fix(RC-25445): use querySelectorPortal in pcln-menu and pcln-popover …

…only when provided as a prop (#1510) * fix(RC-25445): use querySelectorPortal in pcln-menu only when provided as a prop * update * update * update * update * update * update * update
priceline · Sep 4, 2024 · 0cbb97d · 0cbb97d
1 parent 32dbe6a
commit 0cbb97d
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 6 deletions.
diff --git a/common/changes/pcln-design-system/fix-RC-25445-menuSSR_2024-08-12-15-53.json b/common/changes/pcln-design-system/fix-RC-25445-menuSSR_2024-08-12-15-53.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "pcln-design-system",
+      "comment": "use querySelectorPortal in pcln-menu only when provided to fix SSR issues",
+      "type": "patch"
+    }
+  ],
+  "packageName": "pcln-design-system"
+}
diff --git a/common/changes/pcln-menu/fix-RC-25445-menuSSR_2024-08-12-15-53.json b/common/changes/pcln-menu/fix-RC-25445-menuSSR_2024-08-12-15-53.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "pcln-menu",
+      "comment": "use querySelectorPortal in pcln-menu only when provided to fix SSR issues",
+      "type": "patch"
+    }
+  ],
+  "packageName": "pcln-menu"
+}
diff --git a/common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-20.json b/common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-20.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "pcln-popover",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "pcln-popover"
+}
diff --git a/common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-27.json b/common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-27.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "pcln-popover",
+      "comment": "do not query the DOM if querySelectorPortal is falsy",
+      "type": "patch"
+    }
+  ],
+  "packageName": "pcln-popover"
+}
diff --git a/packages/menu/src/Menu/Menu.jsx b/packages/menu/src/Menu/Menu.jsx
@@ -69,14 +69,14 @@ function Menu({
         trapFocus={trapFocus}
         width={width}
         zIndex={zIndex}
-        querySelectorPortal={`.${querySelectorPortal}`}
+        querySelectorPortal={querySelectorPortal ? `.${querySelectorPortal}` : undefined}
         {...props}
       >
         <ClickableNode />
       </Popover>
-      {querySelectorPortal ? (
-        <div style={{ width: 0, display: 'inline-block' }} className={querySelectorPortal} />
-      ) : null}
+      {querySelectorPortal && (
+        <div style={{ width: 0, display: 'inline-block' }} className={querySelectorPortal} data-testid={querySelectorPortal} />
+      )}
     </>
   )
 }

diff --git a/packages/menu/src/Menu/Menu.spec.js b/packages/menu/src/Menu/Menu.spec.js
@@ -121,4 +121,26 @@ describe('Menu', () => {
     const dialog = await screen.findByRole('dialog')
     expect(dialog).toHaveAttribute('data-placement', 'bottom')
   })
+
+  it('renders a div with querySelectorPortal className when querySelectorPortal is provided', async () => {
+    render(
+      <Menu id='menu' idx='1' buttonText='Click Me' placement='bottom' querySelectorPortal='popover-portal'>
+        <MenuItem>Item One</MenuItem>
+        <MenuItem>Item Two</MenuItem>
+      </Menu>
+    )
+
+    expect(screen.getByTestId('popover-portal')).toBeInTheDocument()
+  })
+
+  it('does not render a div with querySelectorPortal className when querySelectorPortal is not provided', async () => {
+    render(
+      <Menu id='menu' idx='1' buttonText='Click Me' placement='bottom'>
+        <MenuItem>Item One</MenuItem>
+        <MenuItem>Item Two</MenuItem>
+      </Menu>
+    )
+
+    expect(screen.queryByTestId('popover-portal')).toBeFalsy()
+  })
 })
diff --git a/packages/popover/src/PopoverContent/PopoverContent.jsx b/packages/popover/src/PopoverContent/PopoverContent.jsx
@@ -69,7 +69,7 @@ function PopoverContent({
 
   // Fallback when cannot find an element
   useLayoutEffect(() => {
-    if (document.querySelector(querySelectorPortal)) {
+    if (querySelectorPortal && document.querySelector(querySelectorPortal)) {
       setPortalSelector(querySelectorPortal)
     } else {
       setPortalSelector('body')

diff --git a/packages/popover/src/PopoverContent/PopoverContent.spec.js b/packages/popover/src/PopoverContent/PopoverContent.spec.js
@@ -71,7 +71,7 @@ describe('PopoverContent', () => {
     expect(container).toBeInTheDocument()
   })
 
-  it('Using different portal fallback to a body', () => {
+  it('falls back to use body as a selector when receiving a mismatched portal selector', () => {
     const Wrapper = () => (
       <>
         <PopoverContent renderContent={() => 'Content'} querySelectorPortal='.wrong-wrapper' />
@@ -84,4 +84,15 @@ describe('PopoverContent', () => {
     const bodyContainer = document.querySelector('body > [role="dialog"]')
     expect(bodyContainer).toBeInTheDocument()
   })
+
+  it('falls back to use body as a selector when not receiving a portal selector', () => {
+    const Wrapper = () => (
+      <>
+        <PopoverContent renderContent={() => 'Content'} />
+      </>
+    )
+    render(<Wrapper />)
+    const bodyContainer = document.querySelector('body > [role="dialog"]')
+    expect(bodyContainer).toBeInTheDocument()
+  })
 })