- Notifications
You must be signed in to change notification settings - Fork12k
bugfix: issue 11801#11849
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
bugfix: issue 11801#11849
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| const options = this.options.setContext(this.getContext()); | ||
| const active = this._active; | ||
| let properties; | ||
| let tooltipItems = []; | ||
| if (!active.length) { | ||
| if (!active.length || !inChartArea) { |
LeeLenaleeeAug 4, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
It seems that this behavior was added intentionally and is not a bug when I look at the code and thepr where this got introduced.
I will look at this later to see if this might break any implementations that rely on that behavior
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.
Thanks for the feedback.
Correct me if I'm wrong, from my understanding, the!active.length is added to make sure the chart can be interactive even when the cursor is out of it (hovering over the legend and high light the corresponding data in the chart).
Adding the extra check will only effect the visibility of the tooltip in this scenario.
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.
Hi@LeeLenaleee, do you think this is a proper change or do you have any further comments on this?
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.
Hi, I tested the changes, what you said is correct, this does still work with externally calling code tosetActiveElements. Although the name is kind of misleading in those cases. Since the bool is calledinChartArea, but when you usesetActiveElements you are not in the chart area.
Also this change seems to introduce a new bug. When you move your mouse to the legend quickly and return to the same slice the hover does not trigger anymore
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.
Thanks@LeeLenaleee , I tested it and tried to fix that, but looks like it's not a simple change. I will change this PR to draft and work on this continuously.
Signed-off-by: Saber Sun <saber.sun@fmr.com>
5f1d470 to66a5204Compare
Add some extra check to make sure the tooltip be hidden while cursor moving outside of the chart. This is to fix the issue#11801