fix: do not set placeholder to empty string by default (#7573) (#7585)

Co-authored-by: Serhii Kulykov <[email protected]>
vaadin · Jul 25, 2024 · 3fb9d66 · 3fb9d66
1 parent 0fa4cd8
commit 3fb9d66
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 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
@@ -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
@@ -35,46 +35,60 @@ describe('accessibility', () => {
 
       describe('placeholder', () => {
         beforeEach(() => {
-          comboBox = fixtureSync(`
-            <vaadin-multi-select-combo-box
-              placeholder="Fruits"
-            ></vaadin-multi-select-combo-box>
-          `);
+          // 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
@@ -2,7 +2,7 @@
 export const snapshots = {};
 
 snapshots["vaadin-multi-select-combo-box host default"] = 
-`<vaadin-multi-select-combo-box placeholder="">
+`<vaadin-multi-select-combo-box>
   <label
     for="input-vaadin-multi-select-combo-box-4"
     id="label-vaadin-multi-select-combo-box-0"
@@ -38,10 +38,7 @@ snapshots["vaadin-multi-select-combo-box host default"] =
 /* end snapshot vaadin-multi-select-combo-box host default */
 
 snapshots["vaadin-multi-select-combo-box host label"] = 
-`<vaadin-multi-select-combo-box
-  has-label=""
-  placeholder=""
->
+`<vaadin-multi-select-combo-box has-label="">
   <label
     for="input-vaadin-multi-select-combo-box-4"
     id="label-vaadin-multi-select-combo-box-0"
@@ -79,10 +76,7 @@ snapshots["vaadin-multi-select-combo-box host label"] =
 /* end snapshot vaadin-multi-select-combo-box host label */
 
 snapshots["vaadin-multi-select-combo-box host helper"] = 
-`<vaadin-multi-select-combo-box
-  has-helper=""
-  placeholder=""
->
+`<vaadin-multi-select-combo-box has-helper="">
   <label
     for="input-vaadin-multi-select-combo-box-4"
     id="label-vaadin-multi-select-combo-box-0"
@@ -128,7 +122,6 @@ snapshots["vaadin-multi-select-combo-box host error"] =
 `<vaadin-multi-select-combo-box
   has-error-message=""
   invalid=""
-  placeholder=""
 >
   <label
     for="input-vaadin-multi-select-combo-box-4"
@@ -169,10 +162,7 @@ snapshots["vaadin-multi-select-combo-box host error"] =
 /* end snapshot vaadin-multi-select-combo-box host error */
 
 snapshots["vaadin-multi-select-combo-box host required"] = 
-`<vaadin-multi-select-combo-box
-  placeholder=""
-  required=""
->
+`<vaadin-multi-select-combo-box required="">
   <label
     for="input-vaadin-multi-select-combo-box-4"
     id="label-vaadin-multi-select-combo-box-0"
@@ -212,7 +202,6 @@ snapshots["vaadin-multi-select-combo-box host disabled"] =
 `<vaadin-multi-select-combo-box
   aria-disabled="true"
   disabled=""
-  placeholder=""
 >
   <label
     for="input-vaadin-multi-select-combo-box-4"
@@ -252,10 +241,7 @@ snapshots["vaadin-multi-select-combo-box host disabled"] =
 /* end snapshot vaadin-multi-select-combo-box host disabled */
 
 snapshots["vaadin-multi-select-combo-box host readonly"] = 
-`<vaadin-multi-select-combo-box
-  placeholder=""
-  readonly=""
->
+`<vaadin-multi-select-combo-box readonly="">
   <label
     for="input-vaadin-multi-select-combo-box-4"
     id="label-vaadin-multi-select-combo-box-0"
@@ -333,7 +319,6 @@ snapshots["vaadin-multi-select-combo-box host opened default"] =
 `<vaadin-multi-select-combo-box
   focused=""
   opened=""
-  placeholder=""
 >
   <label
     for="input-vaadin-multi-select-combo-box-4"