From bd1180dfd1e275d1f9c974ffc1a085b5647047b2 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 11:29:46 -0400 Subject: [PATCH 1/8] fix(RC-25445): use querySelectorPortal in pcln-menu only when provided as a prop --- ...fix-RC-25445-menuSSR_2024-08-12-15-53.json | 10 +++++++++ ...fix-RC-25445-menuSSR_2024-08-12-15-53.json | 10 +++++++++ packages/menu/src/Menu/Menu.jsx | 8 +++---- packages/menu/src/Menu/Menu.spec.js | 22 +++++++++++++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 common/changes/pcln-design-system/fix-RC-25445-menuSSR_2024-08-12-15-53.json create mode 100644 common/changes/pcln-menu/fix-RC-25445-menuSSR_2024-08-12-15-53.json 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 new file mode 100644 index 0000000000..93efc04c45 --- /dev/null +++ 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" +} \ No newline at end of file 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 new file mode 100644 index 0000000000..d8bc00bbc9 --- /dev/null +++ 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" +} \ No newline at end of file diff --git a/packages/menu/src/Menu/Menu.jsx b/packages/menu/src/Menu/Menu.jsx index 685d170b0f..2afdc02c5a 100644 --- 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}`} {...props} > - {querySelectorPortal ? ( -
- ) : null} + {querySelectorPortal && ( +
+ )} ) } diff --git a/packages/menu/src/Menu/Menu.spec.js b/packages/menu/src/Menu/Menu.spec.js index c77ecc7d6a..2c81424f72 100644 --- 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 () => { + const { getByTestId } = render( + + Item One + Item Two + + ) + + expect(getByTestId('popover-portal')).toBeInTheDocument() + }) + + it('does not render a div with querySelectorPortal className when querySelectorPortal is not provided', async () => { + const { queryByTestId } = render( + + Item One + Item Two + + ) + + expect(queryByTestId('popover-portal')).toBeFalsy() + }) }) From 37168b22fda98d64af01c8985173020e29321857 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:17:36 -0400 Subject: [PATCH 2/8] update --- packages/menu/src/Menu/Menu.jsx | 2 +- packages/menu/src/Menu/Menu.spec.js | 8 ++++---- .../popover/src/PopoverContent/PopoverContent.jsx | 2 +- .../src/PopoverContent/PopoverContent.spec.js | 14 ++++++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/menu/src/Menu/Menu.jsx b/packages/menu/src/Menu/Menu.jsx index 2afdc02c5a..551ff8c990 100644 --- a/packages/menu/src/Menu/Menu.jsx +++ b/packages/menu/src/Menu/Menu.jsx @@ -69,7 +69,7 @@ function Menu({ trapFocus={trapFocus} width={width} zIndex={zIndex} - querySelectorPortal={querySelectorPortal && `.${querySelectorPortal}`} + querySelectorPortal={querySelectorPortal ? `.${querySelectorPortal}` : undefined} {...props} > diff --git a/packages/menu/src/Menu/Menu.spec.js b/packages/menu/src/Menu/Menu.spec.js index 2c81424f72..8076c1b3b5 100644 --- a/packages/menu/src/Menu/Menu.spec.js +++ b/packages/menu/src/Menu/Menu.spec.js @@ -123,24 +123,24 @@ describe('Menu', () => { }) it('renders a div with querySelectorPortal className when querySelectorPortal is provided', async () => { - const { getByTestId } = render( + render( Item One Item Two ) - expect(getByTestId('popover-portal')).toBeInTheDocument() + expect(screen.getByTestId('popover-portal')).toBeInTheDocument() }) it('does not render a div with querySelectorPortal className when querySelectorPortal is not provided', async () => { - const { queryByTestId } = render( + render( Item One Item Two ) - expect(queryByTestId('popover-portal')).toBeFalsy() + expect(screen.queryByTestId('popover-portal')).toBeFalsy() }) }) diff --git a/packages/popover/src/PopoverContent/PopoverContent.jsx b/packages/popover/src/PopoverContent/PopoverContent.jsx index d9231504bc..60efacb75a 100644 --- 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 index 605f88f3de..98be04a767 100644 --- a/packages/popover/src/PopoverContent/PopoverContent.spec.js +++ b/packages/popover/src/PopoverContent/PopoverContent.spec.js @@ -84,4 +84,18 @@ describe('PopoverContent', () => { const bodyContainer = document.querySelector('body > [role="dialog"]') expect(bodyContainer).toBeInTheDocument() }) + + it('not using portal fallback to a body', () => { + const Wrapper = () => ( + <> + 'Content'} /> +
+ + ) + render() + const container = document.querySelector('.wrapper > [role="dialog"]') + expect(container).not.toBeInTheDocument() + const bodyContainer = document.querySelector('body > [role="dialog"]') + expect(bodyContainer).toBeInTheDocument() + }) }) From 7b41626fb1581e5d3efa93ced1bfc7efb4c24512 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:20:32 -0400 Subject: [PATCH 3/8] update --- .../fix-RC-25445-menuSSR_2024-08-12-16-20.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-20.json 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 new file mode 100644 index 0000000000..4995e090c5 --- /dev/null +++ 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" +} \ No newline at end of file From 3c6458b7d166776b96d06e7f8f2dca27c67e4c72 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:25:38 -0400 Subject: [PATCH 4/8] update --- packages/popover/src/PopoverContent/PopoverContent.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/popover/src/PopoverContent/PopoverContent.spec.js b/packages/popover/src/PopoverContent/PopoverContent.spec.js index 98be04a767..1a7db8df46 100644 --- a/packages/popover/src/PopoverContent/PopoverContent.spec.js +++ b/packages/popover/src/PopoverContent/PopoverContent.spec.js @@ -89,7 +89,6 @@ describe('PopoverContent', () => { const Wrapper = () => ( <> 'Content'} /> -
) render() From 16b29949210aff484d28729418242425a657065a Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:27:37 -0400 Subject: [PATCH 5/8] update --- .../fix-RC-25445-menuSSR_2024-08-12-16-27.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/pcln-popover/fix-RC-25445-menuSSR_2024-08-12-16-27.json 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 new file mode 100644 index 0000000000..8be8196fb5 --- /dev/null +++ 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" +} \ No newline at end of file From 3c3fd3096363db24c52014f2200ad7482fec7ed1 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:33:30 -0400 Subject: [PATCH 6/8] update --- packages/popover/src/PopoverContent/PopoverContent.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/popover/src/PopoverContent/PopoverContent.spec.js b/packages/popover/src/PopoverContent/PopoverContent.spec.js index 1a7db8df46..1353a99dfd 100644 --- 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 = () => ( <> 'Content'} querySelectorPortal='.wrong-wrapper' /> @@ -85,10 +85,11 @@ describe('PopoverContent', () => { expect(bodyContainer).toBeInTheDocument() }) - it('not using portal fallback to a body', () => { + it('falls back to use body as a selector when not receivinga portal selector', () => { const Wrapper = () => ( <> 'Content'} /> +
) render() From f531336ac50116ce25a6dd546f8ee21e5d8ce6f0 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:34:04 -0400 Subject: [PATCH 7/8] update --- packages/popover/src/PopoverContent/PopoverContent.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/popover/src/PopoverContent/PopoverContent.spec.js b/packages/popover/src/PopoverContent/PopoverContent.spec.js index 1353a99dfd..b30a31887c 100644 --- a/packages/popover/src/PopoverContent/PopoverContent.spec.js +++ b/packages/popover/src/PopoverContent/PopoverContent.spec.js @@ -85,7 +85,7 @@ describe('PopoverContent', () => { expect(bodyContainer).toBeInTheDocument() }) - it('falls back to use body as a selector when not receivinga portal selector', () => { + it('falls back to use body as a selector when not receiving a portal selector', () => { const Wrapper = () => ( <> 'Content'} /> From b2a6d8f42da0b6aca2fd04e1d35bc5a53476febc Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Mon, 12 Aug 2024 12:37:02 -0400 Subject: [PATCH 8/8] update --- packages/popover/src/PopoverContent/PopoverContent.spec.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/popover/src/PopoverContent/PopoverContent.spec.js b/packages/popover/src/PopoverContent/PopoverContent.spec.js index b30a31887c..855af4f564 100644 --- a/packages/popover/src/PopoverContent/PopoverContent.spec.js +++ b/packages/popover/src/PopoverContent/PopoverContent.spec.js @@ -89,12 +89,9 @@ describe('PopoverContent', () => { const Wrapper = () => ( <> 'Content'} /> -
) render() - const container = document.querySelector('.wrapper > [role="dialog"]') - expect(container).not.toBeInTheDocument() const bodyContainer = document.querySelector('body > [role="dialog"]') expect(bodyContainer).toBeInTheDocument() })