Update tooltip content and styling on update() #6181

Evertvdw · 2019-04-02T09:44:37Z

Fixes issue #4991

Another attempt at this issue, credits go to @kurkle.

The tooltip is now updated based on the new data that is visible under the mouse and hides if that point is no longer visible.

To do this the last mouse event is stored and passed to the eventhandler upon update. The data for the tooltip is updated during transition as well as the hover styling of the point.

For a working example of this PR: https://codepen.io/Evertvdw/pen/PgqbWq

Current behaviour: https://codepen.io/Evertvdw/pen/QPbdLQ

Evertvdw · 2019-04-02T11:51:44Z

Found a mistake still that did not set lastActive to an empty initially. This resulted in #3791 breaking again, so needed to fix it.

benmccann · 2019-04-02T15:42:45Z

I believe there was an objection to calling me.eventHandler in #5817 and it looks like that's still being done. Is it possible to call tooltip.update instead? #5817 (comment)

kurkle · 2019-04-10T12:59:15Z

I believe there was an objection to calling me.eventHandler in #5817 and it looks like that's still being done. Is it possible to call tooltip.update instead? #5817 (comment)

I don't think its possible.

benmccann · 2019-04-12T17:44:54Z

@kurkle can you clarify why you don't think it's possible?

kurkle · 2019-04-15T18:38:53Z

@kurkle can you clarify why you don't think it's possible?

To clarify, it certainly is possible, it just does not work correctly 😏

And the reason is: tooltip._active array might contain items that are no longer available and / or under the mouse.
So those active items need to be re-evaluated and that's what eventHandler does (for both hover and tooltip).

Evertvdw · 2019-04-16T06:06:49Z

I guess this PR basically does what was proposed here:

#4991 (comment)

src/core/core.controller.js

src/core/core.tooltip.js

nagix · 2019-04-24T11:01:18Z

src/core/core.controller.js

