Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First whack at the assignment #1

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

First whack at the assignment #1

wants to merge 6 commits into from

Conversation

jmcpeak
Copy link
Owner

@jmcpeak jmcpeak commented Feb 27, 2018

Please take a look Horng.

My apologies for all the white space changes - it is the default react formatter for IntelliJ.

@eoentungz
Copy link

Jason, ran you program and it works. Great Job.
I did misread your code before. Where
addClickHistory = (state, name) => state.clickHistory.concat({ dateTime: new Date().toLocaleTimeString('en-US', options), name: name });
thought you were accessing counter reducer based on the state parameter.
My suggestion is to just pass clickHistory as param to addClickHistory
addClickHistory = (clickHistory, name) => [...clickHistory, { dateTime: new Date().toLocaleTimeString('en-US', options), name: name }];

@jmcpeak
Copy link
Owner Author

jmcpeak commented Mar 6, 2018

Updated PR with your suggestions - cleans it up nicely - why pass around the entire stage object when all we care about is click history - got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants