From 659cbff18c012253fa7592bf8305a92f62df19fe Mon Sep 17 00:00:00 2001
From: gerjanvangeest <Gerjan.van.Geest@ing.com>
Date: Tue, 6 Feb 2024 09:21:37 +0100
Subject: [PATCH] fix(form-core): order aria-labelledby and aria-describedby
 based on slot order instead of dom order

---
 .changeset/witty-houses-argue.md              |  5 ++
 .../form-core/src/FormControlMixin.js         | 14 +++-
 .../form-core/test/FormControlMixin.test.js   | 66 ++++++++++---------
 3 files changed, 53 insertions(+), 32 deletions(-)
 create mode 100644 .changeset/witty-houses-argue.md

diff --git a/.changeset/witty-houses-argue.md b/.changeset/witty-houses-argue.md
new file mode 100644
index 000000000..f89f2a670
--- /dev/null
+++ 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
index fd0c0c03f..52df9d953 100644
--- 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
index 7e83c1e9b..123213732 100644
--- 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']);
-        expect(
-          /** @type {string} */ (_inputNode.getAttribute('aria-describedby')).split(' '),
-        ).to.eql(['myInput', 'internalDescription']);
-
-        // cleanup
-        el.removeFromAriaLabelledBy(myInput);
-        await el.updateComplete;
-        el.removeFromAriaDescribedBy(myInput);
-        await el.updateComplete;
-
-        // opt-out of reorder
-        el.addToAriaLabelledBy(myInput, { reorder: false });
-        await el.updateComplete;
-        el.addToAriaDescribedBy(myInput, { reorder: false });
-        await el.updateComplete;
-
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-labelledby')).split(' '),
         ).to.eql(['internalLabel', 'myInput']);
         expect(
           /** @type {string} */ (_inputNode.getAttribute('aria-describedby')).split(' '),
         ).to.eql(['internalDescription', 'myInput']);
+
+        // cleanup
+        el.removeFromAriaLabelledBy(internalLabel);
+        await el.updateComplete;
+        el.removeFromAriaDescribedBy(internalDescription);
+        await el.updateComplete;
+
+        // opt-out of reorder
+        el.addToAriaLabelledBy(internalLabel, { reorder: false });
+        await el.updateComplete;
+        el.addToAriaDescribedBy(internalDescription, { reorder: false });
+        await el.updateComplete;
+
+        expect(
+          /** @type {string} */ (_inputNode.getAttribute('aria-labelledby')).split(' '),
+        ).to.eql(['myInput', 'internalLabel']);
+        expect(
+          /** @type {string} */ (_inputNode.getAttribute('aria-describedby')).split(' '),
+        ).to.eql(['myInput', 'internalDescription']);
       });
 
       it('respects provided order for external elements', async () => {