-
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
Columns block: reinstate tablet 2 col stacking #41123
Conversation
Size Change: +509 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
@ramon could we do the same as the Gallery block here and add a CSS var for the column blockGap? This var is added to the editor here, and frontend here. If it was possible we should potentially extract these to a shared component/render method as I think they just need the addition of a css var name prop to work in both contexts. Just note there is an open PR to modify the frontend loading of this style slightly. |
Awesome, thanks for raising that @glendaviesnz Yeah I reckon we should try it out. Just for the record and nothing else, I had a PR to test whether we could create 2 x blockGap vars to fix the block-level blockGap bug:
Seems as if it'd also address this error, but the double CSS var hasn't been discussed yet and so the gallery experience might be the way to go. |
@glendaviesnz I took a stab at your suggestion. Does this look/work okay to you? 7fe65e3 I haven't yet taken into account your changes in #41015 |
Seems to work as expected for me: column-after.mp4What do you think about moving the GapStyles component somewhere like |
I'd say it's probably worth it 👍 |
Actually, worth it, but okay for another PR? Just don't want to lose focus of the fix itself. What do you reckon? |
For sure, follow PR fine. |
08b6922
to
4517272
Compare
Tested with core themes 2011 - 2022, and a few custom themes. Looking okay to me, but I'm probably biased 😄 Example block code<!-- wp:columns {"style":{"spacing":{"margin":{"bottom":"47px","top":"115px"}}}} -->
<div class="wp-block-columns" style="margin-top:115px;margin-bottom:47px"><!-- wp:column {"style":{"color":{"background":"#eaed48"}}} -->
<div class="wp-block-column has-background" style="background-color:#eaed48"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#eaed48"}}} -->
<div class="wp-block-column has-background" style="background-color:#eaed48"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- wp:columns {"style":{"spacing":{"blockGap":"151px"}}} -->
<div class="wp-block-columns"><!-- wp:column {"style":{"color":{"background":"#ffccd8"},"spacing":{"padding":{"top":"10px","right":"10px","bottom":"10px","left":"10px"}}}} -->
<div class="wp-block-column has-background" style="background-color:#ffccd8;padding-top:10px;padding-right:10px;padding-bottom:10px;padding-left:10px"><!-- wp:paragraph {"style":{"color":{"text":"#0b0b0b"}}} -->
<p class="has-text-color" style="color:#0b0b0b">Column with custom block gap and padding</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#ffccd8"},"spacing":{"padding":{"top":"10px","right":"10px","bottom":"10px","left":"10px"}}}} -->
<div class="wp-block-column has-background" style="background-color:#ffccd8;padding-top:10px;padding-right:10px;padding-bottom:10px;padding-left:10px"><!-- wp:paragraph {"style":{"color":{"text":"#0b0b0b"}}} -->
<p class="has-text-color" style="color:#0b0b0b">Column with custom block gap and padding</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#ffccd8"},"spacing":{"padding":{"top":"10px","right":"10px","bottom":"10px","left":"10px"}}}} -->
<div class="wp-block-column has-background" style="background-color:#ffccd8;padding-top:10px;padding-right:10px;padding-bottom:10px;padding-left:10px"><!-- wp:paragraph {"style":{"color":{"text":"#0b0b0b"}}} -->
<p class="has-text-color" style="color:#0b0b0b">Column with custom block gap and padding</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- wp:columns {"style":{"spacing":{"blockGap":"14px"}}} -->
<div class="wp-block-columns"><!-- wp:column {"style":{"color":{"background":"#b3002a"},"spacing":{"blockGap":"89px"}}} -->
<div class="wp-block-column has-background" style="background-color:#b3002a"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#b3002a"}}} -->
<div class="wp-block-column has-background" style="background-color:#b3002a"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#b3002a"}}} -->
<div class="wp-block-column has-background" style="background-color:#b3002a"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#b3002a"}}} -->
<div class="wp-block-column has-background" style="background-color:#b3002a"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"style":{"color":{"background":"#b3002a"}}} -->
<div class="wp-block-column has-background" style="background-color:#b3002a"><!-- wp:paragraph -->
<p>Column</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- wp:columns {"style":{"spacing":{"blockGap":"185px"}}} -->
<div class="wp-block-columns"><!-- wp:column {"style":{"color":{"background":"#ecffae"}}} -->
<div class="wp-block-column has-background" style="background-color:#ecffae"><!-- wp:group -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>A paragraph inside a Group block inside a Column.</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>A paragraph inside a Group block inside a Column.</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>A paragraph inside a Group block inside a Column.</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph -->
<p>A paragraph inside a Group block inside a Column.</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
<!-- wp:social-links {"style":{"spacing":{"blockGap":{"top":"68px","left":"68px"}}}} -->
<ul class="wp-block-social-links"><!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /-->
<!-- wp:social-link {"url":"https://wordpress.org","service":"wordpress"} /--></ul>
<!-- /wp:social-links --></div>
<!-- /wp:column --></div>
<!-- /wp:columns --> |
@scruffian Do you reckon this PR is worth pursuing or will it create more headaches for themes? The TL;DR is that we forced stacking for mobile AND tablet in #37436 Before, at tablet widths, we forced a 2 col 50% 50% split. This PR brings us back to that, but via fairly complex route. |
Themes that use columns for layout (Stewart and Remote) are examples of why we removed this rule in the first place. Right now the only tool we have for doing sidebar style layouts is the columns block, and in these cases splitting the columns 50/50 creates quite an unexpected layout on sites using these themes. When we removed the rule we were aware that it would introduce this regression but we also felt like the original rule was a mistake and so removing it seemed like the best course of action. I'm not a fan of bringing it back! We could consider adding a UI to control this, but we also need to be careful to make something that is easy enough to use and understand as well as giving us the flexibility we need. One option to explore might be to set a "minimum width" for columns so that we break when that is reached... |
Hi, I'm the one who reported this issue on trac. Thanks for this PR. While I understand that stacking columns on tablets could be useful for some layouts (e.g. site with sidebar), these layouts are minor and can be handled by the themes. However, I don't think it would be a good idea to totally change the design and break the front-end of millions of websites without users knowing about it and without having the possibility of repairing it. I hope this PR will be included in WordPress 6.0 |
This is a good point, thanks for raising it.
Yeah, a 50%/50% split does seem a bit opinionated. Removing the rule, I think, was less risky since it only applied to a breakpoint window of 181px. Breakpoints are still meaningful, but as the preference for fluid layouts grows, "tablet" will become meaningless and the bandaid will be trickier to rip off. As for an accompanying UI control, it's a great idea. I don't expect @andrewserong to mind if I show off one of his experiments.
I think we missed the boat on WordPress 6.0 unfortunately, but maybe we could strike a balance here in time for the next release by adding a UI control that achieves the tablet 50/50 split (?). Edit: I forgot to mention that, whatever idea we come up with, we should probably compose a dev note on the WordPress Core blog to explain the change. |
Just to keep the discussion / ideas going, I've put up an experimental PR in #41295 to see what it looks like with an added conditional UI control for controlling the number of columns at tablet screen sizes.
While the experimental PR I put up technically kinda works, I think I prefer the idea of columns being able to collapse / expand based on min / max widths. It'd be really neat, say, to be able to have a 4 column layout that on mobile sizes collapses down to I don't quite have enough time to go down another rabbithole with that idea just yet, but if no-one beats me to it, I'd be keen to have a play with min / max widths for columns to see what could potentially be viable there, too. |
Thanks @andrewserong I tried to have a think about an alternative way of controlling the column behaviour at narrower widths, but couldn't come up with anything intuitive so far. Here's this PR in action columns-tablet-2-col-collapse.mp4So that's an effect we'd like to achieve with the given variables. I can't see a way around our defined breakpoints, so maybe something along the lines of #41295 would work, possibly something along the lines of "How many columns would you like at each of the 3 breakpoints?" Seems overly complex, at least the idea. 🤷 |
Thanks for taking a look, Ramon!
Me either! Any of the ideas I've come up with so far wind up creating complex and not-particularly-intuitive interfaces. Though, I suppose if we relied on column-based attributes for minimum and maximum widths, etc, we could abstract it a little by hiding it behind block patterns, but it still gets overly complex quite quickly (and there's lots of edge-case-ey behaviour to deal with).
Yes, it does seem like the crux of the issue is how to make a complex thing flexible and easy to work with to suit a range of different patterns and themes 🤔 |
Perhaps using the columns block for creating sidebar layouts is an abuse of the intention of the block. Maybe another option to explore is a new "layout" block which allows different layout options (for example, main content and sidebar). |
There is another option: block styles columns-tablet-stack.mp4It's a simple change, however I would be wary about future deprecations. For example, if we decide to remove the style we'll have to ensure backwards compatibility. |
…k from #37436 which forces 2 columns rather than a total stack
Remove duplicated comments
4517272
to
37ed4ed
Compare
I've added this since it appears to me to be a fairly frictionless way to add the tablet stack view back in. Happy to dump or revert if it's too much. |
I think that sounds like the best compromise so far — avoids the complexity of adding additional controls as in #41295, and ensures users can choose either behaviour (preserve the old 2 column on tablet behaviour, or avoid that breakpoint altogether). In terms of backwards compatibility and keeping the style around indefinitely, I wonder if the work in #41020 will make that less of an issue — e.g. if the tree-shaking-like approach is used for outputting CSS based on the classnames present in the rendered / stored block markup, then perhaps it'd be easier to keep older CSS around in the repo, because we wouldn't need to worry about the performance issues of it still being there? Just a thought 🙂 I'd be keen to see what folks think about the addition of the block style, too! |
"label": "Default", | ||
"isDefault": true | ||
}, | ||
{ "name": "tablet-stack", "label": "Tablet stack" } |
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.
Just jotting down a brain spasm: I was wondering if we should toggle the style off when isStackedOnMobile !== true
It might cause extra confusion though remembering to toggle it back on.
Just tossing another idea here. |
Just my thoughts on columns: For less complex sites and WP light users, the stackings option and columns management is a good option I think. We are currently adding extended settings to every core block to be more flexible and to have to code less block styles (complex designs mean a lot of custom styles which is a mess in the UI): What we did is to add an opt-in columns control which forces columns to be flex all the time and allow ordering, flex gap and stacking. We use more breakpoints than other sites, but that's not the deal. Those breakpoints are adjustable either in our theme or in the plugin which provides those settings - so not hardcoded. This might be an edge case for most Gutenberg devs IMO, but the biggest thing Gutenberg is missing is real responsive support for medium to heavy complex layouts. This is just our view on the topic, no feature request or critic to specific Gutenberg principles. |
Sorry I looked! That's an interesting approach, thanks for sharing @aristath The motivation behind this PR was to attempt a clean way to reinstate the tablet-view stacking. It's a hard one as columns are used for larger, site-wide layouts and this stacking is undesirable. I'd be interested to see if I can borrow your idea, as possibly a new attribute + UI is called for. What I've got so far with the block style feels a little "band aid" to me.
Thanks for the thoughts and the screenshot @dennisheiden Good to have this to mull over. This issue might interest you as well: There's interest in fluid design, but yeah, it won't happen overnight. |
Closing this as it's stale. If we need to address this in the future we can reopen. |
Related to #40952
What?
This is an experiment to reinstate the tablet breakpoint CSS for the columns block from #37436 which forces two columns rather than a total stack.
Why?
We removed the style as part of #37436 , favouring to stack all column blocks below the desktop breakpoint, however it caused a visual regression.
How?
flex-basis
rule, ensuring we offset any current block gap values.Testing Instructions
Check in classic and block themes that the columns please!
Insert a columns block with more than 1 column. Ensure that 'stack on mobile' is turned on to allow stacking.
With the "Tablet View" applied to the columns block, and between 600px and 781px (the "tablet" view) the columns should stack 2 per row, with any trailing columns stretching the full width.
Turn off 'stack on mobile' and check that the columns no longer stack.
The editor and frontend should behave similarly.
Caveat: adding a custom blockGap value that is greater than
var(--wp--style--block-gap, 2em)
to the columns block will trigger stacking since the spacing is now too great for a 2 per row stacking.Test block code
Screenshots or screencast