From 3fb9d660ba53422af6e47956587cc2763092fc47 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Thu, 25 Jul 2024 08:58:28 +0200 Subject: [PATCH] fix: do not set placeholder to empty string by default (#7573) (#7585) Co-authored-by: Serhii Kulykov --- .../src/vaadin-multi-select-combo-box.js | 6 ++-- .../test/accessibility.test.js | 34 +++++++++++++------ .../multi-select-combo-box.test.snap.js | 25 +++----------- 3 files changed, 33 insertions(+), 32 deletions(-) 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 955847030d..2d67b9dd75 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 @@ -439,7 +439,6 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El */ placeholder: { type: String, - value: '', observer: '_placeholderChanged', }, @@ -794,9 +793,12 @@ class MultiSelectComboBox extends ResizeMixin(InputControlMixin(ThemableMixin(El // Use placeholder for announcing items if (this._hasValue) { const tmpPlaceholder = this._mergeItemLabels(selectedItems); + if (this.__tmpA11yPlaceholder === undefined) { + this.__savedPlaceholder = this.placeholder; + } this.__tmpA11yPlaceholder = tmpPlaceholder; this.placeholder = tmpPlaceholder; - } else { + } else if (this.__tmpA11yPlaceholder !== undefined) { delete this.__tmpA11yPlaceholder; this.placeholder = this.__savedPlaceholder; } diff --git a/packages/multi-select-combo-box/test/accessibility.test.js b/packages/multi-select-combo-box/test/accessibility.test.js index a01e752781..bae8ea03c3 100644 --- a/packages/multi-select-combo-box/test/accessibility.test.js +++ b/packages/multi-select-combo-box/test/accessibility.test.js @@ -35,46 +35,60 @@ describe('accessibility', () => { describe('placeholder', () => { beforeEach(() => { - comboBox = fixtureSync(` - - `); + // Do not use `fixtureSync()` helper to test the case where both placeholder + // and selectedItems are set when the component is initialized, to make sure + // that the placeholder is correctly restored after clearing selectedItems. + comboBox = document.createElement('vaadin-multi-select-combo-box'); + comboBox.placeholder = 'Fruits'; comboBox.items = ['Apple', 'Banana', 'Lemon', 'Orange']; + comboBox.selectedItems = ['Apple', 'Banana']; + document.body.appendChild(comboBox); inputElement = comboBox.inputElement; }); + afterEach(() => { + comboBox.remove(); + }); + it('should set input placeholder when selected items are changed', () => { - comboBox.selectedItems = ['Apple', 'Banana']; expect(inputElement.getAttribute('placeholder')).to.equal('Apple, Banana'); }); it('should restore original placeholder when selected items are removed', () => { - comboBox.selectedItems = ['Apple', 'Banana']; comboBox.selectedItems = []; expect(inputElement.getAttribute('placeholder')).to.equal('Fruits'); }); it('should keep input placeholder when placeholder property is updated', () => { - comboBox.selectedItems = ['Apple', 'Banana']; comboBox.placeholder = 'Options'; expect(inputElement.getAttribute('placeholder')).to.equal('Apple, Banana'); }); it('should restore updated placeholder when placeholder property is updated', () => { - comboBox.selectedItems = ['Apple', 'Banana']; comboBox.placeholder = 'Options'; comboBox.selectedItems = []; expect(inputElement.getAttribute('placeholder')).to.equal('Options'); }); + it('should restore placeholder when selected items are updated and removed', () => { + comboBox.selectedItems = ['Apple']; + comboBox.selectedItems = []; + expect(inputElement.getAttribute('placeholder')).to.equal('Fruits'); + }); + it('should restore empty placeholder when selected items are removed', () => { comboBox.placeholder = ''; - comboBox.selectedItems = ['Apple', 'Banana']; comboBox.selectedItems = []; expect(comboBox.placeholder).to.be.equal(''); expect(inputElement.hasAttribute('placeholder')).to.be.false; }); + + it('should clear placeholder when set to undefined and selected items are removed', () => { + comboBox.placeholder = undefined; + comboBox.selectedItems = []; + expect(comboBox.placeholder).to.be.undefined; + expect(inputElement.hasAttribute('placeholder')).to.be.false; + }); }); }); diff --git a/packages/multi-select-combo-box/test/dom/__snapshots__/multi-select-combo-box.test.snap.js b/packages/multi-select-combo-box/test/dom/__snapshots__/multi-select-combo-box.test.snap.js index 41966bb37d..701b2a4064 100644 --- a/packages/multi-select-combo-box/test/dom/__snapshots__/multi-select-combo-box.test.snap.js +++ b/packages/multi-select-combo-box/test/dom/__snapshots__/multi-select-combo-box.test.snap.js @@ -2,7 +2,7 @@ export const snapshots = {}; snapshots["vaadin-multi-select-combo-box host default"] = -` +`