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

Line chart improvements 2 #315

Merged
merged 8 commits into from
May 4, 2017
Merged

Line chart improvements 2 #315

merged 8 commits into from
May 4, 2017

Conversation

emilyb7
Copy link
Collaborator

@emilyb7 emilyb7 commented May 1, 2017

More improvements to the line chart!

animated arrow on preview version, should let user know that they can click to see more!
animation on detail version, scrolls to latest rating and automatically opens details of that rating (only takes effect when there are 6 or more ratings to see on one goal)

all related to #286

i think i might have done some really bad things here, sorry... 🙈

constructor(props) {
super(props);
this.state = {
customWidth: props.currentGoal.ratings.length < 6
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change? It does not look like it does..
If it does, maybe think about moving it into redux.

If it does not change, it should not be in the state. Either you can move this functionality into mapStateToProps or just make a helper here..

Might be easier if you told me why it is here..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm yeah, i don't know what i was doing 🙃 - have got rid of it now!

@des-des des-des assigned emilyb7 and unassigned des-des May 3, 2017
@emilyb7 emilyb7 assigned des-des and unassigned emilyb7 May 3, 2017
@des-des des-des merged commit 2caa659 into master May 4, 2017
@des-des des-des deleted the line-chart-improvements-2 branch May 4, 2017 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants