[material-ui][docs] Add aria-current for nav tabs demo #39594

Kimzify · 2023-10-24T19:22:54Z

I have followed (at least) the PR section of the contributing guide.

This PR changed aria-selected to aria-current in Tab component.

From the MDN docs:

When you have a group of related elements, such as several links in a breadcrumb or steps in a multi-step flow, with one element in the group styled differently from the others to indicate to the sighted user that this is the current element within its group, the aria-current should be used to inform the assistive technology user what has been indicated via styling.

the aria-selected docs define the following.

The option, tab, and treeitem roles permit user agents to provide an implicit value for aria-selected when specified conditions are met

Closes #32924

https://deploy-preview-39594--material-ui.netlify.app/material-ui/react-tabs/#nav-tabs

mui-bot · 2023-10-24T19:26:37Z

Netlify deploy preview

https://deploy-preview-39594--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 3f02c7f

michaldudak · 2023-10-25T12:48:21Z

ARIA docs explicitly state that the active tab should have the aria-selected attribute set to true:

Authors SHOULD ensure that a selected tab has its aria-selected attribute set to true, that inactive tab elements have their aria-selected attribute set to false, and that the currently selected tab provides a visual indication that it is selected.

Kimzify · 2023-10-25T16:16:12Z

ARIA docs explicitly state that the active tab should have the aria-selected attribute set to true:

Authors SHOULD ensure that a selected tab has its aria-selected attribute set to true, that inactive tab elements have their aria-selected attribute set to false, and that the currently selected tab provides a visual indication that it is selected.

I've updated the PR and add aria-current as a separate props to tab component. Also as aria-current indicates visually styled I've changed style's conditions to be chosen by aria-current rather than aria-selected and I've added some related tests as well. Could you please check it and tell me if there is any issues?

michaldudak · 2023-10-26T15:08:11Z

Why would we need both? Neither ARIA docs nor demos for the tab role mention the aria-current attribute. Is there an accessibility issue you're trying to fix?

Kimzify · 2023-10-27T11:57:34Z

Why would we need both? Neither ARIA docs nor demos for the tab role mention the aria-current attribute. Is there an accessibility issue you're trying to fix?

there was an open issue for that and I have trying to fix #32924
am I getting the issue wrong?

michaldudak · 2023-10-27T12:09:55Z

Sorry, I should have explained this better. I haven't noticed at first that you're fixing #32924.
The issue is only about the navigation tabs demo (https://mui.com/material-ui/react-tabs/#nav-tabs), not the tabs in general.

Kimzify · 2023-10-27T13:10:25Z

Sorry, I should have explained this better. I haven't noticed at first that you're fixing #32924. The issue is only about the navigation tabs demo (https://mui.com/material-ui/react-tabs/#nav-tabs), not the tabs in general.

thank you so much. Just I have another question to clarify solution.

we need Tab component to detect links by itself(with component="a" props) and add aria-current label instead of aria-selected
or
just we need to update document for Nav tabs to contain aria-current beside aria-selected?

Kimzify · 2023-11-12T12:34:52Z

As I was checked, if aria-current and aria-selected both have the same value, aria-selected is preferred to be used. If tabs represent navigations, aria-current="page" should be used to indicate navigation usage.
from w3 doc:

In some use cases for widgets that support aria-selected, current and selected can have different meanings and can both be used within the same set of elements. For example, aria-current="page" can be used in a navigation tree to indicate which page is currently displayed, while aria-selected="true" indicates which page will be displayed if the user activates the treeitem

@michaldudak, could you please check the PR again?

siriwatknp · 2023-11-12T23:59:04Z

docs/data/material/components/tabs/NavTabs.tsx

@@ -34,6 +35,7 @@ function LinkTab(props: LinkTabProps) {
          event.preventDefault();
        }
      }}
+      aria-current={props.selected && "page"}


Suggested change

aria-current={props.selected && "page"}

aria-current={props.selected ? "page" : undefined}

Also, please add aria-selected={undefined} at the end to override the default behavior of the Tab.

<Tab component="a" onClick={(event) => { // Routing libraries handle this, you can remove the onClick handle when using them. if (samePageLinkNavigation(event)) { event.preventDefault(); } }} aria-current={props.selected && "page"} {...props} aria-selected={undefined} />

for the first issue, as I was checked, based on w3 document the default value of aria-current is false.

If the attribute is not present or its value is an empty string or undefined, the default value of false applies and the aria-current state MUST NOT be exposed by user agents or assistive technologies.

and for the second issue, we still need aria-selected tag, because aria-selected and aria-current represents different concepts and meanings here.

based on w3 document:

In some use cases for widgets that support aria-selected, current and selected can have different meanings and can both be used within the same set of elements.

Thanks for the info!

siriwatknp

Based on #32924 (comment), the Tabs should have role="navigation" as well.

When you update the code, please run yarn docs:typescript:formatted.

siriwatknp · 2023-11-13T11:07:50Z

docs/data/material/components/tabs/NavTabs.js

@@ -27,11 +28,17 @@ function LinkTab(props) {
          event.preventDefault();
        }
      }}
+      aria-current={props.selected && 'page'}
+      role="navigation"


