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

feat(material/tabs): allow for content tabindex to be customized #21912

Merged
merged 1 commit into from Apr 19, 2021

Conversation

crisbeto
Copy link
Member

In some cases, adding a tabindex to a tabpanel can improve accessibility, however that is currently impossible with our setup, because the element isn't exposed. These changes add an input that allows consumers to customize the tabindex based on their use case.

We tried to tackle this in an earlier PR (#14808), but it was decided not to proceed, because the tabindex isn't relevant for all use cases. This approach should be a bit more flexible since it allows users to opt into it.

Fixes #21819.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent Accessibility This issue is related to accessibility (a11y) target: minor This PR is targeted for the next minor release labels Feb 15, 2021
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 15, 2021
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

We should probably also add a style for strong-focus-indicators for a tab body with focus

@crisbeto
Copy link
Member Author

Added the strong focus indicator classes to the body.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Feb 17, 2021
@annieyw
Copy link
Contributor

annieyw commented Mar 5, 2021

This is causing a bunch of screenshot failures where the content gets bunched up and does not take up the whole horizontal space as it should. Any ideas what may be the cause?

@crisbeto crisbeto force-pushed the 21819/tabs-content-tabindex branch 2 times, most recently from ffbfe96 to e897fb6 Compare March 28, 2021 11:14
@crisbeto
Copy link
Member Author

Sorry for the delay @annieyw, I just noticed your comment when coming back to rebase the PR. This sounds weird, because the only style changes are:

  1. Adding outline: 0 to the content. This probably isn't the issue.
  2. Adding mat-focus-indicator to the content. This may cause some issues, because it adds a position: relative and ::after style.

I've pushed a change to address point 2. Can we re-run the presubmit?

In some cases, adding a `tabindex` to a `tabpanel` can improve accessibility, however that
is currently impossible with our setup, because the element isn't exposed. These changes
add an input that allows consumers to customize the `tabindex` based on their use case.

We tried to tackle this in an earlier PR (angular#14808), but it was decided not to proceed,
because the `tabindex` isn't relevant for all use cases. This approach should be a bit
more flexible since it allows users to opt into it.

Fixes angular#21819.
@crisbeto crisbeto force-pushed the 21819/tabs-content-tabindex branch from e897fb6 to d27e5e7 Compare April 14, 2021 20:50
@wagnermaciel wagnermaciel merged commit f81f67d into angular:master Apr 19, 2021
@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 May 20, 2021
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 cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(TABS): Tab panel content not accessible to multiple screen readers
4 participants