Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

a11y revamp: Pharos Buttons #594

Merged
merged 38 commits into from
Jan 4, 2024
Merged

a11y revamp: Pharos Buttons #594

merged 38 commits into from
Jan 4, 2024

Conversation

sirrah-tam
Copy link
Contributor

@sirrah-tam sirrah-tam commented Aug 28, 2023 •
edited

This change: (check at least one)

  • Adds a new feature
  • Fixes a bug
  • Improves maintainability
  • Improves documentation
  • Is a release activity

Is this a breaking change? (check one)

  • Yes
  • No

Is the: (complete all)

  • Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
  • Test suite(s) passing?
  • Code coverage maximal?
  • Changeset added?
  • Component status page up to date?

What does this change address?
This updates the Pharos Button to allow consumers to pass in attributes (listed below) following a specific naming convention (a11y-) that assign the appropriate ARIA attribute to the inner <button> element. This addresses the issues with custom web components and the shadow DOM and use of ARIA not being properly being parsed by assistive technologies.

  • a11y-label = aria-label (String)
  • a11y-expanded = aria-expanded (true, false, undefined (default))
  • a11y-pressed = aria-pressed (true, false, mixed, undefined (default))
  • a11y-haspopup = aria-haspopup (true, menu, listbox, tree, grid, dialog, false (default))
  • a11y-disabled = aria-disabled (true, false, undefined (default))

Linked issue #642

How does this change work?
Consumers can add these attributes along with the proper value to the Pharos Button and the appropriate ARIA attribute will be added to the underlying <button> element.

Additional context
This is a breaking change. The previous versions of Pharos has the label attribute that would apply the aria-label attribute to the button. This will need to be updated. Additionally, all uses of ARIA listed above will need to be updated accordingly.

This is an effort to address ADR #0009 (#559)

@sirrah-tam
feat(button): add ability to pass down ARIA attributes
1db7550
@sirrah-tam
feat(button): add reference to new button types
0bf7218 
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.
@sirrah-tam
feat(button): attempt at updating storybook example
0a26b51
@sirrah-tam sirrah-tam requested a review from a team as a code owner August 28, 2023 13:52
@sirrah-tam sirrah-tam requested review from SMQuazi, mtorres3 and shoupeva-ithaka and removed request for a team August 28, 2023 13:52
@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2023 •
edited

🦋 Changeset detected

Latest commit: 630c353

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@ithaka/pharos Major
@ithaka/pharos-site Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chrisjbrown
Copy link
Contributor

don't see any reason verify test would be failing now... triggered a rerun just to check

@daneah daneah added this to the @ithaka/pharos v13.0.0 milestone Sep 15, 2023
@daneah daneah self-requested a review September 15, 2023 18:10
daneah
daneah reviewed Sep 19, 2023
View reviewed changes
packages/pharos/src/components/button/pharos-button.ts Outdated
Comment on lines 296 to 275
// Remove in future release once sufficient time elapsed to update naming convention
const a11yLabel = this.a11yLabel ?? this.label;
const a11yPressed = this.a11yPressed ?? this.pressed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this will be released in a major version, I don't believe we need to support backward compatibility like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was it would encourage faster adoption, it's likely there will be a lot of breaking changes of existing code and the overhead to update everything would require a significant lift.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For others' benefit: This is generally why we release things in a major version, or create the opportunity to upgrade some things prior to the major version adoption by creating "forward compatibility" in a new feature release of the existing version. I'm good with either of those in isolation, but combining the two can lead to a bit of long-term complexity, because if this released in v13 then we can't remove the complexity until v14.

@sirrah-tam sirrah-tam changed the base branch from release/13.0.0 to release/14.0.0 September 29, 2023 21:04
@sirrah-tam sirrah-tam mentioned this pull request Oct 5, 2023
Merged
12 tasks
@sirrah-tam sirrah-tam mentioned this pull request Oct 25, 2023
Closed
@daneah
Copy link
Member

