Skip to content
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.

Sign up for GitHub

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

Jump to bottom

📝 Update aria-label in Popover #2570

Merged
merged 5 commits into from
Oct 13, 2022
Merged

📝 Update aria-label in Popover #2570

merged 5 commits into from
Oct 13, 2022

Conversation

martalalik
Copy link
Contributor

Resolves #2569

@martalalik martalalik marked this pull request as ready for review October 11, 2022 07:26
mimarz
mimarz approved these changes Oct 11, 2022
View reviewed changes
Copy link
Contributor

@mimarz mimarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

One things, when testing with keyboard and voiceover, it doesn't make much sense for the buttons to be called "Click me!" or "Hover me!". Maybe we should change the button text to be something more along the lines of "Open popover"?

oddvernes
oddvernes requested changes Oct 11, 2022
View reviewed changes
Copy link
Collaborator

@oddvernes oddvernes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fix the duplicated ids

@martalalik martalalik force-pushed the docs/ML-2569-popover-a11y branch from 5981fbf to 69705a2 Compare October 11, 2022 11:20
@martalalik
Copy link
Contributor Author

https://www.w3.org/WAI/ARIA/apg/example-index/

oddvernes
oddvernes approved these changes Oct 12, 2022
View reviewed changes
Copy link
Collaborator

@oddvernes oddvernes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@martalalik martalalik merged commit cc44939 into develop Oct 13, 2022
@martalalik martalalik deleted the docs/ML-2569-popover-a11y branch October 13, 2022 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oddvernes oddvernes oddvernes approved these changes

@vnys vnys Awaiting requested review from vnys vnys is a code owner

@mimarz mimarz Awaiting requested review from mimarz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Popover improve a11y in docs
3 participants
@martalalik @mimarz @oddvernes