From 73e9dc5849bbafff8ab7a63ddfb86afd3f1f1791 Mon Sep 17 00:00:00 2001
From: Thijs Louisse <Thijs.Louisse@ing.com>
Date: Mon, 25 Nov 2019 18:55:37 +0100
Subject: [PATCH] fix(field): aria ordering within a FormControl

---
 packages/field/src/FormControlMixin.js       | 19 +++++++--
 packages/field/test/FormControlMixin.test.js | 44 +++++++++++++++++++-
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/packages/field/src/FormControlMixin.js b/packages/field/src/FormControlMixin.js
index a9bffb133..8fe6a210e 100644
--- a/packages/field/src/FormControlMixin.js
+++ b/packages/field/src/FormControlMixin.js
@@ -186,8 +186,10 @@ export const FormControlMixin = dedupeMixin(
       __reflectAriaAttr(attrName, nodes, reorder) {
         if (this._inputNode) {
           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(nodes);
+            nodes = [...getAriaElementsInRightDomOrder(insideNodes), ...outsideNodes];
           }
           const string = nodes.map(n => n.id).join(' ');
           this._inputNode.setAttribute(attrName, string);
@@ -464,7 +466,12 @@ export const FormControlMixin = dedupeMixin(
        * Meant for Application Developers wanting to add to aria-labelledby attribute.
        * @param {Element} element
        */
-      addToAriaLabelledBy(element, { idPrefix, reorder } = { reorder: true }) {
+      addToAriaLabelledBy(element, customConfig = {}) {
+        const { idPrefix, reorder } = {
+          reorder: true,
+          ...customConfig,
+        };
+
         // eslint-disable-next-line no-param-reassign
         element.id = element.id || `${idPrefix}-${this._inputId}`;
         if (!this._ariaLabelledNodes.includes(element)) {
@@ -478,7 +485,13 @@ export const FormControlMixin = dedupeMixin(
        * Meant for Application Developers wanting to add to aria-describedby attribute.
        * @param {Element} element
        */
-      addToAriaDescribedBy(element, { idPrefix, reorder } = { reorder: true }) {
+      addToAriaDescribedBy(element, customConfig = {}) {
+        const { idPrefix, reorder } = {
+          // chronologically sorts children of host element('this')
+          reorder: true,
+          ...customConfig,
+        };
+
         // eslint-disable-next-line no-param-reassign
         element.id = element.id || `${idPrefix}-${this._inputId}`;
         if (!this._ariaDescribedNodes.includes(element)) {
diff --git a/packages/field/test/FormControlMixin.test.js b/packages/field/test/FormControlMixin.test.js
index bb4f63ff5..074bf0972 100644
--- a/packages/field/test/FormControlMixin.test.js
+++ b/packages/field/test/FormControlMixin.test.js
@@ -62,7 +62,49 @@ describe('FormControlMixin', () => {
     });
   });
 
-  it('adds aria-live="politie" to the feedback slot', async () => {
+  it('internally sorts aria-describedby and aria-labelledby ids', async () => {
+    const wrapper = await fixture(html`
+      <div id="wrapper">
+        <div id="additionalLabelA">should go after input internals</div>
+        <div id="additionalDescriptionA">should go after input internals</div>
+        <${tag}>
+          <input slot="input" />
+          <label slot="label">Added to label by default</label>
+          <div slot="feedback">Added to description by default</div>
+        </${tag}>
+        <div id="additionalLabelB">should go after input internals</div>
+        <div id="additionalDescriptionB">should go after input internals</div>
+      </div>`);
+    const el = wrapper.querySelector(elem);
+
+    const { _inputNode } = el;
+    console.log('_inputNode', _inputNode);
+
+    // 1. addToAriaLabelledBy()
+    // external inputs should go in order defined by user
+    el.addToAriaLabelledBy(wrapper.querySelector('#additionalLabelB'));
+    el.addToAriaLabelledBy(wrapper.querySelector('#additionalLabelA'));
+
+    expect(
+      _inputNode.getAttribute('aria-labelledby').indexOf(`label-${el._inputId}`) <
+        _inputNode.getAttribute('aria-labelledby').indexOf('additionalLabelB') <
+        _inputNode.getAttribute('aria-labelledby').indexOf('additionalLabelA'),
+    );
+
+    // 2. addToAriaDescribedBy()
+    // Check if the aria attr is filled initially
+    el.addToAriaDescribedBy(wrapper.querySelector('#additionalDescriptionB'));
+    el.addToAriaDescribedBy(wrapper.querySelector('#additionalDescriptionA'));
+
+    // Should be placed in the end
+    expect(
+      _inputNode.getAttribute('aria-describedby').indexOf(`feedback-${el._inputId}`) <
+        _inputNode.getAttribute('aria-describedby').indexOf('additionalDescriptionB') <
+        _inputNode.getAttribute('aria-describedby').indexOf('additionalDescriptionA'),
+    );
+  });
+
+  it('adds aria-live="polite" to the feedback slot', async () => {
     const lionField = await fixture(html`
       <${tag}>
         ${inputSlot}