Skip to content

Commit

Permalink
TooltipV2: Set the tooltip position when the popover open not when th…
Browse files Browse the repository at this point in the history
…e toggle event is triggered (#4327)

* do not set position if it was set before

* console

* only position set on tooltip show

* remove event listener with the same func signature

* just test

* calculate anchor position after popover-open class is added

* clean up

* comment tidy up

* fix tests

* changeset

* move setting popover auto back in the render

* update snapshots
  • Loading branch information
broccolinisoup committed Mar 12, 2024
1 parent 69f4489 commit f08f85b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/beige-meals-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Tooltip v2: Set the tooltip position when popover-open attribute is added to the tooltip element not the toggle event is triggered
42 changes: 17 additions & 25 deletions packages/react/src/TooltipV2/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const StyledTooltip = styled.span`
/* Overriding the default popover styles */
display: none;
&[popover] {
position: absolute;
padding: 0.5em 0.75em;
width: max-content;
margin: auto;
Expand Down Expand Up @@ -196,7 +197,22 @@ export const Tooltip = React.forwardRef(

const openTooltip = () => {
if (tooltipElRef.current && triggerRef.current && !tooltipElRef.current.matches(':popover-open')) {
tooltipElRef.current.showPopover()
const tooltip = tooltipElRef.current
const trigger = triggerRef.current
tooltip.showPopover()
/*
* TOOLTIP POSITIONING
*/
const settings = {
side: directionToPosition[direction].side,
align: directionToPosition[direction].align,
}
const {top, left, anchorAlign, anchorSide} = getAnchoredPosition(tooltip, trigger, settings)
// This is required to make sure the popover is positioned correctly i.e. when there is not enough space on the specified direction, we set a new direction to position the ::after
const calculatedDirection = positionToDirection[`${anchorSide}-${anchorAlign}` as string]
setCalculatedDirection(calculatedDirection)
tooltip.style.top = `${top}px`
tooltip.style.left = `${left}px`
}
}
const closeTooltip = () => {
Expand Down Expand Up @@ -239,32 +255,8 @@ export const Tooltip = React.forwardRef(
}
}

/*
* TOOLTIP POSITIONING
*/
const tooltip = tooltipElRef.current
const trigger = triggerRef.current
tooltip.setAttribute('popover', 'auto')
const settings = {
side: directionToPosition[direction].side,
align: directionToPosition[direction].align,
}

const positionSet = () => {
const {top, left, anchorAlign, anchorSide} = getAnchoredPosition(tooltip, trigger, settings)

tooltip.style.top = `${top}px`
tooltip.style.left = `${left}px`
// This is required to make sure the popover is positioned correctly i.e. when there is not enough space on the specified direction, we set a new direction to position the ::after
const calculatedDirection = positionToDirection[`${anchorSide}-${anchorAlign}` as string]
setCalculatedDirection(calculatedDirection)
}

tooltip.addEventListener('toggle', positionSet)

return () => {
tooltip.removeEventListener('toggle', positionSet)
}
}, [tooltipElRef, triggerRef, direction, type])

return (
Expand Down
7 changes: 5 additions & 2 deletions packages/react/src/__tests__/ActionMenu.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {render as HTMLRender, waitFor} from '@testing-library/react'
import {render as HTMLRender, waitFor, act} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import {axe} from 'jest-axe'
import React from 'react'
Expand Down Expand Up @@ -360,7 +360,10 @@ describe('ActionMenu', () => {
),
)
const button = component.getByRole('button')
button.focus()
act(() => {
button.focus()
})

expect(component.getByRole('tooltip')).toBeInTheDocument()
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1926,6 +1926,7 @@ exports[`TextInput renders trailingAction icon button 1`] = `
}
.c5[popover] {
position: absolute;
padding: 0.5em 0.75em;
width: -webkit-max-content;
width: -moz-max-content;
Expand Down Expand Up @@ -2920,6 +2921,7 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
}
.c6[popover] {
position: absolute;
padding: 0.5em 0.75em;
width: -webkit-max-content;
width: -moz-max-content;
Expand Down

0 comments on commit f08f85b

Please sign in to comment.