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

fix(material/core): remove tabindex from mat-option #26893

Closed
wants to merge 1 commit into from

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Apr 6, 2023

Remove the tabindex attribute from MatOption. MatOption will no longer set a tabindex. Tabindex is not needed since focus is managed on the parent by setting aria-activedescendant.

Tabindex="-1" seems to be causing a problem in #26861 where VoiceOver with Firefox moves DOM focus from the combobox to the option when opening the listbox popup.

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/autocomplete area: material/core area: material/select dev-app preview When applied, previews of the dev-app are deployed to Firebase labels Apr 6, 2023
@zarend zarend force-pushed the mat-option-remove-tabindex branch 2 times, most recently from ad63734 to 371f7c7 Compare April 6, 2023 22:59
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

Deployed dev-app for 371f7c7 to: https://ng-dev-previews-comp--pr-angular-components-26893-q2nou38p.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 mat-option-remove-tabindex branch from 371f7c7 to 1ace0be Compare April 11, 2023 02:14
@zarend zarend marked this pull request as ready for review April 11, 2023 02:23
@zarend zarend requested a review from crisbeto as a code owner April 11, 2023 02:23
Remove the tabindex attribute added to MatOption components. MatOption
does not need tabindex because the parent component manages focus by
setting `aria-activedescendant` attribute. Previously, MatOption set
tabindex but was also a referenced by aria-activedescendant. This was
not the correct ARIA semantics. Align closer to ARIA spec by using only
aria-activedescendant rather than both.

Tabindex="-1" seems to be causing a problem in angular#26861 where VoiceOver
with Firefox moves DOM focus from the combobox to the option when
opening the listbox popup.

Unblocks angular#26861.
Copy link
Member

@crisbeto crisbeto 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 Apr 12, 2023
@zarend
Copy link
Contributor Author

zarend commented Apr 14, 2023

Closing in favor of #26917

@zarend zarend closed this Apr 14, 2023
@zarend zarend deleted the mat-option-remove-tabindex branch April 14, 2023 18:21
@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 Jun 6, 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/autocomplete area: material/core area: material/select 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.

2 participants