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

Radar scriptable options #6041

Merged
merged 5 commits into from Feb 9, 2019
Merged

Radar scriptable options #6041

merged 5 commits into from Feb 9, 2019

Conversation

nagix
Copy link
Contributor

@nagix nagix commented Feb 3, 2019

Background

This PR introduces scriptable configuration options for points on radar charts. The user can script the styling and interaction options.

Testing

The existing unit tests covering the setHoverStyle dataset controller method were removed as they did not have proper setup for use with scriptable options.

Fixture based tests were written to test:

  • pointBackgroundColor
  • pointBorderColor
  • pointBorderWidth
  • pointRadius
  • pointRotation
  • pointStyle

Documentation

I updated the radar chart documentation to match the style of the line charts.

Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

Awesome work @nagix (and thanks for all the unit tests)

src/controllers/controller.radar.js Show resolved Hide resolved
src/controllers/controller.radar.js Outdated Show resolved Hide resolved
simonbrunel
simonbrunel previously approved these changes Feb 4, 2019
benmccann
benmccann previously approved these changes Feb 4, 2019
src/controllers/controller.radar.js Outdated Show resolved Hide resolved
src/controllers/controller.radar.js Outdated Show resolved Hide resolved
@nagix nagix dismissed stale reviews from benmccann and simonbrunel via 658e85f February 4, 2019 18:26
kurkle
kurkle previously approved these changes Feb 4, 2019
benmccann
benmccann previously approved these changes Feb 4, 2019
@simonbrunel
Copy link
Member

@nagix do you want to add a scriptable example for radar chart? or should we ask @janelledement if she agree to add one as part of #6042?

@nagix
Copy link
Contributor Author

nagix commented Feb 6, 2019

As it seems @janelledement agreed to add more examples, I would leave it to her😉.

@nagix nagix dismissed stale reviews from benmccann and kurkle via 0e87686 February 7, 2019 03:52
Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

Looks really good. Maybe some tests may not fail enough in case of change (e.g. the point rotation with a line symbol) but it should be fine.

@nagix nagix deleted the issue-6041 branch February 10, 2019 13:22
janelledement pushed a commit to janelledement/Chart.js that referenced this pull request Feb 10, 2019
jonrimmer pushed a commit to jonrimmer/Chart.js that referenced this pull request Feb 14, 2019
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
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.

None yet

5 participants