-
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
Fix onHover event not being triggered #4297
Fix onHover event not being triggered #4297
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.
Per my comment in #4926 (at the exact same time this was opened) I think we need to update the docs not the code because otherwise this is a breaking change from v2.0.0
An alternative solution is to say that the docs are right and what we want but make the change in a way that keeps backwards compatibility with the old |
@etimberg, @simonbrunel What do you think of exposing This will allow developers interested only in knowing if the hovering state changed ( |
The core controller was looking at the wrong object (options.hover) to find the function to be called on hover. The function is provided on the top level options object (options.onHover). Fixes #4296
@etimberg Per @simonbrunel's feedback on #4296, I've added the fallback to the "old" |
src/core/core.controller.js
Outdated
// Need to call with native event here to not break backwards compatibility | ||
hoverOptions.onHover.call(me, e.native, me.active); | ||
hoverCallback.call(me, e.native, me.active); |
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.
Might be more concise using helpers.callback(options.onHover || options.hover.onHover, [e.native, me.active], me)
, then no need to test if hoverCallback
is defined/null.
By using the helper function, there's no need to verify if the callback is defined, as the helper already does that.
Fix onHover event not being triggered The core controller was looking at the wrong object (options.hover) to find the function to be called on hover. The function is provided on the top level options object (options.onHover). By using the helper function, there's no need to verify if the callback is defined, as the helper already does that. Fixes chartjs#4296
The core controller was looking at the wrong object (options.hover) to find the function to be called on hover. The function is provided on the top level options object (options.onHover).
Fixes #4296