From f7af2fd3301a8adcbb07afd9179a02765998c736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Mon, 15 Jan 2024 09:48:54 +0100 Subject: [PATCH] feat: add keepFilter option (#7063) (CP: 23.4) (#7085) Cherry-pick of #7063 --- .../combo-box/src/vaadin-combo-box-mixin.js | 15 +- .../vaadin-multi-select-combo-box-internal.js | 32 +++++ .../src/vaadin-multi-select-combo-box.d.ts | 6 + .../src/vaadin-multi-select-combo-box.js | 40 +++++- .../test/selecting-items.test.js | 130 ++++++++++++++++-- 5 files changed, 205 insertions(+), 18 deletions(-) diff --git a/packages/combo-box/src/vaadin-combo-box-mixin.js b/packages/combo-box/src/vaadin-combo-box-mixin.js index 9a2154cfc2..ce230308c4 100644 --- a/packages/combo-box/src/vaadin-combo-box-mixin.js +++ b/packages/combo-box/src/vaadin-combo-box-mixin.js @@ -869,6 +869,15 @@ export const ComboBoxMixin = (subclass) => this._detectAndDispatchChange(); } + /** + * Clears the current filter. Should be used instead of setting the property + * directly in order to allow overriding this in multi-select combo box. + * @protected + */ + _clearFilter() { + this.filter = ''; + } + /** * Reverts back to original value. */ @@ -940,7 +949,7 @@ export const ComboBoxMixin = (subclass) => this.value = this._getItemValue(itemMatchingInputValue); } else { // Revert the input value - this._inputElementValue = this.selectedItem ? this._getItemLabel(this.selectedItem) : this.value || ''; + this._revertInputValueToValue(); } } @@ -948,7 +957,7 @@ export const ComboBoxMixin = (subclass) => this._clearSelectionRange(); - this.filter = ''; + this._clearFilter(); } /** @@ -1097,7 +1106,7 @@ export const ComboBoxMixin = (subclass) => this.selectedItem = null; } - this.filter = ''; + this._clearFilter(); // In the next _detectAndDispatchChange() call, the change detection should pass this._lastCommittedValue = undefined; diff --git a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-internal.js b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-internal.js index 81f429a87b..2a7de4cd65 100644 --- a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-internal.js +++ b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box-internal.js @@ -58,6 +58,14 @@ class MultiSelectComboBoxInternal extends ComboBoxDataProviderMixin(ComboBoxMixi notify: true, }, + /** + * When true, filter string isn't cleared after selecting an item. + */ + keepFilter: { + type: Boolean, + value: false, + }, + /** * When set to `true`, "loading" attribute is set * on the host and the overlay element. @@ -274,6 +282,30 @@ class MultiSelectComboBoxInternal extends ComboBoxDataProviderMixin(ComboBoxMixi super._onEscape(event); } + /** + * Override from combo-box to ignore requests to clear the filter if the + * keepFilter option is enabled. Exceptions are when the dropdown is closed, + * so the filter is still cleared on cancel and focus out. + * @protected + * @override + */ + _clearFilter() { + if (!this.keepFilter || !this.opened) { + super._clearFilter(); + } + } + + /** + * Override method from combo-box to always clear the filter when reverting + * the input value, regardless of the keepFilter option. + * @override + * @protected + */ + _revertInputValueToValue() { + super._revertInputValueToValue(); + this.filter = ''; + } + /** * @protected * @override diff --git a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.d.ts b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.d.ts index 22f80c8c8f..bcfb61ea24 100644 --- a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.d.ts +++ b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.d.ts @@ -268,6 +268,12 @@ declare class MultiSelectComboBox extends HTMLEleme */ i18n: MultiSelectComboBoxI18n; + /** + * When true, filter string isn't cleared after selecting an item. + * @attr {boolean} keep-filter + */ + keepFilter: boolean; + /** * True when loading items from the data provider, false otherwise. */ diff --git a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.js b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.js index 9c1507c04e..1d3c345ebc 100644 --- a/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.js +++ b/packages/multi-select-combo-box/src/vaadin-multi-select-combo-box.js @@ -179,6 +179,7 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El top-group="[[_topGroup]]" opened="{{opened}}" renderer="[[renderer]]" + keep-filter="[[keepFilter]]" theme$="[[_theme]]" on-combo-box-item-selected="_onComboBoxItemSelected" on-change="_onComboBoxChange" @@ -346,6 +347,14 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El }, }, + /** + * When true, filter string isn't cleared after selecting an item. + */ + keepFilter: { + type: Boolean, + value: false, + }, + /** * True when loading items from the data provider, false otherwise. */ @@ -834,10 +843,27 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El return selectedItems.indexOf(item); } - /** @private */ - __clearFilter() { - this.filter = ''; - this.$.comboBox.clear(); + /** + * Clear the internal combo box value and filter. Filter will not be cleared + * when the `keepFilter` option is enabled. Using `force` can enforce clearing + * the filter. + * @param {boolean} force overrides the keepFilter option + * @private + */ + __clearInternalValue(force = false) { + if (!this.keepFilter || force) { + // Clear both combo box value and filter. + this.filter = ''; + this.$.comboBox.clear(); + } else { + // Only clear combo box value. This effectively resets _lastCommittedValue + // which allows toggling the same item multiple times via keyboard. + this.$.comboBox.clear(); + // Restore input to the filter value. Needed when items are + // navigated with keyboard, which overrides the input value + // with the item label. + this.inputElement.value = this.filter; + } } /** @private */ @@ -869,7 +895,7 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El const lastFilter = this._lastFilter; // Do not unselect when manually typing and committing an already selected item. if (lastFilter && lastFilter.toLowerCase() === itemLabel.toLowerCase()) { - this.__clearFilter(); + this.__clearInternalValue(); return; } @@ -882,7 +908,7 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El this.__updateSelection(itemsCopy); // Suppress `value-changed` event. - this.__clearFilter(); + this.__clearInternalValue(); this.__announceItem(itemLabel, isSelected, itemsCopy.length); } @@ -1226,7 +1252,7 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El // Stop the original event event.stopPropagation(); - this.__clearFilter(); + this.__clearInternalValue(true); this.dispatchEvent( new CustomEvent('custom-value-set', { diff --git a/packages/multi-select-combo-box/test/selecting-items.test.js b/packages/multi-select-combo-box/test/selecting-items.test.js index bc5ef8cc18..7c1f151946 100644 --- a/packages/multi-select-combo-box/test/selecting-items.test.js +++ b/packages/multi-select-combo-box/test/selecting-items.test.js @@ -9,6 +9,14 @@ import { getAllItems, getDataProvider, getFirstItem } from './helpers.js'; describe('selecting items', () => { let comboBox, internal, inputElement; + function expectItems(values) { + const items = getAllItems(comboBox); + expect(items.length).to.equal(values.length); + values.forEach((value, idx) => { + expect(items[idx].textContent).to.equal(value); + }); + } + beforeEach(() => { comboBox = fixtureSync(``); internal = comboBox.$.comboBox; @@ -110,6 +118,14 @@ describe('selecting items', () => { comboBox.clear(); expect(comboBox.selectedItems).to.deep.equal([]); }); + + it('should keep the filter and input value when committing an invalid option', async () => { + await sendKeys({ type: 'an' }); + await sendKeys({ down: 'Enter' }); + expect(comboBox.opened).to.be.true; + expect(comboBox.filter).to.equal('an'); + expect(inputElement.value).to.equal('an'); + }); }); describe('dataProvider', () => { @@ -127,14 +143,6 @@ describe('selecting items', () => { }); describe('selected items on top', () => { - function expectItems(values) { - const items = getAllItems(comboBox); - expect(items.length).to.equal(values.length); - values.forEach((value, idx) => { - expect(items[idx].textContent).to.equal(value); - }); - } - beforeEach(() => { comboBox.selectedItemsOnTop = true; }); @@ -309,4 +317,110 @@ describe('selecting items', () => { }); }); }); + + describe('keep filter', () => { + beforeEach(() => { + comboBox.items = ['apple', 'banana', 'lemon', 'orange']; + comboBox.keepFilter = true; + }); + + it('should keep the filter after selecting items', async () => { + await sendKeys({ type: 'an' }); + expectItems(['banana', 'orange']); + + const filterChangeSpy = sinon.spy(); + comboBox.addEventListener('filter-changed', filterChangeSpy); + + await sendKeys({ down: 'ArrowDown' }); + await sendKeys({ down: 'Enter' }); + expect(comboBox.selectedItems).to.deep.equal(['banana']); + expect(comboBox.filter).to.equal('an'); + expect(inputElement.value).to.equal('an'); + expectItems(['banana', 'orange']); + // Filter should never change, otherwise data provider would be called + expect(filterChangeSpy.notCalled).to.be.true; + }); + + it('should clear the filter when closing the overlay', async () => { + await sendKeys({ type: 'an' }); + expectItems(['banana', 'orange']); + + inputElement.blur(); + expect(comboBox.filter).to.equal(''); + expect(inputElement.value).to.equal(''); + }); + + it('should clear a matching filter when closing the overlay', async () => { + await sendKeys({ type: 'apple' }); + + inputElement.blur(); + expect(comboBox.selectedItems).to.deep.equal([]); + expect(comboBox.filter).to.equal(''); + expect(inputElement.value).to.equal(''); + }); + + it('should clear the filter when pressing escape', async () => { + await sendKeys({ type: 'an' }); + expectItems(['banana', 'orange']); + + await sendKeys({ down: 'Escape' }); + expect(comboBox.filter).to.equal(''); + expect(inputElement.value).to.equal(''); + }); + + it('should clear the filter when pressing escape after selecting an item', async () => { + await sendKeys({ type: 'an' }); + expectItems(['banana', 'orange']); + + await sendKeys({ down: 'ArrowDown' }); + await sendKeys({ down: 'Enter' }); + // Pressing escape twice to first deselect item and then close the overlay + await sendKeys({ down: 'Escape' }); + await sendKeys({ down: 'Escape' }); + expect(comboBox.opened).to.be.false; + expect(comboBox.filter).to.equal(''); + expect(inputElement.value).to.equal(''); + }); + + it('should keep the filter and input value when committing an invalid option', async () => { + await sendKeys({ type: 'an' }); + expectItems(['banana', 'orange']); + + await sendKeys({ down: 'Enter' }); + expect(comboBox.opened).to.be.true; + expect(inputElement.value).to.equal('an'); + expect(comboBox.filter).to.equal('an'); + }); + + it('should allow toggling items via keyboard', async () => { + await sendKeys({ down: 'ArrowDown' }); + await sendKeys({ down: 'ArrowDown' }); + await sendKeys({ down: 'Enter' }); + expect(comboBox.selectedItems).to.deep.equal(['apple']); + + await sendKeys({ down: 'Enter' }); + expect(comboBox.selectedItems).to.deep.equal([]); + }); + + it('should restore the input value to the filter after selecting an item', async () => { + await sendKeys({ type: 'an' }); + await sendKeys({ down: 'ArrowDown' }); + await sendKeys({ down: 'Enter' }); + expect(comboBox.selectedItems).to.deep.equal(['banana']); + expect(inputElement.value).to.equal('an'); + }); + + describe('with allowCustomValue', () => { + beforeEach(() => { + comboBox.allowCustomValue = true; + }); + + it('should clear the filter value after entering custom value', async () => { + await sendKeys({ type: 'pear' }); + await sendKeys({ down: 'Enter' }); + expect(comboBox.filter).to.equal(''); + expect(inputElement.value).to.equal(''); + }); + }); + }); });