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.

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

Radio, Checkbox: Fix group label #652

Merged
merged 24 commits into from
Nov 30, 2023

Conversation

sirrah-tam
Copy link
Contributor

@sirrah-tam sirrah-tam commented Nov 27, 2023

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?

Fixes #651

This change ensures the associated hint text or error messages for radio & checkbox groups are calculated as the group's accessible description (accDesc) rather than as part of the accessible name (accName).

How does this change work?
The concatenation for the label as well as the aria-labelledby attribute is removed to allow the legend element to calculate the group's accName. Adding the aria-describedby attribute with its value referencing the ID message will ensure that any hint text or error message that is supplied gets added as the groups accDesc.

Remove concatenation for label reference as well as
the aria-labelledby attribute so thelegend element
is used to calculate accName for group.
Added the aria-describedby attribute with the ID ref
message to look for the associated hint/description
text.
@sirrah-tam sirrah-tam requested a review from a team as a code owner November 27, 2023 16:15
@sirrah-tam sirrah-tam requested review from eslawski, jialin-he and mtorres3 and removed request for a team November 27, 2023 16:15
Copy link

changeset-bot bot commented Nov 27, 2023

🦋 Changeset detected

Latest commit: d4146e0

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

This PR includes changesets to release 1 package
Name Type
@ithaka/pharos 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

@sirrah-tam sirrah-tam changed the title Bugfix for 651 - radio checkbox group label Bugfix: #651 - radio checkbox group label Nov 27, 2023
@daneah daneah changed the title Bugfix: #651 - radio checkbox group label Radio, Checkbox: Fix group label Nov 28, 2023
@daneah daneah requested a review from brentswisher November 28, 2023 19:47
Copy link
Contributor

@brentswisher brentswisher left a comment

Choose a reason for hiding this comment

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

Code looks good! Could a unit test be added to verify the aria-describedby is added/removed as expected?

@daneah daneah requested a review from brentswisher November 30, 2023 17:28
@daneah daneah merged commit 3ad3fd6 into ithaka:develop Nov 30, 2023
sirrah-tam added a commit to sirrah-tam/pharos that referenced this pull request Dec 1, 2023
* docs: add @sirrah-tam as a contributor

* fix: update radio/checkbox labeling

Remove concatenation for label reference as well as
the aria-labelledby attribute so thelegend element
is used to calculate accName for group.
Added the aria-describedby attribute with the ID ref
message to look for the associated hint/description
text.

* chore: add changeset

* fix: use messageID value for aria-describedby ref

Co-authored-by: Brent Swisher <[email protected]>

* fix(radio/checkbox): add ifDefined directive for aria-describedby

* test(radio/checkbox group): add tests for accDesc

* fix: use template literal for groupID expression

Co-authored-by: Dane Hillard <[email protected]>

* fix: use template literal for groupID expression

Co-authored-by: Dane Hillard <[email protected]>

---------

Co-authored-by: Brent Swisher <[email protected]>
Co-authored-by: Dane Hillard <[email protected]>
daneah added a commit that referenced this pull request Dec 19, 2023
* develop:
  A11y revamp: Pharos buttons (non-breaking change) (#628)
  Radio, Checkbox: Fix group label (#652)
  Add elevation tokens and documentation (#643)
  fix(sidenav-link): external link opens in new tab (#645)
  Upgrade to TypeScript 5 (#644)
  feat(cli): add newly created components created using pharos-cli to initComponents files (#630)
  chore: version packages (#640)
  Coach Mark: Fix react component positioning (#638)
  Coach Mark: Documentation fixes (#639)
  chore(deps): bump @babel/traverse from 7.20.0 to 7.23.2 (#637)
  chore: version packages (#636)
  Icon: Add Panorama icon (#631)
  chore: version packages (#629)
  Loading spinner: add small and on background variant (#627)
  chore: version packages (#626)
  Sheet: allow expansion with attribute (#625)
  fix(button): remove fill on subtle disabled button on background (#618)
  chore(deps-dev): bump postcss from 8.4.25 to 8.4.31 (#624)
  chore: version packages (#623)
  Sheet: Add more close options and transition timing function (#620)
@github-actions github-actions bot mentioned this pull request Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkbox & radio group accName includes description/error information
3 participants