-
Notifications
You must be signed in to change notification settings - Fork 92
[WNMGDS-2694] Audit ARIA on generic roles #2978
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
Conversation
Hi @zarahzachz, I've included notes below for an accessibility take. Let me know if there are next steps or further discussion needed. Alert
Autocomplete
Choice List, Date Field, Dropdown, Month Picker
Dialog
Drawer
Tab, Tab Panel
|
// 🥑 | ||
// https://www.w3.org/TR/wai--1.2/#group | ||
// - "aria-invalid (state) (deprecated on this role in ARIA 1.2)" | ||
|
||
return ( | ||
<fieldset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Add role="listbox"
as possible remediation of deprecated aria-invalid
attr on group roles.
|
||
// 🥑 | ||
// https://www.w3.org/TR/wai-aria-1.2/#button | ||
// - "aria-invalid (state) (deprecated on this role in ARIA 1.2)" | ||
'aria-invalid': invalid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Investigate if role="combobox"
still causes issues for VO as remediation for aria-invalid
deprecation. May need further exploration if combobox still has issues.
// 🥑 | ||
// https://www.w3.org/TR/wai-aria-1.2/#option | ||
// - "Default for aria-selected is false." | ||
// Could remove `aria-selected="false"` from the `li` element. | ||
export function renderStatusMessage(message: ReactNode) { | ||
return ( | ||
<li aria-selected="false" className="ds-c-autocomplete__menu-item-message" role="option"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Remove aria-selected
attr as false is implied by its role.
@@ -139,6 +139,9 @@ export const ChoiceList: React.FC<ChoiceListProps> = (props: ChoiceListProps) => | |||
}); | |||
|
|||
return ( | |||
// 🥑 | |||
// https://www.w3.org/TR/wai-aria-1.2/#group | |||
// - "aria-invalid (state) (deprecated on this role in ARIA 1.2)" | |||
<fieldset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: If choice is radio
, set role="radiogroup"
// Is `tabIndex` necessary here? It's not clear why it's being set to -1. | ||
// It's also not clear why the `role` attribute is being set to `document`. | ||
// Should this allow for more flexible heading tags? Locked into `h1` for now. | ||
// Can we remove `role="main"` from the `main` element? | ||
return ( | ||
<NativeDialog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
- Replace
tabIndex
withautofocus
attr - Replace
<h1>
with<h2>
- Remove
role="main"
- Apply
aria-labelledby
todialog
and notdiv
I don't believe the Bootstrap issue exists on native dialog elements, so I don't believe the role="document"
is needed. Will experiment with removing this role.
@@ -69,6 +69,9 @@ export const Drawer = (props: DrawerProps) => { | |||
|
|||
const Heading = `h${props.headingLevel}` as const; | |||
|
|||
// 🥑 | |||
// Why is `tabIndex` needed here? | |||
// `aria-labelledby` not allowed on `role="generic"` element | |||
return ( | |||
<NativeDialog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO:
- Explore replacing
tabindex
withautofocus
- Apply
aria-labelledby
todialog
and notdiv
@@ -79,6 +79,9 @@ export const Tab = forwardRef((props: TabProps, ref: any) => { | |||
{props.children} | |||
</a> | |||
) : ( | |||
// 🥑 | |||
// https://www.w3.org/TR/wai-aria-1.2/#generic | |||
// - "aria-disabled (state) (deprecated on this role in ARIA 1.2)" | |||
<span aria-disabled="true" {...sharedTabProps}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't realize the {...sharedTabProps}
included role="tab"
. Disregard.
@@ -41,6 +41,9 @@ export const TabPanel = (props: TabPanelProps) => { | |||
const classes = classnames('ds-c-tabs__panel', props.className); | |||
|
|||
return ( | |||
// 🥑 | |||
// https://www.w3.org/TR/wai-aria-1.2/#tabpanel | |||
// - "aria-disabled (state) (deprecated on this role in ARIA 1.2)" | |||
<div |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Explore purpose of disabled Tabs. It's recommended not to disable interactive elements as it can confuse users.
aria-disabled
is deprecated on tabpanel
roles. Need to figure out purpose of disabling component before mitigation can be decided.
WNMGDS-2694
Added notes where we could review use of ARIA in components