Skip to content

fix(material/chips): remove button role from editable chips #27317

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

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Jun 16, 2023

fix(material/chips): remove button role from editable chips

Removes the button role from editable input chips. Fix accessibility
issue in ChipRow where the chip action element is mislabeled as a
button (#27106). Remove butotn role by remove DOM node thtat used to
have button role and using the gridcell role element for the primary
chip action instead.

Tested cross-browser with supported screen readers on MacOS and Windows.
Tested on "Chips with input" example by verifying that a chip could be
added then edited after adding. Also verifying that AT read the chip's
aria description.

Testing Environment

  • macOS 13.4 (22F66) / VoiceOver
    • Chrome Version 114.0.5735.133 (Official Build) (arm64)
    • Firefox 114.0.1 (64-bit)
  • windows 10 Enteprise Version 22H2
    • JAWS VERSION 2020.2006.12 ILM
    • NVDA version 2022.3
    • Chrome Version 114.0.5735.134 (Official Build) (64-bit)
    • Firefox 114.0.2 (64-bit)

Fix #27106

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/chips dev-app preview When applied, previews of the dev-app are deployed to Firebase labels Jun 16, 2023
@github-actions
Copy link

github-actions bot commented Jun 16, 2023

Deployed dev-app for 4e2cf9c to: https://ng-dev-previews-comp--pr-angular-components-27317-9f8cfdw6.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@zarend zarend force-pushed the editable-chips-aria branch 6 times, most recently from dd43095 to b056fc2 Compare June 21, 2023 01:42
@zarend zarend requested a review from jelbourn June 21, 2023 01:42
@zarend zarend marked this pull request as ready for review June 21, 2023 01:43
@zarend zarend requested review from mmalerba and crisbeto as code owners June 21, 2023 01:43
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Jun 22, 2023
Removes the button role from editable input chips. Fix accessibility
issue in ChipRow where the chip action element is mislabeled as a
button (angular#27106). Remove butotn role by remove DOM node thtat used to
have button role and using the gridcell role element for the primary
chip action instead.

Tested cross-browser with supported screen readers on MacOS and Windows.
Tested on "Chips with input" example by verifying that a chip could be
added then edited after adding. Also verifying that AT read the chip's
aria description.

Testing Environment
 - macOS 13.4 (22F66) / VoiceOver
   - Chrome Version 114.0.5735.133 (Official Build) (arm64)
   - Firefox 114.0.1 (64-bit)
 - windows 10 Enteprise Version 22H2
   - JAWS VERSION 2020.2006.12 ILM
   - NVDA version 2022.3
   - Chrome Version 114.0.5735.134 (Official Build) (64-bit)
   - Firefox 114.0.2 (64-bit)

Fix angular#27106
@zarend zarend force-pushed the editable-chips-aria branch from b056fc2 to 4e2cf9c Compare June 22, 2023 17:22
@crisbeto
Copy link
Member

I'm merging this PR since the related CL was submitted yesterday.

@crisbeto crisbeto merged commit e0608cb into angular:main Jun 28, 2023
crisbeto pushed a commit that referenced this pull request Jun 28, 2023
Removes the button role from editable input chips. Fix accessibility
issue in ChipRow where the chip action element is mislabeled as a
button (#27106). Remove butotn role by remove DOM node thtat used to
have button role and using the gridcell role element for the primary
chip action instead.

Tested cross-browser with supported screen readers on MacOS and Windows.
Tested on "Chips with input" example by verifying that a chip could be
added then edited after adding. Also verifying that AT read the chip's
aria description.

Testing Environment
 - macOS 13.4 (22F66) / VoiceOver
   - Chrome Version 114.0.5735.133 (Official Build) (arm64)
   - Firefox 114.0.1 (64-bit)
 - windows 10 Enteprise Version 22H2
   - JAWS VERSION 2020.2006.12 ILM
   - NVDA version 2022.3
   - Chrome Version 114.0.5735.134 (Official Build) (64-bit)
   - Firefox 114.0.2 (64-bit)

Fix #27106

(cherry picked from commit e0608cb)
@zarend
Copy link
Contributor Author

zarend commented Jun 28, 2023

Thanks, my bad 🤦

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker area: material/chips dev-app preview When applied, previews of the dev-app are deployed to Firebase target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(material/chips): clicking on an editable chip focuses it and does not begin editing but pressing enter begins the editing state
3 participants