fix(searchBox): stop setting the query twice

We were previously setting the query on every keystroke twice, because
of the keyup + input events.

Now fixed.

This was not triggering any bad behavior but extra work and maybe bad
performance because the way we set queries in the helper (to be worked
on)

+ refactor
+ update tests

src/widgets/search-box/__tests__/search-box-test.js

@@ -209,7 +209,6 @@ describe('searchBox()', () => {
     beforeEach(() => {
       container = document.createElement('div');
       input = createHTMLNodeFromString('<input />');
-      input.addEventListener = sinon.spy();
     });
 
     function simulateInputEvent() {
@@ -218,12 +217,9 @@ describe('searchBox()', () => {
       // When
       widget.init({state, helper, onHistoryChange});
       // Then
-      expect(input.addEventListener.called).toEqual(true);
-      expect(input.addEventListener.args[1].length).toEqual(3);
-      expect(input.addEventListener.args[1][0]).toEqual('input');
-      let fn = input.addEventListener.args[1][1];
-
-      return fn({currentTarget: {value: 'test'}});
+      input.value = 'test';
+      let event = new Event('input');
+      input.dispatchEvent(event);
     }
 
     context('instant search', () => {
@@ -234,9 +230,13 @@ describe('searchBox()', () => {
 
       it('performs a search on any change', () => {
         simulateInputEvent();
-        expect(helper.setQuery.calledOnce).toBe(true);
         expect(helper.search.called).toBe(true);
       });
+
+      it('sets the query on any change', () => {
+        simulateInputEvent();
+        expect(helper.setQuery.calledOnce).toBe(true);
+      });
     });
 
     context('non-instant search', () => {
@@ -258,7 +258,6 @@ describe('searchBox()', () => {
     beforeEach(() => {
       container = document.createElement('div');
       input = createHTMLNodeFromString('<input />');
-      input.addEventListener = sinon.spy();
     });
 
     function simulateKeyUpEvent(args) {
@@ -267,12 +266,9 @@ describe('searchBox()', () => {
       // When
       widget.init({state, helper, onHistoryChange});
       // Then
-      expect(input.addEventListener.called).toEqual(true);
-      expect(input.addEventListener.args[0].length).toEqual(2);
-      expect(input.addEventListener.args[0][0]).toEqual('keyup');
-      let fn = input.addEventListener.args[0][1];
-
-      return fn(args);
+      let event = new Event('keyup', args);
+      Object.defineProperty(event, 'keyCode', {get: () => args.keyCode});
+      input.dispatchEvent(event);
     }
 
     context('instant search', () => {
@@ -283,7 +279,6 @@ describe('searchBox()', () => {
 
       it('do not perform the search on keyup event (should be done by input event)', () => {
         simulateKeyUpEvent({});
-        expect(helper.setQuery.calledOnce).toBe(true);
         expect(helper.search.called).toBe(false);
       });
     });
@@ -296,13 +291,11 @@ describe('searchBox()', () => {
 
       it('performs the search on keyup if <ENTER>', () => {
         simulateKeyUpEvent({keyCode: 13});
-        expect(helper.setQuery.calledOnce).toBe(true);
         expect(helper.search.calledOnce).toBe(true);
       });
 
       it('doesn\'t perform the search on keyup if not <ENTER>', () => {
         simulateKeyUpEvent({});
-        expect(helper.setQuery.calledOnce).toBe(true);
         expect(helper.search.calledOnce).toBe(false);
       });
     });

src/widgets/search-box/search-box.js

@@ -44,6 +44,14 @@ function searchBox({
   autofocus = 'auto',
   searchOnEnterKeyPressOnly = false
 }) {
+  // the 'input' event is triggered when the input value changes
+  // in any case: typing, copy pasting with mouse..
+  // 'onpropertychange' is the IE8 alternative until we support IE8
+  // but it's flawed: http://help.dottoro.com/ljhxklln.php
+  const INPUT_EVENT = window.addEventListener ?
+    'input' :
+    'propertychange';
+
   if (!container) {
     throw new Error(usage);
   }
@@ -124,31 +132,37 @@ function searchBox({
       // Add all the needed attributes and listeners to the input
       this.addDefaultAttributesToInput(input, state.query);
 
-      // Keep keyup to handle searchOnEnterKeyPressOnly
-      input.addEventListener('keyup', (e) => {
-        helper.setQuery(input.value);
-        if (searchOnEnterKeyPressOnly && e.keyCode === KEY_ENTER) {
-          helper.search();
-        }
-
-        // IE8/9 compatibility
-        if (window.attachEvent && e.keyCode === KEY_SUPPRESS) {
-          helper.search();
-        }
-      });
+      // always set the query on every change
+      addListener(input, INPUT_EVENT, setQuery);
 
-      function inputCallback(e) {
-        let target = (e.currentTarget) ? e.currentTarget : e.srcElement;
-        helper.setQuery(target.value);
-        if (!searchOnEnterKeyPressOnly) {
-          helper.search();
-        }
+      // 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, setQuery));
+        addListener(input, 'keyup', ifKey(KEY_SUPPRESS, search));
       }
 
-      if (window.attachEvent) { // IE8/9 compatibility
-        input.attachEvent('onpropertychange', inputCallback);
+      // only search on enter
+      if (searchOnEnterKeyPressOnly) {
+        addListener(input, 'keyup', ifKey(KEY_ENTER, search));
       } else {
-        input.addEventListener('input', inputCallback, false);
+        addListener(input, INPUT_EVENT, search);
+      }
+
+      function setQuery(e) {
+        helper.setQuery(getValue(e));
+      }
+
+      function search() {
+        helper.search();
+      }
+
+      function getValue(e) {
+        return ((e.currentTarget) ? e.currentTarget : e.srcElement).value;
+      }
+
+      function ifKey(expectedKeyCode, fnToExecute) {
+        return actualEvent => actualEvent.keyCode === expectedKeyCode && fnToExecute(actualEvent);
       }
 
       if (isInputTargeted) {
@@ -180,4 +194,12 @@ function searchBox({
   };
 }
 
+function addListener(el, type, fn) {
+  if (el.addEventListener) {
+    el.addEventListener(type, fn);
+  } else {
+    el.attachEvent('on' + type, fn);
+  }
+}
+
 export default searchBox;