From 6aacbc4403df756591e9baa231b101cf954d60d4 Mon Sep 17 00:00:00 2001 From: qa46hx Date: Thu, 24 Sep 2020 13:38:07 +0200 Subject: [PATCH] fix(switch): dispatch checked-changed on checked change --- packages/switch/src/LionSwitch.js | 21 ++++++++++---- packages/switch/src/LionSwitchButton.js | 28 +++++++++++++----- .../switch/test/lion-switch-button.test.js | 17 +++++++++++ packages/switch/test/lion-switch.test.js | 29 +++++++++++++++++++ 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/packages/switch/src/LionSwitch.js b/packages/switch/src/LionSwitch.js index 872e2cff3..d2d778561 100644 --- a/packages/switch/src/LionSwitch.js +++ b/packages/switch/src/LionSwitch.js @@ -51,12 +51,18 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField)) return html`${this._inputGroupTemplate()}`; } + constructor() { + super(); + this.role = 'switch'; + this.checked = false; + this.__handleButtonSwitchCheckedChanged = this.__handleButtonSwitchCheckedChanged.bind(this); + } + connectedCallback() { super.connectedCallback(); - this._inputNode.addEventListener( - 'checked-changed', - this.__handleButtonSwitchCheckedChanged.bind(this), - ); + if (this._inputNode) { + this._inputNode.addEventListener('checked-changed', this.__handleButtonSwitchCheckedChanged); + } if (this._labelNode) { this._labelNode.addEventListener('click', this.__toggleChecked); } @@ -65,6 +71,12 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField)) } disconnectedCallback() { + if (this._inputNode) { + this._inputNode.removeEventListener( + 'checked-changed', + this.__handleButtonSwitchCheckedChanged, + ); + } if (this._labelNode) { this._labelNode.removeEventListener('click', this.__toggleChecked); } @@ -86,7 +98,6 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField)) } _syncButtonSwitch() { - this._inputNode.checked = this.checked; this._inputNode.disabled = this.disabled; } } diff --git a/packages/switch/src/LionSwitchButton.js b/packages/switch/src/LionSwitchButton.js index a51987a2f..3c89ed260 100644 --- a/packages/switch/src/LionSwitchButton.js +++ b/packages/switch/src/LionSwitchButton.js @@ -74,29 +74,43 @@ export class LionSwitchButton extends DisabledWithTabIndexMixin(LitElement) { super(); this.role = 'switch'; this.checked = false; - this.addEventListener('click', this.__handleToggleStateChange); - this.addEventListener('keydown', this.__handleKeydown); - this.addEventListener('keyup', this.__handleKeyup); + this.__toggleChecked = this.__toggleChecked.bind(this); + this.__handleKeydown = this.__handleKeydown.bind(this); + this.__handleKeyup = this.__handleKeyup.bind(this); } connectedCallback() { super.connectedCallback(); this.setAttribute('aria-checked', `${this.checked}`); + this.addEventListener('click', this.__toggleChecked); + this.addEventListener('keydown', this.__handleKeydown); + this.addEventListener('keyup', this.__handleKeyup); } - __handleToggleStateChange() { + disconnectedCallback() { + super.disconnectedCallback(); + this.removeEventListener('click', this.__toggleChecked); + this.removeEventListener('keydown', this.__handleKeydown); + this.removeEventListener('keyup', this.__handleKeyup); + } + + __toggleChecked() { if (this.disabled) { return; } // Force IE11 to focus the component. this.focus(); this.checked = !this.checked; + } + + __checkedStateChange() { this.dispatchEvent( new Event('checked-changed', { composed: true, bubbles: true, }), ); + this.setAttribute('aria-checked', `${this.checked}`); } // eslint-disable-next-line class-methods-use-this @@ -109,7 +123,7 @@ export class LionSwitchButton extends DisabledWithTabIndexMixin(LitElement) { __handleKeyup(e) { if ([32 /* space */, 13 /* enter */].indexOf(e.keyCode) !== -1) { - this.__handleToggleStateChange(); + this.__toggleChecked(); } } @@ -126,8 +140,8 @@ export class LionSwitchButton extends DisabledWithTabIndexMixin(LitElement) { */ requestUpdateInternal(name, oldValue) { super.requestUpdateInternal(name, oldValue); - if (this.isConnected && name === 'checked') { - this.setAttribute('aria-checked', `${this.checked}`); + if (this.isConnected && name === 'checked' && this.checked !== oldValue) { + this.__checkedStateChange(); } } } diff --git a/packages/switch/test/lion-switch-button.test.js b/packages/switch/test/lion-switch-button.test.js index 09a8b05be..859a39ec8 100644 --- a/packages/switch/test/lion-switch-button.test.js +++ b/packages/switch/test/lion-switch-button.test.js @@ -73,6 +73,23 @@ describe('lion-switch-button', () => { checkCall(handlerSpy.getCall(1), false); }); + it('should dispatch "checked-changed" event when checked changed', () => { + const handlerSpy = sinon.spy(); + el.addEventListener('checked-changed', handlerSpy); + el.checked = true; + el.checked = false; + expect(handlerSpy.callCount).to.equal(2); + const checkCall = call => { + expect(call.args).to.have.a.lengthOf(1); + const e = call.args[0]; + expect(e).to.be.an.instanceof(Event); + expect(e.bubbles).to.be.true; + expect(e.composed).to.be.true; + }; + checkCall(handlerSpy.getCall(0), true); + checkCall(handlerSpy.getCall(1), false); + }); + it('should not dispatch "checked-changed" event if disabled', () => { const handlerSpy = sinon.spy(); el.disabled = true; diff --git a/packages/switch/test/lion-switch.test.js b/packages/switch/test/lion-switch.test.js index 95686b872..0622c6094 100644 --- a/packages/switch/test/lion-switch.test.js +++ b/packages/switch/test/lion-switch.test.js @@ -1,4 +1,5 @@ import { expect, fixture, html } from '@open-wc/testing'; +import sinon from 'sinon'; import '../lion-switch.js'; describe('lion-switch', () => { @@ -68,6 +69,34 @@ describe('lion-switch', () => { }); }); + it('should dispatch "checked-changed" event when toggled via button or label', async () => { + const handlerSpy = sinon.spy(); + const el = await fixture(html``); + el.addEventListener('checked-changed', handlerSpy); + el._inputNode.click(); + el._labelNode.click(); + await el.updateComplete; + expect(handlerSpy.callCount).to.equal(2); + const checkCall = call => { + expect(call.args).to.have.a.lengthOf(1); + const e = call.args[0]; + expect(e).to.be.an.instanceof(Event); + expect(e.bubbles).to.be.true; + expect(e.composed).to.be.true; + }; + checkCall(handlerSpy.getCall(0), true); + checkCall(handlerSpy.getCall(1), false); + }); + + it('should dispatch "checked-changed" event when checked changed', async () => { + const handlerSpy = sinon.spy(); + const el = await fixture(html``); + el.addEventListener('checked-changed', handlerSpy); + el.checked = true; + await el.updateComplete; + expect(handlerSpy.callCount).to.equal(1); + }); + it('is submitted by default', async () => { const el = await fixture(html``); expect(el.submitted).to.be.true;