fix: enable using custom id for form-item label (#3701) (#3708)

Co-authored-by: Tomi Virkki <tomivirkki@users.noreply.github.com>
vaadin · Apr 20, 2022 · 8dcaccd · 8dcaccd
1 parent ee9a2d2
commit 8dcaccd
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 10 deletions.
diff --git a/packages/form-layout/src/vaadin-form-item.js b/packages/form-layout/src/vaadin-form-item.js
@@ -177,10 +177,6 @@ class FormItem extends ThemableMixin(PolymerElement) {
      * @private
      */
     this.__fieldNode = null;
-
-    // Ensure every instance has unique ID
-    const uniqueId = (FormItem._uniqueLabelId = 1 + FormItem._uniqueLabelId || 0);
-    this.__labelId = `label-${this.localName}-${uniqueId}`;
   }
 
   /**
@@ -244,7 +240,6 @@ class FormItem extends ThemableMixin(PolymerElement) {
    */
   __onLabelSlotChange() {
     if (this.__labelNode) {
-      this.__labelNode.id = '';
       this.__labelNode = null;
 
       if (this.__fieldNode) {
@@ -255,7 +250,16 @@ class FormItem extends ThemableMixin(PolymerElement) {
     const newLabelNode = this.$.labelSlot.assignedElements()[0];
     if (newLabelNode) {
       this.__labelNode = newLabelNode;
-      this.__labelNode.id = this.__labelId;
+
+      if (this.__labelNode.id) {
+        // The new label node already has an id. Let's use it.
+        this.__labelId = this.__labelNode.id;
+      } else {
+        // The new label node doesn't have an id yet. Generate a unique one.
+        const uniqueId = (FormItem._uniqueLabelId = 1 + FormItem._uniqueLabelId || 0);
+        this.__labelId = `label-${this.localName}-${uniqueId}`;
+        this.__labelNode.id = this.__labelId;
+      }
 
       if (this.__fieldNode) {
         this.__linkLabelToField(this.__fieldNode);

diff --git a/packages/form-layout/test/form-item.test.js b/packages/form-layout/test/form-item.test.js
@@ -120,8 +120,7 @@ describe('form-item', () => {
       newLabel.slot = 'label';
       item.replaceChild(newLabel, label);
       await nextFrame();
-      expect(label.id).to.be.empty;
-      expect(newLabel.id).to.equal(labelId);
+      expect(newLabel.id).to.equal(input.getAttribute('aria-labelledby'));
     });
 
     it('should not set id to the new label element when using appendChild', async () => {
@@ -138,8 +137,36 @@ describe('form-item', () => {
       newLabel.slot = 'label';
       item.insertBefore(newLabel, label);
       await nextFrame();
-      expect(label.id).to.be.empty;
-      expect(newLabel.id).to.equal(labelId);
+      expect(newLabel.id).to.equal(input.getAttribute('aria-labelledby'));
+    });
+
+    it('should not overwrite a custom label id', async () => {
+      const newLabel = document.createElement('label');
+      newLabel.id = 'custom-label-id';
+      newLabel.slot = 'label';
+      item.replaceChild(newLabel, label);
+      await nextFrame();
+      expect(newLabel.id).to.equal('custom-label-id');
+    });
+
+    it('should bind the input with a custom label id ', async () => {
+      const newLabel = document.createElement('label');
+      newLabel.id = 'custom-label-id';
+      newLabel.slot = 'label';
+      item.replaceChild(newLabel, label);
+      await nextFrame();
+      expect(input.getAttribute('aria-labelledby')).to.equal(newLabel.id);
+    });
+
+    it('should not remove id from a label', async () => {
+      const newLabel = document.createElement('label');
+      newLabel.id = 'custom-label-id';
+      newLabel.slot = 'label';
+      item.replaceChild(newLabel, label);
+      await nextFrame();
+      item.replaceChild(label, newLabel);
+      await nextFrame();
+      expect(newLabel.id).to.equal('custom-label-id');
     });
   });