Do not hide tooltip when update is called

[Evertvdw]

Fixes issue #4991

I store the mouse position in the chart eventhandler and when the chart updates after reinitializing the tooltip I create a new 'mousemove' event and dispatch it on the canvas to make the tooltip reappear again after the update.

Only problem that is still there that when there is no point at the mouseposition after the update the tooltip is still there until the next update or mousemove. I found that this is because this line
me._active = me._chart.getElementsAtEventForMode(e, options.mode, options); still returns an element in that case.

You can see this behaviour in the example I also added in this pull request. Keep ur mouse on a point and wait for the x-axis to shift to see the behaviour described above.

Might need a better unit test, I don't have experience on that end, so help would be appreciated.

[benmccann]

@nagix perhaps you could review this one given your experience with https://github.com/nagix/chartjs-plugin-streaming ?

[simonbrunel]

Thanks @Evertvdw for your contribution

Only problem that is still there that when there is no point at the mouseposition after the update the tooltip is still there until the next update or mousemove.

It could be because of the intersect option of the interaction mode: can you prepare a jsfiddle with your updated code to showcase that behavior?

This PR only fixes the tooltip but what about hovering an element after an update? I'm starting to think that this issue may need to be fully handled at the controller level, not in the tooltip.

[nagix]

chartjs-plugin-streaming takes a similar approach (generating a native mousemove event) to update the tooltip, but calling handleEvent using the cached event seems better. I agree with @simonbrunel that this needs to be handled at the controller level.

The _refresh function in core.controller should call eventHandler, not handleEvent, so that both the hover and tooltip states are updated.

Only problem that is still there that when there is no point at the mouseposition after the update the tooltip is still there until the next update or mousemove.

This could happen because the tooltip positioners use elements' view positions at the time of the update and the elements will move to their model positions after the animation. To solve this problem, I suggest the following changes:

1. At the end of core.controller's transition function, call _refresh when all the elements are in the final positions (easingValue is 1) or the chart is animating. This makes sure that the tooltip positioners use elements' model positions. We don't need to call _refresh in core.controller's update function because it is eventually called in the draw function.

if (easingValue === 1 || me.animating) {
    me._refresh();
}

2. In tooltip's handleEvent function, call update not only when the active elements are changed but also the chart is animating.

// Only handle target event on tooltip change
if (changed || me._chart.animating) {

With these changes, the tooltip will follow elements' animation.

[Evertvdw]

Thanks for the reviews and comments, I'm still a bit struggling with the whole proces but hopefully I'm doing it right. @simonbrunel What is the best way to create a fiddle with my version of the Chart.js code? I have no experience with that.

[etimberg]

@Evertvdw when I create a fiddle with my code I run the build command locally and then paste the output script in a <script> tag inside the HTML window of the fiddle. That way I can leave the "JavaScript" section free for the test code.

[Evertvdw]

Ok, tried to implement all the suggestions and made a fiddle of the results.

Here is the fiddle with the code of the last commit: Last commit fiddle

This updates the tooltips correctly but for the line chart with the updating points the tooltip flickers when the chart is updated. The question is if we think this is ok. I would argue that this is not really elegant behaviour. Calling _refresh also in the controllers update function solves this problem.

Here is a fiddle to show that addition: Updated fiddle

@simonbrunel What do you think of the implementation and the above issue?

[simonbrunel]

@Evertvdw do we need to do anything to see the flickering issue? Both jsfiddle behave the same.

But there is another (blocking) issue:

That's because when we click on the legend box, this one generates a new update / draw, which one generates a new click event, and so on until we move the mouse. It's finally maybe not a good idea to trigger a new event (and thinking twice, it's not a new mouse event but an update).

[Evertvdw]

@simonbrunel Somehow the forking of the fiddles went wrong, I corrected this and the links now point to the right fiddle.

About the legend box issue, I did not notice yet but good that you did, I will see if I can get around that somehow.

Edit What you could do is only store the _lastEvent in the controller if the event is not a click. That would result in this: Fiddle

[Evertvdw]

@simonbrunel What are your thoughts on this?

[Evertvdw]

@simonbrunel @etimberg @benmccann Can someone review this? It's been a bit quiet here :)

[kurkle]

PR does not contain code in last fiddle? Please commit those changes too, so we can review. To me the functionality seems fine in https://jsfiddle.net/Evertvdw/azbp0x8w/

[Evertvdw]

I was waiting for feedback on how to solve the problems, but ok I comitted the changes now.

[kurkle]

Added a onHover function to that pie chart that updates a counter.

https://jsfiddle.net/v9sb81w2/

So still needs some work. I don't have any suggestions what exactly 🙅‍♂️

[simonbrunel]

@Evertvdw no solution came to me at that time and forgot about that PR. Though, I still don't know exactly how to implement this, it would need me to have a closer look to the tooltip logic. However, I still think it's not a good approach to simulate a new event by calling eventHandler on update.

[Evertvdw]

If we can't simulate a new event we must update te tooltip contents on update but currently the tooltip is removed when an update is done and I can't figure out yet where and how this is happening.

[Evertvdw]

@simonbrunel Are you planning to have a look at this? I don't currently have a good idea how to fix this. It's still quite annoying that I have to use version 2.7.0 for as long as this is not fixed :(

[benmccann]

@Evertvdw you might want to join the #dev Slack channel. Sometimes it's easier to discuss approaches there than on GitHub

[benmccann]

currently the tooltip is removed when an update is done

I think it happens here: https://github.com/chartjs/Chart.js/blob/master/src/core/core.controller.js#L630. I believe the whole canvas is wiped out and then we draw it from scratch. So I think we can't stop the tooltip from being removed and just need to redraw it

[benmccann]

@Evertvdw can you call tooltip.update instead of eventHandler? It looks to me like that's what eventHandler ends up calling eventually to make the tooltip show

[Evertvdw]

@Evertvdw you might want to join the #dev Slack channel. Sometimes it's easier to discuss approaches there than on GitHub

How do I do that? Never used slack before :)

And regarding your other comments, I will take a look at it and see if I can do that.

[benmccann]

How do I do that? Never used slack before :)

https://github.com/chartjs/Chart.js/blob/master/docs/developers/contributing.md