Merge pull request #9364 from backstage/vinzscam/search-modal-fetch-on-open

SearchModal invoke search only when visible

.changeset/bright-games-eat.md

@@ -0,0 +1,24 @@
+---
+'@backstage/create-app': patch
+---
+
+Remove SearchContextProvider from `<Root />`
+
+The `SidebarSearchModal` exported from `plugin-search` internally renders `SearchContextProvider`, so it can be removed from `Root.tsx`:
+
+```diff
+-import {
+-  SidebarSearchModal,
+-  SearchContextProvider,
+-} from '@backstage/plugin-search';
++import { SidebarSearchModal } from '@backstage/plugin-search';
+
+... omitted ...
+
+       <SidebarGroup label="Search" icon={<SearchIcon />} to="/search">
+-        <SearchContextProvider>
+-          <SidebarSearchModal />
+-        </SearchContextProvider>
++        <SidebarSearchModal />
+       </SidebarGroup>
+```

.changeset/tidy-boxes-sell.md

@@ -0,0 +1,18 @@
+---
+'@backstage/plugin-search': minor
+---
+
+**BREAKING**: `useSearch` doesn't return anymore `open` and `toggleModal`.
+The two properties have been moved to the `useSearchModal` hook.
+
+```
+import { SearchModal, useSearchModal } from '@backstage/plugin-search';
+
+const Foo = () => {
+  const { state, setOpen, toggleModal } = useSearchModal();
+
+  return (
+    <SearchModal {...state} toggleModal={toggleModal} />
+  );
+};
+```

docs/features/search/getting-started.md

@@ -114,15 +114,13 @@ const routes = (
 In `Root.tsx`, add the `SidebarSearchModal` component:
 
 ```bash
-import { SidebarSearchModal, SearchContextProvider } from '@backstage/plugin-search';
+import { SidebarSearchModal } from '@backstage/plugin-search';
 
 export const Root = ({ children }: PropsWithChildren<{}>) => (
   <SidebarPage>
     <Sidebar>
       <SidebarLogo />
-      <SearchContextProvider>
-        <SidebarSearchModal />
-      </SearchContextProvider>
+      <SidebarSearchModal />
       <SidebarDivider />
 ...
 ```

packages/app/src/components/Root/Root.tsx

@@ -34,10 +34,7 @@ import {
   Settings as SidebarSettings,
   UserSettingsSignInAvatar,
 } from '@backstage/plugin-user-settings';
-import {
-  SidebarSearchModal,
-  SearchContextProvider,
-} from '@backstage/plugin-search';
+import { SidebarSearchModal } from '@backstage/plugin-search';
 import { Shortcuts } from '@backstage/plugin-shortcuts';
 import {
   Sidebar,
@@ -89,9 +86,7 @@ export const Root = ({ children }: PropsWithChildren<{}>) => (
     <Sidebar>
       <SidebarLogo />
       <SidebarGroup label="Search" icon={<SearchIcon />} to="/search">
-        <SearchContextProvider>
-          <SidebarSearchModal />
-        </SearchContextProvider>
+        <SidebarSearchModal />
       </SidebarGroup>
       <SidebarDivider />
       <SidebarGroup label="Menu" icon={<MenuIcon />}>

packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx

@@ -28,10 +28,7 @@ import {
   Settings as SidebarSettings,
   UserSettingsSignInAvatar,
 } from '@backstage/plugin-user-settings';
-import {
-  SidebarSearchModal,
-  SearchContextProvider,
-} from '@backstage/plugin-search';
+import { SidebarSearchModal } from '@backstage/plugin-search';
 import {
   Sidebar,
   sidebarConfig,
@@ -84,9 +81,7 @@ export const Root = ({ children }: PropsWithChildren<{}>) => (
     <Sidebar>
       <SidebarLogo />
       <SidebarGroup label="Search" icon={<SearchIcon />} to="/search">
-        <SearchContextProvider>
-          <SidebarSearchModal />
-        </SearchContextProvider>{' '}
+        <SidebarSearchModal />
       </SidebarGroup>
       <SidebarDivider />
       <SidebarGroup label="Menu" icon={<MenuIcon />}>

plugins/search/api-report.md

@@ -145,9 +145,17 @@ export const SearchContextProvider: ({
   initialState,
   children,
 }: React_2.PropsWithChildren<{
-  initialState?: SettableSearchContext | undefined;
+  initialState?: SearchContextState | undefined;
 }>) => JSX.Element;
 
+// @public
+export type SearchContextState = {
+  term: string;
+  types: string[];
+  filters: JsonObject;
+  pageCursor?: string;
+};
+
 // Warning: (ae-missing-release-tag) "SearchFilter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public (undocumented)
@@ -201,6 +209,7 @@ export type SearchFilterWrapperProps = SearchFilterComponentProps & {
 // @public (undocumented)
 export const SearchModal: ({
   open,
+  hidden,
   toggleModal,
 }: SearchModalProps) => JSX.Element;
 
@@ -208,9 +217,8 @@ export const SearchModal: ({
 //
 // @public (undocumented)
 export interface SearchModalProps {
-  // (undocumented)
+  hidden?: boolean;
   open?: boolean;
-  // (undocumented)
   toggleModal: () => void;
 }
 
@@ -320,7 +328,13 @@ export type SidebarSearchProps = {
 // @public (undocumented)
 export const useSearch: () => SearchContextValue;
 
-// Warnings were encountered during analysis:
-//
-// src/components/SearchContext/SearchContext.d.ts:23:5 - (ae-forgotten-export) The symbol "SettableSearchContext" needs to be exported by the entry point index.d.ts
+// @public
+export function useSearchModal(initialState?: boolean): {
+  state: {
+    hidden: boolean;
+    open: boolean;
+  };
+  toggleModal: () => void;
+  setOpen: (open: boolean) => void;
+};
 ```

plugins/search/src/components/SearchBar/SearchBar.tsx

@@ -169,13 +169,16 @@ export type SearchBarProps = Partial<SearchBarBaseProps>;
 export const SearchBar = ({ onChange, ...props }: SearchBarProps) => {
   const { term, setTerm } = useSearch();
 
-  const handleChange = (newValue: string) => {
-    if (onChange) {
-      onChange(newValue);
-    } else {
-      setTerm(newValue);
-    }
-  };
+  const handleChange = useCallback(
+    (newValue: string) => {
+      if (onChange) {
+        onChange(newValue);
+      } else {
+        setTerm(newValue);
+      }
+    },
+    [onChange, setTerm],
+  );
 
   return <SearchBarBase value={term} onChange={handleChange} {...props} />;
 };

plugins/search/src/components/SearchContext/SearchContext.tsx

@@ -31,45 +31,41 @@ import { searchApiRef } from '../../apis';
 
 type SearchContextValue = {
   result: AsyncState<SearchResultSet>;
-  term: string;
   setTerm: React.Dispatch<React.SetStateAction<string>>;
-  types: string[];
   setTypes: React.Dispatch<React.SetStateAction<string[]>>;
-  filters: JsonObject;
   setFilters: React.Dispatch<React.SetStateAction<JsonObject>>;
-  open?: boolean;
-  toggleModal: () => void;
-  pageCursor?: string;
   setPageCursor: React.Dispatch<React.SetStateAction<string | undefined>>;
   fetchNextPage?: React.DispatchWithoutAction;
   fetchPreviousPage?: React.DispatchWithoutAction;
-};
+} & SearchContextState;
 
-type SettableSearchContext = Omit<
-  SearchContextValue,
-  | 'result'
-  | 'setTerm'
-  | 'setTypes'
-  | 'setFilters'
-  | 'toggleModal'
-  | 'setPageCursor'
-  | 'fetchNextPage'
-  | 'fetchPreviousPage'
->;
+/**
+ * The initial state of `SearchContextProvider`.
+ *
+ * @public
+ */
+export type SearchContextState = {
+  term: string;
+  types: string[];
+  filters: JsonObject;
+  pageCursor?: string;
+};
 
 export const SearchContext = createContext<SearchContextValue | undefined>(
   undefined,
 );
 
+const searchInitialState: SearchContextState = {
+  term: '',
+  pageCursor: undefined,
+  filters: {},
+  types: [],
+};
+
 export const SearchContextProvider = ({
-  initialState = {
-    term: '',
-    pageCursor: undefined,
-    filters: {},
-    types: [],
-  },
+  initialState = searchInitialState,
   children,
-}: PropsWithChildren<{ initialState?: SettableSearchContext }>) => {
+}: PropsWithChildren<{ initialState?: SearchContextState }>) => {
   const searchApi = useApi(searchApiRef);
   const [pageCursor, setPageCursor] = useState<string | undefined>(
     initialState.pageCursor,
@@ -77,11 +73,6 @@ export const SearchContextProvider = ({
   const [filters, setFilters] = useState<JsonObject>(initialState.filters);
   const [term, setTerm] = useState<string>(initialState.term);
   const [types, setTypes] = useState<string[]>(initialState.types);
-  const [open, setOpen] = useState<boolean>(false);
-  const toggleModal = useCallback(
-    (): void => setOpen(prevState => !prevState),
-    [],
-  );
 
   const prevTerm = usePrevious(term);
 
@@ -90,7 +81,7 @@ export const SearchContextProvider = ({
       searchApi.query({
         term,
         filters,
-        pageCursor: pageCursor,
+        pageCursor,
         types,
       }),
     [term, filters, types, pageCursor],
@@ -118,8 +109,6 @@ export const SearchContextProvider = ({
     result,
     filters,
     setFilters,
-    open,
-    toggleModal,
     term,
     setTerm,
     types,

plugins/search/src/components/SearchContext/SearchContextForStorybook.stories.tsx

@@ -16,7 +16,7 @@
 import { ApiProvider } from '@backstage/core-app-api';
 import { SearchResultSet } from '@backstage/search-common';
 import { TestApiRegistry } from '@backstage/test-utils';
-import React, { ComponentProps } from 'react';
+import React, { ComponentProps, PropsWithChildren } from 'react';
 import { searchApiRef } from '../../apis';
 import { SearchContextProvider as RealSearchContextProvider } from './SearchContext';
 
@@ -33,13 +33,16 @@ type QueryResultProps = {
 export const SearchContextProvider = (
   props: ComponentProps<typeof RealSearchContextProvider> & QueryResultProps,
 ) => {
-  const { mockedResults, ...contextProps } = props;
-  const query: any = () => Promise.resolve(mockedResults || {});
-  const apiRegistry = TestApiRegistry.from([searchApiRef, { query }]);
-
   return (
-    <ApiProvider apis={apiRegistry}>
-      <RealSearchContextProvider {...contextProps} />
-    </ApiProvider>
+    <SearchApiProvider {...props}>
+      <RealSearchContextProvider children={props.children} />
+    </SearchApiProvider>
   );
 };
+
+export function SearchApiProvider(props: PropsWithChildren<QueryResultProps>) {
+  const { mockedResults, children } = props;
+  const query: any = () => Promise.resolve(mockedResults || {});
+  const apiRegistry = TestApiRegistry.from([searchApiRef, { query }]);
+  return <ApiProvider apis={apiRegistry} children={children} />;
+}

plugins/search/src/components/SearchContext/index.tsx

@@ -19,3 +19,5 @@ export {
   SearchContext,
   useSearch,
 } from './SearchContext';
+
+export type { SearchContextState } from './SearchContext';

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

@@ -18,9 +18,9 @@ import { wrapInTestApp } from '@backstage/test-utils';
 import { Button } from '@material-ui/core';
 import React, { ComponentType } from 'react';
 import { rootRouteRef } from '../../plugin';
-import { useSearch } from '../SearchContext';
-import { SearchContextProvider } from '../SearchContext/SearchContextForStorybook.stories';
+import { SearchApiProvider } from '../SearchContext/SearchContextForStorybook.stories';
 import { SearchModal } from './SearchModal';
+import { useSearchModal } from './useSearchModal';
 
 const mockResults = {
   results: [
@@ -57,23 +57,23 @@ export default {
   decorators: [
     (Story: ComponentType<{}>) =>
       wrapInTestApp(
-        <SearchContextProvider mockedResults={mockResults}>
+        <SearchApiProvider mockedResults={mockResults}>
           <Story />
-        </SearchContextProvider>,
+        </SearchApiProvider>,
         { mountedRoutes: { '/search': rootRouteRef } },
       ),
   ],
 };
 
 export const Default = () => {
-  const { open, toggleModal } = useSearch();
+  const { state, toggleModal } = useSearchModal();
 
   return (
     <>
       <Button variant="contained" color="primary" onClick={toggleModal}>
         Toggle Search Modal
       </Button>
-      <SearchModal open={open} toggleModal={toggleModal} />
+      <SearchModal {...state} toggleModal={toggleModal} />
     </>
   );
 };

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

@@ -24,7 +24,6 @@ import { rootRouteRef } from '../../plugin';
 import { searchApiRef } from '../../apis';
 
 import { SearchModal } from './SearchModal';
-import { SearchContextProvider } from '../SearchContext';
 
 describe('SearchModal', () => {
   const query = jest.fn().mockResolvedValue({ results: [] });
@@ -34,14 +33,16 @@ describe('SearchModal', () => {
     [searchApiRef, { query }],
   );
 
+  beforeEach(() => {
+    query.mockClear();
+  });
+
   const toggleModal = jest.fn();
 
   it('Should render the Modal correctly', async () => {
     await renderInTestApp(
       <ApiProvider apis={apiRegistry}>
-        <SearchContextProvider>
-          <SearchModal open toggleModal={toggleModal} />
-        </SearchContextProvider>
+        <SearchModal open hidden={false} toggleModal={toggleModal} />
       </ApiProvider>,
       {
         mountedRoutes: {
@@ -51,14 +52,13 @@ describe('SearchModal', () => {
     );
 
     expect(screen.getByRole('dialog')).toBeInTheDocument();
+    expect(query).toHaveBeenCalledTimes(1);
   });
 
   it('Calls toggleModal handler', async () => {
     await renderInTestApp(
       <ApiProvider apis={apiRegistry}>
-        <SearchContextProvider>
-          <SearchModal open toggleModal={toggleModal} />
-        </SearchContextProvider>
+        <SearchModal open toggleModal={toggleModal} />
       </ApiProvider>,
       {
         mountedRoutes: {
@@ -66,7 +66,25 @@ describe('SearchModal', () => {
         },
       },
     );
+
+    expect(query).toHaveBeenCalledTimes(1);
     userEvent.keyboard('{esc}');
     expect(toggleModal).toHaveBeenCalledTimes(1);
   });
+
+  it('should render SearchModal hiding its content', async () => {
+    const { getByTestId } = await renderInTestApp(
+      <ApiProvider apis={apiRegistry}>
+        <SearchModal open hidden toggleModal={toggleModal} />
+      </ApiProvider>,
+      {
+        mountedRoutes: {
+          '/search': rootRouteRef,
+        },
+      },
+    );
+
+    expect(getByTestId('search-bar-next')).toBeInTheDocument();
+    expect(getByTestId('search-bar-next')).not.toBeVisible();
+  });
 });

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

@@ -38,7 +38,21 @@ import { Link, useContent } from '@backstage/core-components';
 import { rootRouteRef } from '../../plugin';
 
 export interface SearchModalProps {
+  /**
+   * If true, it renders the modal.
+   */
   open?: boolean;
+  /**
+   * This is supposed to be used together with the open prop.
+   * If `hidden` is true, it hides the modal.
+   * If `open` is false, the value of `hidden` has no effect on the modal.
+   * Use `open` for controlling whether the modal should be rendered or not.
+   */
+  hidden?: boolean;
+  /**
+   * a function invoked when a search item is pressed or when the dialog
+   * should be closed.
+   */
   toggleModal: () => void;
 }
 
@@ -57,7 +71,7 @@ const useStyles = makeStyles(theme => ({
   viewResultsLink: { verticalAlign: '0.5em' },
 }));
 
-export const Modal = ({ open = true, toggleModal }: SearchModalProps) => {
+export const Modal = ({ toggleModal }: SearchModalProps) => {
   const getSearchLink = useRouteRef(rootRouteRef);
   const classes = useStyles();
 
@@ -75,16 +89,7 @@ export const Modal = ({ open = true, toggleModal }: SearchModalProps) => {
   };
 
   return (
-    <Dialog
-      classes={{
-        paperFullWidth: classes.paperFullWidth,
-      }}
-      onClose={toggleModal}
-      aria-labelledby="search-modal-title"
-      open={open}
-      fullWidth
-      maxWidth="lg"
-    >
+    <>
       <DialogTitle>
         <Paper className={classes.container}>
           <SearchBar className={classes.input} />
@@ -139,14 +144,34 @@ export const Modal = ({ open = true, toggleModal }: SearchModalProps) => {
           </Grid>
         </Grid>
       </DialogActions>
-    </Dialog>
+    </>
   );
 };
 
-export const SearchModal = ({ open = true, toggleModal }: SearchModalProps) => {
+export const SearchModal = ({
+  open = true,
+  hidden,
+  toggleModal,
+}: SearchModalProps) => {
+  const classes = useStyles();
+
   return (
-    <SearchContextProvider>
-      <Modal open={open} toggleModal={toggleModal} />
-    </SearchContextProvider>
+    <Dialog
+      classes={{
+        paperFullWidth: classes.paperFullWidth,
+      }}
+      onClose={toggleModal}
+      aria-labelledby="search-modal-title"
+      fullWidth
+      maxWidth="lg"
+      open={open}
+      hidden={hidden}
+    >
+      {open && (
+        <SearchContextProvider>
+          <Modal toggleModal={toggleModal} />
+        </SearchContextProvider>
+      )}
+    </Dialog>
   );
 };

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

@@ -15,3 +15,4 @@
  */
 export { SearchModal } from './SearchModal';
 export type { SearchModalProps } from './SearchModal';
+export { useSearchModal } from './useSearchModal';

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

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2022 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { act, renderHook } from '@testing-library/react-hooks';
+import { useSearchModal } from './useSearchModal';
+
+describe('useSearchModal', () => {
+  it.each([
+    [true, { open: true, hidden: false }],
+    [false, { open: false, hidden: true }],
+  ])(
+    'should return the correct state when initial state is %s',
+    (initialState, result) => {
+      const rendered = renderHook(() => useSearchModal(initialState));
+
+      expect(rendered.result.current.state).toEqual(result);
+    },
+  );
+
+  it('should keep open forever to true once modal is toggled', () => {
+    const rendered = renderHook(() => useSearchModal());
+    act(() => rendered.result.current.toggleModal());
+
+    expect(rendered.result.current.state).toEqual({
+      open: true,
+      hidden: false,
+    });
+
+    act(() => rendered.result.current.toggleModal());
+    expect(rendered.result.current.state).toEqual({
+      open: true,
+      hidden: true,
+    });
+  });
+
+  it('should keep open to false if setOpen(false) is invoked on an initially closed modal', () => {
+    const rendered = renderHook(() => useSearchModal());
+    act(() => rendered.result.current.setOpen(false));
+    expect(rendered.result.current.state).toEqual({
+      open: false,
+      hidden: true,
+    });
+  });
+
+  it('should keep open forever to true even when the modal transition from opened to closed', () => {
+    const rendered = renderHook(() => useSearchModal());
+
+    act(() => rendered.result.current.setOpen(true));
+    expect(rendered.result.current.state).toEqual({
+      open: true,
+      hidden: false,
+    });
+
+    act(() => rendered.result.current.setOpen(false));
+    expect(rendered.result.current.state).toEqual({
+      open: true,
+      hidden: true,
+    });
+  });
+});

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

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2022 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { useCallback, useState } from 'react';
+
+/**
+ * Use this hook to manage the state of {@link SearchModal}
+ * and change its visibility.
+ *
+ * @public
+ *
+ * @param initialState - pass `true` to make the modal initially visible
+ * @returns an object containing the state of the modal together with
+ * functions for changing the visibility of the modal.
+ */
+export function useSearchModal(initialState = false) {
+  const [state, setState] = useState({
+    hidden: !initialState,
+    open: initialState,
+  });
+
+  const toggleModal = useCallback(
+    () =>
+      setState(prevState => ({
+        open: true,
+        hidden: !prevState.hidden,
+      })),
+    [],
+  );
+
+  const setOpen = useCallback(
+    (open: boolean) =>
+      setState(prevState => ({
+        open: prevState.open || open,
+        hidden: !open,
+      })),
+    [],
+  );
+
+  return { state, toggleModal, setOpen };
+}

plugins/search/src/components/SearchType/SearchType.Accordion.test.tsx

@@ -33,7 +33,6 @@ describe('SearchType.Accordion', () => {
     term: '',
     types: [],
     filters: {},
-    toggleModal: jest.fn(),
     setTerm: jest.fn(),
     setTypes: jest.fn(),
     setFilters: jest.fn(),

plugins/search/src/components/SearchType/SearchType.Tabs.test.tsx

@@ -33,7 +33,6 @@ describe('SearchType.Tabs', () => {
     term: '',
     types: [],
     filters: {},
-    toggleModal: jest.fn(),
     setTerm: jest.fn(),
     setTypes: jest.fn(),
     setFilters: jest.fn(),

plugins/search/src/components/SidebarSearchModal/SidebarSearchModal.tsx

@@ -17,15 +17,14 @@ import React from 'react';
 import SearchIcon from '@material-ui/icons/Search';
 import { SidebarItem } from '@backstage/core-components';
 import { IconComponent } from '@backstage/core-plugin-api';
-import { SearchModal } from '../SearchModal';
-import { useSearch } from '../SearchContext';
+import { SearchModal, useSearchModal } from '../SearchModal';
 
 export type SidebarSearchModalProps = {
   icon?: IconComponent;
 };
 
 export const SidebarSearchModal = (props: SidebarSearchModalProps) => {
-  const { open, toggleModal } = useSearch();
+  const { state, toggleModal } = useSearchModal();
   const Icon = props.icon ? props.icon : SearchIcon;
 
   return (
@@ -36,7 +35,7 @@ export const SidebarSearchModal = (props: SidebarSearchModalProps) => {
         text="Search"
         onClick={toggleModal}
       />
-      <SearchModal open={open} toggleModal={toggleModal} />
+      <SearchModal {...state} toggleModal={toggleModal} />
     </>
   );
 };

plugins/search/src/index.ts

@@ -32,13 +32,14 @@ export type {
   SearchBarProps,
 } from './components/SearchBar';
 export { SearchContextProvider, useSearch } from './components/SearchContext';
+export type { SearchContextState } from './components/SearchContext';
 export { SearchFilter, SearchFilterNext } from './components/SearchFilter';
 export type {
   SearchAutocompleteFilterProps,
   SearchFilterComponentProps,
   SearchFilterWrapperProps,
 } from './components/SearchFilter';
-export { SearchModal } from './components/SearchModal';
+export { SearchModal, useSearchModal } from './components/SearchModal';
 export type { SearchModalProps } from './components/SearchModal';
 export { SearchPage as Router } from './components/SearchPage';
 export { SearchResultPager } from './components/SearchResultPager';