Skip to content

Commit

Permalink
Fixing tooltip bug in firefox where it sometimes flickers and stays h…
Browse files Browse the repository at this point in the history
…idden (#2281)

## Summary:
- In Firefox, we were observing that the tooltip would sometimes flicker and stay hidden even though the tooltip anchor is still hovered on. Other browsers didn't seem to have this issue. It could be reproduced in Firefox by placing the tooltip anchor at the top left with long content and hovering over it. 
- When the `mouseenter` and `mouseleave` events were logged, I found that when the issue occurs, there would be subsequent `mouseenter`, `mouseleave`, and `mouseenter` events. 

https://github.com/user-attachments/assets/911f57d3-90b4-4dbc-9d66-8c62928b46c2

What's happening here:
  - The first `mouseenter` opens the tooltip
  - When the `mouseleave` event occurs, it [triggers the tooltip to close after a delay](https://github.com/Khan/wonder-blocks/blob/68dd6059c24c595c62e07b72b9ec17f0ead2bfbb/packages/wonder-blocks-tooltip/src/components/tooltip-anchor.tsx#L258-L261)
  - When the second `mouseenter` event occurs, it updates the active state to true immediately (because [`instant` resolves to true](https://github.com/Khan/wonder-blocks/blob/68dd6059c24c595c62e07b72b9ec17f0ead2bfbb/packages/wonder-blocks-tooltip/src/components/tooltip-anchor.tsx#L237) due to `active && TRACKER.steal(this)` resolving to true)
  - Then, the delayed action of closing the tooltip occurs (triggered by `mouseleave`). Since the `mouseenter` event already finished opening the tooltip, the tooltip stays hidden

The fix
- When the active state is going to be updated, we check first to see if the current active state is already the same as what it needs to be updated to. If it's the same, we cancel any pending actions since the state is already up to date. 
- This fixes it because when the subsequent events occur, the second `mouseenter` event will recognize that the tooltip is already opened and cancel the action to close the tooltip (which was triggered from the prior `mouseleave` event) 

You may also be wondering why multiple events are being triggered in the first place. Here's what I found:
- When we hover over the tooltip anchor, the tooltip popper is activated. It places the tooltip based on the anchor, and then uses a transform to shift it over. This [styling comes from react-popper](https://github.com/Khan/wonder-blocks/blob/68dd6059c24c595c62e07b72b9ec17f0ead2bfbb/packages/wonder-blocks-tooltip/src/components/tooltip-popper.tsx#L148-L159)
<img width="964" alt="image" src="https://github.com/user-attachments/assets/c21bc6a5-15e5-4f95-8801-5b8798e1b11a">

- I think what is happening is when the tooltip opens, it sometimes opens where the cursor is, which causes the `mouseleave` event on the tooltip anchor. Then when it is moved using a CSS transform, the `mouseenter` event is triggered again on the tooltip. 
- I was exploring some of the react-popper options to see if we could prevent this from happening in the first place, but couldn't find anything that worked. Just for my own curiosity, I disabled the css transform and hardcoded the positioning and wasn't able reproduce the multiple events on hover. This isn't ideal though since react-popper handles all the different positioning cases. Also, I think it makes sense for the TooltipAnchor component to be able to handle multiple events in a row in case it ever happens!

As always, please let me know if you have any other ideas or suggestions, this was a tricky one!!


Issue: WB-1731

## Test plan:
- Firefox: When tooltip is in the top left corner, the tooltip should consistently open when the anchor is hovered (`/iframe.html?args=&id=packages-tooltip-tooltip--in-top-corner&viewMode=story`)
- Tooltip continues to work as expected

## Screenshots/Recordings
After:
Tooltip behaves as expected even though multiple mouse events occurred on hover

https://github.com/user-attachments/assets/17d5a0c5-8e4f-4211-acd1-13b084da6887



Logs of events:
**Before:**
<img width="969" alt="before" src="https://github.com/user-attachments/assets/b2724854-2b5b-42b8-a53d-9d7125f2a73a">

**After:**
<img width="975" alt="after" src="https://github.com/user-attachments/assets/e6fe278d-6935-4918-bfa0-daadeeda1d91">

Author: beaesguerra

Reviewers: jandrade

Required Reviewers:

Approved By: jandrade

Checks: ✅ codecov/project, ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Lint (ubuntu-latest, 20.x), ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ⏭️  Publish npm snapshot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2281
  • Loading branch information
beaesguerra authored Jul 31, 2024
1 parent 68dd605 commit be54044
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-hats-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-tooltip": patch
---

Fix tooltip bug where it sometimes flickers and stays hidden when the anchor is hovered over in Firefox
33 changes: 33 additions & 0 deletions __docs__/wonder-blocks-tooltip/tooltip.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {Meta, StoryObj} from "@storybook/react";
import {expect, within, userEvent} from "@storybook/test";

import magnifyingGlass from "@phosphor-icons/core/regular/magnifying-glass.svg";
import info from "@phosphor-icons/core/regular/info.svg";

import Button from "@khanacademy/wonder-blocks-button";
import {View} from "@khanacademy/wonder-blocks-core";
Expand All @@ -13,6 +14,7 @@ import IconButton from "@khanacademy/wonder-blocks-icon-button";
import {OnePaneDialog, ModalLauncher} from "@khanacademy/wonder-blocks-modal";
import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {Body} from "@khanacademy/wonder-blocks-typography";
import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon";

import Tooltip from "@khanacademy/wonder-blocks-tooltip";
import packageConfig from "../../packages/wonder-blocks-tooltip/package.json";
Expand Down Expand Up @@ -476,3 +478,34 @@ const styles = StyleSheet.create({
justifyContent: "center",
},
});

/**
* This story shows the behaviour of the tooltip when it is in the top corner
*/
export const InTopCorner = {
parameters: {
layout: "fullscreen",
chromatic: {
// Disabling snapshot since this is for testing purposes
disableSnapshot: true,
},
},
render: () => (
<View
style={{
position: "absolute",
top: 0,
left: 0,
}}
>
<Tooltip content="This is an example descriptor that's long with more content to see if it will display properly in different browsers">
<PhosphorIcon
icon={info}
size="small"
aria-hidden
style={{":hover": {backgroundColor: color.red}}}
/>
</Tooltip>
</View>
),
};
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,33 @@ describe("tooltip integration tests", () => {
// Assert
expect(screen.queryByRole("tooltip")).not.toBeInTheDocument();
});

it("should have an opened tooltip when subsequent mouseenter, mouseleave, and mouseenter events occur", async () => {
// This a test case that simulates a bug in Firefox where a tooltip will
// sometimes flicker and not stay opened due to the browser triggering
// subsequent mouseenter, mouseleave, and mouseenter events

// Arrange
const ue = userEvent.setup({
advanceTimers: jest.advanceTimersByTime,
});
render(<Tooltip content="hello, world">an anchor</Tooltip>);

// Act
const anchor = await screen.findByText("an anchor");
// Trigger initial mouseenter event on anchor and let the timeout complete
// to activate the tooltip
await ue.hover(anchor);
await jest.runAllTimers();
expect(screen.getByRole("tooltip")).toBeInTheDocument();
// Trigger mouseleave and mouseenter event and run timers only after
// both have been triggered. This simulates the mouseenter event being
// triggered before the tooltip is closed from the mouseleave event
await ue.unhover(anchor);
await ue.hover(anchor);
await jest.runAllTimers();

// Assert
expect(screen.getByRole("tooltip")).toBeInTheDocument();
});
});
16 changes: 14 additions & 2 deletions packages/wonder-blocks-tooltip/src/components/tooltip-anchor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,20 @@ export default class TooltipAnchor
// So, if active is stolen from us, we are changing active state,
// or we are inactive and have a timer, clear the action.
this._clearPendingAction();
} else if (active === this.state.active && !this._timeoutID) {
// Nothing to do if we're already active.
} else if (active === this.state.active) {
if (this._timeoutID) {
// Cancel pending action if the current `this.state.active` is
// already the value we want to set it to (ie. the `active` arg).
// This is okay to cancel because:
// - if the pending action was to set `this.state.active` to the
// same value, it is not needed because it already is up to date
// - if the pending action was to set `this.state.active` to the
// opposite value, it is not needed because there is a more recent
// event that triggered this function with an `active` arg that is
// the same value as the current state.
this._clearPendingAction();
}
// Nothing else to do if active state is up to date.
return;
}

Expand Down

0 comments on commit be54044

Please sign in to comment.