-
Notifications
You must be signed in to change notification settings - Fork 3
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
UI polish for recidivism chart #255
Conversation
Pull Request Test Coverage Report for Build 368791933
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! This is unrelated to the changes, but I was curious about the demographic option being reset whenever a year is changed, and wanted to ask if it's the intended behavior for this chart.
Here's the flow if I wanted to change the year I'm viewing for a particular demographic:
- Select a single year
- Select a demographic
- Deselect a year and select another year
- Demographic resets to Total, so I reselect the demographic
@daschi it is intended but I guess in the use case you are describing it's not strictly necessary. The view resets whenever the number of cohorts is not equal to one, which means setting to zero resets it. Seems kind of silly when you spell it out like that! (It's probably an easy fix, I will look into it and add to this PR if it's sufficiently small) |
good call @daschi I got that change approved and have added it here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! 🥇
Description of the change
Several small polish items concerned behavior in the line chart; the changes are all grouped together here.
Type of change
Related issues
Checklists
Development
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: