fix(form-core): order aria-labelledby and aria-describedby based on s…

…lot order instead of dom order
ing-bank · Feb 19, 2024 · 659cbff · 659cbff
1 parent 5073ea4
commit 659cbff
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 21 deletions.
diff --git a/.changeset/witty-houses-argue.md b/.changeset/witty-houses-argue.md
@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[form-core] order aria-labelledby and aria-describedby based on slot order instead of dom order
diff --git a/packages/ui/components/form-core/src/FormControlMixin.js b/packages/ui/components/form-core/src/FormControlMixin.js
@@ -390,8 +390,20 @@ const FormControlMixinImplementation = superclass =>
           const insideNodes = nodes.filter(n => this.contains(n));
           const outsideNodes = nodes.filter(n => !this.contains(n));
 
+          const insideSlots = insideNodes.map(n => n.assignedSlot || n);
+          const orderedInsideSlots = [...getAriaElementsInRightDomOrder(insideSlots)];
+          /** @type {Element[]} */
+          const orderedInsideNodes = [];
+          orderedInsideSlots.forEach(assignedNode => {
+            insideNodes.forEach(node => {
+              // @ts-ignore
+              if (assignedNode.name === node.slot) {
+                orderedInsideNodes.push(node);
+              }
+            });
+          });
           // eslint-disable-next-line no-param-reassign
-          nodes = [...getAriaElementsInRightDomOrder(insideNodes), ...outsideNodes];
+          nodes = [...orderedInsideNodes, ...outsideNodes];
         }
         const string = nodes.map(n => n.id).join(' ');
         this._inputNode.setAttribute(attrName, string);

diff --git a/packages/ui/components/form-core/test/FormControlMixin.test.js b/packages/ui/components/form-core/test/FormControlMixin.test.js
@@ -349,19 +349,18 @@ describe('FormControlMixin', () => {
         );
       });
 
-      it('sorts internal elements, and allows opt-out', async () => {
+      it('sorts internal elements based on assigned slots, and allows opt-out', async () => {
         const wrapper = await fixture(html`
-        <div id="wrapper">
-          <${tag}>
-            <input slot="input" id="myInput" />
-            <label slot="label" id="internalLabel">Added to label by default</label>
-            <div slot="help-text" id="internalDescription">
-              Added to description by default
-            </div>
-          </${tag}>
-          <div id="externalLabelB">should go after input internals</div>
-          <div id="externalDescriptionB">should go after input internals</div>
-        </div>`);
+          <div id="wrapper">
+            <${tag}>
+              <input slot="input" id="myInput" />
+              <label slot="label" id="internalLabel">Added to label by default</label>
+              <div slot="help-text" id="internalDescription">
+                Added to description by default
+              </div>
+            </${tag}>
+          </div>
+        `);
         const el = /** @type {FormControlMixinClass} */ (wrapper.querySelector(tagString));
         const { _inputNode } = getFormControlMembers(el);
 
@@ -370,36 +369,41 @@ describe('FormControlMixin', () => {
         // A real life scenario would be for instance when
         // a Field or FormGroup would be extended and an extra slot would be added in the template
         const myInput = /** @type {HTMLElement} */ (wrapper.querySelector('#myInput'));
+        const internalLabel = /** @type {HTMLElement} */ (wrapper.querySelector('#internalLabel'));
+        const internalDescription = /** @type {HTMLElement} */ (
+          wrapper.querySelector('#internalDescription')
+        );
+
         el.addToAriaLabelledBy(myInput);
         await el.updateComplete;
         el.addToAriaDescribedBy(myInput);
         await el.updateComplete;
 
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-labelledby')).split(' '),
-        ).to.eql(['myInput', 'internalLabel']);
+        ).to.eql(['internalLabel', 'myInput']);
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-describedby')).split(' '),
-        ).to.eql(['myInput', 'internalDescription']);
+        ).to.eql(['internalDescription', 'myInput']);
 
         // cleanup
-        el.removeFromAriaLabelledBy(myInput);
+        el.removeFromAriaLabelledBy(internalLabel);
         await el.updateComplete;
-        el.removeFromAriaDescribedBy(myInput);
+        el.removeFromAriaDescribedBy(internalDescription);
         await el.updateComplete;
 
         // opt-out of reorder
-        el.addToAriaLabelledBy(myInput, { reorder: false });
+        el.addToAriaLabelledBy(internalLabel, { reorder: false });
         await el.updateComplete;
-        el.addToAriaDescribedBy(myInput, { reorder: false });
+        el.addToAriaDescribedBy(internalDescription, { reorder: false });
         await el.updateComplete;
 
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-labelledby')).split(' '),
-        ).to.eql(['internalLabel', 'myInput']);
+        ).to.eql(['myInput', 'internalLabel']);
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-describedby')).split(' '),
-        ).to.eql(['internalDescription', 'myInput']);
+        ).to.eql(['myInput', 'internalDescription']);
       });
 
       it('respects provided order for external elements', async () => {