Refactor ariaLabel props to aria-label

[ItsMarcoMSF]

Click to expand details

Package

carbon-components, carbon-components-react

Browser

Chrome

Operating System

MacOS

Package version

[email protected] & [email protected]

React version

v16.14.0

Automated testing tool and ruleset

IBM Equal Access Accessibility Checker

Assistive technology

No response

Description

According to @carbon/[email protected] storybook, the Modal component accepts a prop called aria-label.

However, in most other Carbon components in react, the prop ariaLabel is accepted

Can there be at least consistency between components in this area? A deeper dive into the source can also show that a number of components still accepts aria-label as a prop while the storybook says ariaLabel. And then there are other components that only accept one of ariaLabel or aria-label and not both.

According to React's official document on WAI-ARIA: https://reactjs.org/docs/accessibility.html#wai-aria:

Note that all aria-* HTML attributes are fully supported in JSX. Whereas most DOM properties and attributes in React are camelCased, these attributes should be hyphen-cased (also known as kebab-case, lisp-case, etc) as they are in plain HTML

So perhaps, the best approach is to support aria-label in all components?

WCAG 2.1 Violation

No response

Reproduction/example

Not applicable.

Steps to reproduce

Not applicable.

Code of Conduct

• I agree to follow this project's Code of Conduct
• I checked the current issues for duplicate problems

---

Plan of action

As described below, Components with the prop ariaLabel need to be refactored to aria-label. We wanted to ship this as part of v11, #9535, but it fell out of scope. There are currently 13 usages that need to be refactored: https://carbon-react-explorer.vercel.app/props/ariaLabel

Components to update

Preview Give feedback

1. Refactor ariaLabel to aria-label in StructuredListWrapper
2. Refactor ariaLabel to aria-label in OverflowMenu
3. Refactor ariaLabel to aria-label in NumberInput
4. Refactor ariaLabel to aria-label in NotificationButton
5. Refactor ariaLabel to aria-label in FilterableMultiSelect
6. Refactor ariaLabel to aria-label in Dropdown
7. Refactor ariaLabel to aria-label in TableSelectRow
8. Refactor ariaLabel to aria-label in InlineCheckbox
9. Refactor ariaLabel to aria-label in TableSelectAll
10. Refactor ariaLabel to aria-label in TableExpandRow
11. Refactor ariaLabel to aria-label in TableExpandHeader
12. Refactor ariaLabel to aria-label in ComboBox
13. Refactor ariaLabel to aria-label in CodeSnippet

Steps to complete for each component

• Deprecate ariaLabel by updating to use deprecate()
• Add aria-label prop -- e.g. ['aria-label']: PropTypes.string,
• Refactor usages of ariaLabel to prefer aria-label if it's provided. Both props need to work for the time being to remain backwards compatibility

[tay1orjones]

Thanks for bringing this up @ItsMarcoMSF - yes, usages of ariaLabel should be converted to be aria-label instead.

We intended to ship this as part of v11, #9535, but it fell out of scope. There are currently 13 usages that need to be refactored: https://carbon-react-explorer.vercel.app/props/ariaLabel

[tay1orjones]

I just updated the issue title and body with a new "Plan of action" section outlining the work that needs done.

This issue is a lower priority item for our team, but we'll happily accept PRs if you're open to contributing!

[ItsMarcoMSF]

@tay1orjones Thanks for replying to the issue. I'm open to contribute, let me know if you have a timeline in mind!

[tay1orjones]

@ItsMarcoMSF Whatever timeline works for you! We won't be able to pick this up for quite some time unfortunately.

It's all in the contributing.md, but at a high level just fork the repo into your personal account, make a branch, make the changes, commit/push up, and open a PR. As this is a change to props on a component you may need to regenerate the public API snapshot via yarn test -u at the root after making your changes, but we can triage that in the pull request reviews.

Let me know if I can help get you up and running. You could start with a single component and just ref this issue #

[jsehull]

Hey team - good find with the aria inconsistencies. I'm going to start working on some fixes for this.

[jsehull]

@tay1orjones this is a rather large body of work for all of this. I could break it down into multiple PRs for merging to get some things out there door sooner. How would you prefer something like this be handled?

[jsehull]

@tay1orjones Can you clarify what to do with the following regarding aria-label?

1. FilterableMultiSelect: on the list, but does not have it
2. NumberInput: on the list, but does not have it
3. Notifications

• 1. ActionableNotification has aria-label but is not on Issue list above. Update or remove?
• 2. ToastNotification has the propType listed, but it is not currently used. Update or remove?

4. InlineCheckbox is not active in Storybook nor is it listed as an subitem under Checkbox. Ignore? Remove?

• Note: there is a current request for checkboxes in rows. Is this similar enough that it needs to be kept and could be used towards a solution here instead of trying to pass an "inline prop" to the current Checkbox?
• Issue: [Feature]: Checkboxes improperly format as a row#11803

[tay1orjones]

@jsehull Yeah, I would sequence it out into separate PRs per component to get it out the door sooner.

The list of components to refactor was generated by searching the codebase for ariaLabel: PropTypes. Every usage listed there needs to be refactored to use aria-label unless the prop is not used (and not spread onto a child via ...other or similar)

InlineCheckbox is used in a few spots - TableSelectRow, TableSelectAllRow, etc. and can be viewed in those DataTable stories using those components.

Also I think we can just ignore any feature requests for now and focus on updating the existing API in the state it's in.

[jsehull]

Below are some of my progress updates in case others are monitoring.

Items removed:

1. NumberInput

Not started:

In process:
3. all 4 table related items (half done)

• for some I get the new aria-label working but am having trouble keeping the deprecated ariaLabel functional

Completed code

Done (PR created)

1. StructuredListWrapper
2. Dropdown
3. ComboBox
4. CodeSnippet
5. FilterableMultiSelect
6. OverflowMenu
7. Notification
8. InlineCheckbox

[jsehull]

8 of 12 completed and pending PR approvals.

4 of 12 remaining:

• TableSelectRow
• TableSelectAll
• TableExpandRow
• TableExpandHeader

The table section is proving more challenging, though the fix should be nearly the same. I'm having varying degrees of success getting those working.

At best, for some, I get the new aria-label working but am having trouble keeping the deprecated ariaLabel functional

---

For now I'm moving on to other Issues. Anyone is welcome to hop in and try these out!

Whoever works on this next can reference recent fixes as the updates should be similar.

---

Once a solution is found and properly tested, I run the following to ensure that snapshots are updated. It's common to have to fix tests afterwards. DataTable has many tests!

yarn && yarn build && yarn test -u

[tw15egan]

Still a few left to complete

[TannerS]

@tay1orjones @tw15egan do yall mind if i help out with this?

[tw15egan]

Go for it @TannerS 🎉

[fitrahmunir]

@tay1orjones @tw15egan May I help by working on this?

[tw15egan]

Sure thing @fitrahfm 👍🏻