From 8cd22107ea52454426552a3f6b6462b3cf804237 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 28 Oct 2020 11:57:21 +0100 Subject: [PATCH] fix(listbox): enable deselection on Enter/Space for multiselect --- packages/listbox/src/ListboxMixin.js | 8 ++++++-- .../listbox/test-suites/ListboxMixin.suite.js | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index 4fe0ca26f..20f0d370e 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -348,7 +348,7 @@ const ListboxMixinImplementation = superclass => this._uncheckChildren(this.formElements.filter(i => i === index)); index.forEach(i => { if (this.formElements[i]) { - this.formElements[i].checked = true; + this.formElements[i].checked = !this.formElements[i].checked; } }); return; @@ -359,7 +359,11 @@ const ListboxMixinImplementation = superclass => this._uncheckChildren(); } if (this.formElements[index]) { - this.formElements[index].checked = true; + if (this.multipleChoice) { + this.formElements[index].checked = !this.formElements[index].checked; + } else { + this.formElements[index].checked = true; + } } } } diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 59816fd60..7da50a908 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -370,7 +370,7 @@ export function runListboxMixinSuite(customConfig = {}) { it('has a reference to the active option', async () => { const el = await fixture(html` - <${tag} opened has-no-default-selected autocomplete="list"> + <${tag} opened has-no-default-selected autocomplete="none"> <${optionTag} .choiceValue=${'10'} id="first">Item 1 <${optionTag} .choiceValue=${'20'} checked id="second">Item 2 @@ -382,10 +382,6 @@ export function runListboxMixinSuite(customConfig = {}) { // Normalize el.activeIndex = 0; - - // el._activeDescendantOwnerNode.dispatchEvent( - // new KeyboardEvent('keydown', { key: 'ArrowDown' }), - // ); await el.updateComplete; expect(activeDescendantOwner.getAttribute('aria-activedescendant')).to.equal('first'); activeDescendantOwner.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown' })); @@ -829,6 +825,10 @@ export function runListboxMixinSuite(customConfig = {}) { options[1].click(); expect(options[0].checked).to.equal(true); expect(el.modelValue).to.eql(['Artichoke', 'Chard']); + // also deselect + options[1].click(); + expect(options[0].checked).to.equal(true); + expect(el.modelValue).to.eql(['Artichoke']); // Reset // @ts-ignore allow protected members in tests @@ -841,6 +841,10 @@ export function runListboxMixinSuite(customConfig = {}) { listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); expect(options[0].checked).to.equal(true); expect(el.modelValue).to.eql(['Artichoke', 'Chard']); + // also deselect + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + expect(options[0].checked).to.equal(true); + expect(el.modelValue).to.eql(['Artichoke']); // @ts-ignore allow protected if (el._listboxReceivesNoFocus) { @@ -858,6 +862,10 @@ export function runListboxMixinSuite(customConfig = {}) { listbox.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); expect(options[0].checked).to.equal(true); expect(el.modelValue).to.eql(['Artichoke', 'Chard']); + // also deselect + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + expect(options[0].checked).to.equal(true); + expect(el.modelValue).to.eql(['Artichoke']); }); describe('Accessibility', () => {