Merge pull request #16137 from aitherios/managed-search-modal-14903

feat(search): Don't close dialog on ctrl+click at quick search [Option 2]

.changeset/unlucky-snakes-smile.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search': minor
+---
+
+Search modal auto closes on location change

packages/app/src/components/search/SearchModal.tsx

@@ -186,7 +186,7 @@ export const SearchModal = ({ toggleModal }: { toggleModal: () => void }) => {
             </Grid>
           </Grid>
           <Grid item xs>
-            <SearchResult onClick={toggleModal}>
+            <SearchResult>
               <CatalogSearchResultListItem icon={<CatalogIcon />} />
               <TechDocsSearchResultListItem icon={<DocsIcon />} />
               <ToolSearchResultListItem icon={<BuildIcon />} />

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": [

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<HTMLInputElement | HTMLTextAreaElement>) => {
@@ -187,9 +186,7 @@ export const SearchModal = (props: SearchModalProps) => {
     >
       {open && (
         <SearchContextProvider inheritParentContextIfAvailable>
-          {(children && children({ toggleModal })) ?? (
-            <Modal toggleModal={toggleModal} />
-          )}
+          {(children && children({ toggleModal })) ?? <Modal />}
         </SearchContextProvider>
       )}
     </Dialog>

plugins/search/src/components/SearchModal/useSearchModal.test.tsx

@@ -13,9 +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, Router } from 'react-router-dom';
+import { createMemoryHistory } from 'history';
 
 describe('useSearchModal', () => {
   it.each([
@@ -24,14 +26,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 +53,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 +64,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({
@@ -70,4 +80,21 @@ describe('useSearchModal', () => {
       hidden: true,
     });
   });
+
+  it('should hide when location changes', () => {
+    const history = createMemoryHistory({ initialEntries: ['/'] });
+
+    const rendered = renderHook(() => useSearchModal(true), {
+      wrapper: ({ children }) => (
+        <Router location={history.location} navigator={history}>
+          {children}
+        </Router>
+      ),
+    });
+
+    expect(rendered.result.current.state.hidden).toBe(false);
+    act(() => history.push('/new/path'));
+    rendered.rerender();
+    expect(rendered.result.current.state.hidden).toBe(true);
+  });
 });

plugins/search/src/components/SearchModal/useSearchModal.tsx

@@ -15,10 +15,12 @@
  */
 
 import React, { ReactNode, useCallback, useContext, useState } from 'react';
+import { useLocation } from 'react-router-dom';
 import {
   createVersionedContext,
   createVersionedValueMap,
 } from '@backstage/version-bridge';
+import useUpdateEffect from 'react-use/lib/useUpdateEffect';
 
 /**
  * The state of the search modal, as well as functions for changing the modal's
@@ -96,7 +98,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 +108,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 +131,23 @@ 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 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,
+    }));
+  }, [locationKey]);
+
   // Inherit from parent context, if set.
-  return parentContextValue?.state
+  return isParentContextPresent
     ? parentContextValue
     : { state, toggleModal, setOpen };
 }

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