Skip to content
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: fix the tooltip not disappearing issue #3347

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

stephanwlee
Copy link
Contributor

Fixes #3282.

Repro case: when tooltip is shown, slowly move the mouse to an edge of a
chart. We expect the tooltip the disappear when the cursor is on the
edge of the chart.

Cause:

  1. For Polymer 2 and its Shadow DOM compatibility, TensorBoard opted out of the
    event delegation of Plottable. Plottable, by default, attaches a set of event
    listeners on document.body and appropriate callbacks depending on the
    circumstances. In Shadow DOM, the event re-targetting broke (harder to identify
    event.target) so TensorBoard, instead, attaches event listeners on every
    Plottable container, SVGs.

  2. When mouse leaves (mouseout) the container, Plottable remaps the event as
    mouse move and calculate whether the cursor is inside a component
    (Interaction.prototype._isInsideComponent, specifically) to trigger appropriate
    callback. The method, however is flawed since it returns, for a component that
    is at for instance (0, 0) with size of (100, 100), true when pointer is at
    (100, 100). It should only return true for [0, 100), instead. As a result, the
    mouseout event occurred at (100, 100) was treated as an event inside the
    component but all the subsequent mouse movements are not captured since they
    are events that occurred outside of the event target. In vanilla Plottable,
    this bug do not manifest since event delegation on the entire document will
    eventually trigger mouse out when cursor is at, for instance, (101, 100).

Fix:
Overwrote the method, _isInsideComponent, with the correct implementation.

Fixes tensorflow#3282.

Repro: when tooltip is shown, slowly move the mouse to an edge of a chart.  We
expect the tooltip the disappear when the cursor is on the edge of the chart.

Cause:
1. For Polymer 2 and its Shadow DOM compatibility, TensorBoard opted out of the
event delegation of Plottable. Plottable, by default, attaches a set of event
listeners on document.body and invokes appropriate callbacks depending on the
circumstances. However, with the Shadow DOM, the event re-targetting broke
(harder to identify `event.target`), so TensorBoard, instead, attaches the
event listeners on every Plottable container, SVGs.

2. When mouse leaves (mouseout) the container, Plottable remaps the event as
mouse move and calculate whether the cursor is inside a component
(Interaction.prototype._isInsideComponent, specifically) to trigger appropriate
callback. The method, however is flawed since it returns, for a component that
is, for instance, at <0, 0> with size of <100, 100>, true when pointer is at
<100, 100>. It should only return true for [0, 100) for a given dimension,
instead.  As a result, the mouseout event occurred at <100, 100> was treated as
an event inside the component but all the subsequent mouse movements are not
captured since they are events that occurred outside of the event target. In
vanilla Plottable, this bug do not manifest since event delegation on the
entire document will eventually trigger mouse out when cursor is at, for
instance, <101, 100>.

Fix:
Overwrote the method, _isInsideComponent, with the correct implementation.
@stephanwlee stephanwlee requested review from psybuzz and caisq and removed request for psybuzz March 9, 2020 22:43
Copy link
Contributor

@caisq caisq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the investigation and the fix. The explanation in the doc string is very clear.

@stephanwlee stephanwlee merged commit 3f84a34 into tensorflow:master Mar 10, 2020
@stephanwlee stephanwlee deleted the chart branch March 10, 2020 20:14
@wchargin wchargin mentioned this pull request Mar 19, 2020
@mavenlin
Copy link

mavenlin commented Mar 24, 2020

Thanks for this great fix, I observed this for quite a long time and it was quite an annoying bug and I can't wait to install the nightly build that contains this fix.

However, I observe the following behavior.

  1. If the page is shown at default zoom, it works well.
  2. If I zoom out to certain percentage, in my browser at 67%, the tooltip doesn't disappear if I move the mouse out the edge from the right.
  3. To debug, I add some logging to the function you add in this commit
    console.log(p.x, this._componentAttachedTo.width(), return_value)
    and observe the following
240.125 281.125 true
248.125 281.125 true
260.125 281.125 true
268.125 281.125 true
276.125 281.125 true
279.125 281.125 true
-50.875 281.125 false
-43.875 281.125 false
-39.875 281.125 false
-33.875 281.125 false
-27.875 281.125 false
-20.875 281.125 false
-11.875 281.125 false
-9.875 281.125 false
-8.875 281.125 false
-6.875 281.125 false
-5.875 281.125 false
-4.875 281.125 false
-2.875 281.125 false
-1.875 281.125 false
-0.875 281.125 false
0.125 281.125 true
1.125 281.125 true
2.125 281.125 true
4.125 281.125 true
5.125 281.125 true

I think the reason may be that the pointer enters another component before p.x ever gets larger than width, so the tooltip for the first component doesn't disappear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plot hover popup remains if mouse is moved slowly out of the plot through the right side of a plot
4 participants