Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(tabs): Accessibility and keyboard interaction fixes #10706

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

jeyoshimi
Copy link
Contributor

This commit does a few things related to accessibility:

  1. Hides md-dummy-tabs completely from screen readers with aria-hidden.
    With this change, screen readers do not hear the dummy tabs read out
    when using linear navigation. Instead of focusing the dummy tabs, the
    visible tabs are focused. There was a comment that mentions focusing the
    dummy tabs as a way to avoid animation issues; however, I could not
    replicate the mentioned issues.
  2. Fixes the ARIA roles so users can hear "tab 1 of 3" instead of just
    "tab" by moving role="tablist" to the direct parent element (which
    contains the items marked role="tab"). (VoiceOver in particular)
  3. Fixes keyboard interaction to match recommended ARIA practices by
    focusing on the active tab when using TAB navigation, then resolving the
    focus/active tab discrepancy on pressing TAB within the tablist. This
    prevents the behavior where tabbing away from the tablist then back
    results in the previously focused tab getting focus instead of the
    active tab.
  4. Note that there is a SCSS change added in to retain current behavior
    when the active tab is added to the tab order; however, this should
    likely be reconsidered as it actively overrides default focus outline
    behavior.
  5. Removes unsupported ARIA attribute (aria-activedescendant) from
    md-tabs-canvas and removes md-tab-id from dummy tabs to prevent
    duplicate setting of aria-controls.
  6. Moved id to tab item from dummy tab so the aria-labelledby on the tab
    content (role="tabpanel") items is set as per ARIA recommendation.

Tests were updated and fleshed out to check that the active tab is the
only tab allowed in the tab order and that focused/active tabs are
reconciled on tabbing out of the tablist.

Closes #10075.

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label May 30, 2017
@jeyoshimi
Copy link
Contributor Author

Tagging @jelbourn as requested. :)

@jelbourn jelbourn self-requested a review May 31, 2017 20:29
@jelbourn jelbourn added the g3: pr This PR was posted by an internal or external product team. label May 31, 2017
@jelbourn jelbourn self-assigned this May 31, 2017
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 requested a review from crisbeto May 31, 2017 20:43
@jelbourn
Copy link
Member

@crisbeto could you also take a quick look at this?

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, one minor nit.

});

});

describe('leaving tablist', function() {
Copy link
Member

@crisbeto crisbeto May 31, 2017

Choose a reason for hiding this comment

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

nit: I don't think that this needs to be in an extra describe. It's a non-blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it into the above describe. :)

@ThomasBurleson ThomasBurleson modified the milestone: 1.1.5 Jun 3, 2017
This commit does a few things related to accessibility:
1) Hides md-dummy-tabs completely from screen readers with aria-hidden.
With this change, screen readers do not hear the dummy tabs read out
when using linear navigation. Instead of focusing the dummy tabs, the
visible tabs are focused. There was a comment that mentions focusing the
dummy tabs as a way to avoid animation issues; however, I could not
replicate the mentioned issues.
2) Fixes the ARIA roles so users can hear "tab 1 of 3" instead of just
"tab" by moving role="tablist" to the direct parent element (which
contains the items marked role="tab"). (VoiceOver in particular)
3) Fixes keyboard interaction to match recommended ARIA practices by
focusing on the active tab when using TAB navigation, then resolving the
focus/active tab discrepancy on pressing TAB within the tablist. This
prevents the behavior where tabbing away from the tablist then back
results in the previously focused tab getting focus instead of the
active tab.
4) Note that there is a SCSS change added in to retain current behavior
when the active tab is added to the tab order; however, this should
likely be reconsidered as it actively overrides default focus outline
behavior.
5) Removes unsupported ARIA attribute (aria-activedescendant) from
md-tabs-canvas and removes md-tab-id from dummy tabs to prevent
duplicate setting of aria-controls.
6) Moved id to tab item from dummy tab so the aria-labelledby on the tab
content (role="tabpanel") items is set as per ARIA recommendation.

Tests were updated and fleshed out to check that the active tab is the
only tab allowed in the tab order and that focused/active tabs are
reconciled on tabbing out of the tablist.

Closes angular#10075.
@kara kara added pr: merge ready This PR is ready for a caretaker to review and removed needs: presubmit labels Jun 12, 2017
@kara kara merged commit 072f832 into angular:master Jun 12, 2017
@jeyoshimi jeyoshimi deleted the tabs branch June 13, 2017 22:31
@jeyoshimi jeyoshimi restored the tabs branch June 13, 2017 22:31
@jeyoshimi jeyoshimi deleted the tabs branch June 15, 2017 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ g3: pr This PR was posted by an internal or external product team. pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants