diff --git a/.changeset/bright-games-eat.md b/.changeset/bright-games-eat.md new file mode 100644 index 0000000000..3bae70283f --- /dev/null +++ b/.changeset/bright-games-eat.md @@ -0,0 +1,24 @@ +--- +'@backstage/create-app': patch +--- + +Remove SearchContextProvider from `` + +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 ... + + } to="/search"> +- +- +- ++ + +``` diff --git a/.changeset/tidy-boxes-sell.md b/.changeset/tidy-boxes-sell.md new file mode 100644 index 0000000000..82ee16af57 --- /dev/null +++ b/.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 ( + + ); +}; +``` diff --git a/docs/features/search/getting-started.md b/docs/features/search/getting-started.md index 285e399cb4..7c3717e22e 100644 --- a/docs/features/search/getting-started.md +++ b/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<{}>) => ( - - - + ... ``` diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index 57f1fa536f..2441ace27b 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/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<{}>) => ( } to="/search"> - - - + }> diff --git a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx b/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx index 7e98c7d3e6..90738c44d4 100644 --- a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx +++ b/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<{}>) => ( } to="/search"> - - - {' '} + }> diff --git a/plugins/search/api-report.md b/plugins/search/api-report.md index a45f02564f..10ce787a6c 100644 --- a/plugins/search/api-report.md +++ b/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; +}; ``` diff --git a/plugins/search/src/components/SearchBar/SearchBar.tsx b/plugins/search/src/components/SearchBar/SearchBar.tsx index 2ddec22294..a203d27c93 100644 --- a/plugins/search/src/components/SearchBar/SearchBar.tsx +++ b/plugins/search/src/components/SearchBar/SearchBar.tsx @@ -169,13 +169,16 @@ export type SearchBarProps = Partial; 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 ; }; diff --git a/plugins/search/src/components/SearchContext/SearchContext.tsx b/plugins/search/src/components/SearchContext/SearchContext.tsx index 336041bf1f..a9f42fe5ca 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.tsx @@ -31,45 +31,41 @@ import { searchApiRef } from '../../apis'; type SearchContextValue = { result: AsyncState; - term: string; setTerm: React.Dispatch>; - types: string[]; setTypes: React.Dispatch>; - filters: JsonObject; setFilters: React.Dispatch>; - open?: boolean; - toggleModal: () => void; - pageCursor?: string; setPageCursor: React.Dispatch>; 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( 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( initialState.pageCursor, @@ -77,11 +73,6 @@ export const SearchContextProvider = ({ const [filters, setFilters] = useState(initialState.filters); const [term, setTerm] = useState(initialState.term); const [types, setTypes] = useState(initialState.types); - const [open, setOpen] = useState(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, diff --git a/plugins/search/src/components/SearchContext/SearchContextForStorybook.stories.tsx b/plugins/search/src/components/SearchContext/SearchContextForStorybook.stories.tsx index e969e0d8cc..36d670a1ed 100644 --- a/plugins/search/src/components/SearchContext/SearchContextForStorybook.stories.tsx +++ b/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 & QueryResultProps, ) => { - const { mockedResults, ...contextProps } = props; - const query: any = () => Promise.resolve(mockedResults || {}); - const apiRegistry = TestApiRegistry.from([searchApiRef, { query }]); - return ( - - - + + + ); }; + +export function SearchApiProvider(props: PropsWithChildren) { + const { mockedResults, children } = props; + const query: any = () => Promise.resolve(mockedResults || {}); + const apiRegistry = TestApiRegistry.from([searchApiRef, { query }]); + return ; +} diff --git a/plugins/search/src/components/SearchContext/index.tsx b/plugins/search/src/components/SearchContext/index.tsx index 682ade916f..8651e661c1 100644 --- a/plugins/search/src/components/SearchContext/index.tsx +++ b/plugins/search/src/components/SearchContext/index.tsx @@ -19,3 +19,5 @@ export { SearchContext, useSearch, } from './SearchContext'; + +export type { SearchContextState } from './SearchContext'; diff --git a/plugins/search/src/components/SearchModal/SearchModal.stories.tsx b/plugins/search/src/components/SearchModal/SearchModal.stories.tsx index e4b89000d5..aa0ad0479a 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.stories.tsx +++ b/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( - + - , + , { mountedRoutes: { '/search': rootRouteRef } }, ), ], }; export const Default = () => { - const { open, toggleModal } = useSearch(); + const { state, toggleModal } = useSearchModal(); return ( <> - + ); }; diff --git a/plugins/search/src/components/SearchModal/SearchModal.test.tsx b/plugins/search/src/components/SearchModal/SearchModal.test.tsx index b59be710de..d3b2a825e2 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.test.tsx +++ b/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( - - - + , { mountedRoutes: { @@ -51,14 +52,13 @@ describe('SearchModal', () => { ); expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(query).toHaveBeenCalledTimes(1); }); it('Calls toggleModal handler', async () => { await renderInTestApp( - - - + , { 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( + + , + { + mountedRoutes: { + '/search': rootRouteRef, + }, + }, + ); + + expect(getByTestId('search-bar-next')).toBeInTheDocument(); + expect(getByTestId('search-bar-next')).not.toBeVisible(); + }); }); diff --git a/plugins/search/src/components/SearchModal/SearchModal.tsx b/plugins/search/src/components/SearchModal/SearchModal.tsx index 1f0a879f96..2a71e53ea8 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.tsx +++ b/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 ( - + <> @@ -139,14 +144,34 @@ export const Modal = ({ open = true, toggleModal }: SearchModalProps) => { - + ); }; -export const SearchModal = ({ open = true, toggleModal }: SearchModalProps) => { +export const SearchModal = ({ + open = true, + hidden, + toggleModal, +}: SearchModalProps) => { + const classes = useStyles(); + return ( - - - + ); }; diff --git a/plugins/search/src/components/SearchModal/index.tsx b/plugins/search/src/components/SearchModal/index.tsx index ec0053951d..646dff0a62 100644 --- a/plugins/search/src/components/SearchModal/index.tsx +++ b/plugins/search/src/components/SearchModal/index.tsx @@ -15,3 +15,4 @@ */ export { SearchModal } from './SearchModal'; export type { SearchModalProps } from './SearchModal'; +export { useSearchModal } from './useSearchModal'; diff --git a/plugins/search/src/components/SearchModal/useSearchModal.test.tsx b/plugins/search/src/components/SearchModal/useSearchModal.test.tsx new file mode 100644 index 0000000000..424dffff76 --- /dev/null +++ b/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, + }); + }); +}); diff --git a/plugins/search/src/components/SearchModal/useSearchModal.tsx b/plugins/search/src/components/SearchModal/useSearchModal.tsx new file mode 100644 index 0000000000..3871687d1a --- /dev/null +++ b/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 }; +} diff --git a/plugins/search/src/components/SearchType/SearchType.Accordion.test.tsx b/plugins/search/src/components/SearchType/SearchType.Accordion.test.tsx index 6b4171a024..1aae64c463 100644 --- a/plugins/search/src/components/SearchType/SearchType.Accordion.test.tsx +++ b/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(), diff --git a/plugins/search/src/components/SearchType/SearchType.Tabs.test.tsx b/plugins/search/src/components/SearchType/SearchType.Tabs.test.tsx index 54f09023bc..741ee6e83f 100644 --- a/plugins/search/src/components/SearchType/SearchType.Tabs.test.tsx +++ b/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(), diff --git a/plugins/search/src/components/SidebarSearchModal/SidebarSearchModal.tsx b/plugins/search/src/components/SidebarSearchModal/SidebarSearchModal.tsx index b8f20de67d..bbf4eef4e4 100644 --- a/plugins/search/src/components/SidebarSearchModal/SidebarSearchModal.tsx +++ b/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} /> - + ); }; diff --git a/plugins/search/src/index.ts b/plugins/search/src/index.ts index 99c3f5177b..cef0355fb4 100644 --- a/plugins/search/src/index.ts +++ b/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';