-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Global styles: display tooltips for pagination buttons on styles revision #62395
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +45 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
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.
@ramonjd thanks for your PR
I pushed a commit to remove the no longer necessary aria-label
props, as the new label
props are equivalent. LGTM thanks!
Flaky tests detected in dd149f2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9412905191
|
Will try to use the existing icons |
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 think this is ideal from an accessibility and code standpoint, but since it's a design change it might be a good idea to get some design feedback.
Before | After |
---|---|
If these icons feel a bit large, do we need previousSmall
and nextSmall
icons as well as chevronLeftSmall
and chevronRightSmall
? cc @WordPress/gutenberg-design
I think this PR is okay to ship as is, but in the future, we need to apply the same approach and design to other paginations.
Patterns Dialog
Font Library: Install Fonts
Ah, I was wrong, if the icon size is not suitable, we can simply adjust it with the |
A while ago I created an issue to unify the various Pagination components, see #58133. Other improvements should be addressed there and I would love to see more focus on that issue. This is a functional change, as this pagination is fundamentally broken from an accessibility perspective. Thanks everyone for feedback and review. |
Thanks for merging this! |
…sion (#62395) * Showing tooltips for pagination buttons on global styles * Remove redundant aria-label prop. * Use actual SVG icons. * Adjust buttons size. --------- Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
…sion (#62395) * Showing tooltips for pagination buttons on global styles * Remove redundant aria-label prop. * Use actual SVG icons. * Adjust buttons size. --------- Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
…sion (WordPress#62395) * Showing tooltips for pagination buttons on global styles * Remove redundant aria-label prop. * Use actual SVG icons. * Adjust buttons size. --------- Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
This was cherry-picked to the wp/6.6 branch. |
What?
Maybe resolves #62283
Adds tooltips to pagination buttons for Global Styles revisions.
Why?
To expose the button name via tooltip.
How?
Using the
showTooltip
prop on the Button component.Testing Instructions
Screenshots or screencast
2024-06-07.15.10.41.mp4