fix(listbox): no cancellation multi mouse click

This commit is contained in:
Thijs Louisse 2020-10-02 11:25:35 +02:00
parent 20a20844db
commit d1c6b18c0e
3 changed files with 71 additions and 6 deletions

View file

@ -0,0 +1,5 @@
---
'@lion/listbox': patch
---
no cancellation multi mouse click

View file

@ -537,12 +537,20 @@ const ListboxMixinImplementation = superclass =>
*/ */
// eslint-disable-next-line class-methods-use-this, no-unused-vars // eslint-disable-next-line class-methods-use-this, no-unused-vars
_listboxOnClick(ev) { _listboxOnClick(ev) {
const option = /** @type {HTMLElement} */ (ev.target).closest('[role=option]'); /**
const foundIndex = this.formElements.indexOf(option); For now we disable, handling click interactions via delegated click handlers, since
if (foundIndex > -1) { LionOption detects clicks itself and could now potentially be checked as offline dom.
this.activeIndex = foundIndex; When we enable both methods in multiple choice mode, we would 'cancel out' each other.
this.setCheckedIndex(foundIndex); TODO: If we want to allow 'less smart' options (think of role=menu), we should enable this
} again and disable click handling inside LionOption (since there would be no use case for
'orphan' options)
*/
// const option = /** @type {HTMLElement} */ (ev.target).closest('[role=option]');
// const foundIndex = this.formElements.indexOf(option);
// if (foundIndex > -1) {
// this.activeIndex = foundIndex;
// this.setCheckedIndex(foundIndex, 'toggle', );
// }
} }
/** /**

View file

@ -476,6 +476,7 @@ export function runListboxMixinSuite(customConfig = {}) {
expect(options[1].checked).to.be.true; expect(options[1].checked).to.be.true;
}); });
}); });
describe('Space', () => { describe('Space', () => {
it('selects active option when "_listboxReceivesNoFocus" is true', async () => { it('selects active option when "_listboxReceivesNoFocus" is true', async () => {
// When listbox is not focusable (in case of a combobox), the user should be allowed // When listbox is not focusable (in case of a combobox), the user should be allowed
@ -667,6 +668,57 @@ export function runListboxMixinSuite(customConfig = {}) {
expect(el.modelValue).to.eql(['Artichoke', 'Chard']); expect(el.modelValue).to.eql(['Artichoke', 'Chard']);
}); });
it('works via different interaction mechanisms (click, enter, spaces)', async () => {
const el = /** @type {Listbox} */ (await fixture(html`
<${tag} name="foo" multiple-choice>
<${optionTag} .choiceValue="${'Artichoke'}">Artichoke</${optionTag}>
<${optionTag} .choiceValue="${'Chard'}">Chard</${optionTag}>
<${optionTag} .choiceValue="${'Chicory'}">Chicory</${optionTag}>
<${optionTag} .choiceValue="${'Victoria Plum'}">Victoria Plum</${optionTag}>
</${tag}>
`));
const options = el.formElements;
// feature detection select-rich
if (el.navigateWithinInvoker !== undefined) {
// Note we don't have multipleChoice in the select-rich yet.
// TODO: implement in future when requested
return;
}
// click
options[0].click();
options[1].click();
expect(options[0].checked).to.equal(true);
expect(el.modelValue).to.eql(['Artichoke', 'Chard']);
// Reset
el._uncheckChildren();
// Enter
el.activeIndex = 0;
el._listboxNode.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));
el.activeIndex = 1;
el._listboxNode.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));
expect(options[0].checked).to.equal(true);
expect(el.modelValue).to.eql(['Artichoke', 'Chard']);
if (el._listboxReceivesNoFocus) {
return; // if suite is run for combobox, we don't respond to [Space]
}
// Reset
el._uncheckChildren();
// Space
el.activeIndex = 0;
el._listboxNode.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' }));
el.activeIndex = 1;
el._listboxNode.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' }));
expect(options[0].checked).to.equal(true);
expect(el.modelValue).to.eql(['Artichoke', 'Chard']);
});
describe('Accessibility', () => { describe('Accessibility', () => {
it('adds aria-multiselectable="true" to listbox node', async () => { it('adds aria-multiselectable="true" to listbox node', async () => {
const el = /** @type {Listbox} */ (await fixture(html` const el = /** @type {Listbox} */ (await fixture(html`