Suggested change

role="navigation"

role="navigation" should be at Tabs element below, not Tab.

Kimzify · 2023-11-29T12:32:48Z

@siriwatknp @michaldudak, current PR is ready to review again. could you please check it if you have time?

siriwatknp

👍 Thanks for the improvements!

Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com> Co-authored-by: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>

Kimzify marked this pull request as draft October 24, 2023 19:30

Kimzify marked this pull request as ready for review October 24, 2023 19:33

danilo-leal changed the title ~~[docs][Tabs] change aria-selected to aria-current~~ [joy-ui][material-ui][Tabs] Change aria-selected to aria-current Oct 25, 2023

danilo-leal added accessibility component: tabs package: material-ui package: joy-ui labels Oct 25, 2023

danilo-leal requested review from siriwatknp and michaldudak October 25, 2023 12:09

fix: navTabs should have aria-current page value

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

ff1897c

Kimzify force-pushed the tab-aria-current branch from 4a76404 to ff1897c Compare November 12, 2023 12:29

siriwatknp changed the title ~~[joy-ui][material-ui][Tabs] Change aria-selected to aria-current~~ [material-ui][docs] Add aria-current for nav tabs demo Nov 12, 2023

siriwatknp reviewed Nov 12, 2023

View reviewed changes

siriwatknp requested changes Nov 13, 2023

View reviewed changes

doc: add navigation role to navbar

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

36cf986

siriwatknp reviewed Nov 13, 2023

View reviewed changes

fix: pass navigation role to tabs component

Verified

This commit was signed with the committer’s verified signature.

nadine-nguyen Nadine Nguyen

SSH Key Fingerprint: vhgmubL/4wUb78heZVh7hFafYrs3xW0RAAysoDI9c2A
Verified
Learn about vigilant mode

ec5a4d4

remove proptypes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

840d08c

siriwatknp approved these changes Nov 30, 2023

View reviewed changes

danilo-leal added 2 commits December 22, 2023 08:01

Merge branch 'master' into pr/39594

b040838

yarn docs:typescript:formatted

f8ea7e9

danilo-leal added docs and removed package: joy-ui labels Dec 22, 2023

danilo-leal added 2 commits December 22, 2023 08:09

yarn prettier

b4785ed

docs:typescript:formatted again

3f02c7f

danilo-leal merged commit 9e798d8 into mui:master Dec 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[material-ui][docs] Add aria-current for nav tabs demo #39594

[material-ui][docs] Add aria-current for nav tabs demo #39594

Kimzify commented Oct 24, 2023 •

edited by danilo-leal

Loading

mui-bot commented Oct 24, 2023 •

edited

Loading

michaldudak commented Oct 25, 2023

Kimzify commented Oct 25, 2023 •

edited

Loading

michaldudak commented Oct 26, 2023

Kimzify commented Oct 27, 2023

michaldudak commented Oct 27, 2023 •

edited

Loading

Kimzify commented Oct 27, 2023

Kimzify commented Nov 12, 2023

siriwatknp Nov 12, 2023

siriwatknp Nov 13, 2023

Kimzify Nov 13, 2023 •

edited

Loading

Kimzify Nov 14, 2023

siriwatknp Nov 30, 2023

siriwatknp left a comment

siriwatknp Nov 13, 2023 •

edited

Loading

Kimzify Nov 13, 2023

Kimzify commented Nov 29, 2023

siriwatknp left a comment

	aria-current={props.selected && "page"}
	aria-current={props.selected ? "page" : undefined}

[material-ui][docs] Add aria-current for nav tabs demo #39594

[material-ui][docs] Add aria-current for nav tabs demo #39594

Conversation

Kimzify commented Oct 24, 2023 • edited by danilo-leal Loading

mui-bot commented Oct 24, 2023 • edited Loading

Netlify deploy preview

Bundle size report

michaldudak commented Oct 25, 2023

Kimzify commented Oct 25, 2023 • edited Loading

michaldudak commented Oct 26, 2023

Kimzify commented Oct 27, 2023

michaldudak commented Oct 27, 2023 • edited Loading

Kimzify commented Oct 27, 2023

Kimzify commented Nov 12, 2023

siriwatknp Nov 12, 2023

Choose a reason for hiding this comment

siriwatknp Nov 13, 2023

Choose a reason for hiding this comment

Kimzify Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Kimzify Nov 14, 2023

Choose a reason for hiding this comment

siriwatknp Nov 30, 2023

Choose a reason for hiding this comment

siriwatknp left a comment

Choose a reason for hiding this comment

siriwatknp Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Kimzify Nov 13, 2023

Choose a reason for hiding this comment

Kimzify commented Nov 29, 2023

siriwatknp left a comment

Choose a reason for hiding this comment

Kimzify commented Oct 24, 2023 •

edited by danilo-leal

Loading

mui-bot commented Oct 24, 2023 •

edited

Loading

Kimzify commented Oct 25, 2023 •

edited

Loading

michaldudak commented Oct 27, 2023 •

edited

Loading

Kimzify Nov 13, 2023 •

edited

Loading

siriwatknp Nov 13, 2023 •

edited

Loading