fix(form-core): fieldset label as child label suffix

ing-bank · Apr 12, 2021 · fb1522d · fb1522d
1 parent 0dc706b
commit fb1522d
Show file tree

Hide file tree

Showing 7 changed files with 378 additions and 245 deletions.
diff --git a/.changeset/rotten-dots-argue.md b/.changeset/rotten-dots-argue.md
@@ -0,0 +1,5 @@
+---
+'@lion/form-core': patch
+---
+
+**form-core**: fieldset label as child label suffix. Mimics native fieldset a11y
diff --git a/packages/form-core/src/FormControlMixin.js b/packages/form-core/src/FormControlMixin.js
@@ -321,10 +321,10 @@ const FormControlMixinImplementation = superclass =>
       additionalSlots.forEach(additionalSlot => {
         const element = this.__getDirectSlotChild(additionalSlot);
         if (element) {
-          if (element.hasAttribute('data-label') === true) {
+          if (element.hasAttribute('data-label')) {
             this.addToAriaLabelledBy(element, { idPrefix: additionalSlot });
           }
-          if (element.hasAttribute('data-description') === true) {
+          if (element.hasAttribute('data-description')) {
             this.addToAriaDescribedBy(element, { idPrefix: additionalSlot });
           }
         }
@@ -346,6 +346,7 @@ const FormControlMixinImplementation = superclass =>
         if (reorder) {
           const insideNodes = nodes.filter(n => this.contains(n));
           const outsideNodes = nodes.filter(n => !this.contains(n));
+
           // eslint-disable-next-line no-param-reassign
           nodes = [...getAriaElementsInRightDomOrder(insideNodes), ...outsideNodes];
         }
@@ -704,12 +705,7 @@ const FormControlMixinImplementation = superclass =>
      * @param {HTMLElement} element
      * @param {{idPrefix?:string; reorder?: boolean}} customConfig
      */
-    addToAriaLabelledBy(element, customConfig = {}) {
-      const { idPrefix, reorder } = {
-        reorder: true,
-        ...customConfig,
-      };
-
+    addToAriaLabelledBy(element, { idPrefix = '', reorder = true } = {}) {
       // eslint-disable-next-line no-param-reassign
       element.id = element.id || `${idPrefix}-${this._inputId}`;
       if (!this._ariaLabelledNodes.includes(element)) {
@@ -720,18 +716,27 @@ const FormControlMixinImplementation = superclass =>
       }
     }
 
+    /**
+     * Meant for Application Developers wanting to delete from aria-labelledby attribute.
+     * @param {HTMLElement} element
+     */
+    removeFromAriaLabelledBy(element) {
+      if (this._ariaLabelledNodes.includes(element)) {
+        this._ariaLabelledNodes.splice(this._ariaLabelledNodes.indexOf(element), 1);
+        this._ariaLabelledNodes = [...this._ariaLabelledNodes];
+
+        // This value will be read when we need to reflect to attr
+        /** @type {boolean} */
+        this.__reorderAriaLabelledNodes = false;
+      }
+    }
+
     /**
      * Meant for Application Developers wanting to add to aria-describedby attribute.
      * @param {HTMLElement} element
      * @param {{idPrefix?:string; reorder?: boolean}} customConfig
      */
-    addToAriaDescribedBy(element, customConfig = {}) {
-      const { idPrefix, reorder } = {
-        // chronologically sorts children of host element('this')
-        reorder: true,
-        ...customConfig,
-      };
-
+    addToAriaDescribedBy(element, { idPrefix = '', reorder = true } = {}) {
       // eslint-disable-next-line no-param-reassign
       element.id = element.id || `${idPrefix}-${this._inputId}`;
       if (!this._ariaDescribedNodes.includes(element)) {
@@ -742,6 +747,20 @@ const FormControlMixinImplementation = superclass =>
       }
     }
 
+    /**
+     * Meant for Application Developers wanting to delete from aria-labelledby attribute.
+     * @param {HTMLElement} element
+     */
+    removeFromAriaDescribedBy(element) {
+      if (this._ariaDescribedNodes.includes(element)) {
+        this._ariaDescribedNodes.splice(this._ariaDescribedNodes.indexOf(element), 1);
+        this._ariaDescribedNodes = [...this._ariaDescribedNodes];
+        // This value will be read when we need to reflect to attr
+        /** @type {boolean} */
+        this.__reorderAriaLabelledNodes = false;
+      }
+    }
+
     /**
      * @param {string} slotName
      * @return {HTMLElement | undefined}

diff --git a/packages/form-core/src/form-group/FormGroupMixin.js b/packages/form-core/src/form-group/FormGroupMixin.js
@@ -454,6 +454,10 @@ const FormGroupMixinImplementation = superclass =>
       // TODO: Unlink in removeFormElement
       this.__linkChildrenMessagesToParent(child);
       this.validate({ clearCurrentResult: true });
+
+      if (typeof child.addToAriaLabelledBy === 'function' && this._labelNode) {
+        child.addToAriaLabelledBy(this._labelNode, { reorder: false });
+      }
     }
 
     /**
@@ -481,11 +485,15 @@ const FormGroupMixinImplementation = superclass =>
 
     /**
      * @override of FormRegistrarMixin. Connects ValidateMixin
-     * @param {FormRegisteringHost} el
+     * @param {FormRegisteringHost & FormControlHost} el
      */
     removeFormElement(el) {
       super.removeFormElement(el);
       this.validate({ clearCurrentResult: true });
+
+      if (typeof el.removeFromAriaLabelledBy === 'function' && this._labelNode) {
+        el.removeFromAriaLabelledBy(this._labelNode, { reorder: false });
+      }
     }
   };
 

diff --git a/packages/form-core/test-suites/form-group/FormGroupMixin-input.suite.js b/packages/form-core/test-suites/form-group/FormGroupMixin-input.suite.js
@@ -40,7 +40,7 @@ export function runFormGroupMixinInputSuite(cfg = {}) {
     localizeTearDown();
   });
 
-  describe('FormGroupMixin with LionInput', () => {
+  describe('FormGroupMixin with LionField', () => {
     it('serializes undefined values as "" (nb radios/checkboxes are always serialized)', async () => {
       const fieldset = /**  @type {FormGroup} */ (await fixture(html`
         <${tag}>
@@ -55,6 +55,34 @@ export function runFormGroupMixinInputSuite(cfg = {}) {
         'custom[]': ['custom 1', ''],
       });
     });
+
+    it('suffixes child labels with group label, just like in <fieldset>', async () => {
+      const el = /**  @type {FormGroup} */ (await fixture(html`
+        <${tag} label="set">
+          <${childTag} name="A" label="fieldA"></${childTag}>
+          <${childTag} name="B" label="fieldB"></${childTag}>
+        </${tag}>
+      `));
+
+      /**
+       * @param {LionInput} formControl
+       */
+      function getLabels(formControl) {
+        return /** @type {string} */ (formControl._inputNode.getAttribute('aria-labelledby')).split(
+          ' ',
+        );
+      }
+      const field1 = el.formElements[0];
+      const field2 = el.formElements[1];
+
+      expect(getLabels(field1)).to.eql([field1._labelNode.id, el._labelNode.id]);
+      expect(getLabels(field2)).to.eql([field2._labelNode.id, el._labelNode.id]);
+
+      // Test the cleanup on disconnected
+      el.removeChild(field1);
+      await field1.updateComplete;
+      expect(getLabels(field1)).to.eql([field1._labelNode.id]);
+    });
   });
 
   describe('Screen reader relations (aria-describedby) for child inputs and fieldsets', () => {