-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(surveys): add Rating question results viz #17886
Conversation
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 good!
Nonblocking: I think it'd be nice to add some sort of border or box to the graph so it has more structure to it on the page
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.
Some nits that may or may not be useful for the future :)
{ count: dismissed, label: 'Dismissed', color: 'bg-warning' }, | ||
{ count: sent, label: 'Submitted', color: 'bg-success' }, | ||
].map(({ count, label, color }) => ( | ||
{ count: seen, label: 'Viewed', style: { backgroundColor: '#1D4AFF' } }, |
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.
curious why this change? I think we're moving away from specific styles on components to utility classes for those styles (but not 100% sure)
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.
The colors suggested by Cory (#17815) are different from those available in Tailwind.
Probably worth a separate PR to make them available there.
Implemented a bar chart for Rating results as wireframed in #17038
@corywatilo