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

fix(combobox): when multiple choice reset all listbox options on select (#2332)

Browse source 
* fix(combobox): when multiple choice reset all listbox options on select

* Apply suggestions from code review

Co-authored-by: Thijs Louisse <thijs.louisse@ing.com>

* chore: clear options also on click

* chore: adopt code to show list when only when showAllOnEmpty is true

---------

Co-authored-by: Thijs Louisse <thijs.louisse@ing.com>
This commit is contained in:
gerjanvangeest 2024-08-27 10:48:12 +02:00 committed by GitHub
parent 5fa385e923
commit 02a9427a7d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 174 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/brave-plums-drum.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
[combobox] reset listbox options on click/enter for multiple-choice

4
docs/components/combobox/use-cases.md
View file

@ -243,7 +243,7 @@ export const multipleChoice = () => html`
${lazyRender(
listboxData.map(
(entry, i) => html`
<lion-option .choiceValue="${entry}" ?checked="${i === 0}>${entry}"</lion-option>
<lion-option .choiceValue="${entry}" ?checked="${i === 0}">${entry}</lion-option>
`,
),
)}
@ -266,7 +266,7 @@ export const multipleCustomizableChoice = () => html`
${lazyRender(
listboxData.map(
(entry, i) => html`
<lion-option .choiceValue="${entry}" ?checked="${i === 0}>${entry}"</lion-option>
<lion-option .choiceValue="${entry}" ?checked="${i === 0}">${entry}</lion-option>
`,
),
)}

38
packages/ui/components/combobox/src/LionCombobox.js
View file

@ -22,6 +22,7 @@ const matchA11ySpanReverseFns = new WeakMap();
* @typedef {import('@lion/ui/types/form-core.js').ChoiceInputHost} ChoiceInputHost
* @typedef {import('@lion/ui/types/form-core.js').FormControlHost} FormControlHost
* @typedef {import('../types/SelectionDisplay.js').SelectionDisplay} SelectionDisplay
* @typedef {LionOption & { onFilterUnmatch?:function; onFilterMatch?:function }} OptionWithFilterFn
*/
/**
@ -201,9 +202,31 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
reset() {
super.reset();
if (!this.multipleChoice) {
// @ts-ignore _initialModelValue comes from ListboxMixin
this.value = this._initialModelValue;
}
this._resetListboxOptions();
}
/**
* @protected
*/
_resetListboxOptions() {
this.formElements.forEach((/** @type {OptionWithFilterFn} */ option, idx) => {
this._unhighlightMatchedOption(option);
if (!this.showAllOnEmpty || !this.opened) {
// eslint-disable-next-line no-param-reassign
option.style.display = 'none';
} else {
// eslint-disable-next-line no-param-reassign
option.style.display = '';
option.setAttribute('aria-posinset', `${idx + 1}`);
option.setAttribute('aria-setsize', `${this.formElements.length}`);
option.removeAttribute('aria-hidden');
}
});
}
/**
* @enhance FormControlMixin - add slot[name=selection-display]
@ -704,11 +727,13 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
*/
_listboxOnClick(ev) {
super._listboxOnClick(ev);
this._inputNode.focus();
if (!this.multipleChoice) {
this.activeIndex = -1;
this.opened = false;
} else {
this._inputNode.value = '';
this._resetListboxOptions();
}
}
@ -897,7 +922,6 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
const autoselect = this._autoSelectCondition();
const noFilter = this.autocomplete === 'inline' || this.autocomplete === 'none';
/** @typedef {LionOption & { onFilterUnmatch?:function, onFilterMatch?:function }} OptionWithFilterFn */
this.formElements.forEach((/** @type {OptionWithFilterFn} */ option, i) => {
// [1]. Decide whether option should be shown
const matches = this.matchCondition(option, curValue);
@ -983,7 +1007,7 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
// [8]. These values will help computing autofill intentions next autocomplete cycle
this.__prevCboxValueNonSelected = curValue;
// See test 'computation of "user intends autofill" works correctly afer autofill'
// See test 'computation of "user intends autofill" works correctly after autofill'
this.__prevCboxValue = this._inputNode.value;
this.__hadSelectionLastAutofill =
this._inputNode.value.length !== this._inputNode.selectionStart;
@ -1120,16 +1144,16 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
ev.preventDefault();
this.modelValue = this.parser([...this.modelValue, this._inputNode.value]);
this._inputNode.value = '';
this.opened = false;
} else {
super._listboxOnKeyDown(ev);
// TODO: should we clear the input value here when allowCustomChoice is false?
// For now, we don't...
this._resetListboxOptions();
}
if (!this.multipleChoice) {
this.opened = false;
} else {
this._inputNode.value = '';
}
break;
default: {
@ -1168,7 +1192,7 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi
.filter(option => diff.includes(option.choiceValue))
.map(option => this._getTextboxValueFromOption(option))
.join(' ');
this._setTextboxValue(newValue); // or last selected value?
this._setTextboxValue(newValue);
}
}

141
packages/ui/components/combobox/test/lion-combobox.test.js
View file

@ -81,6 +81,54 @@ describe('lion-combobox', () => {
await performChecks();
});
it('hides all options on reset()', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`)
);
const options = el.formElements;
const visibleOptions = () => options.filter(o => o.style.display !== 'none');
mimicUserTyping(el, 'cha');
await el.updateComplete;
expect(visibleOptions().length).to.equal(1);
el.reset();
await el.updateComplete;
expect(visibleOptions().length).to.equal(0);
});
it('shows all options on reset() when showAllOnEmpty is set to true and overlay was open', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo" show-all-on-empty>
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`)
);
const options = el.formElements;
const visibleOptions = () => options.filter(o => o.getAttribute('aria-hidden') !== 'true');
mimicUserTyping(el, 'cha');
await el.updateComplete;
expect(visibleOptions().length).to.equal(1);
expect(el.opened).to.be.true;
el.reset();
await el.updateComplete;
expect(visibleOptions().length).to.equal(4);
});
it('hides listbox on click/enter (when multiple-choice is false)', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
@ -467,13 +515,12 @@ describe('lion-combobox', () => {
mimicUserTyping(el, 'a');
await el.updateComplete;
const visibleOptions = options.filter(o => o.style.display !== 'none');
expect(visibleOptions.length).to.equal(3, 'after input');
const visibleOptions = () => options.filter(o => o.style.display !== 'none');
expect(visibleOptions().length).to.equal(3, 'after input');
el.clear();
await el.updateComplete;
const visibleOptions2 = options.filter(o => o.style.display !== 'none');
expect(visibleOptions2.length).to.equal(0, 'after clear');
expect(visibleOptions().length).to.equal(0, 'after clear');
});
it('resets modelValue and textbox value on reset()', async () => {
@ -508,7 +555,7 @@ describe('lion-combobox', () => {
el2.reset();
expect(el2.modelValue).to.deep.equal(['Artichoke']);
// @ts-ignore [allow-protected] in test
expect(el2._inputNode.value).to.equal('Artichoke');
expect(el2._inputNode.value).to.equal('');
});
it('syncs textbox to modelValue', async () => {
@ -735,7 +782,7 @@ describe('lion-combobox', () => {
await el.updateComplete;
});
it('allows manyu custom selections when multi-choice when requireOptionMatch is false', async () => {
it('allows many custom selections when multi-choice when requireOptionMatch is false', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox
@ -2088,7 +2135,7 @@ describe('lion-combobox', () => {
});
});
it('highlights first occcurence of matching option', async () => {
it('highlights first occurrence of matching option', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo" match-mode="all">
@ -2170,6 +2217,28 @@ describe('lion-combobox', () => {
);
});
it('resets removes highlights on reset()', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo" match-mode="all">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`)
);
const options = el.formElements;
mimicUserTyping(/** @type {LionCombobox} */ (el), 'c');
await el.updateComplete;
expect(options[0]).lightDom.to.equal(`<span aria-label="Artichoke">Arti<b>c</b>hoke</span>`);
el.reset();
await el.updateComplete;
expect(options[0]).lightDom.to.equal(`Artichoke`);
});
it('synchronizes textbox when autocomplete is "inline" or "both"', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
@ -3018,6 +3087,64 @@ describe('lion-combobox', () => {
expect(el.opened).to.equal(true);
});
it('clears textbox after selection of a new item on [enter]', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo" multiple-choice>
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`)
);
const { _inputNode } = getComboboxMembers(el);
const options = el.formElements;
mimicUserTyping(el, 'art');
await el.updateComplete;
expect(el.opened).to.equal(true);
const visibleOptions = () => options.filter(o => o.style.display !== 'none');
expect(visibleOptions().length).to.equal(1);
// N.B. we do only trigger keydown here (and not mimicKeypress (both keyup and down)),
// because this closely mimics what happens in the browser
_inputNode.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));
expect(el.opened).to.equal(true);
expect(visibleOptions().length).to.equal(0);
expect(_inputNode.value).to.equal('');
});
it('clears textbox after selection of a new item on click', async () => {
const el = /** @type {LionCombobox} */ (
await fixture(html`
<lion-combobox name="foo" multiple-choice>
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
<lion-option .choiceValue="${'Victoria Plum'}">Victoria Plum</lion-option>
</lion-combobox>
`)
);
const { _inputNode } = getComboboxMembers(el);
const options = el.formElements;
mimicUserTyping(el, 'art');
await el.updateComplete;
expect(el.opened).to.equal(true);
const visibleOptions = () => options.filter(o => o.style.display !== 'none');
expect(visibleOptions().length).to.equal(1);
visibleOptions()[0].click();
expect(el.opened).to.equal(true);
expect(visibleOptions().length).to.equal(0);
expect(_inputNode.value).to.equal('');
});
it('submits form on [Enter] when listbox is closed', async () => {
const submitSpy = sinon.spy(e => e.preventDefault());
const el = /** @type {HTMLFormElement} */ (