From 997c0c2ef8325b2d402fbd420720c743389db630 Mon Sep 17 00:00:00 2001 From: Vincent Voyer Date: Fri, 8 Jul 2016 09:54:36 +0200 Subject: [PATCH] fix(searchBox): update helper query on every keystroke (#1127) fixes #1015 --- .../search-box/__tests__/search-box-test.js | 108 +++++++++--------- src/widgets/search-box/search-box.js | 30 +++-- 2 files changed, 73 insertions(+), 65 deletions(-) diff --git a/src/widgets/search-box/__tests__/search-box-test.js b/src/widgets/search-box/__tests__/search-box-test.js index 1b67e456b7..47758890e7 100644 --- a/src/widgets/search-box/__tests__/search-box-test.js +++ b/src/widgets/search-box/__tests__/search-box-test.js @@ -16,6 +16,8 @@ function createHTMLNodeFromString(string) { return parent.firstChild; } +const onHistoryChange = () => {}; + describe('searchBox()', () => { beforeEach(function() {this.jsdom = jsdom();}); afterEach(function() {this.jsdom();}); @@ -24,7 +26,6 @@ describe('searchBox()', () => { let state; let helper; let widget; - let onHistoryChange; beforeEach(() => { state = { @@ -38,7 +39,6 @@ describe('searchBox()', () => { }, ...EventEmitter.prototype }; - onHistoryChange = function() {}; }); context('bad usage', () => { @@ -389,56 +389,39 @@ describe('searchBox()', () => { container = document.body.appendChild(document.createElement('input')); }); - function simulateInputEvent(query, stateQuery) { - if (query === undefined) { - query = 'test'; - } - - // Given - if (stateQuery !== undefined) { - helper.state.query = stateQuery; - } else { - helper.state.query = 'tes'; - } - - // When - widget.init({state, helper, onHistoryChange}); - // Then - container.value = query; - let event = new window.Event('input'); - container.dispatchEvent(event); - } - context('instant search', () => { beforeEach(() => { widget = searchBox({container}); }); it('performs a search on any change', () => { - simulateInputEvent(); + simulateInputEvent('test', 'tes', widget, helper, state, container); expect(helper.search.called).toBe(true); }); it('sets the query on any change', () => { - simulateInputEvent(); + simulateInputEvent('test', 'tes', widget, helper, state, container); expect(helper.setQuery.calledOnce).toBe(true); }); it('does nothing when query is the same as state', () => { - simulateInputEvent('test', 'test'); + simulateInputEvent('test', 'test', widget, helper, state, container); expect(helper.setQuery.calledOnce).toBe(false); expect(helper.search.called).toBe(false); }); }); - context('non-instant search', () => { + context('non-instant search and input event', () => { beforeEach(() => { widget = searchBox({container, searchOnEnterKeyPressOnly: true}); + simulateInputEvent('test', 'tes', widget, helper, state, container); }); - it('does not performs (will be handle by keyup event)', () => { - simulateInputEvent(); - expect(helper.setQuery.calledOnce).toBe(false); + it('updates the query', () => { + expect(helper.setQuery.calledOnce).toBe(true); + }); + + it('does not search', () => { expect(helper.search.called).toBe(false); }); }); @@ -447,7 +430,7 @@ describe('searchBox()', () => { it('calls the queryHook', () => { let queryHook = sinon.spy(); widget = searchBox({container, queryHook}); - simulateInputEvent('queryhook input'); + simulateInputEvent('queryhook input', 'tes', widget, helper, state, container); expect(queryHook.calledOnce).toBe(true); expect(queryHook.firstCall.args[0]).toBe('queryhook input'); expect(queryHook.firstCall.args[1]).toBeA(Function); @@ -456,7 +439,7 @@ describe('searchBox()', () => { it('does not perform a search by default', () => { let queryHook = sinon.spy(); widget = searchBox({container, queryHook}); - simulateInputEvent(); + simulateInputEvent('test', 'tes', widget, helper, state, container); expect(helper.setQuery.calledOnce).toBe(false); expect(helper.search.called).toBe(false); }); @@ -464,7 +447,7 @@ describe('searchBox()', () => { it('when calling the provided search function', () => { let queryHook = sinon.spy((query, search) => search(query)); widget = searchBox({container, queryHook}); - simulateInputEvent('oh rly?'); + simulateInputEvent('oh rly?', 'tes', widget, helper, state, container); expect(helper.setQuery.calledOnce).toBe(true); expect(helper.setQuery.firstCall.args[0]).toBe('oh rly?'); expect(helper.search.called).toBe(true); @@ -473,7 +456,7 @@ describe('searchBox()', () => { it('can override the query', () => { let queryHook = sinon.spy((originalQuery, search) => search('hi mom!')); widget = searchBox({container, queryHook}); - simulateInputEvent('come.on.'); + simulateInputEvent('come.on.', 'tes', widget, helper, state, container); expect(helper.setQuery.firstCall.args[0]).toBe('hi mom!'); }); }); @@ -484,24 +467,13 @@ describe('searchBox()', () => { container = document.body.appendChild(document.createElement('input')); }); - function simulateKeyUpEvent(args) { - // Given - helper.state.query = 'foo'; - // When - widget.init({state, helper, onHistoryChange}); - // Then - let event = new window.Event('keyup', args); - Object.defineProperty(event, 'keyCode', {get: () => args.keyCode}); - container.dispatchEvent(event); - } - context('instant search', () => { beforeEach(() => { widget = searchBox({container}); }); it('do not perform the search on keyup event (should be done by input event)', () => { - simulateKeyUpEvent({}); + simulateKeyUpEvent({}, widget, helper, state, container); expect(helper.search.called).toBe(false); }); }); @@ -511,18 +483,14 @@ describe('searchBox()', () => { widget = searchBox({container, searchOnEnterKeyPressOnly: true}); }); - it('sets the query on keyup if ', () => { - simulateKeyUpEvent({keyCode: 13}); - expect(helper.setQuery.calledOnce).toBe(true); - }); - it('performs the search on keyup if ', () => { - simulateKeyUpEvent({keyCode: 13}); + simulateInputEvent('test', 'tes', widget, helper, state, container); + simulateKeyUpEvent({keyCode: 13}, widget, helper, state, container); expect(helper.search.calledOnce).toBe(true); }); it('doesn\'t perform the search on keyup if not ', () => { - simulateKeyUpEvent({}); + simulateKeyUpEvent({}, widget, helper, state, container); expect(helper.setQuery.called).toBe(false); expect(helper.search.called).toBe(false); }); @@ -531,12 +499,9 @@ describe('searchBox()', () => { it('updates the input on history update', () => { let cb; - onHistoryChange = function(fn) { - cb = fn; - }; container = document.body.appendChild(document.createElement('input')); widget = searchBox({container}); - widget.init({state, helper, onHistoryChange}); + widget.init({state, helper, onHistoryChange: fn => cb = fn}); expect(container.value).toBe(''); container.blur(); cb({query: 'iphone'}); @@ -652,3 +617,34 @@ describe('searchBox()', () => { }); }); }); + +function simulateKeyUpEvent(args, widget, helper, state, container) { + // Given + helper.state.query = 'foo'; + // When + widget.init({state, helper, onHistoryChange}); + // Then + let event = new window.Event('keyup', args); + Object.defineProperty(event, 'keyCode', {get: () => args.keyCode}); + container.dispatchEvent(event); +} + +function simulateInputEvent(query, stateQuery, widget, helper, state, container) { + if (query === undefined) { + query = 'test'; + } + + // Given + if (stateQuery !== undefined) { + helper.state.query = stateQuery; + } else { + helper.state.query = 'tes'; + } + + // When + widget.init({state, helper, onHistoryChange}); + // Then + container.value = query; + let event = new window.Event('input'); + container.dispatchEvent(event); +} diff --git a/src/widgets/search-box/search-box.js b/src/widgets/search-box/search-box.js index f96542eb88..ab7f23fbc3 100644 --- a/src/widgets/search-box/search-box.js +++ b/src/widgets/search-box/search-box.js @@ -167,15 +167,20 @@ function searchBox({ init: function({state, helper, onHistoryChange}) { let isInputTargeted = container.tagName === 'INPUT'; let input = this._input = this.getInput(); + let previousQuery; // Add all the needed attributes and listeners to the input this.addDefaultAttributesToInput(input, state.query); - // only update query and search on enter + // 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))); } else { - // always set the query and search on every keystrokes addListener(input, INPUT_EVENT, getInputValueAndCall(maybeSearch)); // handle IE8 weirdness where BACKSPACE key will not trigger an input change.. @@ -186,21 +191,28 @@ function searchBox({ } function maybeSearch(query) { - if (query === helper.state.query) { - return; - } - if (queryHook) { - queryHook(query, search); + queryHook(query, setQueryAndSearch); return; } search(query); } + function setQuery(query) { + if (query !== helper.state.query) { + previousQuery = helper.state.query; + helper.setQuery(query); + } + } + function search(query) { - helper.setQuery(query); - helper.search(); + if (previousQuery !== undefined && previousQuery !== query) helper.search(); + } + + function setQueryAndSearch(query) { + setQuery(query); + search(query); } if (isInputTargeted) {