-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Update 'showLabel' help text #68105
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. |
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.
Left one note but nothing blocking :)
help={ __( | ||
'Toggle off to hide the label text, e.g. "Next Page".' | ||
) } | ||
help={ __( 'Make label text visible to the users.' ) } |
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 like that you remove the CTA from this. Makes it align better with all other instances.
But I have to say I do think the example was useful to know what the label text is actually referring to
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 like that you remove the CTA from this. Makes it align better with all other instances.
CTAs doesn't make sense for similar UI controls.
But I have to say I do think the example was useful to know what the label text is actually referring to
I don't have a strong opinion on the example, happy to append it back. But here's why I removed it:
- This is parent-controlling child blocks, so it's a half example.
- What if you've changed the label to something very different? Would the example still make sense?
Size Change: -3 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
Flaky tests detected in b4dfe27. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12410389716
|
I'd welcome this improvement, thanks for working on this @Mamaduka The main point on the issue is that the UI doesn't show the labeling of the previous/next any longer when the setting is switched to 'arrows'. As such:
At this point, users may completely forget they previously entered inappropriate labels text and the UI doesn't expose the labels text in any way. This PR improves the help text in the UI but I'd like to avoid it closes #51335 when merged because the reported issue isn't solved yet. Can we change the |
help={ __( | ||
'Toggle off to hide the label text, e.g. "Next Page".' | ||
) } | ||
help={ __( 'Make label text visible to the users.' ) } |
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.
isn't to the users
implied?
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. Left a small note.
What?
Part of #51335.
PR updates Query Pagination block's
'showLabel
attribute helps text. The project eliminates thetoggle
keyword from labels and help texts. See #66369.I've also removed the example from a string. The results of toggling to options are visible in the Canvas.
Testing Instructions
Testing Instructions for Keyboard
Same.
Screenshots or screencast