+
+		if (me._lastEvent && me.animating) {
+			// If, during animation, element under mouse changes, let's react to that.
+			me.active = me.getElementsAtEventForMode(me._lastEvent, hoverOptions.mode, hoverOptions);


Why not simply calling eventHandler but duplicating code in handleEvent? I think other logic such as calling the onHover callback is also needed.

That is probably right, I replaced these lines now with a call to eventHandler.

nagix · 2019-04-24T11:09:08Z

src/core/core.controller.js

 		} else {
 			me.active = me.getElementsAtEventForMode(e, hoverOptions.mode, hoverOptions);
+			me._lastEvent = e;


If 'click' event is picked here, this issue will occur.

I cannot reproduce that issue in the codepen I have provided with the PR code, I think that issue there was with refiring the actual click event.

I see this issue when clicking the legend in your codepen.

nagix · 2019-04-24T11:09:48Z

src/core/core.tooltip.js

 		} else {
 			me._active = me._chart.getElementsAtEventForMode(e, options.mode, options);
+			me._lastEvent = e;


Same as above.

benmccann · 2019-05-21T15:32:03Z

@Evertvdw will you be able to address @nagix's comments?

Evertvdw · 2019-05-22T11:30:29Z

@Evertvdw will you be able to address @nagix's comments?

Sorry I forgot about this for a bit, but yes I think I can store the lastEvent only if that is not a click of course. About his first comment I need to look into that, but don't have the time right now.

Evertvdw · 2019-06-05T10:07:12Z

Updated codepen: PR 6181 Updated

ylor · 2019-07-01T23:18:55Z

I'd love to see this merged. Require it for project with a constantly updating dataset.

Thanks for all your hard work @Evertvdw

Evertvdw · 2019-07-02T06:14:19Z

@benmccann @nagix can you review this?

benmccann · 2019-07-02T15:15:40Z

It looks fine to me though I'm not very familiar with this part of the code, so I'll defer to @etimberg @nagix @kurkle on this one. Thanks for having been patient on this one and for following up on it

src/core/core.controller.js

src/core/core.tooltip.js

src/core/core.controller.js

src/core/core.tooltip.js

Evertvdw · 2019-09-12T09:54:16Z

Implemented the rieview, so everything is good now?

kurkle · 2019-09-12T10:56:13Z

Can you update the pen too?

Evertvdw · 2019-09-17T08:58:21Z

Yes, updated the PR pen: https://codepen.io/Evertvdw/pen/PgqbWq

src/core/core.controller.js

kurkle

LGTM

SebZar · 2019-10-15T12:51:03Z

Hi,
can someone fix the codeclimate issue and force the merge.

I' also waiting for this update. :-)

src/core/core.controller.js

benmccann · 2019-10-25T15:24:51Z

@Evertvdw thanks so much for your patience. I think this PR is ready to be merged since it has been approved by @kurkle, @etimberg, and myself. Unfortunately there's a merge conflict. Would you be able to rebase this PR against master so we can merge it?

benmccann · 2019-10-29T12:35:36Z

Rebased and merged in #6635

Evertvdw · 2019-10-29T13:50:36Z

@benmccann Sorry, I forgot about it again, but thanks for doing it!

Weit added 2 commits April 2, 2019 11:20

Update tooltip content and styling on update()

3357e05

Restore clear lastActive

fb8ac49

benmccann added the status: stale label Apr 4, 2019

kurkle added status: awaiting review and removed status: stale labels Apr 10, 2019

kurkle requested a review from nagix April 15, 2019 18:39

benmccann reviewed Apr 19, 2019

View reviewed changes

src/core/core.controller.js Outdated Show resolved Hide resolved

benmccann reviewed Apr 19, 2019

View reviewed changes

src/core/core.controller.js Outdated Show resolved Hide resolved

benmccann reviewed Apr 19, 2019

View reviewed changes

src/core/core.tooltip.js Outdated Show resolved Hide resolved

Fix typos

167af95

nagix reviewed Apr 24, 2019

View reviewed changes

benmccann added status: stale and removed status: awaiting review labels May 4, 2019

Implemented @nagix suggestions

7d4fb13

benmccann removed the status: stale label Jun 5, 2019

etimberg reviewed Jul 2, 2019

View reviewed changes

src/core/core.controller.js Outdated Show resolved Hide resolved

src/core/core.tooltip.js Show resolved Hide resolved

jolav-old-account mentioned this pull request Aug 7, 2019

Color of squares differs from pie's jolav/codetabs#7

Closed

kurkle requested changes Sep 4, 2019

View reviewed changes

src/core/core.controller.js Outdated Show resolved Hide resolved

src/core/core.tooltip.js Outdated Show resolved Hide resolved

src/core/core.tooltip.js Show resolved Hide resolved

etimberg previously approved these changes Sep 7, 2019

View reviewed changes

Change click check, @kurkle request

d6f5b0b

Evertvdw dismissed etimberg’s stale review via d6f5b0b September 12, 2019 09:51

kurkle reviewed Sep 17, 2019

View reviewed changes

src/core/core.controller.js Outdated Show resolved Hide resolved

Fix show/hide dataset, review @kurkle

f8407a3

kurkle approved these changes Sep 18, 2019

View reviewed changes

etimberg approved these changes Sep 19, 2019

View reviewed changes

kurkle requested review from benmccann and nagix October 14, 2019 10:51

benmccann reviewed Oct 17, 2019

View reviewed changes

src/core/core.controller.js Show resolved Hide resolved

benmccann approved these changes Oct 25, 2019

View reviewed changes

RatherLogical mentioned this pull request Oct 29, 2019

Update tooltip content and styling on update() #6635

Merged

benmccann closed this Oct 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update tooltip content and styling on update() #6181

Update tooltip content and styling on update() #6181

Evertvdw commented Apr 2, 2019 •

edited

Loading

Evertvdw commented Apr 2, 2019

benmccann commented Apr 2, 2019

kurkle commented Apr 10, 2019

benmccann commented Apr 12, 2019

kurkle commented Apr 15, 2019

Evertvdw commented Apr 16, 2019

nagix Apr 24, 2019

Evertvdw Jun 5, 2019

nagix Apr 24, 2019

Evertvdw Apr 24, 2019

nagix Apr 24, 2019

nagix Apr 24, 2019

benmccann commented May 21, 2019

Evertvdw commented May 22, 2019

Evertvdw commented Jun 5, 2019

ylor commented Jul 1, 2019

Evertvdw commented Jul 2, 2019

benmccann commented Jul 2, 2019

Evertvdw commented Sep 12, 2019

kurkle commented Sep 12, 2019

Evertvdw commented Sep 17, 2019

kurkle left a comment

SebZar commented Oct 15, 2019

benmccann commented Oct 25, 2019

benmccann commented Oct 29, 2019

Evertvdw commented Oct 29, 2019

Update tooltip content and styling on update() #6181

Update tooltip content and styling on update() #6181

Conversation

Evertvdw commented Apr 2, 2019 • edited Loading

Evertvdw commented Apr 2, 2019

benmccann commented Apr 2, 2019

kurkle commented Apr 10, 2019

benmccann commented Apr 12, 2019

kurkle commented Apr 15, 2019

Evertvdw commented Apr 16, 2019

nagix Apr 24, 2019

Choose a reason for hiding this comment

Evertvdw Jun 5, 2019

Choose a reason for hiding this comment

nagix Apr 24, 2019

Choose a reason for hiding this comment

Evertvdw Apr 24, 2019

Choose a reason for hiding this comment

nagix Apr 24, 2019

Choose a reason for hiding this comment

nagix Apr 24, 2019

Choose a reason for hiding this comment

benmccann commented May 21, 2019

Evertvdw commented May 22, 2019

Evertvdw commented Jun 5, 2019

ylor commented Jul 1, 2019

Evertvdw commented Jul 2, 2019

benmccann commented Jul 2, 2019

Evertvdw commented Sep 12, 2019

kurkle commented Sep 12, 2019

Evertvdw commented Sep 17, 2019

kurkle left a comment

Choose a reason for hiding this comment

SebZar commented Oct 15, 2019

benmccann commented Oct 25, 2019

benmccann commented Oct 29, 2019

Evertvdw commented Oct 29, 2019

Evertvdw commented Apr 2, 2019 •

edited

Loading