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/chip): allow focusing to disabled listbox options #25771

Merged
merged 1 commit into from Oct 13, 2022

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Oct 6, 2022

allow focusing to disabled listbox options

@zarend
Copy link
Contributor Author

zarend commented Oct 6, 2022

This is still a draft, need to do more testing and cleanup the code a bit.

@zarend zarend added P2 The issue is important to a large percentage of users, with a workaround Accessibility This issue is related to accessibility (a11y) area: material/chips labels Oct 6, 2022
@zarend zarend force-pushed the disabled-chip-option branch 2 times, most recently from 2f537cd to 751ff55 Compare October 11, 2022 23:51
Allow user to focus to disabled listbox options. Unlike other chips,
disabled `MatChipOption` remains in the tab order, but it cannot be
clicked. This aligns with WAI ARIA documented best practice to allow
focusing disabled listbox options. Fix issue where screen reader does
not announce disabled item in single selection list.

Summary of API and behavior changes:
 - when disabled, `MatChipOption` sets `aria-disabled="true"` and omits
   `disabled` attribute.
 - Add private `@Input _alowFocusWithDisabled` to `MatChipAction` to
   support focusing the action when it is disabled.
 - `MatChipSet` defines `_skipPredicate` as an instance member which
   dervied classes may override.

`_alowFocusWithDisabled` and `_skipPredicate` are internal API to the
chips.
@zarend zarend marked this pull request as ready for review October 11, 2022 23:53
@zarend
Copy link
Contributor Author

zarend commented Oct 11, 2022

Hi @crisbeto , this is working now, and it's ready for your review

@zarend zarend added the target: minor This PR is targeted for the next minor release label Oct 11, 2022
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

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Oct 12, 2022
@zarend zarend added target: rc This PR is targeted for the next release-candidate and removed target: minor This PR is targeted for the next minor release labels Oct 12, 2022
@zarend zarend merged commit 3f68996 into angular:main Oct 13, 2022
zarend added a commit that referenced this pull request Oct 13, 2022
Allow user to focus to disabled listbox options. Unlike other chips,
disabled `MatChipOption` remains in the tab order, but it cannot be
clicked. This aligns with WAI ARIA documented best practice to allow
focusing disabled listbox options. Fix issue where screen reader does
not announce disabled item in single selection list.

Summary of API and behavior changes:
 - when disabled, `MatChipOption` sets `aria-disabled="true"` and omits
   `disabled` attribute.
 - Add private `@Input _alowFocusWithDisabled` to `MatChipAction` to
   support focusing the action when it is disabled.
 - `MatChipSet` defines `_skipPredicate` as an instance member which
   dervied classes may override.

`_alowFocusWithDisabled` and `_skipPredicate` are internal API to the
chips.

(cherry picked from commit 3f68996)
@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 Nov 13, 2022
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 P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants