diff --git a/.changeset/cold-papayas-judge.md b/.changeset/cold-papayas-judge.md new file mode 100644 index 000000000..8c4d28f6c --- /dev/null +++ b/.changeset/cold-papayas-judge.md @@ -0,0 +1,5 @@ +--- +'@lion/combobox': patch +--- + +do not reopen listbox on focusin edge cases diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index 545edc52a..72e4b5845 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -307,7 +307,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { } /** - * @param {'disabled'|'modelValue'|'readOnly'} name + * @param {'disabled'|'modelValue'|'readOnly'|'focused'} name * @param {unknown} oldValue */ requestUpdateInternal(name, oldValue) { @@ -324,6 +324,9 @@ export class LionCombobox extends OverlayMixin(LionListbox) { } } } + if (name === 'focused' && this.focused) { + this.__requestShowOverlay(); + } } /** @@ -331,11 +334,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) { */ updated(changedProperties) { super.updated(changedProperties); - if (changedProperties.has('focused')) { - if (this.focused) { - this.__requestShowOverlay(); - } - } if (changedProperties.has('opened')) { if (this.opened) { @@ -413,12 +411,18 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * return options.currentValue.length > 4 && super._showOverlayCondition(options); * } * - * @param {{ currentValue: string, lastKey:string|undefined }} options + * @param {{ currentValue?: string, lastKey?: string }} options * @protected * @returns {boolean} */ + // TODO: batch all pending condition triggers in __pendingShowTriggers, reducing race conditions // eslint-disable-next-line class-methods-use-this _showOverlayCondition({ lastKey }) { + const hideOn = ['Tab', 'Escape', 'Enter']; + if (lastKey && hideOn.includes(lastKey)) { + return false; + } + if (this.showAllOnEmpty && this.focused) { return true; } @@ -426,8 +430,8 @@ export class LionCombobox extends OverlayMixin(LionListbox) { if (!lastKey) { return /** @type {boolean} */ (this.opened); } - const doNotShowOn = ['Tab', 'Esc', 'Enter']; - return !doNotShowOn.includes(lastKey); + + return true; } /** @@ -449,11 +453,8 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @param {KeyboardEvent} ev * @protected */ - _textboxOnKeydown(ev) { - if (ev.key === 'Tab') { - this.opened = false; - } - } + // eslint-disable-next-line class-methods-use-this, no-unused-vars + _textboxOnKeydown(ev) {} /** * @param {MouseEvent} ev @@ -461,11 +462,12 @@ export class LionCombobox extends OverlayMixin(LionListbox) { */ _listboxOnClick(ev) { super._listboxOnClick(ev); + + this._inputNode.focus(); if (!this.multipleChoice) { this.activeIndex = -1; - this.opened = false; + this._setOpenedWithoutPropertyEffects(false); } - this._inputNode.focus(); } /** @@ -756,6 +758,15 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this.__setupCombobox(); } + /** + * @enhance OverlayMixin + * @protected + */ + _teardownOverlayCtrl() { + super._teardownOverlayCtrl(); + this.__teardownCombobox(); + } + /** * @enhance OverlayMixin * @protected @@ -784,7 +795,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { const { key } = ev; switch (key) { case 'Escape': - this.opened = false; + this._setOpenedWithoutPropertyEffects(false); this._setTextboxValue(''); break; case 'Enter': @@ -792,7 +803,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { return; } if (!this.multipleChoice) { - this.opened = false; + this._setOpenedWithoutPropertyEffects(false); } break; /* no default */ @@ -891,10 +902,13 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @private */ __requestShowOverlay(ev) { - this.opened = this._showOverlayCondition({ - lastKey: ev && ev.key, - currentValue: this._inputNode.value, - }); + const lastKey = ev && ev.key; + this._setOpenedWithoutPropertyEffects( + this._showOverlayCondition({ + lastKey, + currentValue: this._inputNode.value, + }), + ); } clear() { diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index c909a837a..6a318543c 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -57,7 +57,7 @@ function mimicUserTyping(el, value) { } /** - * @param {HTMLInputElement} el + * @param {HTMLElement} el * @param {string} key */ function mimicKeyPress(el, key) { @@ -157,41 +157,73 @@ async function fruitFixture({ autocomplete, matchMode } = {}) { } describe('lion-combobox', () => { - describe('Options', () => { - describe('showAllOnEmpty', () => { - it('hides options when text in input node is cleared after typing something by default', async () => { - const el = /** @type {LionCombobox} */ (await fixture(html` - - Artichoke - Chard - Chicory - Victoria Plum - - `)); + describe('Options visibility', () => { + it('hides options when text in input node is cleared after typing something by default', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); - const options = el.formElements; - const visibleOptions = () => options.filter(o => o.getAttribute('aria-hidden') !== 'true'); + const options = el.formElements; + const visibleOptions = () => options.filter(o => o.getAttribute('aria-hidden') !== 'true'); - async function performChecks() { - mimicUserTyping(el, 'c'); - await el.updateComplete; - expect(visibleOptions().length).to.equal(4); - mimicUserTyping(el, ''); - await el.updateComplete; - expect(visibleOptions().length).to.equal(0); - } + async function performChecks() { + mimicUserTyping(el, 'c'); + await el.updateComplete; + expect(visibleOptions().length).to.equal(4); + mimicUserTyping(el, ''); + await el.updateComplete; + expect(visibleOptions().length).to.equal(0); + } - // FIXME: autocomplete 'none' should have this behavior as well - // el.autocomplete = 'none'; - // await performChecks(); - el.autocomplete = 'list'; - await performChecks(); - el.autocomplete = 'inline'; - await performChecks(); - el.autocomplete = 'both'; - await performChecks(); - }); + // FIXME: autocomplete 'none' should have this behavior as well + // el.autocomplete = 'none'; + // await performChecks(); + el.autocomplete = 'list'; + await performChecks(); + el.autocomplete = 'inline'; + await performChecks(); + el.autocomplete = 'both'; + await performChecks(); + }); + it('hides listbox on click/enter (when multiple-choice is false)', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + const { _comboboxNode, _listboxNode } = getComboboxMembers(el); + + async function open() { + // activate opened listbox + _comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true })); + mimicUserTyping(el, 'ch'); + return el.updateComplete; + } + + await open(); + expect(el.opened).to.be.true; + const visibleOptions = el.formElements.filter(o => o.style.display !== 'none'); + visibleOptions[0].click(); + expect(el.opened).to.be.false; + await open(); + expect(el.opened).to.be.true; + el.activeIndex = el.formElements.indexOf(visibleOptions[0]); + mimicKeyPress(_listboxNode, 'Enter'); + await el.updateComplete; + expect(el.opened).to.be.false; + }); + + describe('With ".showAllOnEmpty"', () => { it('keeps showing options when text in input node is cleared after typing something', async () => { const el = /** @type {LionCombobox} */ (await fixture(html` @@ -240,6 +272,63 @@ describe('lion-combobox', () => { await el.updateComplete; expect(el.opened).to.be.true; }); + + it('hides overlay on focusin after [Escape]', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + const { _comboboxNode, _inputNode } = getComboboxMembers(el); + + expect(el.opened).to.be.false; + _comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.true; + _inputNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' })); + await el.updateComplete; + expect(el.opened).to.be.false; + }); + + it('hides listbox on click/enter (when multiple-choice is false)', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Artichoke + Chard + Chicory + Victoria Plum + + `)); + + const { _comboboxNode, _listboxNode, _inputNode } = getComboboxMembers(el); + + async function open() { + // activate opened listbox + _comboboxNode.dispatchEvent(new Event('focusin', { bubbles: true, composed: true })); + mimicUserTyping(el, 'ch'); + return el.updateComplete; + } + + await open(); + expect(el.opened).to.be.true; + const visibleOptions = el.formElements.filter(o => o.style.display !== 'none'); + visibleOptions[0].click(); + await el.updateComplete; + expect(el.opened).to.be.false; + + _inputNode.value = ''; + _inputNode.blur(); + await open(); + await el.updateComplete; + expect(el.opened).to.be.true; + + el.activeIndex = el.formElements.indexOf(visibleOptions[0]); + mimicKeyPress(_listboxNode, 'Enter'); + expect(el.opened).to.be.false; + }); }); }); @@ -1766,7 +1855,7 @@ describe('lion-combobox', () => { describe('Multiple Choice', () => { // TODO: possibly later share test with select-rich if it officially supports multipleChoice - it('does not close listbox on click/enter/space', async () => { + it('does not close listbox on click/enter', async () => { const el = /** @type {LionCombobox} */ (await fixture(html` Artichoke @@ -1787,10 +1876,8 @@ describe('lion-combobox', () => { const visibleOptions = el.formElements.filter(o => o.style.display !== 'none'); visibleOptions[0].click(); expect(el.opened).to.equal(true); - // visibleOptions[1].dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); - // expect(el.opened).to.equal(true); - // visibleOptions[2].dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); - // expect(el.opened).to.equal(true); + mimicKeyPress(visibleOptions[1], 'Enter'); + expect(el.opened).to.equal(true); }); });