From d713e4a0cf75f17b59051d3b367006a95351f647 Mon Sep 17 00:00:00 2001
From: Carlos Lopez Jr <clshortfuse@gmail.com>
Date: Fri, 21 Jul 2023 13:05:26 -0400
Subject: [PATCH] fix(button): fix label when using [href]

---
 components/Button.js         |  8 ++++----
 components/IconButton.js     |  2 +-
 mixins/ControlMixin.js       |  6 +++---
 test/spec/Button.test.js     | 16 ++++++++++++++++
 test/spec/IconButton.test.js | 16 ++++++++++++++++
 5 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/components/Button.js b/components/Button.js
index 279cd5ef..4f3d62b5 100644
--- a/components/Button.js
+++ b/components/Button.js
@@ -9,6 +9,8 @@ import StateMixin from '../mixins/StateMixin.js';
 import SurfaceMixin from '../mixins/SurfaceMixin.js';
 import ThemableMixin from '../mixins/ThemableMixin.js';
 
+/* https://m3.material.io/components/buttons/specs */
+
 export default CustomElement
   .extend()
   .mixin(ThemableMixin)
@@ -72,21 +74,19 @@ export default CustomElement
     <mdw-icon mdw-if={hasIcon} id=icon ink={iconInk} disabled={disabledState}
       outlined={outlined} aria-hidden=true svg={svg} src={src}
       svg-path={svgPath} view-box={viewBox} icon={icon}></mdw-icon>
-    <slot id=slot disabled={disabledState} aria-hidden=false>{value}</slot>
+    <slot id=slot disabled={disabledState} aria-hidden=true>{value}</slot>
   `
   .recompose(({ refs: { anchor, shape, state, rippleContainer, surface, control } }) => {
     surface.append(shape);
     shape.append(state, rippleContainer);
     shape.setAttribute('filled', '{filled}');
-    control.setAttribute('hidden', '{href}');
+    control.setAttribute('mdw-if', '{!href}');
     control.setAttribute('role', 'button');
     anchor.setAttribute('mdw-if', '{href}');
     anchor.setAttribute('aria-label', '{_computedAriaLabel}');
     anchor.setAttribute('aria-labelledby', '{_computedAriaLabelledby}');
   })
   .css`
-    /* https://m3.material.io/components/buttons/specs */
-
     :host {
       --mdw-shape__size: var(--mdw-shape__full);
       --mdw-ink: var(--mdw-color__primary);
diff --git a/components/IconButton.js b/components/IconButton.js
index e3da76e3..b1c2da34 100644
--- a/components/IconButton.js
+++ b/components/IconButton.js
@@ -20,7 +20,7 @@ export default Button
     isToggle({ type }) {
       return type === 'checkbox';
     },
-    _computedAriaLabelledBy({ ariaLabel }) {
+    _computedAriaLabelledby({ ariaLabel }) {
       return ariaLabel ? null : 'tooltip';
     },
   })
diff --git a/mixins/ControlMixin.js b/mixins/ControlMixin.js
index 99ef6444..6fc1875d 100644
--- a/mixins/ControlMixin.js
+++ b/mixins/ControlMixin.js
@@ -103,7 +103,7 @@ export default function ControlMixin(Base) {
         }
         return ariaLabel?.trim() || null;
       },
-      _computedAriaLabelledBy({ ariaLabel }) {
+      _computedAriaLabelledby({ ariaLabel }) {
         return ariaLabel ? null : 'slot';
       },
     })
@@ -111,9 +111,9 @@ export default function ControlMixin(Base) {
     .recompose(({ template, html, element }) => {
       template.append(html`
         <${element.controlTagName} id=control
-          aria-labelledby={_computedAriaLabelledBy}
-          part=control
           aria-label={_computedAriaLabel}
+          aria-labelledby={_computedAriaLabelledby}
+          part=control
           form-disabled={disabledState}
           type={type}
           >${element.controlVoidElement ? '' : `</${element.controlTagName}>`}
diff --git a/test/spec/Button.test.js b/test/spec/Button.test.js
index 164cae70..e5b5b6da 100644
--- a/test/spec/Button.test.js
+++ b/test/spec/Button.test.js
@@ -91,6 +91,22 @@ describe('mdw-button', () => {
       const [{ role }] = iterateMeaningfulAXNodes(results);
       assert.equal(role, 'link');
     });
+
+    it('supports aria-label with [href]', async () => {
+      const element = html`<mdw-button href=# aria-label=foo></mdw-button>`;
+      const results = await axTree({ selector: element.tagName });
+      const [{ role, name }] = iterateMeaningfulAXNodes(results);
+      assert.equal(role, 'link');
+      assert.equal(name, 'foo');
+    });
+
+    it('labels based on slot with [href]', async () => {
+      const element = html`<mdw-button href=#>foo</mdw-button>`;
+      const results = await axTree({ selector: element.tagName });
+      const [{ role, name }] = iterateMeaningfulAXNodes(results);
+      assert.equal(role, 'link');
+      assert.equal(name, 'foo');
+    });
   });
 
   /* eslint-disable camelcase */
diff --git a/test/spec/IconButton.test.js b/test/spec/IconButton.test.js
index 80c66e66..e3534075 100644
--- a/test/spec/IconButton.test.js
+++ b/test/spec/IconButton.test.js
@@ -92,6 +92,22 @@ describe('mdw-icon-button', () => {
       const [{ role }] = iterateMeaningfulAXNodes(results);
       assert.equal(role, 'link');
     });
+
+    it('supports aria-label with [href]', async () => {
+      const element = html`<mdw-icon-button href=# aria-label=foo></mdw-icon-button>`;
+      const results = await axTree({ selector: element.tagName });
+      const [{ role, name }] = iterateMeaningfulAXNodes(results);
+      assert.equal(role, 'link');
+      assert.equal(name, 'foo');
+    });
+
+    it('labels based on slot with [href]', async () => {
+      const element = html`<mdw-icon-button href=#>foo</mdw-icon-button>`;
+      const results = await axTree({ selector: element.tagName });
+      const [{ role, name }] = iterateMeaningfulAXNodes(results);
+      assert.equal(role, 'link');
+      assert.equal(name, 'foo');
+    });
   });
 
   /* eslint-disable camelcase */