remove aria attributes from DetailsRowCheck #19936
Conversation
msft-fluent-ui-bot
added
the
needs cherry-pick
msft-fluent-ui-bot
left a comment
msft-fluent-ui-bot
left a comment
There was a problem hiding this comment.
It looks like this change to the 7.0 branch may not have been submitted to master yet. Now that version 8 has released, all changes must be submitted to the master branch first (except in emergencies or if the change is irrelevant to version 8).
Please do one of the following:
- If you've already created a PR to
master, add a link to it - If the change is irrelevant to version 8, add a comment explaining why
- Otherwise, create a PR to
masterwith this same change, and add a link to it
After that, you can dismiss this review and remove the "needs cherry-pick" label (or ask a team member to help do so).
Want to avoid this in the future? Include text like "Cherry-pick of ####" in the PR description (where #### is the real master PR number).
|
This need to be made into v8 because the fix already exists in v8 as part of this PR - #18366 The cherry pick label can be removed. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 305fe54:
|
Asset size changes
Baseline commit: dfa33092a8ce8405681ebec526b187880e14eef3 (build) |
Perf AnalysisNo significant results to display. All results
|
|
|
smhigley
left a comment
smhigley
left a comment
There was a problem hiding this comment.
I'd also recommend removing lines 243-256 in DetailsHeader.base.tsx, so we're not passing in an aria-label/aria-describedby to <DetailsRowCheck> that won't be used.
Thanks for the PR!
I removed ara-describedby. Do you also recommend removed aria-label? The PR to v8 did not remove aria-label |
smhigley
removed
the
needs cherry-pick
khmakoto
dismissed
msft-fluent-ui-bot’s stale review
Already part of #18366
|
Is this change effectively undoing #17623? I want to make sure the issues brought up in that PR are still addressed. |
|
@ThomasMichon nope, it doesn't undo that one (I double checked in the PR demo site link). It only removes the label when |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
|
🎉 Handy links: |
7 participants
Pull request checklist
$ yarn changeDescription of changes
On a single select, the DetailList header section has a check box that is not selectable. This checkbox has aria attibutes but does not have a role attribute causing an accessibility issue aria-allowed-attr. The current fix is to remove those aria attributes and resolve the accessibility issue.
(give an overview)
Focus areas to test
(optional)