From edb43c4e05123aaf315914c1ece1416c95b31e94 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Mon, 19 Apr 2021 11:02:07 +0200 Subject: [PATCH] fix(combobox): no sync unmatched checkedIndex [autocomplet=none --- .changeset/three-apples-appear.md | 6 ++ packages/combobox/src/LionCombobox.js | 26 +++---- packages/combobox/test/lion-combobox.test.js | 75 +++++++++++++------ .../listbox/test-suites/ListboxMixin.suite.js | 20 ++--- 4 files changed, 80 insertions(+), 47 deletions(-) create mode 100644 .changeset/three-apples-appear.md diff --git a/.changeset/three-apples-appear.md b/.changeset/three-apples-appear.md new file mode 100644 index 000000000..295db034b --- /dev/null +++ b/.changeset/three-apples-appear.md @@ -0,0 +1,6 @@ +--- +'@lion/combobox': patch +'@lion/listbox': patch +--- + +syncs last selected choice value for [autocomplet="none|list"] on close diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index 72e4b5845..8f26c0aa1 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -430,7 +430,6 @@ export class LionCombobox extends OverlayMixin(LionListbox) { if (!lastKey) { return /** @type {boolean} */ (this.opened); } - return true; } @@ -466,7 +465,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this._inputNode.focus(); if (!this.multipleChoice) { this.activeIndex = -1; - this._setOpenedWithoutPropertyEffects(false); + this.opened = false; } } @@ -584,8 +583,10 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @protected */ _handleAutocompletion() { - if (this.autocomplete === 'none') { - return; + if ((!this.multipleChoice && this.autocomplete === 'none') || this.autocomplete === 'list') { + if (!this._inputNode.value.startsWith(this.modelValue)) { + this.checkedIndex = -1; + } } const hasSelection = this._inputNode.value.length !== this._inputNode.selectionStart; @@ -607,8 +608,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { const userIntendsInlineAutoFill = this.__computeUserIntendsAutoFill({ prevValue, curValue }); const isInlineAutoFillCandidate = this.autocomplete === 'both' || this.autocomplete === 'inline'; - const autoselect = this._autoSelectCondition(); - // @ts-ignore this.autocomplete === 'none' needs to be there if statement above is removed + const autoselect = this.autocomplete !== 'none' && this._autoSelectCondition(); const noFilter = this.autocomplete === 'inline' || this.autocomplete === 'none'; /** @typedef {LionOption & { onFilterUnmatch?:function, onFilterMatch?:function }} OptionWithFilterFn */ @@ -795,7 +795,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { const { key } = ev; switch (key) { case 'Escape': - this._setOpenedWithoutPropertyEffects(false); + this.opened = false; this._setTextboxValue(''); break; case 'Enter': @@ -803,7 +803,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { return; } if (!this.multipleChoice) { - this._setOpenedWithoutPropertyEffects(false); + this.opened = false; } break; /* no default */ @@ -903,12 +903,10 @@ export class LionCombobox extends OverlayMixin(LionListbox) { */ __requestShowOverlay(ev) { const lastKey = ev && ev.key; - this._setOpenedWithoutPropertyEffects( - this._showOverlayCondition({ - lastKey, - currentValue: this._inputNode.value, - }), - ); + this.opened = 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 6a318543c..99ebac3ba 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -46,6 +46,7 @@ function getComboboxMembers(el) { * @param {LionCombobox} el * @param {string} value */ +// TODO: add keys that actually make sense... function mimicUserTyping(el, value) { const { _inputNode } = getComboboxMembers(el); _inputNode.dispatchEvent(new Event('focusin', { bubbles: true })); @@ -71,45 +72,44 @@ function mimicKeyPress(el, key) { */ async function mimicUserTypingAdvanced(el, values) { const { _inputNode } = getComboboxMembers(el); - const inputNodeLoc = /** @type {HTMLInputElement & {selectionStart:number, selectionEnd:number}} */ (_inputNode); - inputNodeLoc.dispatchEvent(new Event('focusin', { bubbles: true })); + _inputNode.dispatchEvent(new Event('focusin', { bubbles: true })); for (const key of values) { // eslint-disable-next-line no-await-in-loop, no-loop-func await new Promise(resolve => { - const hasSelection = inputNodeLoc.selectionStart !== inputNodeLoc.selectionEnd; + const hasSelection = _inputNode.selectionStart !== _inputNode.selectionEnd; if (key === 'Backspace') { if (hasSelection) { - inputNodeLoc.value = - inputNodeLoc.value.slice( + _inputNode.value = + _inputNode.value.slice( 0, - inputNodeLoc.selectionStart ? inputNodeLoc.selectionStart : undefined, + _inputNode.selectionStart ? _inputNode.selectionStart : undefined, ) + - inputNodeLoc.value.slice( - inputNodeLoc.selectionEnd ? inputNodeLoc.selectionEnd : undefined, - inputNodeLoc.value.length, + _inputNode.value.slice( + _inputNode.selectionEnd ? _inputNode.selectionEnd : undefined, + _inputNode.value.length, ); } else { - inputNodeLoc.value = inputNodeLoc.value.slice(0, -1); + _inputNode.value = _inputNode.value.slice(0, -1); } } else if (hasSelection) { - inputNodeLoc.value = - inputNodeLoc.value.slice( + _inputNode.value = + _inputNode.value.slice( 0, - inputNodeLoc.selectionStart ? inputNodeLoc.selectionStart : undefined, + _inputNode.selectionStart ? _inputNode.selectionStart : undefined, ) + key + - inputNodeLoc.value.slice( - inputNodeLoc.selectionEnd ? inputNodeLoc.selectionEnd : undefined, - inputNodeLoc.value.length, + _inputNode.value.slice( + _inputNode.selectionEnd ? _inputNode.selectionEnd : undefined, + _inputNode.value.length, ); } else { - inputNodeLoc.value += key; + _inputNode.value += key; } - mimicKeyPress(inputNodeLoc, key); - inputNodeLoc.dispatchEvent(new Event('input', { bubbles: true, composed: true })); + mimicKeyPress(_inputNode, key); + _inputNode.dispatchEvent(new Event('input', { bubbles: true, composed: true })); el.updateComplete.then(() => { // @ts-ignore @@ -180,9 +180,8 @@ describe('lion-combobox', () => { expect(visibleOptions().length).to.equal(0); } - // FIXME: autocomplete 'none' should have this behavior as well - // el.autocomplete = 'none'; - // await performChecks(); + el.autocomplete = 'none'; + await performChecks(); el.autocomplete = 'list'; await performChecks(); el.autocomplete = 'inline'; @@ -321,9 +320,9 @@ describe('lion-combobox', () => { _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'); @@ -443,6 +442,36 @@ describe('lion-combobox', () => { expect(el2.modelValue).to.eql([]); expect(_inputNode.value).to.equal(''); }); + + it('syncs textbox to modelValue', async () => { + const el = /** @type {LionCombobox} */ (await fixture(html` + + Aha + Bhb + + `)); + const { _inputNode } = getComboboxMembers(el); + + async function performChecks() { + el.formElements[0].click(); + await el.updateComplete; + + expect(_inputNode.value).to.equal('Aha'); + expect(el.checkedIndex).to.equal(0); + + await mimicUserTyping(el, 'Ah'); + await el.updateComplete; + + await el.updateComplete; + expect(el.checkedIndex).to.equal(-1); + } + + el.autocomplete = 'none'; + await performChecks(); + + el.autocomplete = 'list'; + await performChecks(); + }); }); describe('Overlay visibility', () => { diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 05a983d6b..dd75adfa0 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -365,7 +365,7 @@ export function runListboxMixinSuite(customConfig = {}) { it('has a reference to the active option', async () => { const el = await fixture(html` - <${tag} opened has-no-default-selected autocomplete="none"> + <${tag} opened has-no-default-selected autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${'10'} id="first">Item 1 <${optionTag} .choiceValue=${'20'} checked id="second">Item 2 @@ -387,7 +387,7 @@ export function runListboxMixinSuite(customConfig = {}) { it('puts "aria-setsize" on all options to indicate the total amount of options', async () => { const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} autocomplete="none"> + <${tag} autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -400,7 +400,7 @@ export function runListboxMixinSuite(customConfig = {}) { it('puts "aria-posinset" on all options to indicate their position in the listbox', async () => { const el = await fixture(html` - <${tag} autocomplete="none"> + <${tag} autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -588,7 +588,7 @@ export function runListboxMixinSuite(customConfig = {}) { describe('Enter', () => { it('[Enter] selects active option', async () => { const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened name="foo" autocomplete="none"> + <${tag} opened name="foo" autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue="${'Artichoke'}">Artichoke <${optionTag} .choiceValue="${'Bla'}">Bla <${optionTag} .choiceValue="${'Chard'}">Chard @@ -611,7 +611,7 @@ export function runListboxMixinSuite(customConfig = {}) { // When listbox is not focusable (in case of a combobox), the user should be allowed // to enter a space in the focusable element (texbox) const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened name="foo" ._listboxReceivesNoFocus="${false}" autocomplete="none"> + <${tag} opened name="foo" ._listboxReceivesNoFocus="${false}" autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue="${'Artichoke'}">Artichoke <${optionTag} .choiceValue="${'Bla'}">Bla <${optionTag} .choiceValue="${'Chard'}">Chard @@ -687,7 +687,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); it('navigates through open lists with [ArrowDown] [ArrowUp] keys activates the option', async () => { const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened has-no-default-selected autocomplete="none"> + <${tag} opened has-no-default-selected autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${'Item 1'}>Item 1 <${optionTag} .choiceValue=${'Item 2'}>Item 2 <${optionTag} .choiceValue=${'Item 3'}>Item 3 @@ -715,7 +715,7 @@ export function runListboxMixinSuite(customConfig = {}) { describe('Orientation', () => { it('has a default value of "vertical"', async () => { const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened name="foo" autocomplete="none"> + <${tag} opened name="foo" autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue="${'Artichoke'}">Artichoke <${optionTag} .choiceValue="${'Chard'}">Chard @@ -755,7 +755,7 @@ export function runListboxMixinSuite(customConfig = {}) { it('uses [ArrowLeft] and [ArrowRight] keys when "horizontal"', async () => { const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened name="foo" orientation="horizontal" autocomplete="none"> + <${tag} opened name="foo" orientation="horizontal" autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue="${'Artichoke'}">Artichoke <${optionTag} .choiceValue="${'Chard'}">Chard @@ -932,7 +932,7 @@ export function runListboxMixinSuite(customConfig = {}) { }); } const el = /** @type {LionListbox} */ (await fixture(html` - <${tag} opened selection-follows-focus autocomplete="none"> + <${tag} opened selection-follows-focus autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3 @@ -972,7 +972,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 orientation="horizontal" autocomplete="none" show-all-on-empty> <${optionTag} .choiceValue=${10}>Item 1 <${optionTag} .choiceValue=${20}>Item 2 <${optionTag} .choiceValue=${30}>Item 3