Column: Add border support to column blocks #39967
Conversation
aaronrobertshaw
added
[Block] Columns
|
@jasmussen if you have a few moments to take a look at this, it would be great to get your thoughts before heading too far in any given direction. In particular I have a few questions you might be able to answer.
|
aaronrobertshaw
changed the title
|
Size Change: +325 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
|
Hey, thanks for the ping. Took this for a spin, trying toth the all-sides border controls and the left/right individual side controls: First off, impressive work, and this does unlock some fun things. Nice work. On the one hand:
On the other hand, the way the columns block is built doesn't lend itself well to how borders behave in responsive situations, as you flag yourself. As in the example you share, with vertical separators between columns, you'd likely expect those to shift orientation when collapsing to a single column, rather than being right of the first row and left of the last row. Especially considering the high level goal of having most responsiveness work out of the box (#34641), you can definitely get into situations where things behave in a way that you weren't expecting. But does that mean we don't add border support just because you can get in trouble? I don't know, because you could get yourself into that very same situation today with the Row block by adding group blocks inside: So maybe it's fine? To an extent, the fact that the Group block supports borders, could mean both Columns and Column blocks support it too. About border radius, the biggest issue is that it doesn't affect content inside, only the box itself. So if you apply a large radius you might assume it crops content inside. We could potentially explore a separate "Clip content" toggle (which would apply But your point about the separators is still valid — but maybe that needs a different solution? Notably something like this would be nice to accomplish:
The only way I can think of to accomplish the above would be blue site background, white columns background, and blue column backgrounds, and using a combination of columns padding and spacing to control the border width. Not ideal, but I can't think of a way to get borders to overlap in a flex situation:
@kjellr do you have time for a quick look? Would column border controls be useful? |
|
Testing. This is impressive!
I assume that later on we will see similar controls in the Radius option. Seeing the corners curve etc.
+Something similar for Padding and margins controls. I look forward to seeing the individual column borders featured included! It makes styling columns so much more powerful and flexible! Thank you! |
|
Yes, borders for columns are useful. |
|
Seems fine to try and land this one. |
Is there something we can do automatically here in combination with the "stack on mobile" control? Maybe if that's set to true, we automatically hide left/right borders if they're the only border set for the column? Might get too opinionated. 🤔 |
|
Thanks for taking the time to give this a run @jasmussen 👍
I'm not quite sure how we'd achieve that. I could imagine something like setting some CSS vars from the supplied border values and then some additional CSS to effect the desired switch in orientation. There've been issues adding CSS vars inline previously so we might not make it far down that road. There'd probably be some RTL issues to finesse as well. I also share the same concerns as @kjellr here in that things could get very opinionated. Is simply hiding all Column (not Columns) borders if "stack on mobile" is enabled, as this PR does now, a satisfactory first step?
I'm inclined to think the positives and value added by this feature outweigh the concerns at the moment. Particularly if by default on mobile there is no difference to what is achievable now (no individual column borders, or the unresponsive ones added by wrapping contents in group blocks).
Perhaps we can leave out the border-radius support on individual columns until there is a proven demand for it or we have a means of enabling the clipping of content. I'd be happy to move that into a separate PR to discuss further.
For the designs above there are two open PRs that I believe would help here. The first makes vertically positioned columns stretch to the full height of the I don't think we are too far off here for non-mobile viewports. The individual side border controls give us quite a bit of flexibility to achieve these kinds of designs. Screen.Recording.2022-04-06.at.7.31.51.pm.mp4Ignoring the responsive styling of borders the biggest challenge for me was getting the vertical spacing within columns right. Having a |
As noted above, this PR hides the individual column borders with the "stack on mobile" option enabled. At the moment, it's an all or nothing proposition. We could tweak that to where it is only left and right borders that get hidden. |
Let's give this a try and see how it feels. 👍 |
I've explored a few options today but none felt any more intuitive than simply hiding left and right borders. In its current state this PR hides
The above allows columns that have a complete consistent border to continue to be shown on mobile. There is also still the edge case with not being able to collapse borders given the flex layout. Screen.Recording.2022-04-07.at.2.07.59.pm.mp4 |
|
From the video above (thank you for providing), I wonder if it isn't better to start without those border handling changes. There are possibly cases where you want those borders to be visible on mobile, and where it feels confusing to not see them even if the inspector suggests they should be there. I think the general idea is good, but perhaps it's best to start without and see if we can go from there. |
|
I've removed the responsive styles that hide the borders as well as the border radius support. We can add either of these back in via follow-ups. |
|
Cool. Let's land this: group already supports this, and with the reduced footprint of the control, this one is able to carry it as well. |
|
Thanks for working through this one @jasmussen 👍 I'll merge it once #37770 lands. |
|
Just tested and works well. I like that the responsive styles to hide the borders were removed. I feel the following will be a common layout that would break on mobile if borders were automatically hidden. I would love to see radius control in the future though 😉
|
Thanks for giving this a test @ndiego 👍
For the record, the layout in your screenshot would not have been broken by the responsive styles that were removed. Each of the columns shown has a uniform flat border. The responsive styles I removed only affected individually configured borders i.e. non-uniform borders, so borders that were applied via |
aaronrobertshaw
force-pushed
the
try/individual-border-support
branch
from
9de7061
to
103bf31
Compare
aaronrobertshaw
requested review from
oandregal,
TimothyBJacobs,
spacedmonkey,
ellatrix and
ajlende
as code owners
aaronrobertshaw
requested review from
mkaz,
ryanwelcher and
juanmaguitar
as code owners
aaronrobertshaw
force-pushed
the
try/individual-border-support
branch
from
ee1573a
to
ea485d5
Compare
aaronrobertshaw
force-pushed
the
add/border-support-to-column-blocks
branch
from
bfee1e7
to
2bc19bb
Compare
There was a problem hiding this comment.
This is testing really well for me.
✅ Border colors/style/width work for Columns and Column blocks in the Post editor and frontend. Radius for Columns.
✅ Global styles and site editor show borders as expected.
And with stacking
This will open up a lot of design potential far beyond the screenshots I've added 🤣
Thank you!
|
Unless I'm mistaken, it looks like this only adds Radius support for ColumnS and not an individual Column. I'm trying to understand why that decision was made, because I'd love to be able to easily create designs like this by setting background colors and border radii for columns. (Putting a Group block in the column and rounding corners there doesn't work because that does not guarantee the groups all have the same height) |
|
Hi @MadtownLems 👋 You are correct, this PR only added border color, style, and width support to the Column block. Nothing was changed for the parent Columns block itself.
During the review of this PR, there was a small issue raised around border-radius not affecting content inside the column and the possible need to allow clipping column content. The opt-in to border radius support was removed from this PR so that it can be explored separately and proper discussion take place around clipping content, any introduction of new sidebar controls, where that might best be placed (at the Column or Columns level?) etc. Here's the description of the issue from the earlier review:
Here's where I suggested removing it in favour of a separate exploration:
The design you've highlighted is definitely something we wish to facilitate. We just need to work through the above concern before shipping it. I hope that helps explain the decision and maybe makes the wait for border radius on Column blocks a touch easier. Thanks for the feedback 👍 |
|
Is there any date when this will be merged into a core? |
|
@DeoThemes I believe this change didn't make it in time to land in WordPress 6.0 as it depended on the new border control component which needed a little more time (more discussion over in #37770 (comment)), but will land as part of 6.1. I don't believe a date has been announced yet. |
Fixes: #21889
Related:
What?
Opts into border support for individual column blocks.
Why?
To achieve block patterns and designs where columns use borders as visual separators.
How?
Leverages the new
BorderBoxControland individual border side block supports to offer control over innerColumnblock borders.Questions
0width border when stacked on mobile.!importantTesting Instructions
Columnsand some innerColumnblocks with contentColumnblocksColumnsblock stacking on mobileExample columns code
Screenshots or screencast
Demo
Screen.Recording.2022-04-01.at.3.27.32.pm.mp4
Responsive Styling Issue
Example of behaviour without forcing borders on mobile to be hidden
Screen.Recording.2022-04-01.at.3.29.59.pm.mp4