From 43417510cd4b7cc7e3e79db512da859711a89427 Mon Sep 17 00:00:00 2001 From: Josh Johnson Date: Sun, 10 Dec 2017 16:41:39 +0000 Subject: [PATCH] Various unit test improvements + focus flipping --- package.json | 5 +- src/scripts/src/choices.js | 23 ++- src/scripts/src/choices.test.js | 149 +++++++++++------- src/scripts/src/components/container.js | 1 + src/scripts/src/components/container.test.js | 6 + src/scripts/src/components/dropdown.test.js | 6 + src/scripts/src/components/input.js | 21 +-- src/scripts/src/components/input.test.js | 64 ++++++-- src/scripts/src/components/list.test.js | 6 + .../src/components/wrapped-element.test.js | 6 + 10 files changed, 189 insertions(+), 98 deletions(-) diff --git a/package.json b/package.json index f96dc5c..03e60b9 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,9 @@ "scripts": { "start": "node server.js", "lint": "eslint assets/**/*.js", - "test": "nyc mocha --require ./config/test.js --compilers js:babel-core/register \"./src/**/**/**/**/*.test.js\"", - "test:watch": "npm run test -- --watch", + "test": "mocha --require ./config/test.js --compilers js:babel-core/register \"./src/**/**/**/**/*.test.js\"", + "test:watch": "npm run test -- --watch --inspect=5556", + "coverage": "nyc npm run test", "css:watch": "nodemon -e scss -x \"npm run css:build\"", "css:build": "npm run css:sass -s && npm run css:prefix -s && npm run css:min -s", "css:sass": "node-sass --output-style expanded --include-path scss src/styles/scss/base.scss src/styles/css/base.css && node-sass --output-style expanded --include-path scss src/styles/scss/choices.scss src/styles/css/choices.css", diff --git a/src/scripts/src/choices.js b/src/scripts/src/choices.js index 5424b92..b694536 100644 --- a/src/scripts/src/choices.js +++ b/src/scripts/src/choices.js @@ -672,14 +672,18 @@ class Choices { * @return {Object} Class instance * @public */ - showDropdown(focusInput = false) { + showDropdown(focusInput) { if (this.dropdown.isActive) { return this; } - this.containerOuter.open(this.dropdown.getVerticalPos()); this.dropdown.show(); - this.input.activate(focusInput); + this.containerOuter.open(this.dropdown.getVerticalPos()); + + if (focusInput && this.canSearch) { + this.input.focus(); + } + this.passedElement.triggerEvent(EVENTS.showDropdown, {}); return this; @@ -690,14 +694,19 @@ class Choices { * @return {Object} Class instance * @public */ - hideDropdown(blurInput = false) { + hideDropdown(blurInput) { if (!this.dropdown.isActive) { return this; } - this.containerOuter.close(); this.dropdown.hide(); - this.input.deactivate(blurInput); + this.containerOuter.close(); + + if (blurInput && this.canSearch) { + this.input.removeActiveDescendant(); + this.input.blur(); + } + this.passedElement.triggerEvent(EVENTS.hideDropdown, {}); return this; @@ -763,7 +772,7 @@ class Choices { * @public */ setChoiceByValue(value) { - if (this.isTextElement || !this.initialised) { + if (!this.initialised || this.isTextElement) { return this; } diff --git a/src/scripts/src/choices.test.js b/src/scripts/src/choices.test.js index 5bb69e9..ccd1f5d 100644 --- a/src/scripts/src/choices.test.js +++ b/src/scripts/src/choices.test.js @@ -9,26 +9,31 @@ describe('choices', () => { let output; let passedElement; + const returnsInstance = () => { + it('returns this', () => { + expect(output).to.eql(instance); + }); + }; + describe('public methods', () => { - const returnsInstance = () => { - it('returns this', () => { - expect(output).to.eql(instance); - }); - }; + beforeEach(() => { + passedElement = document.createElement('input'); + passedElement.type = 'text'; + passedElement.className = 'js-choices'; + document.body.appendChild(passedElement); + + instance = new Choices(passedElement); + }); afterEach(() => { output = null; + instance = null; }); describe('init', () => { const callbackOnInitSpy = spy(); beforeEach(() => { - passedElement = document.createElement('input'); - passedElement.type = 'text'; - passedElement.className = 'js-choices'; - document.body.appendChild(passedElement); - instance = new Choices(passedElement, { callbackOnInit: callbackOnInitSpy, }); @@ -322,13 +327,13 @@ describe('choices', () => { describe('showDropdown', () => { let containerOuterOpenSpy; let dropdownShowSpy; - let inputActivateSpy; + let inputFocusSpy; let passedElementTriggerEventStub; beforeEach(() => { containerOuterOpenSpy = spy(instance.containerOuter, 'open'); dropdownShowSpy = spy(instance.dropdown, 'show'); - inputActivateSpy = spy(instance.input, 'activate'); + inputFocusSpy = spy(instance.input, 'focus'); passedElementTriggerEventStub = stub(); instance.passedElement.triggerEvent = passedElementTriggerEventStub; @@ -337,13 +342,11 @@ describe('choices', () => { afterEach(() => { containerOuterOpenSpy.restore(); dropdownShowSpy.restore(); - inputActivateSpy.restore(); + inputFocusSpy.restore(); instance.passedElement.triggerEvent.reset(); }); describe('dropdown active', () => { - let output; - beforeEach(() => { instance.dropdown.isActive = true; output = instance.showDropdown(); @@ -356,7 +359,7 @@ describe('choices', () => { it('returns early', () => { expect(containerOuterOpenSpy.called).to.equal(false); expect(dropdownShowSpy.called).to.equal(false); - expect(inputActivateSpy.called).to.equal(false); + expect(inputFocusSpy.called).to.equal(false); expect(passedElementTriggerEventStub.called).to.equal(false); }); }); @@ -379,28 +382,38 @@ describe('choices', () => { expect(dropdownShowSpy.called).to.equal(true); }); - it('activates input', () => { - expect(inputActivateSpy.called).to.equal(true); - }); - it('triggers event on passedElement', () => { expect(passedElementTriggerEventStub.called).to.equal(true); expect(passedElementTriggerEventStub.lastCall.args[0]).to.eql(EVENTS.showDropdown); expect(passedElementTriggerEventStub.lastCall.args[1]).to.eql({}); }); + + describe('passing true focusInput flag with canSearch set to true', () => { + beforeEach(() => { + instance.dropdown.isActive = false; + instance.canSearch = true; + output = instance.showDropdown(true); + }); + + it('focuses input', () => { + expect(inputFocusSpy.called).to.equal(true); + }); + }); }); }); describe('hideDropdown', () => { let containerOuterCloseSpy; let dropdownHideSpy; - let inputDeactivateSpy; + let inputBlurSpy; + let inputRemoveActiveDescendantSpy; let passedElementTriggerEventStub; beforeEach(() => { containerOuterCloseSpy = spy(instance.containerOuter, 'close'); dropdownHideSpy = spy(instance.dropdown, 'hide'); - inputDeactivateSpy = spy(instance.input, 'deactivate'); + inputBlurSpy = spy(instance.input, 'blur'); + inputRemoveActiveDescendantSpy = spy(instance.input, 'removeActiveDescendant'); passedElementTriggerEventStub = stub(); instance.passedElement.triggerEvent = passedElementTriggerEventStub; @@ -409,7 +422,8 @@ describe('choices', () => { afterEach(() => { containerOuterCloseSpy.restore(); dropdownHideSpy.restore(); - inputDeactivateSpy.restore(); + inputBlurSpy.restore(); + inputRemoveActiveDescendantSpy.restore(); instance.passedElement.triggerEvent.reset(); }); @@ -424,7 +438,7 @@ describe('choices', () => { it('returns early', () => { expect(containerOuterCloseSpy.called).to.equal(false); expect(dropdownHideSpy.called).to.equal(false); - expect(inputDeactivateSpy.called).to.equal(false); + expect(inputBlurSpy.called).to.equal(false); expect(passedElementTriggerEventStub.called).to.equal(false); }); }); @@ -447,30 +461,45 @@ describe('choices', () => { expect(dropdownHideSpy.called).to.equal(true); }); - it('deactivates input', () => { - expect(inputDeactivateSpy.called).to.equal(true); - }); - it('triggers event on passedElement', () => { expect(passedElementTriggerEventStub.called).to.equal(true); expect(passedElementTriggerEventStub.lastCall.args[0]).to.eql(EVENTS.hideDropdown); expect(passedElementTriggerEventStub.lastCall.args[1]).to.eql({}); }); + + describe('passing true blurInput flag with canSearch set to true', () => { + beforeEach(() => { + instance.dropdown.isActive = true; + instance.canSearch = true; + output = instance.hideDropdown(true); + }); + + it('removes active descendants', () => { + expect(inputRemoveActiveDescendantSpy.called).to.equal(true); + }); + + it('blurs input', () => { + expect(inputBlurSpy.called).to.equal(true); + }); + }); }); }); describe('toggleDropdown', () => { - let hideDropdownSpy; - let showDropdownSpy; + let hideDropdownStub; + let showDropdownStub; beforeEach(() => { - hideDropdownSpy = spy(instance, 'hideDropdown'); - showDropdownSpy = spy(instance, 'showDropdown'); + hideDropdownStub = stub(); + showDropdownStub = stub(); + + instance.hideDropdown = hideDropdownStub; + instance.showDropdown = showDropdownStub; }); afterEach(() => { - hideDropdownSpy.restore(); - showDropdownSpy.restore(); + instance.hideDropdown.reset(); + instance.showDropdown.reset(); }); describe('dropdown active', () => { @@ -480,7 +509,7 @@ describe('choices', () => { }); it('hides dropdown', () => { - expect(hideDropdownSpy.called).to.equal(true); + expect(hideDropdownStub.called).to.equal(true); }); returnsInstance(output); @@ -493,7 +522,7 @@ describe('choices', () => { }); it('shows dropdown', () => { - expect(showDropdownSpy.called).to.equal(true); + expect(showDropdownStub.called).to.equal(true); }); returnsInstance(output); @@ -724,7 +753,7 @@ describe('choices', () => { describe('highlightAll', () => { let storeGetItemsStub; - let highlightItemSpy; + let highlightItemStub; const items = [ { @@ -739,30 +768,31 @@ describe('choices', () => { beforeEach(() => { storeGetItemsStub = stub().returns(items); - highlightItemSpy = spy(instance, 'highlightItem'); + highlightItemStub = stub(); + instance.highlightItem = highlightItemStub; instance.store.getItems = storeGetItemsStub; output = instance.highlightAll(); }); afterEach(() => { - highlightItemSpy.restore(); + highlightItemStub.reset(); instance.store.getItems.reset(); }); returnsInstance(output); it('highlights each item in store', () => { - expect(highlightItemSpy.callCount).to.equal(items.length); - expect(highlightItemSpy.firstCall.args[0]).to.equal(items[0]); - expect(highlightItemSpy.lastCall.args[0]).to.equal(items[1]); + expect(highlightItemStub.callCount).to.equal(items.length); + expect(highlightItemStub.firstCall.args[0]).to.equal(items[0]); + expect(highlightItemStub.lastCall.args[0]).to.equal(items[1]); }); }); describe('unhighlightAll', () => { let storeGetItemsStub; - let unhighlightItemSpy; + let unhighlightItemStub; const items = [ { @@ -777,24 +807,25 @@ describe('choices', () => { beforeEach(() => { storeGetItemsStub = stub().returns(items); - unhighlightItemSpy = spy(instance, 'unhighlightItem'); + unhighlightItemStub = stub(); + instance.unhighlightItem = unhighlightItemStub; instance.store.getItems = storeGetItemsStub; output = instance.unhighlightAll(); }); afterEach(() => { - unhighlightItemSpy.restore(); + instance.unhighlightItem.reset(); instance.store.getItems.reset(); }); returnsInstance(output); it('unhighlights each item in store', () => { - expect(unhighlightItemSpy.callCount).to.equal(items.length); - expect(unhighlightItemSpy.firstCall.args[0]).to.equal(items[0]); - expect(unhighlightItemSpy.lastCall.args[0]).to.equal(items[1]); + expect(unhighlightItemStub.callCount).to.equal(items.length); + expect(unhighlightItemStub.firstCall.args[0]).to.equal(items[0]); + expect(unhighlightItemStub.lastCall.args[0]).to.equal(items[1]); }); }); @@ -829,6 +860,7 @@ describe('choices', () => { inputClearSpy = spy(instance.input, 'clear'); storeDispatchStub = stub(); instance.store.dispatch = storeDispatchStub; + output = instance.clearInput(); }); afterEach(() => { @@ -836,10 +868,7 @@ describe('choices', () => { instance.store.dispatch.reset(); }); - it('returnsInstance(output)', () => { - output = instance.clearInput(); - expect(output).to.eql(instance); - }); + returnsInstance(output); describe('text element', () => { beforeEach(() => { @@ -951,9 +980,10 @@ describe('choices', () => { returnsInstance(output); - it('sets loading state', () => { + it('sets loading state', (done) => { requestAnimationFrame(() => { expect(handleLoadingStateStub.called).to.equal(true); + done(); }); }); @@ -1036,12 +1066,16 @@ describe('choices', () => { }); }); - describe('when already initialised', () => { + describe('when already initialised and not text element', () => { + beforeEach(() => { + instance.initialised = true; + instance.isTextElement = false; + }); + describe('passing a string value', () => { const value = 'Test value'; beforeEach(() => { - instance.initialised = true; output = instance.setChoiceByValue(value); }); @@ -1060,7 +1094,6 @@ describe('choices', () => { ]; beforeEach(() => { - instance.initialised = true; output = instance.setChoiceByValue(values); }); @@ -1434,14 +1467,14 @@ describe('choices', () => { }); }); - describe('passing truthy replaceChoices flag', () => { + describe('passing true replaceChoices flag', () => { it('choices are cleared', () => { instance.setChoices(choices, value, label, true); expect(clearChoicesStub.called).to.equal(true); }); }); - describe('passing falsey replaceChoices flag', () => { + describe('passing false replaceChoices flag', () => { it('choices are not cleared', () => { instance.setChoices(choices, value, label, false); expect(clearChoicesStub.called).to.equal(false); diff --git a/src/scripts/src/components/container.js b/src/scripts/src/components/container.js index f72ac56..e0dc0dd 100644 --- a/src/scripts/src/components/container.js +++ b/src/scripts/src/components/container.js @@ -79,6 +79,7 @@ export default class Container { shouldFlip = true; } + return shouldFlip; } diff --git a/src/scripts/src/components/container.test.js b/src/scripts/src/components/container.test.js index be2a02d..f9b0a6c 100644 --- a/src/scripts/src/components/container.test.js +++ b/src/scripts/src/components/container.test.js @@ -30,6 +30,12 @@ describe('components/container', () => { expect(instance.classNames).to.eql(DEFAULT_CLASSNAMES); }); + describe('getElement', () => { + it('returns DOM reference of element', () => { + expect(instance.getElement()).to.eql(choicesElement); + }); + }); + describe('addEventListeners', () => { let addEventListenerStub; diff --git a/src/scripts/src/components/dropdown.test.js b/src/scripts/src/components/dropdown.test.js index 4798504..d2e3e6d 100644 --- a/src/scripts/src/components/dropdown.test.js +++ b/src/scripts/src/components/dropdown.test.js @@ -32,6 +32,12 @@ describe('components/dropdown', () => { expect(instance.classNames).to.eql(DEFAULT_CLASSNAMES); }); + describe('getElement', () => { + it('returns DOM reference of element', () => { + expect(instance.getElement()).to.eql(choicesElement); + }); + }); + describe('getVerticalPos', () => { let top; let offset; diff --git a/src/scripts/src/components/input.js b/src/scripts/src/components/input.js index d98fe37..4a40eb1 100644 --- a/src/scripts/src/components/input.js +++ b/src/scripts/src/components/input.js @@ -71,21 +71,6 @@ export default class Input { this.isFocussed = false; } - activate(focusInput) { - // Optionally focus the input if we have a search input - if (focusInput && this.parentInstance.canSearch && document.activeElement !== this.element) { - this.element.focus(); - } - } - - deactivate(blurInput) { - this.removeActiveDescendant(); - // Optionally blur the input if we have a search input - if (blurInput && this.parentInstance.canSearch && document.activeElement === this.element) { - this.element.blur(); - } - } - enable() { this.element.removeAttribute('disabled'); this.isDisabled = false; @@ -102,6 +87,12 @@ export default class Input { } } + blur() { + if (this.isFocussed) { + this.element.blur(); + } + } + /** * Set value of input to blank * @return {Object} Class instance diff --git a/src/scripts/src/components/input.test.js b/src/scripts/src/components/input.test.js index 6f15995..336ede2 100644 --- a/src/scripts/src/components/input.test.js +++ b/src/scripts/src/components/input.test.js @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import sinon from 'sinon'; +import { stub } from 'sinon'; import Input from './input'; import { DEFAULT_CLASSNAMES, DEFAULT_CONFIG } from '../constants'; @@ -18,23 +18,31 @@ describe('components/input', () => { instance = new Input(choicesInstance, choicesElement, DEFAULT_CLASSNAMES); }); - it('assigns choices instance to class', () => { - expect(instance.parentInstance).to.eql(choicesInstance); + describe('constructor', () => { + it('assigns choices instance to class', () => { + expect(instance.parentInstance).to.eql(choicesInstance); + }); + + it('assigns choices element to class', () => { + expect(instance.element).to.eql(choicesElement); + }); + + it('assigns classnames to class', () => { + expect(instance.classNames).to.eql(DEFAULT_CLASSNAMES); + }); }); - it('assigns choices element to class', () => { - expect(instance.element).to.eql(choicesElement); - }); - - it('assigns classnames to class', () => { - expect(instance.classNames).to.eql(DEFAULT_CLASSNAMES); + describe('getElement', () => { + it('returns DOM reference of element', () => { + expect(instance.getElement()).to.eql(choicesElement); + }); }); describe('addEventListeners', () => { let addEventListenerStub; beforeEach(() => { - addEventListenerStub = sinon.stub(instance.element, 'addEventListener'); + addEventListenerStub = stub(instance.element, 'addEventListener'); }); afterEach(() => { @@ -55,7 +63,7 @@ describe('components/input', () => { let removeEventListenerStub; beforeEach(() => { - removeEventListenerStub = sinon.stub(instance.element, 'removeEventListener'); + removeEventListenerStub = stub(instance.element, 'removeEventListener'); }); afterEach(() => { @@ -76,7 +84,7 @@ describe('components/input', () => { let setWidthStub; beforeEach(() => { - setWidthStub = sinon.stub(instance, 'setWidth'); + setWidthStub = stub(instance, 'setWidth'); }); afterEach(() => { @@ -106,7 +114,7 @@ describe('components/input', () => { beforeEach(() => { eventMock = { - preventDefault: sinon.stub(), + preventDefault: stub(), target: instance.element, }; }); @@ -144,6 +152,30 @@ describe('components/input', () => { }); }); + // describe('activate', () => { + // describe('when passed focusInput argument is true, canSearch is true and current element is not in focus', () => { + // let focusSpy; + // beforeEach(() => { + // instance.parentInstance.canSearch = true; + // focusSpy = spy(instance.element, 'focus'); + // }); + + // afterEach(() => { + // focusSpy.restore(); + // }); + + // it('focuses element', () => { + // expect(focusSpy.callCount).to.equal(0); + // instance.activate(true); + // expect(focusSpy.callCount).to.equal(1); + // }); + // }); + // }); + + describe('deactivate', () => { + + }); + describe('enable', () => { beforeEach(() => { instance.element.setAttribute('disabled', ''); @@ -180,7 +212,7 @@ describe('components/input', () => { let focusStub; beforeEach(() => { - focusStub = sinon.stub(instance.element, 'focus'); + focusStub = stub(instance.element, 'focus'); }); afterEach(() => { @@ -198,7 +230,7 @@ describe('components/input', () => { let setWidthStub; beforeEach(() => { - setWidthStub = sinon.stub(instance, 'setWidth'); + setWidthStub = stub(instance, 'setWidth'); }); afterEach(() => { @@ -230,7 +262,7 @@ describe('components/input', () => { const inputWidth = '200px'; beforeEach(() => { - getWidthStub = sinon.stub(instance, 'getWidth').returns(inputWidth); + getWidthStub = stub(instance, 'getWidth').returns(inputWidth); }); afterEach(() => { diff --git a/src/scripts/src/components/list.test.js b/src/scripts/src/components/list.test.js index a97c5a5..8c6e874 100644 --- a/src/scripts/src/components/list.test.js +++ b/src/scripts/src/components/list.test.js @@ -29,6 +29,12 @@ describe('components/list', () => { expect(instance.classNames).to.eql(DEFAULT_CLASSNAMES); }); + describe('getElement', () => { + it('returns DOM reference of element', () => { + expect(instance.getElement()).to.eql(choicesElement); + }); + }); + describe('clear', () => { it('clears element\'s inner HTML', () => { const innerHTML = 'test'; diff --git a/src/scripts/src/components/wrapped-element.test.js b/src/scripts/src/components/wrapped-element.test.js index 9908145..e786f42 100644 --- a/src/scripts/src/components/wrapped-element.test.js +++ b/src/scripts/src/components/wrapped-element.test.js @@ -18,6 +18,12 @@ describe('components/wrappedElement', () => { instance = new WrappedElement(choicesInstance, choicesElement, DEFAULT_CLASSNAMES); }); + describe('getElement', () => { + it('returns DOM reference of element', () => { + expect(instance.getElement()).to.eql(choicesElement); + }); + }); + describe('conceal', () => { let originalStyling;