daneah commented Dec 5, 2023

Let's figure out to make sure the diff here is up to date, free of conflicts, and doesn't show others' changes—hard to review until then.

@sirrah-tam
feat(button): replace property for ButtonVariant
5b095b8
@sirrah-tam
feat(button): allow backwards compatibility
1ce8b78 
Gives warning if using deprecated attributes, updating aria-pressed
@sirrah-tam
feat(button): update storybook aria-pressed
b3455a7
@sirrah-tam
feat(button): remove fallback from major release
cbb694c
@sirrah-tam
feat(button): add a11y attributes typing
6b216fc
@sirrah-tam
test(button): add tests for new aria attributes
4affa69
@sirrah-tam
chore: merge changes made on GitHub
f9328cd
@sirrah-tam
fix(a11y attributes): update AriaHiddenState name
029e368
@sirrah-tam
fix(button): remove ts ignore
5edf3ba
@sirrah-tam
chore: merge upstream
2c1902c 
Merge branch 'release/14.0.0' into feature/a11y-button
@sirrah-tam
feat(button): remove backwards compat
4a0a60c
@sirrah-tam
feat(button): add aria-disabled back
6eb7746
@sirrah-tam
test(button): add a11y-label to new components
91d2ede
@sirrah-tam
fix: remove sidenav button from bad merge
348e0d5
@sirrah-tam
test(button): fix aria-pressed in toggle buttons
e03ca81
sirrah-tam and others added 3 commits January 4, 2024 10:06
@sirrah-tam sirrah-tam requested a review from daneah January 4, 2024 15:48
@daneah
Copy link
Member

daneah commented Jan 4, 2024

There're a few linting warnings/errors that I think I'm confused about as to why they're arising here in your PR. Several are readily fixed, which I already did but accidentally pushed as a new branch rather than back to your fork; I'll fix that, but ultimately there's a test that might need a closer look that I couldn't fix in the couple of minutes I spent.

@sirrah-tam
Copy link
Contributor Author

There're a few linting warnings/errors that I think I'm confused about as to why they're arising here in your PR. Several are readily fixed, which I already did but accidentally pushed as a new branch rather than back to your fork; I'll fix that, but ultimately there's a test that might need a closer look that I couldn't fix in the couple of minutes I spent.

It looks like the one error also appears in the release/14.0.0 branch, the one related to the any type. Not sure what's what there. But did manage to clean up the earlier errors referencing unknown ARIA after removing the aria-description bits.

@daneah
Copy link
Member

daneah commented Jan 4, 2024

the one related to the any type

Yeah, that's the one. If that's the case, let's get this merged in after I push up what I've fixed, since it's not new to this change, and we can attack that error on a different timeline.

