Try toggle instead of dropdown to show stylebook. #67810
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tellthemachines
added
[Type] Enhancement
tellthemachines
requested review from
jasmussen,
ramonjd and
jorgefilipecosta
|
Size Change: -20 B (0%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 13cd621. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12269301402
|
ramonjd
approved these changes
Dec 11, 2024
There was a problem hiding this comment.
LGTM and works well in my testing.
Makes sense to minimize the code and effort for the user (1 vs 2 clicks).
I'll lean on @jameskoster a bit for final review, as he was involved in the initial effort
|
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. |
|
I think this toggle is good, but it's not clear that specifically the toggle control is the right one, and the label "Show Style Book" is a bit long. One instinct is to use this icon: This is mainly to make it similar to what's in the in-editor global styles panel now. But I also recognize the icon may not be ideal. Could it be the same type of toggle button, just with a text label that says "Style Book"? |
|
The toggle control makes it feel like a setting, which doesn't quite capture the nature of this feature. I agree that reusing the existing toggle is probably the best approach, given it's familiarity. We can revisit the icon separately if needs be. |
|
Thanks for the feedback folks! I've updated to use a button with text in the latest commit: 
|
|
Thanks for the adjustment. I think the idea was actually to use the same button we find in the Styles panel in the Editor, this one: 
What do you think? |
I was going off Joen's comment above:
I think that icon looks weird and I never understood why it stands for the style book, but I have a bad relationship with icons in general 😅 Perhaps we can make it an icon by default and make sure it becomes a a text label if button text labels are enabled. |
Ok I've updated the PR with this! |
|
Oh sorry, perhaps there's a crossed wire here :D I agree the icon isn't the best, and could be revisited. But I also think there's value in maintaining consistency between this styles panel and the one found in the editor. In other words; if we want a text button here, why not in the editor too? That said if y'all prefer a text button I'm happy to defer, and apologies for the detour. |
|
I think it's worth reaching consistency here first, supporting Jay's point, and then revisit the icon and behavior. |
|
We can further discuss the button in #67980. |
|
I don't know what the stakes are here, but is it worth pausing this PR until we've reached a point of consistency in #67980? Just my opinion: Pros of what's presently in trunk:
Cons:
On the general topic of alignment with the editor, there are efforts to bring revisions to the top-level:
The second PR, I think, proposes to relocate the revisions icon and make a text button in the styles panel in the editor. That will change the available realestate in the header as well. |
the latest update in this PR makes the button consistent with the editor one. They are icons by default, and switch to text when the text labels preference is enabled. Given this provides a UX improvement as it is, I believe we should merge it and then discuss iterating on the icon as a follow-up. |
3 tasks
yogeshbhutkar
pushed a commit
to yogeshbhutkar/gutenberg
that referenced
this pull request
Dec 18, 2024
* Try toggle instead of dropdown to show stylebook. * Use button instead of actual toggle * Make button icon except if text labels are enabled
yogeshbhutkar
pushed a commit
to yogeshbhutkar/gutenberg
that referenced
this pull request
Dec 18, 2024
* Try toggle instead of dropdown to show stylebook. * Use button instead of actual toggle * Make button icon except if text labels are enabled
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Tiny PR to test replacing the dropdown that allows switching to the stylebook in the styles screen with a simple toggle, because a dropdown feels like overkill when there are only two options 😅
To test, activate a block theme and go to the Styles section in the site editor.
Screenshots or screencast