-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
refactor(data-table): DataTable components to use aria-label instead of ariaLabel #16332
refactor(data-table): DataTable components to use aria-label instead of ariaLabel #16332
Conversation
Update to deprecate ariaLabel in favor of aria-label. No breaking changes, aria-label will have priority if provided.
All contributors have signed the DCO. |
I have read the DCO document and I hereby sign the DCO. |
recheck |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Could you please give context on why the versioning of the eslint-plugin-react-refresh
is changed?
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.
I had some issue to run yarn install and commit was failing with an error related to that module.
It was related to something else on my local (I just checkout the repo) but I was keeping this version since it's the latest minor update.
I think the change in that file can be reverted if we want, but it will not have any impact since we use the caret ^
prefix, we are already installing the latest minor update, so it was still installing that version.
Let me know if I'm wrong and if we want to revert it, please :) Thanks for the comment!
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.
LGTM 🚀
add my user as contributor for code and a11y. No changes in the logic compared to last commit
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks so much for this! I'll incorporate my suggestions and then merge
packages/react/src/components/DataTable/__tests__/DataTable-test.js
Outdated
Show resolved
Hide resolved
c35d1c7
This is my first contribution here, I would like to learn more.
In the scope of this PR I've marked
ariaLabel
as deprecated (but still usable) and addaria-label
No breaking changes,
aria-label
will have priority if provided.Closes #12802
I think this PR close the issue above with the updates to the DataTable components.
Changelog
New
Changed
Removed
Testing / Reviewing
I've added a unit-test scenario for all cases to check: