-
Notifications
You must be signed in to change notification settings - Fork 762
fix(datagrid): add an aria-label to the column chooser dialog #5195
Conversation
Add an aria-label to the column chooser dialog. Closes: vmware-archive#4754 Signed-off-by: Alex Mellnik <a.r.mellnik@gmail.com>
<div class="clr-sr-only" tabindex="-1" #menuDescription> | ||
{{ commonStrings.keys.showColumnsMenuDescription }} | ||
</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.
Where does this and related removals come from? I can't find them as part of the linked issue.
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.
Previously, the description for the column toggle panel was being done with this invisible div that was programatically focused when the column chooser was opened. The problem with this is that while it's read when the panel is opened, when the user exits the panel the screen reader will tell them that they have exited a dialog but not what dialog it was that they exited. This change moves the description to the dialog itself so that it's read both when the user enters it and leaves it.
There's still another one of these invisible divs in the dialog after this change, but a better way of handling that would probably to use the Angular CDK's aria-live service rather than trying to get the same effect by shifting focus to an invisible 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.
Sounds good. Btw, we're staying away from making our own aria-live implementations or services, because we don't want to impose any such implementations to the user. So, your solution is more in sync with our current strategy. I like it when code gets removed.
If you found this helpful I'd appreciate it if you would add the hacktoberfest-accepted -- I'm attempting to mine a shirt. Thanks -A |
I've not forgotten this PR, I am trying to fix in parallel some other related issues with the same dialog and I want to see what interference might there be both ways. |
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.
OK, I don't think it will be conflicting with the other changes I'll be doing here.
Looks good to me, too.
Add an aria-label to the column chooser dialog.
Closes: #4754
Signed-off-by: Alex Mellnik a.r.mellnik@gmail.com
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The column chooser dialog has the dialog role, but there's no label associated with it. There is a invisible div inside the dialog that describes it, but since it's not an aria-label its not read again as the screen reader virtual focus exits the dialog.
What is the new behavior?
Removes the invisible div and moves the message to the aria-label of the dialog.
Does this PR introduce a breaking change?
Other information