-
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
Block Styles: Make block style variations control scale for increased number of variations #57780
Block Styles: Make block style variations control scale for increased number of variations #57780
Conversation
This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit. Thank you! ❤️ |
Size Change: +389 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
4299abf
to
8a8a5c9
Compare
Flaky tests detected in 8a8a5c9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7637638077
|
e6ba6a7
to
79f5765
Compare
08be716
to
fb14f6b
Compare
a694f89
to
1c8b54b
Compare
This partially reverts commit d3fa9c2.
8a8a5c9
to
ea76a86
Compare
I've rebased this PR and switched the approach back to using the v2 CustomSelect components given the extended block style variations feature can now afford to wait for those components before landing. |
I chatted with @mtias today about this. I think we should add these as style selectors in the "Color" panel, above the color presets, rather than in the "Styles" panel, alongside any potential other styles. This would reduce potential UI confusion with group blocks that have the more traditional styles applied, and it also fits in visually with the efforts to add color (and typeset) presets to the editor. I don't think I need the preview either, as it's easy enough to switch back to the default (the first one in the list). What do you think? |
Thanks for the feedback @richtabor 👍
Honestly, I'm not sure how the latest suggestion reduces confusion. I'm certainly more confused, so please bear with me 🙏 Block style variations can cover the full gamut of theme.json styling. Yes, the examples to date were just using colors but they could be purely typographic, change only borders like the existing image rounded block style or a new "frame" style a theme author could create, or style everything (spacing, colors, typography, borders, inner elements, block types etc). Given that, I don't personally think it makes sense to have them under Colors and assumed that is why block style variations resided separately to begin with.
Can you expand on this for me? It sounds like you're suggesting that we conditionally separate block style variations and show some under the styles panel and others elsewhere (colors in this case). After gradually getting some clarity on block style variations being the right mechanism for section styling it seems like we're moving away from that again and needing smaller subsets of styles within theme.json that can be mixed and matched properly.
I can remove the preview from the block style variations control if desired. I can see it providing a more complete picture of all of a block style variation's styles when it covers more than colors though. |
I would +1 @aaronrobertshaw comment. A summary of the timeline so far. We started with focusing on just colours via colour sets/ways (basically what you've proposed here @richtabor ). We then moved in a direction to cover properties beyond colours. We then changed direction technically to make use of block style variations instead of introducing something new. I think we've landed in a decent spot in that we have the flexibility of other properties if theme authors need it, but most importantly we are still solving the primary need which resolves around colours. I think we continue making use of the existing block style variation picker, and if at a later date we decide to more prominently highlight colours included in the block style variations we can do that (much like we are doing for font/colours at the global variations level). |
Just to confirm, do you mean we should stick with the buttons for block style variations? I'm happy to shelve this PR and revisit as required in the future. It could be a little premature to optimize the UI control for excessive numbers of block styles before we've run into that problem in the wild 🤷 |
We are talking about two related but different things here. The mechanism is just style variations, so the implementation is fine and can carry on. It's going to be presented based on what it contains, though. That's what #56604 aims to capture. So if a style variation only has color changes, it's presented as a color preset. That means in global styles it won't be within the "browse styles" but within the colors panel, where we have "color palette". Same for typography. Check out the PR at #56622 Now in block instances we should follow this same organization. I think design wise we should use an element button and have the color presets on a flyout, not directly on the sidebar, which feels a bit overwhelming. Given we don't yet have this organization, I don't think it should block this branch, but we should converge with #56622. |
No I meant stick with what you've built. |
Thanks for the continued discussion and patience here @mtias 👍
Are the two things referred to theme style variations and block style variations?
This might be the bit that confuses me a little. Style variations are generally the "theme" style variations, however the implementation here is for "block" style variations. They primarily address two different user pain points i.e. quickly creating a theme-level look and feel vs easily switching out styles across a section of a page, post, etc.
This is all related to theme style variations and a different scope to this PR (and its base PR).
I did see that PR a couple of months back, and even worked on a proof of concept based on it, applying colorways and typesets in a mix and match way for block instances however the early feedback on that was the user problem was already solved by the work being done here on block style variations. I'll give it another fresh look though.
I'm not sure this makes as much sense at the block instance level as it does at the theme level in global styles. Block style variations can be registered from multiple sources, core, themes, plugins etc. The complete set of block style variations might include variations that aren't purely color styles. That could lead to both the styles panel having some block style variation options and some under colors (or typography) yet we don't have a means of merging and applying multiple block style variations. Having variations across different panels, or having seemingly unrelated controls impacting others sounds like a pretty sub par experience, so I'm not quite seeing just yet, how block style variations could converge with #56622. My primary concern is making sure that whatever the grand plan is, the underlying implementation of block style variations doesn't cause problems in getting to the end goal. If the block style variations implementation is still relatively independent to #56622, which I think it is, it should be ok to move forward. P.S. Can we rebrand all the various variations? 🙏 There seem to be a number of GitHub issues (e.g. #59186) and confusing discussions because of the term "variation" being used in multiple locations.
|
@aaronrobertshaw of course! Appreciate the patience as well :) It is all just style variations in the end (a Agreed we need to group all of the disparate issues related to style variations, block variations, and everything in between. Finally, for the specific UI here, I think the only thing we should tweak is that while the resting element is fine: I think the dropdown (or flyout, probably), should use the full style variations card for clarity. Right now these seem to imply that it only affects colors, but it can affect any property. I'm also ok with iterating on this after merge if we are on the same page. |
Just noting that there is some work involving color cards happening in #59498. It would be good to align this PR with the results of that if possible. |
As noted on the main tracking issue, these UI updates will be put on hold for the time being. So for now I'll close this PR with a view to reopen down the road if it makes sense. |
Depends on:
Related:
What?
Updates the Block Styles UI to use a Dropdown to display available options.
Each block style variation option will also render a card indicating the background, text, and link colors set by that block style variation.
Given the possibility for block style variations to mix other styles with colors, or not set color styles at all, the combination of a "color card" and label was settled on.
Why?
With the expansion of block style variation functionality in #57908. It is anticipated that blocks will have an increasing number of block styles available to them. Currently, in the block inspector, each block style is given a button to enable its selection. As the number of available block style variations increases, so does the real estate these controls take up.
How?
BlockStylesDropdown
component in the block editorTesting Instructions
Screenshots or screencast
Demo:
Screen.Recording.2024-02-29.at.5.27.07.pm.mp4
Previous iteration of this PR description for the `CustomSelect` approach
What?
Leverages the new v2
CustomSelect
andCustomSelectItem
components to provide a custom select control for block style variations.Initially, it was deemed to early to use these components and using a Dropdown might have been a better option. However, given the delays to the block style variations feature, the new v2 CustomSelect components are much closer to being ready and should land shortly before the variations work will in 6.6.
Why?
With the expansion of block style variation functionality in #57908. It is anticipated that blocks will have an increasing number of block styles available to them. Currently, in the block inspector, each block style is given a button to enable its selection. As the number of available block style variations increases, so does the real estate these controls take up.
How?
popoverProps
Testing Instructions
Screenshots or screencast
Demo:
Screen.Recording.2024-02-21.at.5.45.29.pm.mp4
Original iteration of this PR description for the Dropdown approach
What?
This is an alternate approach to updating the block style variations control in the block inspector to better handle an increasing number of block styles.
Why?
With the expansion of block style variation functionality in #57908. It is anticipated that blocks will have an increasing number of block styles available to them. Currently, in the block inspector each block style is given a button to enable its selection. As the number of available block style variations increases so too does the real estate these controls takes up.
The initial approach in #57331 was to use a
CustomSelectControl
however that component (including its newv2
version) require some updates before it will be suitable. This PR offers a similar approach using aDropdown
instead.How?
Testing Instructions
Screenshots or screencast
Demo:
Screen.Recording.2024-01-11.at.9.58.22.pm.mp4