From 34aebcc5204dbdd73ab3500457b61497ccc5c9a7 Mon Sep 17 00:00:00 2001 From: fcamgz Date: Mon, 13 Oct 2025 16:17:10 -0400 Subject: [PATCH 1/4] Fixed SearchModal not cleaning up body overflow #31402 **Changes:** - Added `isDialogOpen = open && !hidden` logic to properly unmount Dialog component - Removed `hidden` prop from Dialog component and use `isDialogOpen` for both `open` prop and conditional rendering - Added 2 tests verifying Dialog is completely unmounted (not just hidden) when closed #31402 Signed-off-by: fcamgz --- .changeset/orange-friends-march.md | 11 +++++++ .../SearchModal/SearchModal.test.tsx | 32 +++++++++++++++++++ .../components/SearchModal/SearchModal.tsx | 7 ++-- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 .changeset/orange-friends-march.md diff --git a/.changeset/orange-friends-march.md b/.changeset/orange-friends-march.md new file mode 100644 index 0000000000..e2fc659439 --- /dev/null +++ b/.changeset/orange-friends-march.md @@ -0,0 +1,11 @@ +--- +'@backstage/plugin-search': patch +--- + +Fix SearchModal Dialog not unmounting properly on close + +Fixed issue where Dialog was hidden but not unmounted, preventing +Material-UI from removing overflow:hidden from body element. +Added tests to verify Dialog is properly removed from DOM. + +Signed-off-by: fcamgz diff --git a/plugins/search/src/components/SearchModal/SearchModal.test.tsx b/plugins/search/src/components/SearchModal/SearchModal.test.tsx index 8d8e1bb9b9..4629017408 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.test.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.test.tsx @@ -248,4 +248,36 @@ describe('SearchModal', () => { expect(navigate).toHaveBeenCalledWith('/search?query=term'); }); + + it('should completely unmount the Dialog from DOM when open prop is false', async () => { + await renderInTestApp( + + , + { + mountedRoutes: { + '/search': rootRouteRef, + }, + }, + ); + + // Dialog should not exist in the DOM at all (unmounted, not just hidden) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('should completely unmount the Dialog from DOM when hidden prop is true', async () => { + await renderInTestApp( + + , + { + mountedRoutes: { + '/search': rootRouteRef, + }, + }, + ); + + // Dialog should not exist in the DOM at all (unmounted, not just hidden) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); }); diff --git a/plugins/search/src/components/SearchModal/SearchModal.tsx b/plugins/search/src/components/SearchModal/SearchModal.tsx index af7e1c6e10..86fa4f1b73 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.tsx @@ -211,6 +211,8 @@ export const SearchModal = (props: SearchModalProps) => { const classes = useStyles(); + const isDialogOpen = open && !hidden; + return ( { aria-modal="true" fullWidth maxWidth="lg" - open={open} - hidden={hidden} + open={isDialogOpen} > - {open && ( + {isDialogOpen && ( {(children && children({ From d7c7c7e0717984dcbdbc6b161d33ceac0cdc289e Mon Sep 17 00:00:00 2001 From: fcamgz Date: Mon, 13 Oct 2025 17:06:56 -0400 Subject: [PATCH 2/4] Remove outdated SearchModal test that expected hidden Dialog to remain in DOM Removed test that checked for hidden Dialog still being in DOM. The new behavior properly unmounts the Dialog when hidden, so this test is now covered by the new unmounting tests. Signed-off-by: fcamgz --- .../components/SearchModal/SearchModal.test.tsx | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/plugins/search/src/components/SearchModal/SearchModal.test.tsx b/plugins/search/src/components/SearchModal/SearchModal.test.tsx index 4629017408..6a17276d3b 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.test.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.test.tsx @@ -151,22 +151,6 @@ describe('SearchModal', () => { expect(toggleModal).toHaveBeenCalledTimes(1); }); - it('should render SearchModal hiding its content', async () => { - const { getByTestId } = await renderInTestApp( - - , - { - mountedRoutes: { - '/search': rootRouteRef, - }, - }, - ); - - expect(getByTestId('search-bar-next')).toBeInTheDocument(); - expect(getByTestId('search-bar-next')).not.toBeVisible(); - }); - it('should focus on its search bar when opened', async () => { await renderInTestApp( From 13d1483605e8c2b485888647cc1eb8f32d22ada4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 22 Mar 2026 10:45:17 +0100 Subject: [PATCH 3/4] Apply suggestion from @Rugvip Signed-off-by: Patrik Oldsberg --- .changeset/orange-friends-march.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.changeset/orange-friends-march.md b/.changeset/orange-friends-march.md index e2fc659439..929db51666 100644 --- a/.changeset/orange-friends-march.md +++ b/.changeset/orange-friends-march.md @@ -2,10 +2,4 @@ '@backstage/plugin-search': patch --- -Fix SearchModal Dialog not unmounting properly on close - -Fixed issue where Dialog was hidden but not unmounted, preventing -Material-UI from removing overflow:hidden from body element. -Added tests to verify Dialog is properly removed from DOM. - -Signed-off-by: fcamgz +Fixed `SearchModalDialog` breaking the page by not closing correctly. From 86711b1769dc1a3812292f728d73861bb9d919df Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 22 Mar 2026 22:07:02 +0100 Subject: [PATCH 4/4] Update .changeset/orange-friends-march.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Patrik Oldsberg --- .changeset/orange-friends-march.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/orange-friends-march.md b/.changeset/orange-friends-march.md index 929db51666..d3813063e2 100644 --- a/.changeset/orange-friends-march.md +++ b/.changeset/orange-friends-march.md @@ -2,4 +2,4 @@ '@backstage/plugin-search': patch --- -Fixed `SearchModalDialog` breaking the page by not closing correctly. +Fixed the `SearchModal` leaving the page in a broken state by not restoring body overflow and aria-hidden attributes when closing.