From 159d6839c8a3087c084896b5be21e12bfa1d3d0d Mon Sep 17 00:00:00 2001 From: Matthieu Date: Fri, 11 Jun 2021 12:44:54 -0400 Subject: [PATCH] fix: better support when options change dynamically --- .changeset/beige-students-vanish.md | 5 ++ docs/components/inputs/combobox/features.md | 2 +- packages/combobox/src/LionCombobox.js | 38 ++++++++++++--- packages/combobox/test/lion-combobox.test.js | 49 ++++++++++++++++++++ 4 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 .changeset/beige-students-vanish.md diff --git a/.changeset/beige-students-vanish.md b/.changeset/beige-students-vanish.md new file mode 100644 index 000000000..c1e60cb36 --- /dev/null +++ b/.changeset/beige-students-vanish.md @@ -0,0 +1,5 @@ +--- +'@lion/combobox': minor +--- + +Better support when options change dynamically diff --git a/docs/components/inputs/combobox/features.md b/docs/components/inputs/combobox/features.md index 65129b97b..a2aa7cf0e 100644 --- a/docs/components/inputs/combobox/features.md +++ b/docs/components/inputs/combobox/features.md @@ -266,7 +266,7 @@ function fetchMyData(val) { if (rejectPrev) { rejectPrev(); } - const results = comboboxData.filter(item => item.toLowerCase().includes(val.toLowerCase())); + const results = listboxData.filter(item => item.toLowerCase().includes(val.toLowerCase())); return new Promise((resolve, reject) => { rejectPrev = reject; setTimeout(() => { diff --git a/packages/combobox/src/LionCombobox.js b/packages/combobox/src/LionCombobox.js index 53ed0beb4..bbc6bf958 100644 --- a/packages/combobox/src/LionCombobox.js +++ b/packages/combobox/src/LionCombobox.js @@ -286,6 +286,16 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @private */ this.__prevCboxValue = ''; + /** + * @type {boolean} + * @private + */ + this.__hadUserIntendsInlineAutoFill = false; + /** + * @type {boolean} + * @private + */ + this.__listboxContentChanged = false; /** @type {EventListener} * @private @@ -386,6 +396,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { // Only update list in render cycle this._handleAutocompletion(); this.__shouldAutocompleteNextUpdate = false; + this.__listboxContentChanged = false; } if (typeof this._selectionDisplayNode?.onComboboxElementUpdated === 'function') { @@ -479,6 +490,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { _onListboxContentChanged() { super._onListboxContentChanged(); this.__shouldAutocompleteNextUpdate = true; + this.__listboxContentChanged = true; } // eslint-disable-next-line no-unused-vars @@ -609,7 +621,11 @@ export class LionCombobox extends OverlayMixin(LionListbox) { prevValue.length && curValue.length && prevValue[0].toLowerCase() !== curValue[0].toLowerCase(); - return userIsAddingChars || userStartsNewWord; + return ( + userIsAddingChars || + userStartsNewWord || + (this.__listboxContentChanged && this.__hadUserIntendsInlineAutoFill) + ); } /* eslint-enable no-param-reassign, class-methods-use-this */ @@ -629,7 +645,11 @@ export class LionCombobox extends OverlayMixin(LionListbox) { _handleAutocompletion() { const hasSelection = this._inputNode.value.length !== this._inputNode.selectionStart; - const curValue = this._inputNode.value; + const inputValue = this._inputNode.value; + const inputSelectionStart = this._inputNode.selectionStart; + const curValue = + hasSelection && inputSelectionStart ? inputValue.slice(0, inputSelectionStart) : inputValue; + const prevValue = hasSelection || this.__hadSelectionLastAutofill ? this.__prevCboxValueNonSelected @@ -734,6 +754,7 @@ export class LionCombobox extends OverlayMixin(LionListbox) { this.__prevCboxValue = this._inputNode.value; this.__hadSelectionLastAutofill = this._inputNode.value.length !== this._inputNode.selectionStart; + this.__hadUserIntendsInlineAutoFill = userIntendsInlineAutoFill; // [9]. Reposition overlay if (this._overlayCtrl && this._overlayCtrl._popper) { @@ -745,10 +766,15 @@ export class LionCombobox extends OverlayMixin(LionListbox) { * @private */ __textboxInlineComplete(option = this.formElements[this.activeIndex]) { - const prevLen = this._inputNode.value.length; - this._inputNode.value = this._getTextboxValueFromOption(option); - this._inputNode.selectionStart = prevLen; - this._inputNode.selectionEnd = this._inputNode.value.length; + const newValue = this._getTextboxValueFromOption(option); + + // Make sure that we don't lose inputNode.selectionStart and inputNode.selectionEnd + if (this._inputNode.value !== newValue) { + const prevLen = this._inputNode.value.length; + this._inputNode.value = newValue; + this._inputNode.selectionStart = prevLen; + this._inputNode.selectionEnd = this._inputNode.value.length; + } } /** diff --git a/packages/combobox/test/lion-combobox.test.js b/packages/combobox/test/lion-combobox.test.js index 7d1b3230c..31de70e98 100644 --- a/packages/combobox/test/lion-combobox.test.js +++ b/packages/combobox/test/lion-combobox.test.js @@ -1457,6 +1457,11 @@ describe('lion-combobox', () => { this.requestUpdate(); } + fillAllOptions() { + this.options = [...listboxData]; + this.requestUpdate(); + } + get combobox() { return /** @type {LionCombobox} */ (this.shadowRoot?.querySelector('#combobox')); } @@ -1488,6 +1493,50 @@ describe('lion-combobox', () => { await el.updateComplete; expect(spy).to.have.been.calledTwice; }); + + it('should handle dynamic options', async () => { + // Arrange + const el = /** @type {MyEl} */ (await fixture(html`<${wrappingTag}>`)); + await el.combobox.registrationComplete; + + // Act (start typing) + mimicUserTyping(el.combobox, 'l'); + // simulate fetching data from server + el.clearOptions(); + await el.updateComplete; + await el.updateComplete; + el.fillAllOptions(); + await el.updateComplete; + await el.updateComplete; + + // Assert + const { _inputNode } = getComboboxMembers(el.combobox); + expect(_inputNode.value).to.equal('lorem'); + expect(_inputNode.selectionStart).to.equal(1); + expect(_inputNode.selectionEnd).to.equal(_inputNode.value.length); + expect(getFilteredOptionValues(el.combobox)).to.eql(['lorem', 'dolor']); + + // Act (continue typing) + mimicUserTyping(el.combobox, 'lo'); + // simulate fetching data from server + el.clearOptions(); + await el.updateComplete; + await el.updateComplete; + el.fillAllOptions(); + await el.updateComplete; + await el.updateComplete; + + // Assert + expect(_inputNode.value).to.equal('lorem'); + expect(_inputNode.selectionStart).to.equal(2); + expect(_inputNode.selectionEnd).to.equal(_inputNode.value.length); + expect(getFilteredOptionValues(el.combobox)).to.eql(['lorem', 'dolor']); + + // We don't autocomplete when characters are removed + mimicUserTyping(el.combobox, 'l'); // The user pressed backspace (number of chars decreased) + expect(_inputNode.value).to.equal('l'); + expect(_inputNode.selectionStart).to.equal(_inputNode.value.length); + }); }); describe('Subclassers', () => {