From 55fd115e4715ddc3a48d8208f45de83b73cf5b05 Mon Sep 17 00:00:00 2001 From: Mathieu Puech Date: Wed, 2 Dec 2020 00:17:44 -0500 Subject: [PATCH 1/3] fix(listbox): using selection-follows-focus with horizontal orientation --- packages/listbox/src/ListboxMixin.js | 2 +- .../listbox/test-suites/ListboxMixin.suite.js | 41 ++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index 292705fdf..6b3e697a3 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -547,7 +547,7 @@ const ListboxMixinImplementation = superclass => /* no default */ } - const keys = ['ArrowUp', 'ArrowDown', 'Home', 'End']; + const keys = ['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight', 'Home', 'End']; if (keys.includes(key) && this.selectionFollowsFocus && !this.multipleChoice) { this.setCheckedIndex(this.activeIndex); } diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index aedee3726..74148196c 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -912,7 +912,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); } const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened selection-follows-focus autocomplete="none"> + <${tag} opened selection-follows-focus orientation="horizontal" autocomplete="none"> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -936,6 +936,45 @@ export function runListboxMixinSuite(customConfig = {}) { expect(el.checkedIndex).to.equal(0); expectOnlyGivenOneOptionToBeChecked(options, 0); }); + it('navigates through list with [ArrowLeft] [ArrowRight] keys when horizontal: activates and checks the option', async () => { + /** + * @param {LionOption[]} options + * @param {number} selectedIndex + */ + function expectOnlyGivenOneOptionToBeChecked(options, selectedIndex) { + options.forEach((option, i) => { + if (i === selectedIndex) { + expect(option.checked).to.be.true; + } else { + expect(option.checked).to.be.false; + } + }); + } + const el = /** @type {LionListbox} */ (await fixture(html` + <${tag} opened selection-follows-focus autocomplete="none"> + <${optionTag} .choiceValue=${10}>Item 1 + <${optionTag} .choiceValue=${20}>Item 2 + <${optionTag} .choiceValue=${30}>Item 3 + + `)); + + const { listbox } = getProtectedMembers(el); + const options = el.formElements; + // Normalize start values between listbox, slect and combobox and test interaction below + el.activeIndex = 0; + el.checkedIndex = 0; + expect(el.activeIndex).to.equal(0); + expect(el.checkedIndex).to.equal(0); + expectOnlyGivenOneOptionToBeChecked(options, 0); + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight' })); + expect(el.activeIndex).to.equal(1); + expect(el.checkedIndex).to.equal(1); + expectOnlyGivenOneOptionToBeChecked(options, 1); + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft' })); + expect(el.activeIndex).to.equal(0); + expect(el.checkedIndex).to.equal(0); + expectOnlyGivenOneOptionToBeChecked(options, 0); + }); it('checks first and last option with [Home] and [End] keys', async () => { const el = await fixture(html` <${tag} opened selection-follows-focus> From e808df7a99b2aea2199ffd7e2b71c21f6d4f7ad1 Mon Sep 17 00:00:00 2001 From: Mathieu Puech Date: Wed, 2 Dec 2020 00:43:12 -0500 Subject: [PATCH 2/3] fix(listbox): become active on click --- packages/listbox/README.md | 2 +- packages/listbox/src/LionOption.js | 2 ++ packages/listbox/src/ListboxMixin.js | 4 ++- .../listbox/test-suites/ListboxMixin.suite.js | 26 +++++++++++++++++-- packages/listbox/test/lion-option.test.js | 4 ++- 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/listbox/README.md b/packages/listbox/README.md index b1dbe0d8f..befcade63 100644 --- a/packages/listbox/README.md +++ b/packages/listbox/README.md @@ -131,7 +131,7 @@ See [wai aria spec](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_foll export const selectionFollowsFocus = () => html` Apple - Artichoke + Artichoke Asparagus Banana Beets diff --git a/packages/listbox/src/LionOption.js b/packages/listbox/src/LionOption.js index 02b0e2c8e..27e676d81 100644 --- a/packages/listbox/src/LionOption.js +++ b/packages/listbox/src/LionOption.js @@ -125,8 +125,10 @@ export class LionOption extends DisabledMixin(ChoiceInputMixin(FormRegisteringMi const parentForm = /** @type {unknown} */ (this.__parentFormGroup); if (parentForm && /** @type {ChoiceGroupHost} */ (parentForm).multipleChoice) { this.checked = !this.checked; + this.active = !this.active; } else { this.checked = true; + this.active = true; } } } diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index 6b3e697a3..33fe22b76 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -362,7 +362,9 @@ const ListboxMixinImplementation = superclass => this._uncheckChildren(); } if (this.formElements[index]) { - if (this.multipleChoice) { + if (this.formElements[index].disabled) { + this._uncheckChildren(); + } else 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 74148196c..f57bce38d 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -912,7 +912,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); } const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened selection-follows-focus orientation="horizontal" autocomplete="none"> + <${tag} opened selection-follows-focus autocomplete="none"> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -951,7 +951,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); } const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened selection-follows-focus autocomplete="none"> + <${tag} opened selection-follows-focus orientation="horizontal" autocomplete="none"> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -1075,6 +1075,28 @@ export function runListboxMixinSuite(customConfig = {}) { // Checked index stays where it was expect(el.checkedIndex).to.equal(0); }); + it('does not check disabled options when selection-follow-focus is enabled', async () => { + const el = await fixture(html` + <${tag} opened autocomplete="inline" .selectionFollowsFocus="${true}"> + <${optionTag} .choiceValue=${'Item 1'} checked>Item 1 + <${optionTag} .choiceValue=${'Item 2'} disabled>Item 2 + <${optionTag} .choiceValue=${'Item 3'}>Item 3 + + `); + + const { listbox } = getProtectedMembers(el); + + // Normalize activeIndex across multiple implementers of ListboxMixinSuite + el.activeIndex = 0; + + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown' })); + expect(el.activeIndex).to.equal(1); + expect(el.checkedIndex).to.equal(-1); + + listbox.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown' })); + expect(el.activeIndex).to.equal(2); + expect(el.checkedIndex).to.equal(2); + }); }); describe('Programmatic interaction', () => { diff --git a/packages/listbox/test/lion-option.test.js b/packages/listbox/test/lion-option.test.js index f6c8edb2d..441482682 100644 --- a/packages/listbox/test/lion-option.test.js +++ b/packages/listbox/test/lion-option.test.js @@ -86,14 +86,16 @@ describe('lion-option', () => { expect(el.hasAttribute('active')).to.be.false; }); - it('does become checked on [click]', async () => { + it('does become checked and active on [click]', async () => { const el = /** @type {LionOption} */ (await fixture( html``, )); expect(el.checked).to.be.false; + expect(el.active).to.be.false; el.click(); await el.updateComplete; expect(el.checked).to.be.true; + expect(el.active).to.be.true; }); it('fires active-changed event', async () => { From 3c2a33a77e8385fc932c5d36d0af34e5db90d094 Mon Sep 17 00:00:00 2001 From: Mathieu Puech Date: Wed, 2 Dec 2020 08:05:12 -0500 Subject: [PATCH 3/3] chore(listbox): add changeset --- .changeset/popular-needles-hug.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/popular-needles-hug.md diff --git a/.changeset/popular-needles-hug.md b/.changeset/popular-needles-hug.md new file mode 100644 index 000000000..1e42255ea --- /dev/null +++ b/.changeset/popular-needles-hug.md @@ -0,0 +1,7 @@ +--- +'@lion/listbox': patch +--- + +- Fix keyboard navigation when `selection-follows-focus` and `orientation="horizontal"` are set on a `` +- Fix keyboard navigation with `selection-follows-focus` and disabled options +- On click of an option, it become active