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

Hover styling for dataset in 'dataset' mode #6527

Merged
merged 5 commits into from
Oct 25, 2019
Merged

Conversation

kurkle
Copy link
Member

@kurkle kurkle commented Sep 18, 2019

Dataset level hover styles for dataset mode.

Pen

Closes: #2136
Partially solves: #3880 (hover only triggered by points still, not filled area as requested)

@benmccann
Copy link
Contributor

I just had an idea. What if we call _resolveDatasetElementOptions again, but with a new argument denoting that the dataset is currently being hovered? Then we would support all chart types instead of just line and it would be scriptable by the user.

We could automatically look for options of the same name prefixed with hover and give them priority. We could also add hover: true to the context so that it can be scriptable by the user

etimberg
etimberg previously approved these changes Oct 24, 2019
@kurkle kurkle requested a review from benmccann October 25, 2019 11:53
@benmccann
Copy link
Contributor

I think it'd be nice to update setHoverStyle to use _resolveDataElementOptions similar to the way setDatasetHoverStyle works

I also think some of the other docs need to be updated as well like bar chart now that this is implemented generically. Maybe we can document hover generically?

@kurkle
Copy link
Member Author

kurkle commented Oct 25, 2019

I think it'd be nice to update setHoverStyle to use _resolveDataElementOptions similar to the way setDatasetHoverStyle works

Agreed, but its not quite in the scope of this PR

I also think some of the other docs need to be updated as well like bar chart now that this is implemented generically. Maybe we can document hover generically?

Bar does not utilize dataset element, so this does not apply there. Not quite sure how to document hover generically.

@@ -52,6 +52,13 @@ The line chart allows a number of properties to be specified for each dataset. T
| [`borderWidth`](#line-styling) | `number` | Yes | - | `3`
| [`cubicInterpolationMode`](#cubicinterpolationmode) | `string` | Yes | - | `'default'`
| [`fill`](#line-styling) | <code>boolean&#124;string</code> | Yes | - | `true`
| [`hoverBackgroundColor`](#line-styling) | [`Color`](../general/colors.md) | Yes | - | `undefined`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we document hoverCubicInterpolationMode and hoverFill?

Copy link
Member Author

Choose a reason for hiding this comment

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

Neither of those work. We are not going through update on hover, just updating the model with options and then render.
So bezier control points don't get updated.
I did not investigate why hoverFill does not work.

exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
* Hover styling for dataset in 'dataset' mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Line Chart - hoverBorderColor possible?
3 participants