ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1114 from ing-bank/fix/listboxSelectionFollowFocusHorizontal

Browse source 
fix(listbox): using selection-follows-focus with horizontal orientation
This commit is contained in:
Joren Broekema 2020-12-02 16:25:32 +01:00 committed by GitHub
commit 053bcee6cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 78 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
.changeset/popular-needles-hug.md Normal file
View file

@ -0,0 +1,7 @@
---
'@lion/listbox': patch
---
- Fix keyboard navigation when `selection-follows-focus` and `orientation="horizontal"` are set on a `<lion-listbox>`
- Fix keyboard navigation with `selection-follows-focus` and disabled options
- On click of an option, it become active

2
packages/listbox/README.md
View file

@ -131,7 +131,7 @@ See [wai aria spec](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_foll
export const selectionFollowsFocus = () => html`
<lion-listbox name="combo" label="Selection follows focus" selection-follows-focus>
<lion-option .choiceValue=${'Apple'}>Apple</lion-option>
<lion-option .choiceValue=${'Artichoke'}>Artichoke</lion-option>
<lion-option .choiceValue=${'Artichoke'} disabled>Artichoke</lion-option>
<lion-option .choiceValue=${'Asparagus'}>Asparagus</lion-option>
<lion-option .choiceValue=${'Banana'}>Banana</lion-option>
<lion-option .choiceValue=${'Beets'}>Beets</lion-option>

2
packages/listbox/src/LionOption.js
View file

@ -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;
}
}
}

6
packages/listbox/src/ListboxMixin.js
View file

@ -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;
@ -547,7 +549,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);
}

61
packages/listbox/test-suites/ListboxMixin.suite.js
View file

@ -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 orientation="horizontal" autocomplete="none">
<${optionTag} .choiceValue=${10}>Item 1</${optionTag}>
<${optionTag} .choiceValue=${20}>Item 2</${optionTag}>
<${optionTag} .choiceValue=${30}>Item 3</${optionTag}>
</${tag}>
`));
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>
@ -1036,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}>
<${optionTag} .choiceValue=${'Item 2'} disabled>Item 2</${optionTag}>
<${optionTag} .choiceValue=${'Item 3'}>Item 3</${optionTag}>
</${tag}>
`);
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', () => {

4
packages/listbox/test/lion-option.test.js
View file

@ -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`<lion-option .choiceValue=${10}></lion-option>`,
));
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 () => {