fix: hide unmeasured tooltips during exit - #10502
Conversation
|
I investigated the failing s2-docs check locally by running the same build on both my PR branch and upstream/main. Both fail at |
| expect(button).toHaveAttribute('tabindex', '0'); | ||
| }); | ||
|
|
||
| it('hides unmeasured tooltips during exit animation', () => { |
There was a problem hiding this comment.
This test is pretty specific to the implementation, probably better to reproduce the actual user flow in the test with mocked timers if possible
There was a problem hiding this comment.
Makes sense. I agree the test should cover the actual focus flow rather than asserting the visibility style directly. I'll hold off on changing the test until we settle on the implementation approach.
| ); | ||
|
|
||
| let tooltip = getByRole('tooltip', {hidden: true}); | ||
| expect(tooltip).toHaveStyle('visibility: hidden'); |
There was a problem hiding this comment.
expect.toBeVisible
https://testing-library.com/docs/ecosystem-jest-dom/
There was a problem hiding this comment.
if we do the either of the suggestions I had instead, we could instead listen to onOpenChange in TooltipTrigger and assert it was never called
| style={ | ||
| { | ||
| ...overlayProps.style, | ||
| visibility: !placement && !state.isOpen ? 'hidden' : undefined, |
There was a problem hiding this comment.
definitely an interesting approach
some other approaches I'd had:
- delay opening a tooltip on focus until setTimeout(0) and check that focus is still there
- fire a custom event that tells a TooltipTrigger that focus will just be passing through, don't bother opening a tooltip
I'm not yet sure which approach I like best, so it's a question for the team.
Closes #10496
When a Tooltip starts its exit animation before its position has been calculated, it can remain mounted with the fallback
position: fixed; top: 0; left: 0styles and briefly appear at the top-left of the viewport.This change hides the tooltip when it is closed and does not have a computed placement:
Normally positioned tooltips and their exit animations are unchanged.
Pull Request Checklist:
Test Instructions:
Run:
yarn jest packages/react-aria-components/test/Tooltip.test.jsThe regression test
hides unmeasured tooltips during exit animationshould fail onupstream/mainand pass with this change.Also verified:
yarn jest packages/react-aria-components/test/Toolbar.test.tsxyarn jest packages/react-aria/test/overlays/useOverlayPosition.test.tsxyarn jest packages/react-aria-components/test/Popover.test.jsyarn oxlint packages/react-aria-components/src/Tooltip.tsx packages/react-aria-components/test/Tooltip.test.jsyarn oxfmt --check packages/react-aria-components/src/Tooltip.tsx packages/react-aria-components/test/Tooltip.test.js