diff --git a/.changeset/wild-olives-applaud.md b/.changeset/wild-olives-applaud.md new file mode 100644 index 0000000000..14637cc842 --- /dev/null +++ b/.changeset/wild-olives-applaud.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search': patch +--- + +Fix search page to respond to searches made from sidebar search diff --git a/plugins/search/src/components/SearchBar/SearchBar.tsx b/plugins/search/src/components/SearchBar/SearchBar.tsx index a1db818488..bdff003b7a 100644 --- a/plugins/search/src/components/SearchBar/SearchBar.tsx +++ b/plugins/search/src/components/SearchBar/SearchBar.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ChangeEvent, useState } from 'react'; +import React, { ChangeEvent, useEffect, useState } from 'react'; import { useDebounce } from 'react-use'; import { InputBase, InputAdornment, IconButton } from '@material-ui/core'; import SearchIcon from '@material-ui/icons/Search'; @@ -31,6 +31,10 @@ export const SearchBar = ({ className, debounceTime = 0 }: Props) => { const { term, setTerm } = useSearch(); const [value, setValue] = useState(term); + useEffect(() => { + setValue(prevValue => (prevValue !== term ? term : prevValue)); + }, [term]); + useDebounce(() => setTerm(value), debounceTime, [value]); const handleQuery = (e: ChangeEvent) => { diff --git a/plugins/search/src/components/SearchContext/SearchContext.tsx b/plugins/search/src/components/SearchContext/SearchContext.tsx index 619471d490..45a74f5355 100644 --- a/plugins/search/src/components/SearchContext/SearchContext.tsx +++ b/plugins/search/src/components/SearchContext/SearchContext.tsx @@ -54,7 +54,7 @@ export const SearchContextProvider = ({ term: '', pageCursor: '', filters: {}, - types: ['*'], + types: [], }, children, }: PropsWithChildren<{ initialState?: SettableSearchContext }>) => { diff --git a/plugins/search/src/components/SearchPage/SearchPage.test.tsx b/plugins/search/src/components/SearchPage/SearchPage.test.tsx index 4c1ae38005..c387d57b60 100644 --- a/plugins/search/src/components/SearchPage/SearchPage.test.tsx +++ b/plugins/search/src/components/SearchPage/SearchPage.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { renderInTestApp } from '@backstage/test-utils'; import { useLocation, useOutlet } from 'react-router'; -import { useSearch, SearchContextProvider } from '../SearchContext'; +import { useSearch } from '../SearchContext'; import { SearchPage } from './'; jest.mock('react-router', () => ({ @@ -29,6 +29,11 @@ jest.mock('react-router', () => ({ useOutlet: jest.fn().mockReturnValue('Route Children'), })); +const setTermMock = jest.fn(); +const setTypesMock = jest.fn(); +const setFiltersMock = jest.fn(); +const setPageCursorMock = jest.fn(); + jest.mock('../SearchContext', () => ({ ...jest.requireActual('../SearchContext'), SearchContextProvider: jest @@ -36,9 +41,13 @@ jest.mock('../SearchContext', () => ({ .mockImplementation(({ children }) => children), useSearch: jest.fn().mockReturnValue({ term: '', + setTerm: (term: any) => setTermMock(term), types: [], + setTypes: (types: any) => setTypesMock(types), filters: {}, + setFilters: (filters: any) => setFiltersMock(filters), pageCursor: '', + setPageCursor: (pageCursor: any) => setPageCursorMock(pageCursor), }), })); @@ -58,7 +67,7 @@ describe('SearchPage', () => { window.history.replaceState = origReplaceState; }); - it('uses initial term state from location', async () => { + it('sets term state from location', async () => { // Given this initial location.search value... const expectedFilterField = 'anyKey'; const expectedFilterValue = 'anyValue'; @@ -75,13 +84,11 @@ describe('SearchPage', () => { // When we render the page... await renderInTestApp(); - // Then search context should be initialized with these values... - const calls = (SearchContextProvider as jest.Mock).mock.calls[0]; - const actualInitialState = calls[0].initialState; - expect(actualInitialState.term).toEqual(expectedTerm); - expect(actualInitialState.types).toEqual(expectedTypes); - expect(actualInitialState.pageCursor).toEqual(expectedPageCursor); - expect(actualInitialState.filters).toStrictEqual(expectedFilters); + // Then search context should be set with these values... + expect(setTermMock).toHaveBeenCalledWith(expectedTerm); + expect(setTypesMock).toHaveBeenCalledWith(expectedTypes); + expect(setPageCursorMock).toHaveBeenCalledWith(expectedPageCursor); + expect(setFiltersMock).toHaveBeenCalledWith(expectedFilters); }); it('renders provided router element', async () => { @@ -103,6 +110,10 @@ describe('SearchPage', () => { types: ['software-catalog'], pageCursor: 'page2-or-something', filters: { anyKey: 'anyValue' }, + setTerm: setTermMock, + setTypes: setTypesMock, + setFilters: setFiltersMock, + setPageCursor: setPageCursorMock, }); const expectedLocation = encodeURI( '?query=bieber&types[]=software-catalog&pageCursor=page2-or-something&filters[anyKey]=anyValue', diff --git a/plugins/search/src/components/SearchPage/SearchPage.tsx b/plugins/search/src/components/SearchPage/SearchPage.tsx index 8a85879a48..fb18833da6 100644 --- a/plugins/search/src/components/SearchPage/SearchPage.tsx +++ b/plugins/search/src/components/SearchPage/SearchPage.tsx @@ -14,7 +14,8 @@ * limitations under the License. */ -import React from 'react'; +import React, { useEffect } from 'react'; +import { usePrevious } from 'react-use'; import qs from 'qs'; import { useLocation, useOutlet } from 'react-router'; import { SearchContextProvider, useSearch } from '../SearchContext'; @@ -22,46 +23,72 @@ import { JsonObject } from '@backstage/config'; import { LegacySearchPage } from '../LegacySearchPage'; export const UrlUpdater = () => { - const { term, types, pageCursor, filters } = useSearch(); + const location = useLocation(); + const { + term, + setTerm, + types, + setTypes, + pageCursor, + setPageCursor, + filters, + setFilters, + } = useSearch(); - const newParams = qs.stringify( - { - query: term, - types, - pageCursor, - filters, - }, - { arrayFormat: 'brackets' }, - ); - const newUrl = `${window.location.pathname}?${newParams}`; + const prevQueryParams = usePrevious(location.search); + useEffect(() => { + // Only respond to changes to url query params + if (location.search === prevQueryParams) { + return; + } - // We directly manipulate window history here in order to not re-render - // infinitely (state => location => state => etc). The intention of this - // code is just to ensure the right query/filters are loaded when a user - // clicks the "back" button after clicking a result. - window.history.replaceState(null, document.title, newUrl); + const query = + qs.parse(location.search.substring(1), { arrayLimit: 0 }) || {}; + + if (query.filters) { + setFilters(query.filters as JsonObject); + } + + if (query.query) { + setTerm(query.query as string); + } + + if (query.pageCursor) { + setPageCursor(query.pageCursor as string); + } + + if (query.types) { + setTypes(query.types as string[]); + } + }, [prevQueryParams, location, setTerm, setTypes, setPageCursor, setFilters]); + + useEffect(() => { + const newParams = qs.stringify( + { + query: term, + types, + pageCursor, + filters, + }, + { arrayFormat: 'brackets' }, + ); + const newUrl = `${window.location.pathname}?${newParams}`; + + // We directly manipulate window history here in order to not re-render + // infinitely (state => location => state => etc). The intention of this + // code is just to ensure the right query/filters are loaded when a user + // clicks the "back" button after clicking a result. + window.history.replaceState(null, document.title, newUrl); + }, [term, types, pageCursor, filters]); return null; }; export const SearchPage = () => { - const location = useLocation(); const outlet = useOutlet(); - const query = qs.parse(location.search.substring(1), { arrayLimit: 0 }) || {}; - const filters = (query.filters as JsonObject) || {}; - const queryString = (query.query as string) || ''; - const pageCursor = (query.pageCursor as string) || ''; - const types = (query.types as string[]) || []; - - const initialState = { - term: queryString || '', - types, - pageCursor, - filters, - }; return ( - + {outlet || }