Skip to content

Commit

Permalink
fix(searchbox): SearchBox should not change the state if SearchOnEnte…
Browse files Browse the repository at this point in the history
…rKeyPress

This patch revisits the previous solution to prevent impact on the URL
Sync mechanism. It uses the `change` event again and impacts only the
behavior of the SearchBox.

The solution also contains a refactoring that introduces a default
value for the queryHook which reduces the control flow complexity.
  • Loading branch information
Alexandre Stanislawski committed Jun 2, 2017
1 parent d6ff626 commit 0536b74
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
5 changes: 3 additions & 2 deletions src/lib/url-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ function getFullURL(relative) {
// IE <= 11 has no location.origin or buggy
function getLocationOrigin() {
// eslint-disable-next-line max-len
return `${window.location.protocol}//${window.location.hostname}${window.location.port ? `:${window.location.port}` : ''}`;
const port = window.location.port ? `:${window.location.port}` : '';
return `${window.location.protocol}//${window.location.hostname}${port}`;
}

// see InstantSearch.js file for urlSync options
Expand Down Expand Up @@ -126,7 +127,7 @@ class URLSync {
if (this.firstRender) {
this.firstRender = false;
this.onHistoryChange(this.onPopState.bind(this, helper));
helper.on('search', state => this.renderURLFromState(state));
helper.on('change', state => this.renderURLFromState(state));
}
}

Expand Down
12 changes: 8 additions & 4 deletions src/widgets/search-box/__tests__/search-box-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,8 @@ describe('searchBox()', () => {
simulateInputEvent('test', 'tes', widget, helper, state, container);
});

it('updates the query', () => {
expect(helper.setQuery.calledOnce).toBe(true);
it('does not update the query in the state', () => {
expect(helper.setQuery.calledOnce).toBe(false);
});

it('does not search', () => {
Expand Down Expand Up @@ -477,10 +477,14 @@ describe('searchBox()', () => {
widget = searchBox({container, searchOnEnterKeyPressOnly: true});
});

it('performs the search on keyup if <ENTER>', () => {
it('does not perform search when typing', () => {
simulateInputEvent('test', 'tes', widget, helper, state, container);
expect(helper.search.callCount).toBe(0);
});

it('performs the search on keyup if <ENTER>', () => {
simulateKeyUpEvent({keyCode: 13}, widget, helper, state, container);
expect(helper.search.calledOnce).toBe(true);
expect(helper.search.callCount).toBe(1);
});

it('doesn\'t perform the search on keyup if not <ENTER>', () => {
Expand Down
19 changes: 6 additions & 13 deletions src/widgets/search-box/search-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const KEY_SUPPRESS = 8;
* @return {Object}
*/

const defaultQueryHook = function (query, setQueryAndSearch) {
setQueryAndSearch(query);
};

const usage = `Usage:
searchBox({
container,
Expand All @@ -56,7 +60,7 @@ function searchBox({
wrapInput = true,
autofocus = 'auto',
searchOnEnterKeyPressOnly = false,
queryHook,
queryHook = defaultQueryHook,
}) {
// the 'input' event is triggered when the input value changes
// in any case: typing, copy pasting with mouse..
Expand Down Expand Up @@ -172,11 +176,6 @@ function searchBox({
// Add all the needed attributes and listeners to the input
this.addDefaultAttributesToInput(input, state.query);

// always set the query every keystrokes when there's no queryHook
if (!queryHook) {
addListener(input, INPUT_EVENT, getInputValueAndCall(setQuery));
}

// search on enter
if (searchOnEnterKeyPressOnly) {
addListener(input, 'keyup', ifKey(KEY_ENTER, getInputValueAndCall(maybeSearch)));
Expand All @@ -186,18 +185,12 @@ function searchBox({
// handle IE8 weirdness where BACKSPACE key will not trigger an input change..
// can be removed as soon as we remove support for it
if (INPUT_EVENT === 'propertychange' || window.attachEvent) {
addListener(input, 'keyup', ifKey(KEY_SUPPRESS, getInputValueAndCall(setQuery)));
addListener(input, 'keyup', ifKey(KEY_SUPPRESS, getInputValueAndCall(maybeSearch)));
}
}

function maybeSearch(query) {
if (queryHook) {
queryHook(query, setQueryAndSearch);
return;
}

search(query);
queryHook(query, setQueryAndSearch);
}

function setQuery(query) {
Expand Down

0 comments on commit 0536b74

Please sign in to comment.