From 9dbdcccd200eabe3a8f5237443c6850ea28779d4 Mon Sep 17 00:00:00 2001 From: Renan Mendes Carvalho Date: Wed, 21 Dec 2022 14:56:19 +0100 Subject: [PATCH 1/6] feature(SearchModal): Auto close the search modal upon route change Signed-off-by: Renan Mendes Carvalho --- .../app/src/components/search/SearchModal.tsx | 2 +- .../components/SearchModal/SearchModal.tsx | 7 ++-- .../components/SearchModal/useSearchModal.tsx | 42 +++++++++++++++---- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/app/src/components/search/SearchModal.tsx b/packages/app/src/components/search/SearchModal.tsx index ea5a156c55..fea80f9952 100644 --- a/packages/app/src/components/search/SearchModal.tsx +++ b/packages/app/src/components/search/SearchModal.tsx @@ -186,7 +186,7 @@ export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => { - + } /> } /> } /> diff --git a/plugins/search/src/components/SearchModal/SearchModal.tsx b/plugins/search/src/components/SearchModal/SearchModal.tsx index c32c9a5cc3..8035484333 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.tsx @@ -92,7 +92,7 @@ const useStyles = makeStyles(theme => ({ viewResultsLink: { verticalAlign: '0.5em' }, })); -export const Modal = ({ toggleModal }: SearchModalProps) => { +export const Modal = () => { const classes = useStyles(); const navigate = useNavigate(); const { transitions } = useTheme(); @@ -107,9 +107,8 @@ export const Modal = ({ toggleModal }: SearchModalProps) => { }); const handleSearchResultClick = useCallback(() => { - toggleModal(); setTimeout(focusContent, transitions.duration.leavingScreen); - }, [toggleModal, focusContent, transitions]); + }, [focusContent, transitions]); const handleSearchBarKeyDown = useCallback( (e: KeyboardEvent) => { @@ -188,7 +187,7 @@ export const SearchModal = (props: SearchModalProps) => { {open && ( {(children && children({ toggleModal })) ?? ( - + )} )} diff --git a/plugins/search/src/components/SearchModal/useSearchModal.tsx b/plugins/search/src/components/SearchModal/useSearchModal.tsx index db01765f0e..91a2c38bfa 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.tsx @@ -14,7 +14,14 @@ * limitations under the License. */ -import React, { ReactNode, useCallback, useContext, useState } from 'react'; +import React, { + ReactNode, + useCallback, + useContext, + useState, + useEffect, +} from 'react'; +import { useLocation } from 'react-router-dom'; import { createVersionedContext, createVersionedValueMap, @@ -96,7 +103,8 @@ export const SearchModalProvider = (props: SearchModalProviderProps) => { /** * Use this hook to manage the state of {@link SearchModal} - * and change its visibility. + * and change its visibility. Monitors route changes setting the hidden state + * to avoid having to call toggleModal on every result click. * * @public * @@ -105,10 +113,6 @@ export const SearchModalProvider = (props: SearchModalProviderProps) => { * functions for changing the visibility of the modal. */ export function useSearchModal(initialState = false) { - // Check for any existing parent context. - const parentContext = useContext(SearchModalContext); - const parentContextValue = parentContext?.atVersion(1); - const [state, setState] = useState({ hidden: !initialState, open: initialState, @@ -132,8 +136,32 @@ export function useSearchModal(initialState = false) { [], ); + // Check for any existing parent context. + const parentContext = useContext(SearchModalContext); + const parentContextValue = parentContext?.atVersion(1); + const isParentContextPresent = !!parentContextValue?.state; + + // Monitor route pathname changes to automatically hide the modal. + const { pathname } = useLocation(); + const [openedPathname, setOpenedPathname] = useState(null); + const { open, hidden } = state; + const isModalOpened = !isParentContextPresent && open && !hidden; + useEffect(() => { + if (isModalOpened) { + setOpenedPathname(pathname); + } + }, [isModalOpened, pathname]); + useEffect(() => { + if (isModalOpened && openedPathname && openedPathname !== pathname) { + setState(prevState => ({ + open: prevState.open, + hidden: true, + })); + } + }, [isModalOpened, openedPathname, pathname]); + // Inherit from parent context, if set. - return parentContextValue?.state + return isParentContextPresent ? parentContextValue : { state, toggleModal, setOpen }; } From 4c916d51875ad2522470fdb7a2b669ec5ff8b7a2 Mon Sep 17 00:00:00 2001 From: Renan Mendes Carvalho Date: Thu, 9 Mar 2023 16:21:59 +0100 Subject: [PATCH 2/6] feature(SearchModal): Simplify solution Co-authored-by: Harry Hogg Signed-off-by: Renan Mendes Carvalho --- .../SearchModal/useSearchModal.test.tsx | 17 ++++++++++---- .../components/SearchModal/useSearchModal.tsx | 23 ++++++------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/plugins/search/src/components/SearchModal/useSearchModal.test.tsx b/plugins/search/src/components/SearchModal/useSearchModal.test.tsx index 424dffff76..26c40d87b9 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.test.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.test.tsx @@ -16,6 +16,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; import { useSearchModal } from './useSearchModal'; +import { BrowserRouter } from 'react-router-dom'; describe('useSearchModal', () => { it.each([ @@ -24,14 +25,18 @@ describe('useSearchModal', () => { ])( 'should return the correct state when initial state is %s', (initialState, result) => { - const rendered = renderHook(() => useSearchModal(initialState)); + const rendered = renderHook(() => useSearchModal(initialState), { + wrapper: BrowserRouter, + }); expect(rendered.result.current.state).toEqual(result); }, ); it('should keep open forever to true once modal is toggled', () => { - const rendered = renderHook(() => useSearchModal()); + const rendered = renderHook(() => useSearchModal(), { + wrapper: BrowserRouter, + }); act(() => rendered.result.current.toggleModal()); expect(rendered.result.current.state).toEqual({ @@ -47,7 +52,9 @@ describe('useSearchModal', () => { }); it('should keep open to false if setOpen(false) is invoked on an initially closed modal', () => { - const rendered = renderHook(() => useSearchModal()); + const rendered = renderHook(() => useSearchModal(), { + wrapper: BrowserRouter, + }); act(() => rendered.result.current.setOpen(false)); expect(rendered.result.current.state).toEqual({ open: false, @@ -56,7 +63,9 @@ describe('useSearchModal', () => { }); it('should keep open forever to true even when the modal transition from opened to closed', () => { - const rendered = renderHook(() => useSearchModal()); + const rendered = renderHook(() => useSearchModal(), { + wrapper: BrowserRouter, + }); act(() => rendered.result.current.setOpen(true)); expect(rendered.result.current.state).toEqual({ diff --git a/plugins/search/src/components/SearchModal/useSearchModal.tsx b/plugins/search/src/components/SearchModal/useSearchModal.tsx index 91a2c38bfa..5298768700 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.tsx @@ -26,6 +26,7 @@ import { createVersionedContext, createVersionedValueMap, } from '@backstage/version-bridge'; +import { useUpdateEffect } from 'react-use'; /** * The state of the search modal, as well as functions for changing the modal's @@ -143,22 +144,12 @@ export function useSearchModal(initialState = false) { // Monitor route pathname changes to automatically hide the modal. const { pathname } = useLocation(); - const [openedPathname, setOpenedPathname] = useState(null); - const { open, hidden } = state; - const isModalOpened = !isParentContextPresent && open && !hidden; - useEffect(() => { - if (isModalOpened) { - setOpenedPathname(pathname); - } - }, [isModalOpened, pathname]); - useEffect(() => { - if (isModalOpened && openedPathname && openedPathname !== pathname) { - setState(prevState => ({ - open: prevState.open, - hidden: true, - })); - } - }, [isModalOpened, openedPathname, pathname]); + useUpdateEffect(() => { + setState(prevState => ({ + open: prevState.open, + hidden: true, + })); + }, [pathname]); // Inherit from parent context, if set. return isParentContextPresent From 3b9e96c536d3640f72d1f3779bfa89c6cecfcf0e Mon Sep 17 00:00:00 2001 From: Renan Mendes Carvalho Date: Thu, 9 Mar 2023 16:30:19 +0100 Subject: [PATCH 3/6] feature(SearchModal): Reverting Modal component toggleModal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Signed-off-by: Renan Mendes Carvalho --- plugins/search/src/components/SearchModal/SearchModal.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/search/src/components/SearchModal/SearchModal.tsx b/plugins/search/src/components/SearchModal/SearchModal.tsx index 8035484333..3513eec65d 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.tsx @@ -186,9 +186,7 @@ export const SearchModal = (props: SearchModalProps) => { > {open && ( - {(children && children({ toggleModal })) ?? ( - - )} + {(children && children({ toggleModal })) ?? } )} From 430b9f28901eef2c3152f7ad185a363980acc636 Mon Sep 17 00:00:00 2001 From: Renan Mendes Carvalho Date: Fri, 10 Mar 2023 16:23:54 +0100 Subject: [PATCH 4/6] test(useSearchModal): Add new test case for location change Signed-off-by: Renan Mendes Carvalho --- .../SearchModal/useSearchModal.test.tsx | 22 +++++++++++++++++-- .../components/SearchModal/useSearchModal.tsx | 8 +------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/plugins/search/src/components/SearchModal/useSearchModal.test.tsx b/plugins/search/src/components/SearchModal/useSearchModal.test.tsx index 26c40d87b9..0de41e3589 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.test.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.test.tsx @@ -13,10 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +import React from 'react'; import { act, renderHook } from '@testing-library/react-hooks'; import { useSearchModal } from './useSearchModal'; -import { BrowserRouter } from 'react-router-dom'; +import { BrowserRouter, Router } from 'react-router-dom'; +import { createMemoryHistory } from 'history'; describe('useSearchModal', () => { it.each([ @@ -79,4 +80,21 @@ describe('useSearchModal', () => { hidden: true, }); }); + + it('should hide when location changes', () => { + const history = createMemoryHistory({ initialEntries: ['/'] }); + + const rendered = renderHook(() => useSearchModal(true), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + expect(rendered.result.current.state.hidden).toBe(false); + act(() => history.push('/new/path')); + rendered.rerender(); + expect(rendered.result.current.state.hidden).toBe(true); + }); }); diff --git a/plugins/search/src/components/SearchModal/useSearchModal.tsx b/plugins/search/src/components/SearchModal/useSearchModal.tsx index 5298768700..0700898c0b 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.tsx @@ -14,13 +14,7 @@ * limitations under the License. */ -import React, { - ReactNode, - useCallback, - useContext, - useState, - useEffect, -} from 'react'; +import React, { ReactNode, useCallback, useContext, useState } from 'react'; import { useLocation } from 'react-router-dom'; import { createVersionedContext, From d6b73b0380da35ce09e79ec28663a5654e048d44 Mon Sep 17 00:00:00 2001 From: Renan Mendes Carvalho Date: Fri, 10 Mar 2023 16:39:38 +0100 Subject: [PATCH 5/6] changeset(plugin-search): Search modal auto closes on location change Signed-off-by: Renan Mendes Carvalho --- .changeset/unlucky-snakes-smile.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/unlucky-snakes-smile.md diff --git a/.changeset/unlucky-snakes-smile.md b/.changeset/unlucky-snakes-smile.md new file mode 100644 index 0000000000..ec0f9bf947 --- /dev/null +++ b/.changeset/unlucky-snakes-smile.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search': minor +--- + +Search modal auto closes on location change From 24e3daa6dcf9c7fb2fbe1c0938c753878040ffbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 3 Apr 2023 12:10:44 +0200 Subject: [PATCH 6/6] fixup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/search/package.json | 1 + .../search/src/components/SearchModal/useSearchModal.tsx | 9 +++++---- yarn.lock | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/search/package.json b/plugins/search/package.json index d4367778b7..7616a03ffd 100644 --- a/plugins/search/package.json +++ b/plugins/search/package.json @@ -67,6 +67,7 @@ "@testing-library/user-event": "^14.0.0", "@types/node": "^16.11.26", "cross-fetch": "^3.1.5", + "history": "^5.0.0", "msw": "^1.0.0" }, "files": [ diff --git a/plugins/search/src/components/SearchModal/useSearchModal.tsx b/plugins/search/src/components/SearchModal/useSearchModal.tsx index 0700898c0b..279484bd47 100644 --- a/plugins/search/src/components/SearchModal/useSearchModal.tsx +++ b/plugins/search/src/components/SearchModal/useSearchModal.tsx @@ -20,7 +20,7 @@ import { createVersionedContext, createVersionedValueMap, } from '@backstage/version-bridge'; -import { useUpdateEffect } from 'react-use'; +import useUpdateEffect from 'react-use/lib/useUpdateEffect'; /** * The state of the search modal, as well as functions for changing the modal's @@ -136,14 +136,15 @@ export function useSearchModal(initialState = false) { const parentContextValue = parentContext?.atVersion(1); const isParentContextPresent = !!parentContextValue?.state; - // Monitor route pathname changes to automatically hide the modal. - const { pathname } = useLocation(); + // Monitor route changes to automatically hide the modal. + const location = useLocation(); + const locationKey = `${location.pathname}${location.search}${location.hash}`; useUpdateEffect(() => { setState(prevState => ({ open: prevState.open, hidden: true, })); - }, [pathname]); + }, [locationKey]); // Inherit from parent context, if set. return isParentContextPresent diff --git a/yarn.lock b/yarn.lock index a70bcecc97..beb214edda 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8450,6 +8450,7 @@ __metadata: "@types/node": ^16.11.26 "@types/react": ^16.13.1 || ^17.0.0 cross-fetch: ^3.1.5 + history: ^5.0.0 msw: ^1.0.0 qs: ^6.9.4 react-use: ^17.2.4