Make event handlers private

src/scripts/src/choices.js

@@ -344,10 +344,6 @@ class Choices {
    * @public
    */
   removeActiveItemsByValue(value) {
-    if (!value) {
-      return this;
-    }
-
     this._store.activeItems
       .filter(item => item.value === value)
       .forEach(item => this._removeItem(item));
@@ -448,7 +444,7 @@ class Choices {
     if (this.dropdown.isActive) {
       this.hideDropdown();
     } else {
-      this.showDropdown(true);
+      this.showDropdown(true); // code smell 🤢
     }
 
     return this;
@@ -462,13 +458,12 @@ class Choices {
    * @public
    */
   getValue(valueOnly = false) {
-    const items = this._store.activeItems;
-
-    const values = items.reduce((selectedItems, item) => {
-      const itemValue = valueOnly ? item.value : item;
-      selectedItems.push(itemValue);
-      return selectedItems;
-    }, []);
+    const values = this._store.activeItems
+      .reduce((selectedItems, item) => {
+        const itemValue = valueOnly ? item.value : item;
+        selectedItems.push(itemValue);
+        return selectedItems;
+      }, []);
 
     return this._isSelectOneElement ? values[0] : values;
   }

src/scripts/src/choices.test.js

@@ -1204,51 +1204,41 @@ describe('choices', () => {
     });
 
     describe('removeActiveItemsByValue', () => {
-      describe('passing invalid value', () => {
-        beforeEach(() => {
-          output = instance.removeActiveItemsByValue(null);
-        });
+      let activeItemsStub;
+      let removeItemStub;
+      const value = 'Removed';
+      const items = [
+        {
+          id: '1',
+          value: 'Not removed',
+        },
+        {
+          id: '2',
+          value: 'Removed',
+        },
+        {
+          id: '3',
+          value: 'Removed',
+        },
+      ];
 
-        returnsInstance(output);
+      beforeEach(() => {
+        removeItemStub = stub();
+        activeItemsStub = stub(instance._store, 'activeItems').get(() => items);
+        instance._removeItem = removeItemStub;
+
+        output = instance.removeActiveItemsByValue(value);
       });
 
-      describe('passing valid value', () => {
-        let activeItemsStub;
-        let removeItemStub;
-        const value = 'Removed';
-        const items = [
-          {
-            id: '1',
-            value: 'Not removed',
-          },
-          {
-            id: '2',
-            value: 'Removed',
-          },
-          {
-            id: '3',
-            value: 'Removed',
-          },
-        ];
+      afterEach(() => {
+        activeItemsStub.reset();
+        instance._removeItem.reset();
+      });
 
-        beforeEach(() => {
-          removeItemStub = stub();
-          activeItemsStub = stub(instance._store, 'activeItems').get(() => items);
-          instance._removeItem = removeItemStub;
-
-          output = instance.removeActiveItemsByValue(value);
-        });
-
-        afterEach(() => {
-          activeItemsStub.reset();
-          instance._removeItem.reset();
-        });
-
-        it('removes each active item in store with matching value', () => {
-          expect(removeItemStub.callCount).to.equal(2);
-          expect(removeItemStub.firstCall.args[0]).to.equal(items[1]);
-          expect(removeItemStub.secondCall.args[0]).to.equal(items[2]);
-        });
+      it('removes each active item in store with matching value', () => {
+        expect(removeItemStub.callCount).to.equal(2);
+        expect(removeItemStub.firstCall.args[0]).to.equal(items[1]);
+        expect(removeItemStub.secondCall.args[0]).to.equal(items[2]);
       });
     });
 

src/scripts/src/components/container.js

@@ -10,16 +10,16 @@ export default class Container {
     this.isDisabled = false;
     this.isLoading = false;
 
-    this.onFocus = this.onFocus.bind(this);
-    this.onBlur = this.onBlur.bind(this);
+    this._onFocus = this._onFocus.bind(this);
+    this._onBlur = this._onBlur.bind(this);
   }
 
   /**
    * Add event listeners
   */
   addEventListeners() {
-    this.element.addEventListener('focus', this.onFocus);
-    this.element.addEventListener('blur', this.onBlur);
+    this.element.addEventListener('focus', this._onFocus);
+    this.element.addEventListener('blur', this._onBlur);
   }
 
   /**
@@ -28,22 +28,8 @@ export default class Container {
 
   /** */
   removeEventListeners() {
-    this.element.removeEventListener('focus', this.onFocus);
-    this.element.removeEventListener('blur', this.onBlur);
-  }
-
-  /**
-   * Set focussed state
-   */
-  onFocus() {
-    this.isFocussed = true;
-  }
-
-  /**
-   * Remove blurred state
-   */
-  onBlur() {
-    this.isFocussed = false;
+    this.element.removeEventListener('focus', this._onFocus);
+    this.element.removeEventListener('blur', this._onBlur);
   }
 
   /**
@@ -177,4 +163,18 @@ export default class Container {
     this.element.removeAttribute('aria-busy');
     this.isLoading = false;
   }
+
+  /**
+   * Set focussed state
+   */
+  _onFocus() {
+    this.isFocussed = true;
+  }
+
+  /**
+   * Remove blurred state
+   */
+  _onBlur() {
+    this.isFocussed = false;
+  }
 }

src/scripts/src/components/container.test.js

@@ -77,7 +77,7 @@ describe('components/container', () => {
   describe('onFocus', () => {
     it('sets isFocussed flag to true', () => {
       expect(instance.isFocussed).to.equal(false);
-      instance.onFocus();
+      instance._onFocus();
       expect(instance.isFocussed).to.equal(true);
     });
   });
@@ -85,7 +85,7 @@ describe('components/container', () => {
   describe('onBlur', () => {
     it('sets isFocussed flag to false', () => {
       instance.isFocussed = true;
-      instance.onBlur();
+      instance._onBlur();
       expect(instance.isFocussed).to.equal(false);
     });
   });

src/scripts/src/components/input.js

@@ -10,10 +10,10 @@ export default class Input {
     this.isDisabled = false;
 
     // Bind event listeners
-    this.onPaste = this.onPaste.bind(this);
-    this.onInput = this.onInput.bind(this);
-    this.onFocus = this.onFocus.bind(this);
-    this.onBlur = this.onBlur.bind(this);
+    this._onPaste = this._onPaste.bind(this);
+    this._onInput = this._onInput.bind(this);
+    this._onFocus = this._onFocus.bind(this);
+    this._onBlur = this._onBlur.bind(this);
   }
 
   set placeholder(placeholder) {
@@ -29,49 +29,17 @@ export default class Input {
   }
 
   addEventListeners() {
-    this.element.addEventListener('input', this.onInput);
-    this.element.addEventListener('paste', this.onPaste);
-    this.element.addEventListener('focus', this.onFocus);
-    this.element.addEventListener('blur', this.onBlur);
+    this.element.addEventListener('input', this._onInput);
+    this.element.addEventListener('paste', this._onPaste);
+    this.element.addEventListener('focus', this._onFocus);
+    this.element.addEventListener('blur', this._onBlur);
   }
 
   removeEventListeners() {
-    this.element.removeEventListener('input', this.onInput);
-    this.element.removeEventListener('paste', this.onPaste);
-    this.element.removeEventListener('focus', this.onFocus);
-    this.element.removeEventListener('blur', this.onBlur);
-  }
-
-  /**
-   * Input event
-   * @return
-   * @private
-   */
-  onInput() {
-    if (this.type !== 'select-one') {
-      this.setWidth();
-    }
-  }
-
-  /**
-   * Paste event
-   * @param  {Object} e Event
-   * @return
-   * @private
-   */
-  onPaste(e) {
-    // Disable pasting into the input if option has been set
-    if (e.target === this.element && this.preventPaste) {
-      e.preventDefault();
-    }
-  }
-
-  onFocus() {
-    this.isFocussed = true;
-  }
-
-  onBlur() {
-    this.isFocussed = false;
+    this.element.removeEventListener('input', this._onInput);
+    this.element.removeEventListener('paste', this._onPaste);
+    this.element.removeEventListener('focus', this._onFocus);
+    this.element.removeEventListener('blur', this._onBlur);
   }
 
   enable() {
@@ -146,4 +114,36 @@ export default class Input {
   removeActiveDescendant() {
     this.element.removeAttribute('aria-activedescendant');
   }
+
+  /**
+   * Input event
+   * @return
+   * @private
+   */
+  _onInput() {
+    if (this.type !== 'select-one') {
+      this.setWidth();
+    }
+  }
+
+  /**
+   * Paste event
+   * @param  {Object} e Event
+   * @return
+   * @private
+   */
+  _onPaste(e) {
+    // Disable pasting into the input if option has been set
+    if (e.target === this.element && this.preventPaste) {
+      e.preventDefault();
+    }
+  }
+
+  _onFocus() {
+    this.isFocussed = true;
+  }
+
+  _onBlur() {
+    this.isFocussed = false;
+  }
 }

src/scripts/src/components/input.test.js

@@ -90,7 +90,7 @@ describe('components/input', () => {
     describe('when element is select one', () => {
       it('does not set input width', () => {
         instance.type = 'select-one';
-        instance.onInput();
+        instance._onInput();
         expect(setWidthStub.callCount).to.equal(0);
       });
     });
@@ -98,7 +98,7 @@ describe('components/input', () => {
     describe('when element is not a select one', () => {
       it('sets input width', () => {
         instance.type = 'text';
-        instance.onInput();
+        instance._onInput();
         expect(setWidthStub.callCount).to.equal(1);
       });
     });
@@ -117,7 +117,7 @@ describe('components/input', () => {
     describe('when pasting is disabled and target is the element', () => {
       it('prevents default pasting behaviour', () => {
         instance.preventPaste = true;
-        instance.onPaste(eventMock);
+        instance._onPaste(eventMock);
         expect(eventMock.preventDefault.callCount).to.equal(1);
       });
     });
@@ -125,7 +125,7 @@ describe('components/input', () => {
     describe('when pasting is enabled', () => {
       it('does not prevent default pasting behaviour', () => {
         instance.preventPaste = false;
-        instance.onPaste(eventMock);
+        instance._onPaste(eventMock);
         expect(eventMock.preventDefault.callCount).to.equal(0);
       });
     });
@@ -134,7 +134,7 @@ describe('components/input', () => {
   describe('onFocus', () => {
     it('sets isFocussed flag to true', () => {
       expect(instance.isFocussed).to.equal(false);
-      instance.onFocus();
+      instance._onFocus();
       expect(instance.isFocussed).to.equal(true);
     });
   });
@@ -142,7 +142,7 @@ describe('components/input', () => {
   describe('onBlur', () => {
     it('sets isFocussed flag to false', () => {
       instance.isFocussed = true;
-      instance.onBlur();
+      instance._onBlur();
       expect(instance.isFocussed).to.equal(false);
     });
   });