@sirrah-tam sirrah-tam merged commit f8342cc into ithaka:release/14.0.0 Jan 4, 2024
1 of 2 checks passed
@sirrah-tam sirrah-tam mentioned this pull request Jan 22, 2024
Closed
daneah added a commit that referenced this pull request Feb 21, 2024
@daneah
Merge branch 'release/14.0.0' into fix/tab-overflow
a0024e1 
* release/14.0.0:
  fix(chromatic): reintroduce build-storybook script (#656)
  Fix broken whitespace in Icon documentation (#616)
  chore(deps): bump msgpackr from 1.7.2 to 1.10.1 (#654)
  Loading Spinner, Sidenav, Docs: use is-on-background correctly (#657)
  a11y revamp: Pharos Buttons (#594)
  Sidenav: remove sidenav button (#478)
  Global: update onBackground to isOnBackground (#578)
  Remove `paint` value from contain properties (#614)
  Footer: Remove google translate (#586)
  fix: use public registry
  chore: update storybook deps
  fix: update incorrect imports
  Site: fix site build (#481)
  Toast: Update Toaster to use internally consistent scoping (#475)
  Tabs: move selected state management to pharos-tabs (#468)
  Infra: Bump TypeScript, add main exports (#472)
brentswisher added a commit that referenced this pull request Mar 7, 2024
@brentswisher @michael-iden
@daneah @mtorres3 @satya-achanta-venkata @shoupeva-ithaka @jialin-he @sirrah-tam @dependabot @david-corneail @ymouzakis @mariadevadoss @github-actions
Release/14.0.0 (#697)
a2eca68 
* Infra: Bump TypeScript, add main exports (#472)

* feat: bump typscript, add main exports

* feat: use yarn registry

* feat: export style paths

* feat: comment mixins to test

* feat: export sass files

* feat: export react components

* feat: expose utils styles

* feat: sort out exports and file extensions

* feat: remove dupe export

* feat: use pattern to export styles

* feat: explicitly export pharos.scss

* chore: add changeset

* Tabs: move selected state management to pharos-tabs (#468)

* fix: tab event

* fix: update selected tab via parent

* fix(tabs): update property and attribute selectors

* fix(tabs): default to selected, then selected-tab, then 0

* fix(tabs): selected-tab already defaults to 0

* fix(tabs): select initial tab panel

* fix(tabs): only trigger event if tab not selected

* fix(tabs): update test

* fix(tabs): add pharos-tabs-tab-selected event

* fix(tabs): fire tab selected in updated)

* chore: add changeset

* Update packages/pharos/src/components/tabs/pharos-tabs.ts

Co-authored-by: Dane Hillard <github@danehillard.com>

Co-authored-by: Mike Iden <mike.iden@ithaka.org>
Co-authored-by: Dane Hillard <github@danehillard.com>

* Toast: Update Toaster to use internally consistent scoping (#475)

* fix(storybook): fix updateable toast stories

* fix(toast): update to use internal scoping

* chore: add changeset

* docs: fix casing of function name in comment

* docs: update code comment to reflect scoped registry behavior

* Site: fix site build (#481)

* fix(pharos-site): revert pharos-site changes for typescript

* fix(pharos-site): leave exports in place

* fix(pharos-site): add end line

* fix: update incorrect imports

* chore: update storybook deps

* fix: use public registry

* Footer: Remove google translate (#586)

* feat(footer): remove deprecated Google Translate widget

* chore: add changeset

* Remove `paint` value from contain properties (#614)

* fix: remove `paint` value from contain properties

Of all the `contain` property values, `paint` is the most noticeably
problematic when overused because it can bite people by cropping off the
content they intended to render within a component that has `paint`
containment. For components that rely on composition at the consumer,
this has caused issues more than a few times.

* chore: add changeset

* Global: update onBackground to isOnBackground (#578)

* feat(global): update onBackground attribute to isOnBackground

* feat(global): update tokens import for pharos-site/.../color

* feat(global): revert pharos-site onBackground changes

* feat(global): remove .js on token import

* feat(global): update on-background to is-on-background

* feat(global): remove is-on-background from pharos-site

* feat(global): change onBackground to isOnBackground

* fix(global): change is-on-background to on-background in tokens

* Sidenav: remove sidenav button (#478)

* refactor(sidenav): remove sidenav button

* refactor(sidenav-button): remove sus comma

* refactor(sidenav-button): remove test for sidenav button

* fix(sidenav): remove automatic sliding behavior

* feat(sidenav): render button conditionally and add unit tests

* refactor(sidenav-button): add changeset

* Remove console log statement

Co-authored-by: Dane Hillard <github@danehillard.com>

* fix(sidenav): fix storybook

---------

Co-authored-by: Evan Shoup <evan.shoup@ithaka.org>
Co-authored-by: Jialin He <jialin.he@ithaka.org>
Co-authored-by: Jialin He <38861633+jialin-he@users.noreply.github.com>
Co-authored-by: Evan Shoup <112417900+shoupeva-ithaka@users.noreply.github.com>
Co-authored-by: Dane Hillard <github@danehillard.com>

* 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 <github@danehillard.com>

* Update packages/pharos/src/components/toast/pharos-toast-button.ts

Co-authored-by: Dane Hillard <github@danehillard.com>

* fix(btn): remove aria-description

---------

Co-authored-by: Dane Hillard <github@danehillard.com>

* Loading Spinner, Sidenav, Docs: use is-on-background correctly (#657)

* fix(loading-spinner, sidenav, docs): use is-on-background correctly

* chore: add changeset

* fix: address linting issues

* chore(deps): bump msgpackr from 1.7.2 to 1.10.1 (#654)

Bumps [msgpackr](https://github.com/kriszyp/msgpackr) from 1.7.2 to 1.10.1.
- [Release notes](https://github.com/kriszyp/msgpackr/releases)
- [Commits](https://github.com/kriszyp/msgpackr/commits/v1.10.1)

---
updated-dependencies:
- dependency-name: msgpackr
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix broken whitespace in Icon documentation (#616)

* Update icon.docs.mdx

added empty space to fix run on from "an" "aria"

* Update packages/pharos-site/static/guidelines/icon.docs.mdx

---------

Co-authored-by: Dane Hillard <github@danehillard.com>

* fix(chromatic): reintroduce build-storybook script (#656)

* Remove old onBackground attribute from sidenav storybook story (#671)

* fix(sidenav): remove onBackground attribute

When merging the latest develop into this release branch the sidenav
component ended up with both the old onBackground and new isOnBackground
versions of the attribute in its storybook story. This removes the old
version so it is correct.

* chore: add changeset

* Update link, dropdown-menu-nav, and popover to use a11y-label - Breaking (#676)

* feat: add & update a11y-label, remove label

* feat: update remaining components to use a11y-label

* chore: add changeset

* fix: update a11yLabel references

* fix: update a11y-label descriptions for each component

* fix(popover): clean up the aria-labelledby attribute

* fix(link): update SB to pass in a11yLabel values (#677)

* fix(link): update SB to pass in a11yLabel values

* chore: add changeset

* DropdownMenu: add nav category to replace links (#479)

* refactor(dropdown-menu): add dropdownmenunavheading

* refactor(dropdown-menu): rename nav heading to nav category

* refactor(dropdown-menu): add slot name

* refactor(dropdown-menu): correctly name component

* refactor(dropdown-menu): remove hrefs from categories

* refactor(dropdown-menu): test stories component rename

* refactor(dropdown-menu): add import

* chore: merge branch release-v13

* Revert "chore: merge branch release-v13"

This reverts commit 617d9be.

* chore: add scoping

* fix: init storybook category

* fix: attribute naming

* fix: add is-active property/attribute to category

Also, remove pharos-icon from nav-link as it is no longer necessary

* refactor: move aria attributes to button

* refactor: style dropdown menu button

* refactor: add slot category to header test

* refactor: remove aria attributes from container

* refactor: add scoping

* refactor: add category slot

* refactor(dropdown-menu): check has hover over attribute

* fix: add nothing value from lit

* fix: remove unnecessary href from DropdownMenuNavCategory

---------

Co-authored-by: Dane Hillard <github@danehillard.com>
Co-authored-by: Mat Harris <mat.harris@ithaka.org>

* Require icon accessibility attributes (#679)

* feat(icon): throw error for inaccessible icons

Previously, the a11y-title and a11y-hiddden attributes were
added and the description attribute was deprecated. This change removes
the description attribute and throws an error if an icon is used without
proper accessibility attributes.

* fix: remove deprecated attribute from tests

There was a default description of "" for icons but now that
has been removed.

* fix(dropdown-menu-nav): add a11y-hidden to icon

Because there will always be a category in the button text
and the icon is purely decorative, it should be hidden from
screen readers.

* hotfix(icon): fix typo in test description

Co-authored-by: Mat Harris <mat.harris@ithaka.org>

* chore: add changeset

---------

Co-authored-by: Mat Harris <mat.harris@ithaka.org>

* Combobox: Add default elevation (#683)

* feat(combobox): add default elevation

Adds elevation token level 3 to the combobox component.

* feat(combobox): remove border on combobox list

* chore: update changeset

---------

Co-authored-by: Markell Torres <markell.torres@ithaka.org>

* Modal: Add default elevation (#681)

* feat(modal): add elevation token

Adds elevation token level 5 to the modal component.

* chore: add changeset

* feat(modal-elevation): remove border and move elevation to modal__content class

---------

Co-authored-by: Markell Torres <markell.torres@ithaka.org>

* feat(popover): add elevation token (#682)

Adds elevation token level 4 to the popover component.

* Dropdown Menu: Add default elevation (#684)

* feat(dropdown-menu): add elevation token

Adds a default elevation to the dropdown-menu set
to elevation level 3.

* feat(dropdown-menu): remove border

With the new elevation styles, the border is no longer needed.

* Tabs: Remove overflow-y from pharos-tabs' tab__panels class (#690)

* style(tabs): remove overflow-y

* docs(tabs): add changeset

* Add ability to use a11y-disabled and maintain default disabled styling (#691)

* feat(btn): a11y-disabled remove disabled attr, checks to see if aria-disabled is being used and if so removes the disabled attribute

* chore: add changeset

* Update Storybook stories with new sidenav display logic (#696)

* fix(sidenav): update react report storybook example

* fix(sidenav): update web component reports storybook example

Update the sidenav in the Reports example to handle showing/hiding at
a certain window size. This used to be the default behaviors of the
sidenav before the v14 release`

* fix(layout): default sidenav layouts to open

Because the sidnav no longer contains the logic to open and close,
the layout stories were broken. Now, the default state is open, although
the mobile experience could probably be improved.

* fix(sidenav): update storybook mobile breakpoint to match wc version

* chore: add changeset

* fix(sidenav): default stories to use close button

Because the default story a hidden the sidenav, the close button should
be enabled by default.

* fix(sidenav): update import syntax

* refactor: update sidenav stories to use a ResizeObserver

* chore: trigger new build

* chore(deps-dev): bump vite from 4.3.9 to 4.5.2 (#672)

Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 4.3.9 to 4.5.2.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/v4.5.2/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v4.5.2/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* DropdownMenuNav: Add pharos-elevation-level-3 token (#670)

* fix(dropdown-menu-nav): add elevation 3 to dropdown-menu-nav

* add changeset

* fix(dropdown-menu): add elevation 3 to dropdown-menu

* Update link, dropdown-menu-nav, and popover to use a11y-label (#675)

* chore: merge upstream

Squashed commit of the following:

* feat: update label to a11y-label

Added the a11y-label attribute to replace
the label attribute when needing to
update a components aria-label

* feat: update references from label to a11y-label

For components and tests that referenced the
label attribute this updates those to now use
the a11y-label attribute

* chore: add changeset

* fix: update a11y-label descriptions for each component

* Fix the all-contributors badge (#680)

* fix(docs): update all-contributors badge to show proper count

The all-contributors badge was stuck at a count of 20, as we seem to
have been using a hard-coded image. This updates the badge to a proper
dynamic badge with the right count.

* chore(docs): add color parameter to badge

* Icon: Add "add to folder" icon (#678)

* feat(icon): add add to folder icon

* docs(icon): update change log for addin add to folder icon

* Update .changeset/good-readers-lay.md

---------

Co-authored-by: Dane Hillard <github@danehillard.com>

* chore: version packages (#685)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* chore(deps): bump ip from 1.1.8 to 1.1.9 (#687)

Bumps [ip](https://github.com/indutny/node-ip) from 1.1.8 to 1.1.9.
- [Commits](indutny/node-ip@v1.1.8...v1.1.9)

---
updated-dependencies:
- dependency-name: ip
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(icon): add media query style for WHCM (#689)

* fix(icon): add media query style for Windows High Contrast Mode

* chore: add changeset

* chore(deps): bump es5-ext from 0.10.62 to 0.10.64 (#692)

Bumps [es5-ext](https://github.com/medikoo/es5-ext) from 0.10.62 to 0.10.64.
- [Release notes](https://github.com/medikoo/es5-ext/releases)
- [Changelog](https://github.com/medikoo/es5-ext/blob/main/CHANGELOG.md)
- [Commits](medikoo/es5-ext@v0.10.62...v0.10.64)

---
updated-dependencies:
- dependency-name: es5-ext
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): upgrade to Yarn 4 (#694)

* chore(deps): upgrade to Yarn 4

Some recent ecosystem changes no longer support Yarn v1 moving forward.
Coupled with a potential desire to use Yarn PnP / Zero installs (#635),
this upgrade puts us in good shape to keep moving.

* fix(infra): update Yarn cache dir in GitHub Actions

* fix(infra): update Yarn cache dir in GitHub Actions

* fix(infra): update Yarn installation flags

* fix: prevent report example from being tree-shaken

Without an actual export of the report example, it was being tree-shaken
when storybook was being built.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Dane Hillard <github@danehillard.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Markell Torres <54967638+mtorres3@users.noreply.github.com>
Co-authored-by: Mat Harris <mat.harris@ithaka.org>
Co-authored-by: mariadevadoss <132926833+mariadevadoss@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* hotfix: update sidenav button to use a11yLabel

Co-authored-by: Mat Harris <mat.harris@ithaka.org>

* Require an accessible label be present for icon-only buttons (#699)

* feat(button): require a11y-label for icon buttons

Because the icon in an icon button is set to aria-hidden="true", it is
not visible to screen readers. This change requires an a11y-label for
icon buttons to ensure that the button is accessible.

* fix: unit test buttons were inaccessible

* fix: add accessible label to icon button group stories

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Christopher Brown <christopher.brown@ithaka.org>
Co-authored-by: Mike Iden <mike.iden@ithaka.org>
Co-authored-by: Dane Hillard <github@danehillard.com>
Co-authored-by: Markell Torres <markell.torres@ithaka.org>
Co-authored-by: Satya Achanta <satya.achantavenkata@ithaka.org>
Co-authored-by: Markell Torres <54967638+mtorres3@users.noreply.github.com>
Co-authored-by: Evan Shoup <evan.shoup@ithaka.org>
Co-authored-by: Jialin He <jialin.he@ithaka.org>
Co-authored-by: Jialin He <38861633+jialin-he@users.noreply.github.com>
Co-authored-by: Evan Shoup <112417900+shoupeva-ithaka@users.noreply.github.com>
Co-authored-by: Mat Harris <mat.harris@ithaka.org>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: david-corneail <104939119+david-corneail@users.noreply.github.com>
Co-authored-by: Yanni Mouzakis <64924035+ymouzakis@users.noreply.github.com>
Co-authored-by: mariadevadoss <132926833+mariadevadoss@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@brentswisher brentswisher mentioned this pull request Mar 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daneah daneah daneah approved these changes

@mtorres3 mtorres3 mtorres3 approved these changes

@shoupeva-ithaka shoupeva-ithaka shoupeva-ithaka approved these changes

@SMQuazi SMQuazi Awaiting requested review from SMQuazi SMQuazi was automatically assigned from ithaka/pharos-maintainers

Assignees
No one assigned
Labels
accessibility BREAKING component/button
Projects
None yet
Milestone
@ithaka/pharos v14.0.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sirrah-tam @chrisjbrown @daneah @mtorres3 @shoupeva-ithaka