From 60c3137d64a2e18e46a2fb6c319a86f8c442e5f6 Mon Sep 17 00:00:00 2001 From: "Michael J. Giarlo" Date: Wed, 30 Sep 2020 15:25:26 -0700 Subject: [PATCH] Remember resource and template search state Fixes #2227 Also, allow clearing the search via a new button. See screencast to watch feature in action: TBD --- .../feature/searchAndOpenTemplate.test.js | 13 +++- .../feature/searchAndViewResource.test.js | 18 ++++- cypress/integration/end2end.spec.js | 4 +- cypress/integration/leftNav.spec.js | 1 - src/components/search/Search.jsx | 73 +++++++++++++------ src/components/templates/TemplateSearch.jsx | 40 +++++++--- 6 files changed, 106 insertions(+), 43 deletions(-) diff --git a/__tests__/feature/searchAndOpenTemplate.test.js b/__tests__/feature/searchAndOpenTemplate.test.js index 087db1758..77f2af82c 100644 --- a/__tests__/feature/searchAndOpenTemplate.test.js +++ b/__tests__/feature/searchAndOpenTemplate.test.js @@ -45,9 +45,11 @@ describe('searching and opening a resource', () => { it('adds the template to recently used template history', async () => { renderApp(store, history) + const queryString = 'title' + // Search for a template const input = screen.getByPlaceholderText('Enter id, label, URI, remark, or author') - await fireEvent.change(input, { target: { value: 'title' } }) + await fireEvent.change(input, { target: { value: queryString } }) await screen.findByText('resourceTemplate:bf2:Title:Note') // open the template @@ -55,10 +57,17 @@ describe('searching and opening a resource', () => { fireEvent.click(link) await act(() => promise) - // return the the RT list + // return to the RT list const rtLink = await screen.findByText('Resource Templates', { selector: 'a' }) fireEvent.click(rtLink) + // confirm RT query is still in place (stored in state and not cleared) + expect(input.value).toEqual(queryString) + + // Clear search button empties the search field + fireEvent.click(screen.getByTestId('Clear query string', { selector: 'button' })) + expect(screen.getByPlaceholderText('Enter id, label, URI, remark, or author').value).toEqual('') + // see the recently used RTs const histTemplateBtn = await screen.findByText('Most recently used resource templates') fireEvent.click(histTemplateBtn) diff --git a/__tests__/feature/searchAndViewResource.test.js b/__tests__/feature/searchAndViewResource.test.js index a4829df07..1f8a061cf 100644 --- a/__tests__/feature/searchAndViewResource.test.js +++ b/__tests__/feature/searchAndViewResource.test.js @@ -21,11 +21,11 @@ describe('searching and viewing a resource', () => { error: undefined, }) - it('renders a modal without edit controls', async () => { - // Setup search component to return known resource - const uri = 'http://localhost:3000/resource/c7db5404-7d7d-40ac-b38e-c821d2c3ae3f' - sinopiaSearch.getSearchResultsWithFacets.mockResolvedValue(resourceSearchResults(uri)) + // Setup search component to return known resource + const uri = 'http://localhost:3000/resource/c7db5404-7d7d-40ac-b38e-c821d2c3ae3f' + sinopiaSearch.getSearchResultsWithFacets.mockResolvedValue(resourceSearchResults(uri)) + it('renders a modal without edit controls', async () => { renderApp() fireEvent.click(screen.getByText('Linked Data Editor', { selector: 'a' })) @@ -75,5 +75,15 @@ describe('searching and viewing a resource', () => { fireEvent.click(screen.getByLabelText('Edit', { selector: 'button', exact: true })) expect(screen.getByText('Uber template1', { selector: 'h3' })).toBeInTheDocument() expect(screen.getByText('Copy URI', { selector: 'button' })).toBeInTheDocument() + + // Switch back to search page + fireEvent.click(screen.getByText('Search', { selector: 'a' })) + + // Confirm search query is still in place (stored in state and not cleared) + expect(await screen.getByLabelText('Query').value).toEqual(uri) + + // Clear search button empties the search field + fireEvent.click(screen.getByTestId('Clear query string', { selector: 'button' })) + expect(await screen.getByLabelText('Query').value).toEqual('') }) }) diff --git a/cypress/integration/end2end.spec.js b/cypress/integration/end2end.spec.js index 58eccfc38..9ec4eab37 100644 --- a/cypress/integration/end2end.spec.js +++ b/cypress/integration/end2end.spec.js @@ -67,7 +67,6 @@ describe('End-to-end test', () => { cy.url().should('include', '/templates') cy.get('#searchInput') - .type('resourceTemplate:bf2:WorkTitle') .should('have.value', 'resourceTemplate:bf2:WorkTitle') // eslint-disable-next-line cypress/no-unnecessary-waiting @@ -115,6 +114,9 @@ describe('End-to-end test', () => { cy.get('a').contains('Search').click() cy.url().should('include', '/search') + // Test search clear button + cy.get('button[title="Clear query string"]').click() + // Indexing latency is possible problem here. // Force is necessary because reflow of search inputs is suboptimal. cy.get('input#searchInput') diff --git a/cypress/integration/leftNav.spec.js b/cypress/integration/leftNav.spec.js index 4c7dd9465..91fe9a7b1 100644 --- a/cypress/integration/leftNav.spec.js +++ b/cypress/integration/leftNav.spec.js @@ -50,7 +50,6 @@ describe('Left-nav test', () => { cy.url().should('include', '/templates') cy.get('#searchInput') - .type('resourceTemplate:testing:uber1') .should('have.value', 'resourceTemplate:testing:uber1') // eslint-disable-next-line cypress/no-unnecessary-waiting diff --git a/src/components/search/Search.jsx b/src/components/search/Search.jsx index 136d78544..a519e2861 100644 --- a/src/components/search/Search.jsx +++ b/src/components/search/Search.jsx @@ -10,45 +10,57 @@ import { fetchSinopiaSearchResults as fetchSinopiaSearchResultsCreator, fetchQASearchResults as fetchQASearchResultsCreator, } from 'actionCreators/search' -import { clearSearchResults as clearSearchResultsAction } from 'actions/search' +import { clearSearchResults as clearSearchResultsAction, setSearchResults } from 'actions/search' import SinopiaSearchResults from './SinopiaSearchResults' import QASearchResults from './QASearchResults' import SearchResultsPaging from './SearchResultsPaging' import SearchResultsMessage from './SearchResultsMessage' import Alert from '../Alert' import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' -import { faSearch } from '@fortawesome/free-solid-svg-icons' +import { faTrashAlt, faSearch } from '@fortawesome/free-solid-svg-icons' import searchConfig from '../../../static/searchConfig.json' -import { selectSearchError, selectSearchUri, selectSearchOptions } from 'selectors/search' +import { + selectSearchError, selectSearchQuery, selectSearchUri, selectSearchOptions, +} from 'selectors/search' const Search = (props) => { const dispatch = useDispatch() - const fetchQASearchResults = (queryString, uri, startOfRange) => dispatch(fetchQASearchResultsCreator(queryString, uri, { ...searchOptions, startOfRange })) const searchOptions = useSelector((state) => selectSearchOptions(state, 'resource')) + const error = useSelector((state) => selectSearchError(state, 'resource')) + const searchUri = useSelector((state) => selectSearchUri(state, 'resource')) + const lastQueryString = useSelector((state) => selectSearchQuery(state, 'resource')) - const fetchSinopiaSearchResults = (queryString, startOfRange) => dispatch(fetchSinopiaSearchResultsCreator( - queryString, { ...searchOptions, startOfRange }, - )) + const clearSearchResults = useCallback(() => dispatch(clearSearchResultsAction('resource')), [dispatch]) - const fetchNewSinopiaSearchResults = (queryString) => dispatch(fetchSinopiaSearchResultsCreator( - queryString, - )) + const topRef = useRef(null) + const defaultUri = 'sinopia' - const clearSearchResults = useCallback(() => dispatch(clearSearchResultsAction('resource')), [dispatch]) + const [queryString, setQueryString] = useState(lastQueryString || '') + const [uri, setUri] = useState(searchUri || defaultUri) - const error = useSelector((state) => selectSearchError(state, 'resource')) - const searchUri = useSelector((state) => selectSearchUri(state, 'resource')) + useEffect(() => { + if (!queryString) clearSearchResults() + }, [clearSearchResults, queryString]) - const topRef = useRef(null) + const fetchQASearchResults = (queryString, uri, startOfRange) => { + dispatch(fetchQASearchResultsCreator(queryString, uri, { ...searchOptions, startOfRange })).then((response) => { + if (response) dispatch(setSearchResults('resource', uri, response.results, response.totalHits, {}, queryString, { startOfRange }, response.error)) + }) + } - const [queryString, setQueryString] = useState('') - const [uri, setUri] = useState('sinopia') + const fetchSinopiaSearchResults = (queryString, startOfRange) => { + dispatch(fetchSinopiaSearchResultsCreator(queryString, { ...searchOptions, startOfRange })).then((response) => { + if (response) dispatch(setSearchResults('resource', null, response.results, response.totalHits, {}, queryString, { startOfRange }, response.error)) + }) + } - useEffect(() => { - clearSearchResults() - }, [clearSearchResults]) + const fetchNewSinopiaSearchResults = (queryString) => { + dispatch(fetchSinopiaSearchResultsCreator(queryString)).then((response) => { + if (response) dispatch(setSearchResults('resource', null, response.results, response.totalHits, {}, queryString, {}, response.error)) + }) + } const handleKeyPress = (event) => { if (event.key === 'Enter') { @@ -66,7 +78,7 @@ const Search = (props) => { if (queryString === '') { return } - if (uri === 'sinopia') { + if (uri === defaultUri) { fetchNewSinopiaSearchResults(queryString) } else { fetchQASearchResults(queryString, uri, 0) @@ -85,7 +97,7 @@ const Search = (props) => { const options = searchConfig.map((config) => ()) let results - if (searchUri === 'sinopia') { + if (searchUri === defaultUri) { results = (
@@ -116,7 +128,7 @@ const Search = (props) => { value={uri} onChange={ (event) => setUri(event.target.value) } onBlur={ (event) => setUri(event.target.value) }> - + {options}
@@ -127,14 +139,29 @@ const Search = (props) => {
setQueryString(event.target.value) } - onKeyPress={handleKeyPress} /> + onKeyPress={handleKeyPress} + placeholder="Enter query string" + value={ queryString } /> +
diff --git a/src/components/templates/TemplateSearch.jsx b/src/components/templates/TemplateSearch.jsx index de70f105e..aa9de4bb6 100644 --- a/src/components/templates/TemplateSearch.jsx +++ b/src/components/templates/TemplateSearch.jsx @@ -10,9 +10,10 @@ import Alert from '../Alert' import SinopiaResourceTemplates from './SinopiaResourceTemplates' import SearchResultsPaging from 'components/search/SearchResultsPaging' import NewResourceTemplateButton from './NewResourceTemplateButton' -import { selectSearchError } from 'selectors/search' +import { selectSearchError, selectSearchQuery } from 'selectors/search' import PropTypes from 'prop-types' - +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' +import { faTrashAlt } from '@fortawesome/free-solid-svg-icons' const TemplateSearch = (props) => { const dispatch = useDispatch() @@ -20,15 +21,17 @@ const TemplateSearch = (props) => { // search, but causes result to be ignored. const tokens = useRef([]) - const [query, setQueryText] = useState('') + const error = useSelector((state) => selectSearchError(state, 'template')) + const lastQueryString = useSelector((state) => selectSearchQuery(state, 'template')) + + const [queryString, setQueryString] = useState(lastQueryString || '') const [startOfRange, setStartOfRange] = useState(0) - const error = useSelector((state) => selectSearchError(state, 'template')) const clearSearchResults = useCallback(() => dispatch(clearSearchResultsAction('template')), [dispatch]) useEffect(() => { - clearSearchResults() - }, [clearSearchResults]) + if (!queryString) clearSearchResults() + }, [clearSearchResults, queryString]) useEffect(() => { // Cancel all current searches @@ -39,10 +42,10 @@ const TemplateSearch = (props) => { // Create a token for this set of searches const token = { cancel: false } tokens.current.push(token) - getTemplateSearchResults(query, { startOfRange }).then((response) => { - if (!token.cancel) dispatch(setSearchResults('template', null, response.results, response.totalHits, {}, null, { startOfRange }, response.error)) + getTemplateSearchResults(queryString, { startOfRange }).then((response) => { + if (!token.cancel) dispatch(setSearchResults('template', null, response.results, response.totalHits, {}, queryString, { startOfRange }, response.error)) }) - }, [dispatch, query, startOfRange]) + }, [dispatch, queryString, startOfRange]) const changePage = (startOfRange) => { setStartOfRange(startOfRange) @@ -50,7 +53,7 @@ const TemplateSearch = (props) => { const updateSearch = (e) => { setStartOfRange(0) - setQueryText(e.target.value) + setQueryString(e.target.value) } return ( @@ -63,9 +66,22 @@ const TemplateSearch = (props) => {
 
- + placeholder="Enter id, label, URI, remark, or author" + value={ queryString } /> + + +