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
[BUG] options.hover.animationDuration not respected #4300
Comments
@etimberg @simonbrunel I've been looking at possible solutions for this, and this is what I could understand from the codebase:
I'd like your opinion regarding possible ways to fix this:
Personally, I prefer 3 since it provides more flexibility and decouples the "global" animation from the "hover" animation, but you guys are the ones with the final word. Let me know what you think. |
I like option 3,where the animation is a new property to keep backwards compatibility with the previous code. |
In this specific case, I would vote for option 2: if we implement option 3, you will still need to manually call In option 3, what is the If it's a plain object, then it will need to propagate to the I deeply think that all the chart animations should be independent and each element should be animable separately. However it would certainly require a huge refactor, maybe breaking changes, so would be better to wait v3 for that. Anyway, if we are going to implement option 3, I would not add an extra parameter but instead gather all options under the first argument as plain object (same for update: function(config) {
if (!config || typeof config !== "object") {
// backward compatibility
config = {
duration: config,
lazy: arguments[1]
};
}
var duration = config.duration;
var lazy = config.lazy;
// {...}
} |
Yes, my idea was to have a plain However, this issue already exists in the current Chart.js version: if |
You right, |
I got this working for my use case: different duration and easing function for the hover event. |
See discussion in the issue for context and possible approaches. When invoking update() inside an event handler, such as onHover, `options.hover.animationDuration` was not being respected. Given that some use cases may require additional animation properties for the manual update call, this commit changes that method signature to accept a configuration object. This object provides backwards compatibility with duration and lazy properties, and also introduces the easing property so that the event animation is different from the global one. Add tests that guarantee that when update is called manually with arguments, it properly builds the _bufferedRequest or calls render with the proper arguments. It includes test cases for when update is called with legacy arguments (duration and lazy) instead of the config object. .update() documentation was previously updated but .render() was left out. Since the backwards compatible change was also made to render(), this commit adds documentation for it.
Resolved in #4362 |
See discussion in the issue for context and possible approaches. When invoking update() inside an event handler, such as onHover, `options.hover.animationDuration` was not being respected. Given that some use cases may require additional animation properties for the manual update call, this commit changes that method signature to accept a configuration object. This object provides backwards compatibility with duration and lazy properties, and also introduces the easing property so that the event animation is different from the global one. Add tests that guarantee that when update is called manually with arguments, it properly builds the _bufferedRequest or calls render with the proper arguments. It includes test cases for when update is called with legacy arguments (duration and lazy) instead of the config object. .update() documentation was previously updated but .render() was left out. Since the backwards compatible change was also made to render(), this commit adds documentation for it.
When the
onHover
/options.hover.onHover
callback invokes_chart.update()
the duration of the hover animation is not being respected and it uses theanimation.duration
value.Expected Behavior
The duration of the hover animation should be the one provided in
options.hover.animationDuration
instead ofanimation.duration
.Current Behavior
The hover animation has the same duration has the initial loading animation.
Possible Solution
When
_chart.update()
is called, the code enters thebufferedRequest
branch incore.controller.js#L772
. Therefore, therender
method is called with a duration that is not the one provided for the hover effect.Steps to Reproduce (for bugs)
Example gist: https://gist.github.com/ricardocosta89/f4751f9a73113df32c9be5cda10ddecf
Context
I wish to have a different animation duration for the hover effect.
Environment
The text was updated successfully, but these errors were encountered: