Resolve undefined error (#528)

* Remove run-p from test command

* Remove dropdown interaction tests

* Tidy utils

* Use merge lib

* Remove string casting

* Sanitise in constants

* Housekeeping

* Add non-string value tests

.eslintrc

@@ -42,6 +42,7 @@
     "no-plusplus": "off",
     "no-unused-expressions": "off",
     "no-underscore-dangle": "off",
+    "consistent-return": "off",
     "prettier/prettier": [
       "error",
       {

cypress/integration/select-multiple.spec.js

@@ -68,7 +68,7 @@ describe('Choices - select multiple', () => {
               .click();
           });
 
-          it('allows me select choices from a dropdown', () => {
+          it('allows selecting choices from dropdown', () => {
             cy.get('[data-test-hook=basic]')
               .find('.choices__list--multiple .choices__item')
               .last()
@@ -211,44 +211,6 @@ describe('Choices - select multiple', () => {
         });
       });
 
-      /*
-        There is currently a bug with opening/closing/toggling dropdowns
-
-        @todo Investigate why
-      */
-      describe('interacting with dropdown', () => {
-        describe('opening dropdown', () => {
-          it('opens dropdown', () => {
-            cy.get('[data-test-hook=basic]')
-              .find('button.open-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('.choices__list--dropdown')
-              .should('be.visible');
-          });
-        });
-
-        describe('closing dropdown', () => {
-          it('closes dropdown', () => {
-            cy.get('[data-test-hook=basic]')
-              .find('button.open-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('button.close-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('.choices__list--dropdown')
-              .should('not.be.visible');
-          });
-        });
-      });
-
       describe('disabling', () => {
         describe('on disable', () => {
           it('disables the search input', () => {
@@ -754,6 +716,40 @@ describe('Choices - select multiple', () => {
       });
     });
 
+    describe('non-string values', () => {
+      beforeEach(() => {
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices')
+          .click();
+      });
+
+      it('displays expected amount of choices in dropdown', () => {
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--dropdown .choices__list')
+          .children()
+          .should('have.length', 4);
+      });
+
+      it('allows selecting choices from dropdown', () => {
+        let $selectedChoice;
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--dropdown .choices__list')
+          .children()
+          .first()
+          .then($choice => {
+            $selectedChoice = $choice;
+          })
+          .click();
+
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--single .choices__item')
+          .last()
+          .should($item => {
+            expect($item.text().trim()).to.equal($selectedChoice.text().trim());
+          });
+      });
+    });
+
     describe('within form', () => {
       describe('selecting choice', () => {
         describe('on enter key', () => {

cypress/integration/select-one.spec.js

@@ -45,7 +45,7 @@ describe('Choices - select one', () => {
       describe('selecting choices', () => {
         const selectedChoiceText = 'Choice 1';
 
-        it('allows me select choices from a dropdown', () => {
+        it('allows selecting choices from dropdown', () => {
           cy.get('[data-test-hook=basic]')
             .find('.choices__list--dropdown .choices__list')
             .children()
@@ -129,44 +129,6 @@ describe('Choices - select one', () => {
         });
       });
 
-      /*
-        There is currently a bug with opening/closing/toggling dropdowns
-
-        @todo Investigate why
-      */
-      describe('interacting with dropdown', () => {
-        describe('opening dropdown', () => {
-          it('opens dropdown', () => {
-            cy.get('[data-test-hook=basic]')
-              .find('button.open-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('.choices__list--dropdown')
-              .should('be.visible');
-          });
-        });
-
-        describe('closing dropdown', () => {
-          it('closes dropdown', () => {
-            cy.get('[data-test-hook=basic]')
-              .find('button.open-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('button.close-dropdown')
-              .focus()
-              .click();
-
-            cy.get('[data-test-hook=basic]')
-              .find('.choices__list--dropdown')
-              .should('not.be.visible');
-          });
-        });
-      });
-
       describe('disabling', () => {
         describe('on disable', () => {
           it('disables the search input', () => {
@@ -418,7 +380,7 @@ describe('Choices - select one', () => {
           .should('not.exist');
       });
 
-      it('allows me select choices from a dropdown', () => {
+      it('allows selecting choices from dropdown', () => {
         cy.get('[data-test-hook=search-disabled]')
           .find('.choices__list--dropdown .choices__list')
           .children()
@@ -766,6 +728,40 @@ describe('Choices - select one', () => {
       });
     });
 
+    describe('non-string values', () => {
+      beforeEach(() => {
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices')
+          .click();
+      });
+
+      it('displays expected amount of choices in dropdown', () => {
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--dropdown .choices__list')
+          .children()
+          .should('have.length', 4);
+      });
+
+      it('allows selecting choices from dropdown', () => {
+        let $selectedChoice;
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--dropdown .choices__list')
+          .children()
+          .first()
+          .then($choice => {
+            $selectedChoice = $choice;
+          })
+          .click();
+
+        cy.get('[data-test-hook=non-string-values]')
+          .find('.choices__list--single .choices__item')
+          .last()
+          .should($item => {
+            expect($item.text().trim()).to.equal($selectedChoice.text().trim());
+          });
+      });
+    });
+
     describe('within form', () => {
       describe('selecting choice', () => {
         describe('on enter key', () => {

package.json

@@ -12,7 +12,7 @@
     "bundlesize": "bundlesize",
     "cypress:run": "$(npm bin)/cypress run",
     "cypress:open": "$(npm bin)/cypress open",
-    "test": "run-p test:unit test:e2e",
+    "test": "npm run test:unit && npm run test:e2e",
     "test:unit": "mocha --require ./config/jsdom.js --require @babel/register $(find src -name '*.test.js') --exit",
     "test:unit:watch": "npm run test:unit -- --watch --inspect=5556",
     "test:e2e": "run-p --race start cypress:run",

public/test/select-multiple.html

@@ -31,8 +31,6 @@
         <h2>Select multiple inputs</h2>
         <div data-test-hook="basic">
           <label for="choices-basic">Basic</label>
-          <button class="open-dropdown push-bottom">Open dropdown</button>
-          <button class="close-dropdown push-bottom">Close dropdown</button>
           <button class="disable push-bottom">Disable</button>
           <button class="enable push-bottom">Enable</button>
           <select class="form-control" name="choices-basic" id="choices-basic" multiple>
@@ -176,6 +174,11 @@
           <select class="form-control" name="choices-custom-properties" id="choices-custom-properties" multiple></select>
         </div>
 
+        <div data-test-hook="non-string-values">
+          <label for="choices-non-string-values">Non-string values</label>
+          <select class="form-control" name="choices-non-string-values" id="choices-non-string-values"></select>
+        </div>
+
         <div data-test-hook="within-form">
           <form>
             <label for="choices-within-form">Within form</label>
@@ -207,14 +210,6 @@
       document.addEventListener('DOMContentLoaded', function() {
         const choicesBasic = new Choices('#choices-basic');
 
-        document.querySelector('button.open-dropdown').addEventListener('click', () => {
-          choicesBasic.showDropdown();
-        });
-
-        document.querySelector('button.close-dropdown').addEventListener('click', () => {
-          choicesBasic.hideDropdown();
-        });
-
         document.querySelector('button.disable').addEventListener('click', () => {
           choicesBasic.disable();
         });
@@ -303,8 +298,35 @@
               customProperties: {
                 country: 'Portugal',
               },
-            }
+            },
-          ]
+          ],
+        });
+
+        new Choices('#choices-non-string-values', {
+          choices: [
+            {
+              id: 1,
+              label: 'Number',
+              value: 1,
+            },
+            {
+              id: 2,
+              label: 'Boolean',
+              value: true,
+            },
+            {
+              id: 3,
+              label: 'Object',
+              value: {
+                test: true,
+              },
+            },
+            {
+              id: 4,
+              label: 'Array',
+              value: ['test'],
+            },
+          ],
         });
 
         new Choices('#choices-within-form');

public/test/select-one.html

@@ -31,8 +31,6 @@
         <h2>Select one inputs</h2>
         <div data-test-hook="basic">
           <label for="choices-basic">Basic</label>
-          <button class="open-dropdown push-bottom">Open dropdown</button>
-          <button class="close-dropdown push-bottom">Close dropdown</button>
           <button class="disable push-bottom">Disable</button>
           <button class="enable push-bottom">Enable</button>
           <select class="form-control" name="choices-basic" id="choices-basic">
@@ -180,6 +178,11 @@
           <select class="form-control" name="choices-custom-properties" id="choices-custom-properties"></select>
         </div>
 
+        <div data-test-hook="non-string-values">
+          <label for="choices-non-string-values">Non-string values</label>
+          <select class="form-control" name="choices-non-string-values" id="choices-non-string-values"></select>
+        </div>
+
         <div data-test-hook="within-form">
           <form>
             <label for="choices-within-form">Within form</label>
@@ -211,14 +214,6 @@
       document.addEventListener('DOMContentLoaded', function() {
         const choicesBasic = new Choices('#choices-basic');
 
-        document.querySelector('button.open-dropdown').addEventListener('click', () => {
-          choicesBasic.showDropdown();
-        });
-
-        document.querySelector('button.close-dropdown').addEventListener('click', () => {
-          choicesBasic.hideDropdown();
-        });
-
         document.querySelector('button.disable').addEventListener('click', () => {
           choicesBasic.disable();
         });
@@ -319,6 +314,33 @@
           ]
         });
 
+        new Choices('#choices-non-string-values', {
+          choices: [
+            {
+              id: 1,
+              label: 'Number',
+              value: 1,
+            },
+            {
+              id: 2,
+              label: 'Boolean',
+              value: true,
+            },
+            {
+              id: 3,
+              label: 'Object',
+              value: {
+                test: true,
+              },
+            },
+            {
+              id: 4,
+              label: 'Array',
+              value: ['test'],
+            },
+          ],
+        });
+
         new Choices('#choices-within-form');
 
         new Choices('#choices-set-choice-by-value').setChoiceByValue('Choice 2');

src/scripts/choices.js

@@ -29,7 +29,6 @@ import {
   getType,
   isType,
   strToEl,
-  extend,
   sortByScore,
   generateId,
   findAncestorByAttrName,
@@ -1787,7 +1786,7 @@ class Choices {
       userTemplates = callbackOnCreateTemplates.call(this, strToEl);
     }
 
-    this.config.templates = extend(TEMPLATES, userTemplates);
+    this.config.templates = merge(TEMPLATES, userTemplates);
   }
 
   _createElements() {

src/scripts/components/input.js

@@ -1,15 +1,12 @@
-import { calcWidthOfInput, stripHTML } from '../lib/utils';
+import { calcWidthOfInput, sanitise } from '../lib/utils';
 
 export default class Input {
   constructor({ element, type, classNames, placeholderValue }) {
     Object.assign(this, { element, type, classNames, placeholderValue });
-
     this.element = element;
     this.classNames = classNames;
     this.isFocussed = this.element === document.activeElement;
     this.isDisabled = false;
-
-    // Bind event listeners
     this._onPaste = this._onPaste.bind(this);
     this._onInput = this._onInput.bind(this);
     this._onFocus = this._onFocus.bind(this);
@@ -21,11 +18,11 @@ export default class Input {
   }
 
   set value(value) {
-    this.element.value = `${value}`;
+    this.element.value = value;
   }
 
   get value() {
-    return stripHTML(this.element.value);
+    return sanitise(this.element.value);
   }
 
   addEventListeners() {
@@ -134,7 +131,6 @@ export default class Input {
 
   _onPaste(event) {
     const { target } = event;
-    // Disable pasting into the input if option has been set
     if (target === this.element && this.preventPaste) {
       event.preventDefault();
     }

src/scripts/components/wrapped-input.js

@@ -1,5 +1,4 @@
 import WrappedElement from './wrapped-element';
-import { reduceToValues } from './../lib/utils';
 
 export default class WrappedInput extends WrappedElement {
   constructor({ element, classNames, delimiter }) {
@@ -8,11 +7,11 @@ export default class WrappedInput extends WrappedElement {
   }
 
   set value(items) {
-    const itemsFiltered = reduceToValues(items);
+    const itemValues = items.map(({ value }) => value);
-    const itemsFilteredString = itemsFiltered.join(this.delimiter);
+    const joinedValues = itemValues.join(this.delimiter);
 
-    this.element.setAttribute('value', itemsFilteredString);
+    this.element.setAttribute('value', joinedValues);
-    this.element.value = itemsFilteredString;
+    this.element.value = joinedValues;
   }
 
   // @todo figure out why we need this? Perhaps a babel issue

src/scripts/constants.js

@@ -1,4 +1,4 @@
-import { stripHTML, sortByAlpha } from './lib/utils';
+import { sanitise, sortByAlpha } from './lib/utils';
 
 export const DEFAULT_CLASSNAMES = {
   containerOuter: 'choices',
@@ -65,7 +65,7 @@ export const DEFAULT_CONFIG = {
   itemSelectText: 'Press to select',
   uniqueItemText: 'Only unique values can be added',
   customAddItemText: 'Only values matching specific conditions can be added',
-  addItemText: value => `Press Enter to add <b>"${stripHTML(value)}"</b>`,
+  addItemText: value => `Press Enter to add <b>"${sanitise(value)}"</b>`,
   maxItemText: maxItemCount => `Only ${maxItemCount} values can be added`,
   itemComparer: (choice, item) => choice === item,
   fuseOptions: {

src/scripts/lib/utils.js

@@ -1,10 +1,7 @@
-/* eslint-disable */
+export const getRandomNumber = (min, max) =>
+  Math.floor(Math.random() * (max - min) + min);
 
-export const getRandomNumber = function(min, max) {
+export const generateChars = length => {
-  return Math.floor(Math.random() * (max - min) + min);
-};
-
-export const generateChars = function(length) {
   let chars = '';
 
   for (let i = 0; i < length; i++) {
@@ -15,7 +12,7 @@ export const generateChars = function(length) {
   return chars;
 };
 
-export const generateId = function(element, prefix) {
+export const generateId = (element, prefix) => {
   let id =
     element.id ||
     (element.name && `${element.name}-${generateChars(2)}`) ||
@@ -26,56 +23,14 @@ export const generateId = function(element, prefix) {
   return id;
 };
 
-export const getType = function(obj) {
+export const getType = obj => Object.prototype.toString.call(obj).slice(8, -1);
-  return Object.prototype.toString.call(obj).slice(8, -1);
-};
 
-export const isType = function(type, obj) {
+export const isType = (type, obj) =>
-  const clas = getType(obj);
+  obj !== undefined && obj !== null && getType(obj) === type;
-  return obj !== undefined && obj !== null && clas === type;
-};
 
-export const isElement = (element) => {
+export const isElement = element => element instanceof Element;
-  return element instanceof Element;
-};
 
-export const extend = function() {
+export const wrap = (element, wrapper = document.createElement('div')) => {
-  const extended = {};
-  const length = arguments.length;
-
-  /**
-   * Merge one object into another
-   * @param  {Object} obj  Object to merge into extended object
-   */
-  const merge = function(obj) {
-    for (const prop in obj) {
-      if (Object.prototype.hasOwnProperty.call(obj, prop)) {
-        // If deep merge and property is an object, merge properties
-        if (isType('Object', obj[prop])) {
-          extended[prop] = extend(true, extended[prop], obj[prop]);
-        } else {
-          extended[prop] = obj[prop];
-        }
-      }
-    }
-  };
-
-  // Loop through each passed argument
-  for (let i = 0; i < length; i++) {
-    // store argument at position i
-    const obj = arguments[i];
-
-    // If we are in fact dealing with an object, merge it.
-    if (isType('Object', obj)) {
-      merge(obj);
-    }
-  }
-
-  return extended;
-};
-
-export const wrap = function(element, wrapper) {
-  wrapper = wrapper || document.createElement('div');
   if (element.nextSibling) {
     element.parentNode.insertBefore(wrapper, element.nextSibling);
   } else {
@@ -84,12 +39,7 @@ export const wrap = function(element, wrapper) {
   return wrapper.appendChild(element);
 };
 
-export const findAncestor = function(el, cls) {
+export const findAncestorByAttrName = (el, attr) => {
-  while ((el = el.parentElement) && !el.classList.contains(cls));
-  return el;
-};
-
-export const findAncestorByAttrName = function(el, attr) {
   let target = el;
 
   while (target) {
@@ -104,7 +54,9 @@ export const findAncestorByAttrName = function(el, attr) {
 };
 
 export const getAdjacentEl = (startEl, className, direction = 1) => {
-  if (!startEl || !className) return;
+  if (!startEl || !className) {
+    return;
+  }
 
   const parent = startEl.parentNode.parentNode;
   const children = Array.from(parent.querySelectorAll(className));
@@ -116,7 +68,9 @@ export const getAdjacentEl = (startEl, className, direction = 1) => {
 };
 
 export const isScrolledIntoView = (el, parent, direction = 1) => {
-  if (!el) return;
+  if (!el) {
+    return;
+  }
 
   let isVisible;
 
@@ -132,26 +86,30 @@ export const isScrolledIntoView = (el, parent, direction = 1) => {
   return isVisible;
 };
 
-export const stripHTML = html =>
+export const sanitise = value => {
-  html
+  if (!isType('String', value)) {
+    return value;
+  }
+
+  return value
     .replace(/&/g, '&amp;')
     .replace(/>/g, '&rt;')
     .replace(/</g, '&lt;')
     .replace(/"/g, '&quot;');
+};
 
-export const strToEl = (function() {
+export const strToEl = (() => {
   const tmpEl = document.createElement('div');
-  return function(str) {
+  return str => {
     const cleanedInput = str.trim();
-    let r;
     tmpEl.innerHTML = cleanedInput;
-    r = tmpEl.children[0];
+    const firldChild = tmpEl.children[0];
 
     while (tmpEl.firstChild) {
       tmpEl.removeChild(tmpEl.firstChild);
     }
 
-    return r;
+    return firldChild;
   };
 })();
 
@@ -164,7 +122,7 @@ export const calcWidthOfInput = (input, callback) => {
   let width = input.offsetWidth;
 
   if (value) {
-    const testEl = strToEl(`<span>${stripHTML(value)}</span>`);
+    const testEl = strToEl(`<span>${sanitise(value)}</span>`);
     testEl.style.position = 'absolute';
     testEl.style.padding = '0';
     testEl.style.top = '-9999px';
@@ -203,8 +161,8 @@ export const calcWidthOfInput = (input, callback) => {
 };
 
 export const sortByAlpha = (a, b) => {
-  const labelA = (a.label || a.value).toLowerCase();
+  const labelA = `${a.label || a.value}`.toLowerCase();
-  const labelB = (b.label || b.value).toLowerCase();
+  const labelB = `${b.label || b.value}`.toLowerCase();
 
   if (labelA < labelB) {
     return -1;
@@ -241,15 +199,6 @@ export const getWindowHeight = () => {
   );
 };
 
-export const reduceToValues = (items, key = 'value') => {
-  const values = items.reduce((prev, current) => {
-    prev.push(current[key]);
-    return prev;
-  }, []);
-
-  return values;
-};
-
 export const fetchFromObject = (object, path) => {
   const index = path.indexOf('.');
 
@@ -284,7 +233,5 @@ export const diff = (a, b) => {
   const aKeys = Object.keys(a).sort();
   const bKeys = Object.keys(b).sort();
 
-  return aKeys.filter((i) => {
+  return aKeys.filter(i => bKeys.indexOf(i) < 0);
-    return bKeys.indexOf(i) < 0;
+};
-  });
-}

src/scripts/lib/utils.test.js

@@ -1,14 +1,13 @@
 import { expect } from 'chai';
 import { stub } from 'sinon';
 import {
-  reduceToValues,
   getRandomNumber,
   generateChars,
   generateId,
   getType,
   isType,
   isElement,
-  stripHTML,
+  sanitise,
   sortByAlpha,
   sortByScore,
   fetchFromObject,
@@ -18,54 +17,6 @@ import {
 } from './utils';
 
 describe('utils', () => {
-  describe('reduceToValues', () => {
-    const items = [
-      {
-        id: 1,
-        choiceId: 1,
-        groupId: -1,
-        value: 'Item one',
-        label: 'Item one',
-        active: false,
-        highlighted: false,
-        customProperties: null,
-        placeholder: false,
-        keyCode: null,
-      },
-      {
-        id: 2,
-        choiceId: 2,
-        groupId: -1,
-        value: 'Item two',
-        label: 'Item two',
-        active: true,
-        highlighted: false,
-        customProperties: null,
-        placeholder: false,
-        keyCode: null,
-      },
-      {
-        id: 3,
-        choiceId: 3,
-        groupId: -1,
-        value: 'Item three',
-        label: 'Item three',
-        active: true,
-        highlighted: true,
-        customProperties: null,
-        placeholder: false,
-        keyCode: null,
-      },
-    ];
-
-    it('returns an array of item values', () => {
-      const expectedResponse = [items[0].value, items[1].value, items[2].value];
-
-      const actualResponse = reduceToValues(items);
-      expect(actualResponse).to.eql(expectedResponse);
-    });
-  });
-
   describe('getRandomNumber', () => {
     it('returns random number between range', () => {
       for (let index = 0; index < 10; index++) {
@@ -154,10 +105,10 @@ describe('utils', () => {
     });
   });
 
-  describe('stripHTML', () => {
+  describe('sanitise', () => {
     it('strips HTML from value', () => {
       const value = '<script>somethingMalicious();</script>';
-      const output = stripHTML(value);
+      const output = sanitise(value);
       expect(output).to.equal(
         '&lt;script&rt;somethingMalicious();&lt;/script&rt;',
       );
@@ -247,25 +198,6 @@ describe('utils', () => {
     });
   });
 
-  describe('reduceToValues', () => {
-    it('reduces an array of objects to an array of values using given key', () => {
-      const values = [
-        { name: 'The Strokes' },
-        { name: 'Arctic Monkeys' },
-        { name: 'Oasis' },
-        { name: 'Tame Impala' },
-      ];
-
-      const output = reduceToValues(values, 'name');
-      expect(output).to.eql([
-        'The Strokes',
-        'Arctic Monkeys',
-        'Oasis',
-        'Tame Impala',
-      ]);
-    });
-  });
-
   describe('fetchFromObject', () => {
     it('fetches value from object using given path', () => {
       const object = {