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(tabs): unable to tab to tab content #14808

Closed
wants to merge 1 commit into from

Conversation

crisbeto
Copy link
Member

Currently the tab's content doesn't have a tabindex which means that if it doesn't have any focusable elements, users with assistive technology will skip over the content completely. These changes add a tabindex to the content of the currently-selected tab, based on the example of accessible tabs from the ARIA best practices.

@crisbeto crisbeto added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release labels Jan 12, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 12, 2019
Copy link
Contributor

@andrewseguin andrewseguin left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewseguin andrewseguin added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jan 15, 2019
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jan 19, 2019
Along the same line as angular#14808. Fixes not being able to reach the content of a step if it doesn't have focusable content already. Based on the [tabs example from the accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html).
jelbourn pushed a commit that referenced this pull request Feb 6, 2019
Along the same line as #14808. Fixes not being able to reach the content of a step if it doesn't have focusable content already. Based on the [tabs example from the accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html).
josephperrott pushed a commit that referenced this pull request Mar 5, 2019
Along the same line as #14808. Fixes not being able to reach the content of a step if it doesn't have focusable content already. Based on the [tabs example from the accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html).
josephperrott pushed a commit that referenced this pull request Mar 5, 2019
Along the same line as #14808. Fixes not being able to reach the content of a step if it doesn't have focusable content already. Based on the [tabs example from the accessibility guidelines](https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html).
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@crisbeto crisbeto added the P2 The issue is important to a large percentage of users, with a workaround label Jun 12, 2019
@mmalerba
Copy link
Contributor

mmalerba commented Jun 13, 2019

So it seems there's a team in google3 that is placing an empty tab in one of their test apps which they run accessibility tests against. Because the tab is empty, mat-tab-body has 0 height, resulting in this error:

Warning: AX_FOCUS_01 (These elements are focusable but either invisible or obscured by another element) failed on the following element:
#mat-tab-content-0-0

I'm not sure if this is something we should bother addressing, or just consider it an error on the user's part if they do this in a real app. @jelbourn thoughts?

@jelbourn
Copy link
Member

It sounds like a problem in their app rather than the tabs itself

@andrewseguin andrewseguin added this to To do in July PRs Jul 1, 2019
@andrewseguin andrewseguin moved this from Jeremy - To do to Andrew - To do in July PRs Jul 1, 2019
@jelbourn jelbourn removed the action: merge The PR is ready for merge by the caretaker label Jul 9, 2019
@jelbourn
Copy link
Member

jelbourn commented Jul 9, 2019

I actually double-checked this against the internal examples built by Google's a11y people and they don't put tabindex on the content. This aligns with my understanding that only interactive elements should be tab stops, and that screen-reader users will navigate to static content via the screen-reader shortcuts (not tab).

I have my next a11y sync next Monday, so I'll double check.

@crisbeto
Copy link
Member Author

crisbeto commented Jul 9, 2019

For future reference, this is based on the following example from the W3C: https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html

@jelbourn
Copy link
Member

@crisbeto I confirmed that we shouldn't make the content tabbable by default. However, when the content is scrollable and it doesn't contain any interactive elements, making the content a tab stop is the only way for users to scroll the content with the keyboard. It seems like the best thing here,then, would be to document what people should do in that case rather than trying to figure it out at the library level.

Currently the tab's content doesn't have a `tabindex` which means that if it doesn't have any focusable elements, users with assistive technology will skip over the content completely. These changes add a `tabindex` to the content of the currently-selected tab, based on [the example of accessible tabs from the ARIA best practices](https://www.w3.org/TR/wai-aria-practices-1.1/examples/tabs/tabs-2/tabs.html).
@andrewseguin
Copy link
Contributor

@crisbeto Should this PR be closed in favor of adding better documentation for adding a tabstop when the content is scrollable?

@crisbeto
Copy link
Member Author

I think we should close it in favor of not doing anything. We have a similar pattern to this in the stepper and there's a pending PR to remove it since it ends up being more annoying than useful.

@crisbeto crisbeto closed this Jun 12, 2020
July PRs automation moved this from Andrew - To do to Done Jun 12, 2020
@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 13, 2020
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) cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
July PRs
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants