a11y revamp: Pharos Buttons (#594)

* feat(button): add ability to pass down ARIA attributes * feat(button): add reference to new button types Including the ExpandedState type reference for other components that consume the button. These threw errors when initially compiling so there may be other such commponents that eventually need a similar update. * feat(button): attempt at updating storybook example * feat(button): remove test code for popupstate * feat(button): add aria-haspopup * chore(changeset): add changeset * feat(button): update label attr to a11y-label * feat(button): replace property for ButtonVariant * feat(button): allow backwards compatibility Gives warning if using deprecated attributes, updating aria-pressed * feat(button): update storybook aria-pressed * feat(button): remove fallback from major release * feat(button): add a11y attributes typing * test(button): add tests for new aria attributes * feat(button): add ability to pass down ARIA attributes * feat(button): add reference to new button types Including the ExpandedState type reference for other components that consume the button. These threw errors when initially compiling so there may be other such commponents that eventually need a similar update. * feat(button): attempt at updating storybook example * feat(button): remove test code for popupstate * feat(button): add aria-haspopup * chore(changeset): add changeset * feat(button): update label attr to a11y-label * feat(button): replace property for ButtonVariant * feat(button): allow backwards compatibility Gives warning if using deprecated attributes, updating aria-pressed * feat(button): update storybook aria-pressed * feat(button): remove fallback from major release * feat(button): add a11y attributes typing * test(button): add tests for new aria attributes * fix(a11y attributes): update AriaHiddenState name * fix(button): remove ts ignore * feat(button): remove backwards compat * feat(button): add aria-disabled back * test(button): add a11y-label to new components * fix: remove sidenav button from bad merge * test(button): fix aria-pressed in toggle buttons * Update packages/pharos/src/components/sidenav/PharosSidenav.react.stories.jsx Co-authored-by: Dane Hillard <[email protected]> * Update packages/pharos/src/components/toast/pharos-toast-button.ts Co-authored-by: Dane Hillard <[email protected]> * fix(btn): remove aria-description --------- Co-authored-by: Dane Hillard <[email protected]>
ithaka · Jan 4, 2024 · f8342cc · f8342cc
1 parent a7279ce
commit f8342cc
Show file tree

Hide file tree

Showing 34 changed files with 145 additions and 124 deletions.
diff --git a/.changeset/mighty-planes-poke.md b/.changeset/mighty-planes-poke.md
@@ -0,0 +1,7 @@
+---
+'@ithaka/pharos': major
+---
+
+Add additional ARIA attributes for Pharos button
+and use a new naming convention for these specific
+pharos attributes.
diff --git a/packages/pharos/src/components/alert/pharos-alert.ts b/packages/pharos/src/components/alert/pharos-alert.ts
@@ -114,7 +114,7 @@ export class PharosAlert extends ScopedRegistryMixin(FocusMixin(PharosElement))
           variant="subtle"
           icon="close"
           icon-condensed
-          label="Close alert"
+          a11y-label="Close alert"
           class="alert__button"
           @click=${this.close}
         ></pharos-button>`

diff --git a/packages/pharos/src/components/button/PharosButton.react.stories.jsx b/packages/pharos/src/components/button/PharosButton.react.stories.jsx
@@ -45,7 +45,7 @@ export const Base = {
       fullWidth={args.fullWidth}
       href={args.href}
       hreflang={args.hreflang}
-      label={args.label}
+      a11yLabel={args.a11yLabel}
       large={args.large}
       isOnBackground={args.isOnBackground}
       ping={args.ping}

diff --git a/packages/pharos/src/components/button/pharos-button.test.ts b/packages/pharos/src/components/button/pharos-button.test.ts
@@ -24,10 +24,14 @@ describe('pharos-button', () => {
       await expect(component).to.be.accessible();
     });
 
-    it('is accessible as an icon button', async () => {
-      component.icon = 'download';
-      component.label = 'download';
+    it('is renders aria-label on button and is accessible', async () => {
+      const label = 'download';
+      component.icon = label;
+      component.a11yLabel = label;
       await component.updateComplete;
+      await expect(
+        component.renderRoot.querySelector('button')?.getAttribute('aria-label')
+      ).to.equal(label);
       await expect(component).to.be.accessible();
     });
 
@@ -37,6 +41,33 @@ describe('pharos-button', () => {
       await expect(component).to.be.accessible();
     });
 
+    it('is accessible when using aria-disabled', async () => {
+      component.a11yDisabled = 'true';
+      await component.updateComplete;
+      await expect(
+        component.renderRoot.querySelector('button')?.getAttribute('aria-disabled')
+      ).to.equal('true');
+      await expect(component).to.be.accessible();
+    });
+
+    it('is accessible when using aria-expanded', async () => {
+      component.a11yExpanded = 'true';
+      await component.updateComplete;
+      await expect(
+        component.renderRoot.querySelector('button')?.getAttribute('aria-expanded')
+      ).to.equal('true');
+      await expect(component).to.be.accessible();
+    });
+
+    it('is accessible when using aria-haspopup', async () => {
+      component.a11yHaspopup = 'menu';
+      await component.updateComplete;
+      await expect(
+        component.renderRoot.querySelector('button')?.getAttribute('aria-haspopup')
+      ).to.equal('menu');
+      await expect(component).to.be.accessible();
+    });
+
     it('is accessible as the secondary variant', async () => {
       component.variant = 'secondary';
       await component.updateComplete;

diff --git a/packages/pharos/src/components/button/pharos-button.ts b/packages/pharos/src/components/button/pharos-button.ts
@@ -108,14 +108,6 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
   @property({ type: Boolean, reflect: true })
   public large = false;
 
-  /**
-   * @deprecated
-   * Indicates the aria label to apply to the button.
-   * @attr label
-   */
-  @property({ type: String, reflect: true })
-  public label?: string;
-
   /**
    * Indicates the aria label to apply to the button.
    * @attr a11y-label
@@ -124,11 +116,11 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
   public a11yLabel?: string;
 
   /**
-   * Indicates the aria description to apply to the button.
-   * @attr a11y-description
+   * Indicates this button is a toggle button and whether it is pressed or not.
+   * @attr value
    */
-  @property({ type: String, reflect: true, attribute: 'a11y-description' })
-  public a11yDescription?: string;
+  @property({ type: String, reflect: true, attribute: 'a11y-pressed' })
+  public a11yPressed: AriaPressedState = undefined;
 
   /**
    * Indicates the aria expanded state to apply to the button.
@@ -137,6 +129,13 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
   @property({ type: String, reflect: true, attribute: 'a11y-expanded' })
   public a11yExpanded: AriaExpandedState = undefined;
 
+  /**
+   * Indicates the aria expanded state to apply to the button.
+   * @attr a11y-disabled
+   */
+  @property({ type: String, reflect: true, attribute: 'a11y-disabled' })
+  public a11yDisabled: AriaDisabledState = undefined;
+
   /**
    * Indicates the aria expanded state to apply to the button.
    * @attr a11y-haspopup
@@ -165,21 +164,6 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
   @property({ type: String, reflect: true })
   public value?: string;
 
-  /**
-   * @deprecated
-   * Indicates this button is a toggle button and whether it is pressed or not.
-   * @attr value
-   */
-  @property({ type: String, reflect: true })
-  public pressed: AriaPressedState = undefined;
-
-  /**
-   * Indicates this button is a toggle button and whether it is pressed or not.
-   * @attr value
-   */
-  @property({ type: String, reflect: true, attribute: 'a11y-pressed' })
-  public a11yPressed: AriaPressedState = undefined;
-
   @query('#button-element')
   private _button!: HTMLButtonElement | HTMLAnchorElement;
 
@@ -285,10 +269,6 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
   }
 
   protected override render(): TemplateResult {
-    // TODO: Remove in future release once sufficient time elapsed to update naming convention
-    const a11yLabel = this.a11yLabel ?? this.label;
-    const a11yPressed = this.a11yPressed ?? this.pressed;
-
     return this.href
       ? html`
           <a
@@ -300,11 +280,11 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
             ping=${ifDefined(this.ping)}
             rel=${ifDefined(this.rel)}
             target=${ifDefined(this.target)}
-            aria-label=${ifDefined(a11yLabel)}
-            aria-description=${ifDefined(this.a11yDescription)}
-            aria-pressed=${ifDefined(a11yPressed)}
+            aria-label=${ifDefined(this.a11yLabel)}
+            aria-pressed=${ifDefined(this.a11yPressed)}
             aria-expanded=${ifDefined(this.a11yExpanded)}
             aria-haspopup=${ifDefined(this.a11yHaspopup)}
+            aria-disabled=${ifDefined(this.a11yDisabled)}
             @keyup=${this._handleKeyup}
           >
             ${this.buttonContent}
@@ -318,11 +298,11 @@ export class PharosButton extends ScopedRegistryMixin(FocusMixin(AnchorElement))
             ?autofocus=${this.autofocus}
             ?disabled=${this.disabled}
             type="${ifDefined(this.type)}"
-            aria-label=${ifDefined(a11yLabel)}
-            aria-description=${ifDefined(this.a11yDescription)}
-            aria-pressed=${ifDefined(a11yPressed)}
+            aria-label=${ifDefined(this.a11yLabel)}
+            aria-pressed=${ifDefined(this.a11yPressed)}
             aria-expanded=${ifDefined(this.a11yExpanded)}
             aria-haspopup=${ifDefined(this.a11yHaspopup)}
+            aria-disabled=${ifDefined(this.a11yDisabled)}
           >
             ${this.buttonContent}
           </button>

diff --git a/packages/pharos/src/components/button/pharos-button.wc.stories.jsx b/packages/pharos/src/components/button/pharos-button.wc.stories.jsx
@@ -30,11 +30,12 @@ export const Base = {
         ?full-width=${ifDefined(args.fullWidth)}
         href=${ifDefined(args.href)}
         hreflang=${ifDefined(args.hreflang)}
-        label=${ifDefined(args.label)}
+        a11y-label=${ifDefined(args.a11yLabel)}
+        a11y-expanded=${ifDefined(args.a11yExpanded)}
+        a11y-pressed=${ifDefined(args.a11yPressed)}
         ?large=${ifDefined(args.large)}
         ?is-on-background=${ifDefined(args.isOnBackground)}
         ping=${ifDefined(args.ping)}
-        pressed=${ifDefined(args.pressed)}
         target=${ifDefined(args.target)}
         type=${ifDefined(args.type)}
         variant=${ifDefined(args.variant)}
@@ -226,7 +227,7 @@ export const IconOnly = {
     ...Base.args,
     text: undefined,
     icon: 'download',
-    label: 'download',
+    a11yLabel: 'download',
   },
 };
 

diff --git a/packages/pharos/src/components/combobox/pharos-combobox.ts b/packages/pharos/src/components/combobox/pharos-combobox.ts
@@ -258,7 +258,7 @@ export class PharosCombobox extends ScopedRegistryMixin(FormMixin(FormElement))
           variant="subtle"
           icon="close"
           ?disabled=${this.disabled}
-          label="Clear option"
+          a11y-label="Clear option"
           @click=${this._handleClearClick}
           @mousedown=${this._handleClearClick}
         >
@@ -277,7 +277,7 @@ export class PharosCombobox extends ScopedRegistryMixin(FormMixin(FormElement))
           type="button"
           variant="subtle"
           class="search__button"
-          label="Search"
+          a11y-label="Search"
           ?disabled=${this.disabled}
           @click=${this.onChange}
         ></pharos-button>

diff --git a/packages/pharos/src/components/image-card/pharos-image-card.ts b/packages/pharos/src/components/image-card/pharos-image-card.ts
@@ -367,7 +367,7 @@ export class PharosImageCard extends ScopedRegistryMixin(FocusMixin(PharosElemen
           icon="ellipses-vertical"
           variant="subtle"
           icon-condensed
-          label="More actions"
+          a11y-label="More actions"
           @click=${this._handleClick}
         ></pharos-button>`
       : html`<slot name="action-button"></slot>`;

diff --git a/packages/pharos/src/components/input-group/PharosInputGroup.react.stories.jsx b/packages/pharos/src/components/input-group/PharosInputGroup.react.stories.jsx
@@ -38,7 +38,7 @@ export const Base = {
           name="search-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
     </div>
@@ -60,7 +60,7 @@ export const Prominent = {
           name="search-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
     </div>
@@ -82,7 +82,7 @@ export const Validity = {
           name="search-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
       <PharosInputGroup validated value="[email protected]" name="my-input-group">
@@ -91,7 +91,7 @@ export const Validity = {
           name="search-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
     </div>
@@ -117,13 +117,13 @@ export const Composition = {
           name="close-button"
           icon="close"
           variant="subtle"
-          label="close"
+          a11y-label="close"
         ></PharosButton>
         <PharosButton
           name="search-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
       <PharosInputGroup name="my-input-group-select">
@@ -137,7 +137,7 @@ export const Composition = {
           name="search-with-select-button"
           icon="search"
           variant="subtle"
-          label="search"
+          a11y-label="search"
         ></PharosButton>
       </PharosInputGroup>
       <PharosInputGroup name="my-input-group-prepend">
@@ -146,7 +146,7 @@ export const Composition = {
           slot="prepend"
           icon="book"
           variant="subtle"
-          label="book"
+          a11y-label="book"
         ></PharosButton>
         <span slot="label">Prepend</span>
       </PharosInputGroup>

diff --git a/packages/pharos/src/components/input-group/pharos-input-group.test.ts b/packages/pharos/src/components/input-group/pharos-input-group.test.ts
@@ -10,7 +10,7 @@ describe('pharos-input-group', () => {
     component = await fixture(html`
       <test-pharos-input-group>
         <span slot="label">Search</span>
-        <test-pharos-button icon="search" variant="subtle" label="search"></test-pharos-button>
+        <test-pharos-button icon="search" variant="subtle" a11y-label="search"></test-pharos-button>
       </test-pharos-input-group>
     `);
   });
@@ -38,7 +38,7 @@ describe('pharos-input-group', () => {
             slot="prepend"
             icon="search"
             variant="subtle"
-            label="search"
+            a11y-label="search"
           ></test-pharos-button>
         </test-pharos-input-group>
       `