Try toggle instead of dropdown to show stylebook. #67810
+46
−44
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. |
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