Add block spacing and layout to Post Template #49050
Conversation
|
Size Change: -16.6 kB (-1%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
tellthemachines
force-pushed
the
try/post-template-grid
branch
from
cbae2a9
to
6ce40e5
Compare
|
Updated this draft to use column width instead of number of columns, to align better with #49018. If this approach is good, I can start working out deprecation details soon 😅 |
|
Flaky tests detected in 82b6193. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5140490121
|
tellthemachines
force-pushed
the
try/grid-layout
branch
from
47bfe5a
to
52bfcc7
Compare
tellthemachines
force-pushed
the
try/post-template-grid
branch
from
6ce40e5
to
bec3923
Compare
|
We should use the same control that's on the new grid layout (and it should probably go in the "Layout" panel:
And it seems funky having the Layout Inner blocks controls with the grid controls—the grid block does not have these:
Is there a cheap way to inherit that same Layout panel from the Grid block? It'll continue to get enhancements moving forward as well. |
|
Perhaps the control should fallback to a default column width value, if switched from list to grid. Would prevent cases where you can't tell that it gridded, if the column width was large: CleanShot.2023-03-28.at.15.26.25.mp4 |
The problem here is that the Query block itself has a flow/constrained layout so it can set the content width of its children, and it's using custom logic to toggle between the list and the grid display, but those list/grid styles are actually applied to the child Post Template block. To complicate things further, Post Template itself has a locked flow layout type, probably in order to leverage the default theme block spacing value when it's set to list display. Untangling all this is going to be complicated! It helps to be clear about the ideal state we want to move towards, so with that in mind here are some questions:
Ultimately the important thing is deciding whereabouts in this interdependent nested block structure we want the controls to display; I think it would make sense to have them all together. It would greatly simplify the interface if Post Template could be invisible and non-configurable, but at this point I'm not sure we can change that without breaking back compat 🤔 |
| const updatedLayoutType = | ||
| layoutType === 'list' || layoutType === 'default' ? 'default' : 'grid'; | ||
|
|
||
| useEffect( () => { |
There was a problem hiding this comment.
I don't love this but don't think there's a way around it unless we move the list/grid controls into Post Template instead of keeping them in Query.
There was a problem hiding this comment.
It might be worth looking into exploring moving them down 🤔
|
Looking at ways to leverage this work for #44899; I've now added block spacing support to the Post Template block. Controls can be found in the Post Template sidebar, under "Dimensions". |
tellthemachines
force-pushed
the
try/post-template-grid
branch
from
4865f98
to
48c0f73
Compare
tellthemachines
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Very interesting questions and problems to solve! Some first impressions from testing it out:
Personally, I'd expect the controls to exist at the Post Template block level, though it might be challenging for folks to discover. The Query block has a lot of complexity to the settings with options for post type, ordering, filters, etc, so I quite like the idea of the styling being deferred to the Post Template level, so that there's a semantic difference there (Query is where you manage the query, Post Template is where you manage the appearance). But that could just be me!
I feel like in an ideal case we might have both options. At the moment, switching to minimum column width could be perceived as a regression depending on the pattern or layout in use, since it's a widely used block and there are plugins that depend on it (e.g. WooCommerce). For example, here's how the default grid / 3 column pattern appears on trunk versus this PR when using a wide alignment:
Another thought: for backwards compatibility, are there any existing aspects of using |
|
Thanks for the feedback @andrewserong !
Sure, but for now I'll settle whatever gets us to a mergeable solution without too much back and forth around how to materialise those options in the UI 😅
Hmm, the default column width could be changed to match the 3 col default better, but yeah it will always behave differently.
Not with the current implementation of flex in Post Template. In the future, for most alignment options we can rely on grid for the same features as flex; I think flex could only really be useful in super edge case situations such as having only 2 posts in the query and wanting them to be spaced-between? But that seems like a pretty unlikely use case. |
Good point, it'd be good to try to keep the change as simple as possible if we can to avoid scope creep. In that case, would it be worth exploring retaining the number of columns behaviour, even if it is a bit different to the minimum column width controls in the Grid block? I'm mostly wondering if that would help with the backwards compatibility concern for blocks that might already have a custom column count set. |
|
I added an unstable "columns" property to layout and some extra logic to make it work. Given the experimental nature of grid (and layout in general 😅 ) this shouldn't work against future experimentation. That solves the back compat issue, all except for the responsive behaviour that isn't working yet. I'm undecided on whether to add some custom logic to Post Template or to add it as part of layout support; if we ever want to explicitly support defining a fixed number of columns we'll probably want it to have some responsivity. I guess the main decision left to make is whether to leave the layout controls where they are or bring them down into Post Template. Deprecations etc. to be dealt with once we're clear on how much to change here 😄 |
They can be very fiddly, so good idea to hold off until it's clear which direction to go in!
This looks like a good approach to me! One quick question: since these values will be stored in post content, would it help in the long-term to give it something closer to a final name, possibly |
Good question. My main aim for now is to signal that this property could change and shouldn't be relied on. But yeah it's worth making the not- |
Dev noteAdded layout and block spacing support to Post Template blockPreviously, Post Template block had custom layout styles that allowed for either a "list" or a "grid" (implemented behind the scenes with CSS flex) layout, with controls living in its parent Query block. For 6.3, layout and block spacing support have been added to Post Template, and its controls now live in the Post Template toolbar. There is still a choice of "list" and "grid" styles, but "grid" is now implemented with CSS grid. |
* Add static column number option * Address feedback and fix Group placeholder * Revert non-responsive option * Try using grid layout in Post Template * Update Post Template to use auto-fill columns * Add block spacing to post template * Show gap control by default * Add unstable columns grid property * Rename layout column attribute. * Add a media query to reproduce current responsiveness * Move layout controls to Post Template block * Reduce max columns to 6. * Remove unstable prefix * fix cols breaking out of container * Try adding deprecation * Fix broken loop * Update fixtures * Code improvements * Add some comments to the CSS. * Add back deleted line * Remove legacy attributes in deprecation * Fix deprecation logic * Update fixtures * Fallback gap for classic themes * fix spacing * match old default value * Update PHP test strings. * Fix tag discrepancy in fixtures
|
Stupid question, was this feature reverted from WordPress core at some point? 🤔 In 6.4 I only see the option to choose the number of columns. Not the minimum column width for some reason? |
|
Not a stupid question at all @fabiankaegy! The grid option in Post Template has a slightly different configuration than the experimental Group grid variation. They both use grid layout, but this solution was thought best to preserve back compat, because Post Template always had the option to pick the number of columns. I think ideally there should be a way to switch between both grid configurations; with #42385 still in (slow) progress we haven't settled the best way to expose that in the UI. |
|
@tellthemachines Thanks for that clarification. Just to make sure, this means there also isn't a simple attribute one can change in a pattern for example to get the different grid behavior? |
|
You can change the behaviour by editing the block attributes in the markup; the only thing that hasn't been implemented yet is the UI for switching between grid types. Essentially you'd need to replace the So you'd end up with the Post Template layout attribute looking something like |
|
@tellthemachines I'm working on a block with a requirement that responsiveness be implemented with a configurable number of maximum columns. This isn't something that layout supports, however, I did come up with a solution by setting Would you say that's a valid assumption, and if so, what would you suggest? I'm fine with opening up a PR if you agree supporting both |
|
@ObliviousHarmony Container query based responsiveness is under discussion as a feature to add to core, but it will take a while to ship so my suggestion in the meantime would be to add some custom styles to your block in a way that can be removed later once core support is in place. |
|
Thanks for getting back to me so quickly @tellthemachines. Sorry, I wrote this comment way too early in the morning and shared very little information; I even gave some CSS without any context 😅. I appreciate your suggestion here, however, a container query doesn't quite meet my requirements. The feedback we've received has steered us towards having a maximum number of columns that decreases the number of columns if one of them would fall below a minimum width. We're currently migrating a Query Loop like block from grid-template-columns: repeat( auto-fill, minmax( max( 150px, ( 100% - 3.75em ) / 4 ), 1fr ) );The approach basically just boils down to using a percentage width (4 columns in this case) (with a minimum, 150px in this case) to select the maximum number of columns. When it can't fit that many columns it creates new rows and decreases the number of columns accordingly. This is much simpler than a container query. It provides an easy user experience in the form of a single slider rather than potentially complicated configuration using container sizes. I don't think As noted, I do have a working implementation by passing part of the above style to |
|
@ObliviousHarmony that's an interesting use case!
So which variable would you allow users to configure, column count or minimum width? I wasn't thinking of exposing the container query configuration to users; rather it could be a dynamic query based on the column count/min width values such as we already have for grid span resizing. |
In our case we would be letting them set the column count and handle setting the minimum width internally. Since the
Even so, it still feels like one of the powerful aspects of a CSS Grid is the ability for the browser to handle column overflow behavior. Since that's the specific kind of responsiveness I'm suggesting here it seems like a reasonable feature to add. What do you think? Would you be opposed to me opening a PR adding a case for both |
The primary goal for layout (and other block supports) is to provide a way to configure layout options in the UI. Block developers can define a layout type and choose to expose the controls or not, and theme authors can further choose to hide layout controls altogether or for specific blocks. The way block supports are built relies on a close correspondence between settings such as It's very likely that at some point additional responsiveness will be added to the "Manual" grid option, but it's not yet clear how that might be exposed in the UI or as an API for developers. That should be a part of the wider work on responsive controls being discussed in #19909. The reason I mentioned container queries above is partly because the discussion over there is leaning towards them, but also because responsive styles for grid need to take into account the behaviour of multi-span children, and once #59483 is moved out of experimental status, blocks with fixed positions in the grid. Container queries are a good way of ensuring consistent behaviour between the parent grid and its children on resize. |
|
In our design the control is expressed as the number of maximum columns. With the context you've provided, how would you feel about a new |
@ObliviousHarmony this is a timely comment, because #60941 was started yesterday proposing something similar 😄 It's a bit buggy right now because the logic to toggle between Auto and Manual modes is still looking at For context, that draft resulted from this conversation, where we're looking at changing Manual mode to more easily allow for positioning blocks in fixed points in the grid. Feedback is welcome in either place! |
This is timely. I wrote essentially the exact same thing, even down to using the presence of |
What?
Fixes #44899 and #44557.
Note: the deprecation had to be done at Query level, because that's where the legacy layout type is stored. This doesn't really work on the server side though, so I opted to leave legacy classnames and styles for unchanged blocks on the front end.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast