-
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
Query Pagination: allow hiding the label text #50779
Conversation
Size Change: +9.41 kB (+1%) Total Size: 1.39 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.
The span
element that wraps the arrow has aria-hidden="true"
in the markup, which according to the spec should remove it from the accessibility tree.
The presence of the aria-hidden attribute hides content from assistive technology but doesn't visually hide anything.
aria-hidden="true"
should not be used on elements that can receive focus. Additionally, since this attribute is inherited by an element's children, it should not be added onto the parent or ancestor of a focusable element.
I think aria-hidden
should be set to false
if we expect to be able to tab to the arrow itself when the text label is hidden.
It does not affect the functionality, at least not in Chrome and Safari, but it would be safe to adhere to the spec regardless.
Thanks for testing this out, @vcanales!
I believe because the It would probably be best practice to have a text label, but as we're offering a design that only uses the icon, I think adding an
|
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.
The option and the arla-labels work well in my test, thank you.
Flaky tests detected in ece2e7f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5118981101
|
* Wrap showLabel logic in a useEffect * Only render PlainText if showLabel is true * Update packages/block-library/src/query-pagination/edit.js Co-authored-by: Nik Tsekouras <[email protected]> * Add showLabel to deps --------- Co-authored-by: Nik Tsekouras <[email protected]>
* Add showLabel attribute * Add QueryPaginationLabelControl * Add showLabel to query-pagination-next * Update docs * Add showLabel to query-pagination-previous * Always show label if there is no pagination arrow * Update test fixtures * Update toggle label and description * Make comment backticks consistent
) * Wrap showLabel logic in a useEffect * Only render PlainText if showLabel is true * Update packages/block-library/src/query-pagination/edit.js Co-authored-by: Nik Tsekouras <[email protected]> * Add showLabel to deps --------- Co-authored-by: Nik Tsekouras <[email protected]>
What?
This adds an extra display option to the Query Pagination block which allows users to hide the text for the pagination links. This then unlocks several display options for the pagination links:
Why?
Closes #49266.
How?
This adds a new boolean attribute,
showLabel
, to the Query Pagination block. The default value istrue
, and when set tofalse
, it will hide the text labels.showLabel
is controlled by a toggle control which is only displayed when an arrow icon type is selected. The attribute cannot be set tofalse
if there is nopaginationArrow
set, to prevent the pagination link from being empty when no arrow or label is set.Testing Instructions
Testing Instructions for Keyboard
Editor testing instructions:
Front end testing instructions:
Screenshots or screencast
Settings panel:
Screen.Recording.2023-05-19.at.11.17.35.mov