Skip to content

fix(TooltipV2): delay tooltip opening time by 50ms#5350

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.

Sign up for GitHub

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

Merged
francinelucca merged 11 commits into from
Dec 13, 2024
Merged

fix(TooltipV2): delay tooltip opening time by 50ms #5350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d4e1eb7
fix(TooltipV2): delay tooltip opening time by ms
francinelucca Dec 4, 2024
3fcdbdd
Create five-mayflies-flash.md
francinelucca Dec 4, 2024
f62bf38
Merge branch 'main' of github.com:primer/react into francinelucca/443…
francinelucca Dec 4, 2024
4d4cbcc
fix(Overlay): use fire event for click in test and set popover to fal…
francinelucca Dec 5, 2024
a5bbca5
fix(Overlay): remove redundant await
francinelucca Dec 5, 2024
bb7ebcb
Merge branch 'main' into francinelucca/4432-bug-tooltip-on-iconbutton…
francinelucca Dec 5, 2024
2073d41
Merge branch 'main' into francinelucca/4432-bug-tooltip-on-iconbutton…
francinelucca Dec 9, 2024
7bc59c4
fix(TooltipV2): use safeTimeout
francinelucca Dec 10, 2024
5f5738d
Merge branch 'main' into francinelucca/4432-bug-tooltip-on-iconbutton…
francinelucca Dec 10, 2024
15ce545
Merge branch 'main' of github.com:primer/react into francinelucca/443…
francinelucca Dec 11, 2024
bed7c74
fix(TooltipV2): change type of timeoutRef
francinelucca Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/five-mayflies-flash.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

fix(TooltipV2): delay tooltip opening time by ms
2 changes: 1 addition & 1 deletion packages/react/src/Overlay/Overlay.test.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -128,7 +128,7 @@ describe('Overlay', () => {
expect(container.getByText('Add to list')).toBeInTheDocument()

// open second menu
await user.click(container.getByText('Create list'))
fireEvent.click(container.getByText('Create list'))

@francineluccafrancineluccaDec 5, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I tried for minute to get this to work with user.click, I replicated this test manually in storybook and it works as expected. I noticed userEvent was triggering different events than manual testing did 🤷🏽‍♀️

expect(container.getByPlaceholderText('Name this list')).toBeInTheDocument()

// hitting escape on input should close the second menu but not the first
Expand Down
19 changes: 17 additions & 2 deletions packages/react/src/TooltipV2/Tooltip.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,7 @@ import classes from './Tooltip.module.css'
import {useFeatureFlag} from '../FeatureFlags'
import {KeybindingHint, type KeybindingHintProps} from '../KeybindingHint'
import VisuallyHidden from '../_VisuallyHidden'
import useSafeTimeout from '../hooks/useSafeTimeout'

const CSS_MODULE_FEATURE_FLAG = 'primer_react_css_modules_team'

Expand DownExpand Up@@ -213,6 +214,10 @@ export const Tooltip = React.forwardRef(

const [isPopoverOpen, setIsPopoverOpen] = useState(false)

const timeoutRef = React.useRef<number | null>(null)

const {safeSetTimeout, safeClearTimeout} = useSafeTimeout()

const openTooltip = () => {
try {
if (
Expand DownExpand Up@@ -265,6 +270,8 @@ export const Tooltip = React.forwardRef(
) {
tooltipElRef.current.hidePopover()
setIsPopoverOpen(false)
} else {
setIsPopoverOpen(false)
Comment on lines +273 to +274

@francineluccafrancineluccaDec 5, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Fixes bug where overlay would stay "stuck" and not able to close via keyboard due to popOver state being open but it not actually being in the DOM

}
} catch (error) {
// older browsers don't support the :popover-open selector and will throw, even though we use a polyfill
Expand DownExpand Up@@ -362,10 +369,18 @@ export const Tooltip = React.forwardRef(
child.props.onFocus?.(event)
},
onMouseEnter: (event: React.MouseEvent) => {
openTooltip()
child.props.onMouseEnter?.(event)
// show tooltip after mosue has been hovering for at least 50ms
// (prevent showing tooltip when mouse is just passing through)
timeoutRef.current = safeSetTimeout(() => {
openTooltip()
child.props.onMouseEnter?.(event)
}, 50)
},
onMouseLeave: (event: React.MouseEvent) => {
if (timeoutRef.current) {
safeClearTimeout(timeoutRef.current)
timeoutRef.current = null
}
closeTooltip()
child.props.onMouseLeave?.(event)
},
Expand Down