From 9c61c5628f1137d952bf3290abefdfc66a080f50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arma=C4=9Fan?= <broccolinisoup@github.com>
Date: Tue, 12 Mar 2024 10:31:08 +1000
Subject: [PATCH] TooltipV2: Set the tooltip position when the popover open not
 when the 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
---
 .changeset/beige-meals-doubt.md               |  5 +++
 packages/react/src/TooltipV2/Tooltip.tsx      | 42 ++++++++-----------
 .../react/src/__tests__/ActionMenu.test.tsx   |  7 +++-
 .../__snapshots__/TextInput.test.tsx.snap     |  2 +
 4 files changed, 29 insertions(+), 27 deletions(-)
 create mode 100644 .changeset/beige-meals-doubt.md

diff --git a/.changeset/beige-meals-doubt.md b/.changeset/beige-meals-doubt.md
new file mode 100644
index 00000000000..ea5b7ecc8ec
--- /dev/null
+++ b/.changeset/beige-meals-doubt.md
@@ -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
diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx
index af1c76bb316..9b08a83528c 100644
--- a/packages/react/src/TooltipV2/Tooltip.tsx
+++ b/packages/react/src/TooltipV2/Tooltip.tsx
@@ -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;
@@ -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 = () => {
@@ -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 (
diff --git a/packages/react/src/__tests__/ActionMenu.test.tsx b/packages/react/src/__tests__/ActionMenu.test.tsx
index 86c10088391..e2c871c931f 100644
--- a/packages/react/src/__tests__/ActionMenu.test.tsx
+++ b/packages/react/src/__tests__/ActionMenu.test.tsx
@@ -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'
@@ -360,7 +360,10 @@ describe('ActionMenu', () => {
       ),
     )
     const button = component.getByRole('button')
-    button.focus()
+    act(() => {
+      button.focus()
+    })
+
     expect(component.getByRole('tooltip')).toBeInTheDocument()
   })
 
diff --git a/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap
index 1f42c1e6019..e47aa7278d0 100644
--- a/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap
+++ b/packages/react/src/__tests__/__snapshots__/TextInput.test.tsx.snap
@@ -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;
@@ -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;