From 34cc0210271d5baf97b04c3c6bc5cfc30b7f3bac Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Wed, 21 Oct 2020 20:12:38 +0300 Subject: [PATCH 01/10] fix: set value on dataprovider callback --- src/vaadin-combo-box-data-provider-mixin.html | 26 +++++++++++++++---- src/vaadin-combo-box-mixin.html | 9 +++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/vaadin-combo-box-data-provider-mixin.html b/src/vaadin-combo-box-data-provider-mixin.html index e32095819..03f33a5cc 100644 --- a/src/vaadin-combo-box-data-provider-mixin.html +++ b/src/vaadin-combo-box-data-provider-mixin.html @@ -107,11 +107,24 @@ /** @private */ _dataProviderFilterChanged() { - if (this.dataProvider && this.opened) { - this.size = undefined; - this._pendingRequests = {}; - this.clearCache(); + if(!this._shouldFetchData()) { + return; + } + + this.size = undefined; + this._pendingRequests = {}; + this.clearCache(); + } + + /** @private */ + _shouldFetchData() { + + if(!this.dataProvider) { + return false; } + + return this.opened || + (this.filter && this.filter.length); } /** @private */ @@ -161,6 +174,9 @@ if (this._isValidValue(this.value) && this._getItemValue(this.selectedItem) !== this.value) { this._selectItemForValue(this.value); } + if (!this.opened && !this.hasAttribute('focused')) { + this._commitValue(); + } this.size = size; delete this._pendingRequests[page]; @@ -197,7 +213,7 @@ filteredItems.push(this.__placeHolder); } this.filteredItems = filteredItems; - if (this.opened) { + if (this._shouldFetchData()) { this._loadPage(0); } else { this._forceNextRequest = true; diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html index caccbe1ba..702e2b4d9 100644 --- a/src/vaadin-combo-box-mixin.html +++ b/src/vaadin-combo-box-mixin.html @@ -552,7 +552,7 @@ /** @private */ _closeOrCommit() { - if (this.autoOpenDisabled && !this.opened) { + if (this.autoOpenDisabled && !this.opened && !this.loading) { this._commitValue(); } else { this.close(); @@ -685,9 +685,12 @@ this.value = ''; } } else { + const itemsMatchedByLabel = this.filteredItems + && this.filteredItems.filter(item => this._getItemLabel(item) === this._inputElementValue) + || []; if (this.allowCustomValue // to prevent a repetitive input value being saved after pressing ESC and Tab. - && !(this.filteredItems && this.filteredItems.filter(item => this._getItemLabel(item) === this._inputElementValue).length)) { + && !itemsMatchedByLabel.length) { const e = new CustomEvent('custom-value-set', {detail: this._inputElementValue, composed: true, cancelable: true, bubbles: true}); this.dispatchEvent(e); @@ -696,6 +699,8 @@ this._selectItemForValue(customValue); this.value = customValue; } + } else if (!this.open && itemsMatchedByLabel.length == 1) { + this.value = this._getItemValue(itemsMatchedByLabel[0]); } else { this._inputElementValue = this.selectedItem ? this._getItemLabel(this.selectedItem) : (this.value || ''); } From d77799bc7b39618b95c791db2ac5caa754dc6081 Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Wed, 21 Oct 2020 20:14:58 +0300 Subject: [PATCH 02/10] Fix styling issues --- src/vaadin-combo-box-data-provider-mixin.html | 4 ++-- src/vaadin-combo-box-mixin.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vaadin-combo-box-data-provider-mixin.html b/src/vaadin-combo-box-data-provider-mixin.html index 03f33a5cc..72b5fc690 100644 --- a/src/vaadin-combo-box-data-provider-mixin.html +++ b/src/vaadin-combo-box-data-provider-mixin.html @@ -107,7 +107,7 @@ /** @private */ _dataProviderFilterChanged() { - if(!this._shouldFetchData()) { + if (!this._shouldFetchData()) { return; } @@ -119,7 +119,7 @@ /** @private */ _shouldFetchData() { - if(!this.dataProvider) { + if (!this.dataProvider) { return false; } diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html index 702e2b4d9..b9f5b19de 100644 --- a/src/vaadin-combo-box-mixin.html +++ b/src/vaadin-combo-box-mixin.html @@ -687,7 +687,7 @@ } else { const itemsMatchedByLabel = this.filteredItems && this.filteredItems.filter(item => this._getItemLabel(item) === this._inputElementValue) - || []; + || []; if (this.allowCustomValue // to prevent a repetitive input value being saved after pressing ESC and Tab. && !itemsMatchedByLabel.length) { From 690a67891b29134309476e3c84ae00b45706f4bf Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Thu, 22 Oct 2020 10:47:40 +0300 Subject: [PATCH 03/10] Fixed error --- src/vaadin-combo-box-mixin.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html index b9f5b19de..89408e480 100644 --- a/src/vaadin-combo-box-mixin.html +++ b/src/vaadin-combo-box-mixin.html @@ -699,7 +699,7 @@ this._selectItemForValue(customValue); this.value = customValue; } - } else if (!this.open && itemsMatchedByLabel.length == 1) { + } else if (!this.opened && itemsMatchedByLabel.length == 1) { this.value = this._getItemValue(itemsMatchedByLabel[0]); } else { this._inputElementValue = this.selectedItem ? this._getItemLabel(this.selectedItem) : (this.value || ''); From 1e045f7bb1e6d976d1cd98841b405575e64515f4 Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Thu, 22 Oct 2020 10:47:57 +0300 Subject: [PATCH 04/10] Added test --- test/lazy-loading.html | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/lazy-loading.html b/test/lazy-loading.html index 3bebf47f7..178a0d1ef 100644 --- a/test/lazy-loading.html +++ b/test/lazy-loading.html @@ -801,6 +801,20 @@ const pages = spyDataProvider.getCalls().map(call => call.args[0].page); expect(pages).to.contain(1); }); + + it('should set value if not opened nor focused', () => { + const dataProvider = (params, callback) => { + comboBox.blur(); + callback(['item 1'], 1); + }; + + comboBox.dataProvider = dataProvider; + comboBox._inputElementValue = 'item 1'; + comboBox.filter = 'item 1'; + expect(comboBox.opened).to.be.false; + expect(comboBox.hasAttribute('focused')).to.be.false; + expect(comboBox.value).to.equal('item 1'); + }); }); describe('after empty data set loaded', () => { From 3daba80561d8dfe394b5bca8131eca75165d1495 Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Thu, 22 Oct 2020 11:45:29 +0300 Subject: [PATCH 05/10] Disable change if allowCustomValue --- src/vaadin-combo-box-mixin.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html index 89408e480..4843df76f 100644 --- a/src/vaadin-combo-box-mixin.html +++ b/src/vaadin-combo-box-mixin.html @@ -699,7 +699,7 @@ this._selectItemForValue(customValue); this.value = customValue; } - } else if (!this.opened && itemsMatchedByLabel.length == 1) { + } else if (!this.allowCustomValue && !this.opened && itemsMatchedByLabel.length == 1) { this.value = this._getItemValue(itemsMatchedByLabel[0]); } else { this._inputElementValue = this.selectedItem ? this._getItemLabel(this.selectedItem) : (this.value || ''); From f0a40a071d4e687ca5e5505fcfa893c139ac4d3b Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Fri, 23 Oct 2020 20:51:06 +0300 Subject: [PATCH 06/10] chore: revision fixes --- test/lazy-loading.html | 79 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/test/lazy-loading.html b/test/lazy-loading.html index 2be8d2ac0..7c36c1619 100644 --- a/test/lazy-loading.html +++ b/test/lazy-loading.html @@ -852,20 +852,87 @@ const pages = spyDataProvider.getCalls().map(call => call.args[0].page); expect(pages).to.contain(1); }); + }); + + describe('after closed without focus', () => { + let returnedItems; + const bluringDataProvider = (params, callback) => { + comboBox.blur(); + callback(returnedItems, returnedItems.length); + }; + + beforeEach(() => { + returnedItems = ['item 12']; + comboBox.focus(); + comboBox.opened = true; + comboBox.dataProvider = bluringDataProvider; + comboBox.opened = false; + comboBox.focus(); + }); it('should set value if not opened nor focused', () => { - const dataProvider = (params, callback) => { - comboBox.blur(); - callback(['item 1'], 1); - }; + comboBox.autoOpenDisabled = false; + expect(comboBox.autoOpenDisabled).to.be.false; + expect(comboBox.hasAttribute('focused')).to.be.true; - comboBox.dataProvider = dataProvider; + comboBox._inputElementValue = 'item 12'; + comboBox.filter = 'item 12'; + expect(comboBox.opened).to.be.false; + expect(comboBox.hasAttribute('focused')).to.be.false; + expect(comboBox.value).to.equal('item 12'); + }); + + it('should set value if not opened nor focused with auto-open-disabled', () => { + comboBox.autoOpenDisabled = true; + expect(comboBox.autoOpenDisabled).to.be.true; + expect(comboBox.hasAttribute('focused')).to.be.true; + + comboBox._inputElementValue = 'item 12'; + comboBox.filter = 'item 12'; + expect(comboBox.opened).to.be.false; + expect(comboBox.hasAttribute('focused')).to.be.false; + expect(comboBox.value).to.equal('item 12'); + }); + + it('should keep empty value if it is not an exact match', () => { + expect(comboBox.hasAttribute('focused')).to.be.true; + comboBox._inputElementValue = 'item'; + comboBox.filter = 'item'; + expect(comboBox.opened).to.be.false; + expect(comboBox.hasAttribute('focused')).to.be.false; + expect(comboBox.value).to.equal(''); + }); + + it('should keep previous value if it is not an exact match', () => { + comboBox.filteredItems = ['other value','item 12']; + comboBox.value = 'other value'; + expect(comboBox.value).to.equal('other value'); + + returnedItems = ['item 12']; comboBox._inputElementValue = 'item 1'; + expect(comboBox.hasAttribute('focused')).to.be.true; comboBox.filter = 'item 1'; expect(comboBox.opened).to.be.false; expect(comboBox.hasAttribute('focused')).to.be.false; - expect(comboBox.value).to.equal('item 1'); + expect(comboBox.value).to.equal('other value'); }); + + it('should keep previous value after if allow-custom-value is set', () => { + comboBox.allowCustomValue = true; + + comboBox.open(); + comboBox.inputElement.value = 'other value'; + comboBox.inputElement.dispatchEvent(new CustomEvent('input')); + comboBox.close(); + + comboBox.focus() + comboBox.inputElement.value = 'item 12'; + comboBox.filter = 'item 12'; + + expect(comboBox.value).to.eql('other value'); + expect(comboBox.inputElement.value).to.eql('other value'); + }); + }); describe('after empty data set loaded', () => { From e3c9ebfb9b1e2418bcb1135d8f143d13ff17773a Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Mon, 26 Oct 2020 12:29:35 +0200 Subject: [PATCH 07/10] Fix spaces --- test/lazy-loading.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/lazy-loading.html b/test/lazy-loading.html index 7c36c1619..224c5ec9f 100644 --- a/test/lazy-loading.html +++ b/test/lazy-loading.html @@ -862,7 +862,7 @@ }; beforeEach(() => { - returnedItems = ['item 12']; + returnedItems = ['item 12']; comboBox.focus(); comboBox.opened = true; comboBox.dataProvider = bluringDataProvider; @@ -904,11 +904,11 @@ }); it('should keep previous value if it is not an exact match', () => { - comboBox.filteredItems = ['other value','item 12']; + comboBox.filteredItems = ['other value', 'item 12']; comboBox.value = 'other value'; expect(comboBox.value).to.equal('other value'); - returnedItems = ['item 12']; + returnedItems = ['item 12']; comboBox._inputElementValue = 'item 1'; expect(comboBox.hasAttribute('focused')).to.be.true; comboBox.filter = 'item 1'; @@ -919,13 +919,13 @@ it('should keep previous value after if allow-custom-value is set', () => { comboBox.allowCustomValue = true; - + comboBox.open(); comboBox.inputElement.value = 'other value'; comboBox.inputElement.dispatchEvent(new CustomEvent('input')); comboBox.close(); - comboBox.focus() + comboBox.focus(); comboBox.inputElement.value = 'item 12'; comboBox.filter = 'item 12'; From 94ea7bed94cc64ffad529b8439a058744b36a62f Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Mon, 26 Oct 2020 17:53:28 +0200 Subject: [PATCH 08/10] fix tests --- test/lazy-loading.html | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/lazy-loading.html b/test/lazy-loading.html index 224c5ec9f..66d68eecf 100644 --- a/test/lazy-loading.html +++ b/test/lazy-loading.html @@ -854,7 +854,7 @@ }); }); - describe('after closed without focus', () => { + describe('using data provider, lost focus before data is returned', () => { let returnedItems; const bluringDataProvider = (params, callback) => { comboBox.blur(); @@ -870,10 +870,9 @@ comboBox.focus(); }); - it('should set value if not opened nor focused', () => { + it('should set value without auto-open-disabled', () => { comboBox.autoOpenDisabled = false; expect(comboBox.autoOpenDisabled).to.be.false; - expect(comboBox.hasAttribute('focused')).to.be.true; comboBox._inputElementValue = 'item 12'; comboBox.filter = 'item 12'; @@ -882,10 +881,9 @@ expect(comboBox.value).to.equal('item 12'); }); - it('should set value if not opened nor focused with auto-open-disabled', () => { + it('should set value with auto-open-disabled', () => { comboBox.autoOpenDisabled = true; expect(comboBox.autoOpenDisabled).to.be.true; - expect(comboBox.hasAttribute('focused')).to.be.true; comboBox._inputElementValue = 'item 12'; comboBox.filter = 'item 12'; @@ -895,7 +893,6 @@ }); it('should keep empty value if it is not an exact match', () => { - expect(comboBox.hasAttribute('focused')).to.be.true; comboBox._inputElementValue = 'item'; comboBox.filter = 'item'; expect(comboBox.opened).to.be.false; @@ -910,23 +907,30 @@ returnedItems = ['item 12']; comboBox._inputElementValue = 'item 1'; - expect(comboBox.hasAttribute('focused')).to.be.true; comboBox.filter = 'item 1'; expect(comboBox.opened).to.be.false; expect(comboBox.hasAttribute('focused')).to.be.false; expect(comboBox.value).to.equal('other value'); }); - it('should keep previous value after if allow-custom-value is set', () => { + it('should keep previous value if allow-custom-value is set', () => { comboBox.allowCustomValue = true; - + const setInputValue = value => { + if (comboBox.inputElement.tagName === 'IRON-INPUT') { + comboBox.inputElement._initSlottedInput(); + comboBox.inputElement.inputElement.value = value; + } else { + comboBox.inputElement.value = value; + } + }; comboBox.open(); - comboBox.inputElement.value = 'other value'; + setInputValue('other value'); comboBox.inputElement.dispatchEvent(new CustomEvent('input')); comboBox.close(); + expect(comboBox.value).to.eql('other value'); comboBox.focus(); - comboBox.inputElement.value = 'item 12'; + setInputValue('item 12'); comboBox.filter = 'item 12'; expect(comboBox.value).to.eql('other value'); From 09b8bc57d009993db8cdb21647973c22f771e00d Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Thu, 29 Oct 2020 13:05:44 +0200 Subject: [PATCH 09/10] Unify behaviour regardless of auto-open-disabled --- src/vaadin-combo-box-data-provider-mixin.html | 4 ++-- src/vaadin-combo-box-mixin.html | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/vaadin-combo-box-data-provider-mixin.html b/src/vaadin-combo-box-data-provider-mixin.html index e7b6f0206..214677dff 100644 --- a/src/vaadin-combo-box-data-provider-mixin.html +++ b/src/vaadin-combo-box-data-provider-mixin.html @@ -72,14 +72,14 @@ static get observers() { return [ '_dataProviderFilterChanged(filter, dataProvider)', - '_dataProviderClearFilter(dataProvider, opened, value)', + '_dataProviderClearFilter(dataProvider, value)', '_warnDataProviderValue(dataProvider, value)', '_ensureFirstPage(opened)', ]; } /** @private */ - _dataProviderClearFilter(dataProvider, opened, value) { + _dataProviderClearFilter(dataProvider, value) { // Can't depend on filter in this observer as we don't want // to clear the filter whenever it's set if (dataProvider && this.filter) { diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html index 4843df76f..1f29a8fb5 100644 --- a/src/vaadin-combo-box-mixin.html +++ b/src/vaadin-combo-box-mixin.html @@ -552,7 +552,7 @@ /** @private */ _closeOrCommit() { - if (this.autoOpenDisabled && !this.opened && !this.loading) { + if (!this.opened && !this.loading) { this._commitValue(); } else { this.close(); @@ -666,7 +666,9 @@ this.close(); } - this._commitValue(); + if (!this.loading) { + this._commitValue(); + } } /** @private */ From 8f9cdc28d9096ef675e3c834ade37283cf84bc3b Mon Sep 17 00:00:00 2001 From: Tulio Garcia Date: Thu, 29 Oct 2020 15:07:09 +0200 Subject: [PATCH 10/10] Fix tests --- src/vaadin-combo-box-data-provider-mixin.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vaadin-combo-box-data-provider-mixin.html b/src/vaadin-combo-box-data-provider-mixin.html index 214677dff..603f63b74 100644 --- a/src/vaadin-combo-box-data-provider-mixin.html +++ b/src/vaadin-combo-box-data-provider-mixin.html @@ -72,17 +72,17 @@ static get observers() { return [ '_dataProviderFilterChanged(filter, dataProvider)', - '_dataProviderClearFilter(dataProvider, value)', + '_dataProviderClearFilter(dataProvider, opened, value)', '_warnDataProviderValue(dataProvider, value)', '_ensureFirstPage(opened)', ]; } /** @private */ - _dataProviderClearFilter(dataProvider, value) { + _dataProviderClearFilter(dataProvider, opened, value) { // Can't depend on filter in this observer as we don't want // to clear the filter whenever it's set - if (dataProvider && this.filter) { + if (dataProvider && !this.loading && this.filter) { this.size = undefined; this._pendingRequests = {}; this.filter = '';