search

carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.8k stars 1.8k forks source link

[Feature Request]: ariaLabel removal or deprecation log changes #17584

Open ev-codes opened 1 week ago

ev-codes commented 1 week ago

The problem

getSelectionProps() (part of @carbon/react) returns ariaLabel as one of the properties.

However, <TableSelectAll> logs a deprecation warning to console.warn when ariaLabel is passed into it. Unnecessary logs make it more difficult to find more important issues and slow down CI workflows.

The solution

Either:

Examples

Code:

<TableSelectAll {...getSelectionProps()} aria-label="entity table select all" />

Logs from test:

console.warn
    Warning: This prop syntax has been deprecated. Please use the new `aria-label`.

Application/PAL

No response

Business priority

None

Available extra resources

No response

Code of Conduct

github-actions[bot] commented 1 week ago

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

sstrubberg commented 1 week ago

Hey @ev-codes, the warning is intentional and it meant to let you know that you need to change aria-label from ariaLabel. Stop by office hours if you need any more clarity!

AlanGreene commented 1 week ago

@sstrubberg As far as I understand it, the issue reported here is that the instance of ariaLabel triggering the warning is originating from Carbon's own implementation of the getSelectionProps function.

See for example https://github.com/carbon-design-system/carbon/blob/647881bbea53d99ad4e75c5c4726926538d269d1/packages/react/src/components/DataTable/DataTable.tsx#L165-L166

The values returned include both ariaLabel and the expected aria-label.

A similar issue exists for a number of the other functions, e.g. getRowsProps, getExpandHeaderProps, etc.

Please reopen this issue.

tay1orjones commented 1 week ago

Yeah, sorry for the confusion, this is something we need to fix in all the applicable DataTable prop getters.

<TableSelectAll> and the other DataTable components are correct in logging a deprecation warning for ariaLabel. aria-label should be used instead.

I'm not sure why the initial deprecation work didn't modify getSelectionProps() to no longer return ariaLabel and use aria-label instead.