-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(material/paginator): add role="group"
to host
#22512
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
Conversation
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. Do we need a test for it?
In testing `<mat-paginator>` with VoiceOver, I found that the interaction was much nicer when the paginator controls were semantically grouped with `role="group"`. This change adds the role and updates the documentation to explicitly instruct users to apply a meaningful label.
299fa57
to
a282188
Compare
Added a test Caretaker note: when this is synced into Google, we should communicate the change to users |
LGTM |
In testing `<mat-paginator>` with VoiceOver, I found that the interaction was much nicer when the paginator controls were semantically grouped with `role="group"`. This change adds the role and updates the documentation to explicitly instruct users to apply a meaningful label. (cherry picked from commit f06ff2a)
In testing `<mat-paginator>` with VoiceOver, I found that the interaction was much nicer when the paginator controls were semantically grouped with `role="group"`. This change adds the role and updates the documentation to explicitly instruct users to apply a meaningful label. (cherry picked from commit f06ff2a)
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
In testing
<mat-paginator>
with VoiceOver, I found that the interaction was much nicer when the paginator controls were semantically grouped withrole="group"
. This change adds the role and updates the documentation to explicitly instruct users to apply a meaningful label.I was on the fence about adding a default label (something like "Pagination control"). I settled on this approach because it's the most consistent with our other controls that require developers to specify a label for host elements, even though we do have
MatPaginatorIntl
for the labels for child components within the control.