-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Doughnut scriptable options #5966
Doughnut scriptable options #5966
Conversation
Looks good to me except for a few minor errors. Are you going to add fixture tests and a sample? |
I will add fixture tests but I don't have anytime today so I can't get to it until tomorrow. I'll see what we have for the bar & bubble charts in terms of this. The interaction tests should cover a fair bit of it already. |
Looks good, I agree with @nagix that we should also add image based unit test, similar to these ones. |
6d6ad65
to
85b98ff
Compare
@simonbrunel @nagix I added some scriptable fixture tests for |
I've added tests for |
Addressed concerns regarding missing tests
bf592de
to
6c3ea48
Compare
borderColor, and borderWidth.
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 very good, except for the duplication of couple of lines, reported by codeclimate.
We'll see how it turns out when more scriptable options are implemented and refactor later if needed.
Background
This PR introduces scriptable configuration options for doughnut charts. The user can script the styling, interaction, and border alignment options.
Testing
Unit tests were written to cover these features and were based on the existing bubble controller tests. The existing unit tests covering the
setHoverStyle
dataset controller method were removed as they did not have proper setup for use with scriptable options.Documentation
I updated the doughnut chart documentation to match the style of the bubble & bar charts as they already support scriptable options.