Fix Search Navigation Disconnect Between URL and Search Input (#11009)

This commit is contained in:
Nick Taylor 2020-10-22 12:37:26 -04:00 committed by GitHub
parent f54549cca3
commit 82d659be8f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 72 additions and 7 deletions

View file

@ -21,6 +21,7 @@ export class Search extends Component {
constructor(props) {
super(props);
this.enableSearchPageChecker = true;
this.syncSearchUrlWithInput = this.syncSearchUrlWithInput.bind(this);
}
componentWillMount() {
@ -48,9 +49,33 @@ export class Search extends Component {
searchPageChecker();
}
/**
* Synchronizes the search input value with the search term defined in the URL.
*/
syncSearchUrlWithInput() {
// TODO: Consolidate search functionality.
// Note that push states for search occur in _search.html.erb
// in initializeSortingTabs(query)
const { searchBoxId } = this.props;
const searchTerm = getInitialSearchTerm(window.location.search);
// We set the value outside of React state so that there is no flickering of placeholder
// to search term.
const searchBox = document.getElementById(searchBoxId);
searchBox.value = searchTerm;
// Even though we set the search term directly via the DOM, it still needs to reside
// in component state.
this.setState({
searchTerm,
});
}
componentDidMount() {
this.registerGlobalKeysListener();
InstantClick.on('change', this.enableSearchPageListener);
window.addEventListener('popstate', this.syncSearchUrlWithInput);
}
enableSearchPageListener = () => {
@ -81,8 +106,9 @@ export class Search extends Component {
}
}
componentDidUnmount() {
componentWillUnmount() {
document.removeEventListener('keydown', this.globalKeysListener);
window.removeEventListener('popstate', this.syncSearchUrlWithInput);
InstantClick.off('change', this.enableSearchPageListener);
}

View file

@ -9,6 +9,7 @@ describe('<Search />', () => {
global.filterXSS = jest.fn();
global.InstantClick = jest.fn(() => ({
on: jest.fn(),
off: jest.fn(),
preload: jest.fn(),
display: jest.fn(),
}))();
@ -78,4 +79,34 @@ describe('<Search />', () => {
expect(searchInput.value).toEqual('hello');
expect(Search.prototype.search).toHaveBeenCalledWith('Enter', 'hello');
});
it('should be listening for history state changes', async () => {
// This is an implementation detail, but I want to make sure that this
// listener is registered as it affects the UI.
jest.spyOn(window, 'addEventListener');
render(<Search />);
expect(window.addEventListener).toHaveBeenCalledTimes(1);
expect(window.addEventListener).toHaveBeenCalledWith(
'popstate',
expect.any(Function),
);
});
it('should stop listening for history state changes when the component is destroyed', async () => {
// This is an implementation detail, but I want to make sure that this
// listener is unregistered as it affects the UI.
jest.spyOn(window, 'removeEventListener');
const { unmount } = render(<Search />);
unmount();
expect(window.removeEventListener).toHaveBeenCalledTimes(1);
expect(window.removeEventListener).toHaveBeenCalledWith(
'popstate',
expect.any(Function),
);
});
});

View file

@ -58,6 +58,15 @@ describe('Search utilities', () => {
const actual = getInitialSearchTerm(querystring);
expect(actual).toEqual(expected);
});
it(`should return an empty string when the search term is not defined`, () => {
/* eslint-disable-next-line no-global-assign */
filterXSS = jest.fn(() => undefined);
const querystring = `?q=`;
const actual = getInitialSearchTerm(querystring);
expect(actual).toEqual('');
});
});
describe(`When the querystring key 'q' does not exist`, () => {

View file

@ -70,7 +70,7 @@ export function getInitialSearchTerm(querystring) {
matches !== null && matches.length === 2
? decodeURIComponent(matches[1].replace(/\+/g, '%20'))
: '';
const query = filterXSS(rawSearchTerm);
const query = filterXSS(rawSearchTerm) || '';
const divForDecode = document.createElement('div');
divForDecode.innerHTML = query;
@ -86,11 +86,10 @@ export function preloadSearchResults({
const encodedQuery = fixedEncodeURIComponent(
searchTerm.replace(/^[ ]+|[ ]+$/g, ''),
);
InstantClick.preload(
`${location.origin}/search?q=${encodedQuery}${getFilterParameters(
location.href,
)}`,
);
const searchUrl = `${
location.origin
}/search?q=${encodedQuery}${getFilterParameters(location.href)}`;
InstantClick.preload(searchUrl);
}
export function createSearchUrl(dataHash) {