Block Library: Columns: Force 50% column width at mid-range viewport #20597
Conversation
aduth
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
|
Size Change: +23 B (0%) Total Size: 865 kB
ℹ️ View Unchanged
|
aduth
added
[Block] Columns
| // As with mobile styles, this must be important since the Column | ||
| // assigns its own width as an inline style, which should take effect | ||
| // starting at `break-medium`. | ||
| flex-basis: calc(50% - #{$grid-unit-20}) !important; | ||
| flex-grow: 0; |
There was a problem hiding this comment.
Note that there are two other styles here that will now only take effect between $break-small and $break-medium - 1:
flex-grow:nth-child( even ) { margin-left }
As far as I see it, this is not only fine, but in-fact preferable, since these styles are specific to how the columns are presented in the mid-range viewport (with 50% width).
Practically speaking, it should have no effective difference either, since:
flex-growis overridden atbreak-mediummargin-leftis assigned to the same value$grid-unit-20 * 2atbreak-mediumfor:not(:first-child), which includes (is a superset of):nth-child(even)
There was a problem hiding this comment.
Code looks good, if complex :)
Thank you for doing this.
jorgefilipecosta
added
the
Backport to WP 6.7 Beta/RC
|
@jorgefilipecosta FWIW I was mostly expecting this to have an impact on front-end of the previewed post. I guess if the stylesheet is loaded in the editor too, it may have a benefit there as well. |
jorgefilipecosta
removed
the
Backport to WP 6.7 Beta/RC
|
Hi there, |
|
Hi @dianeco , my understanding is that the intended behavior was always that there would be two equal-width columns shown at mid-range viewports. This didn't work correctly before, resulting in a behavior such as the one you describe, but also resulting in issues like that summarized in #18416 (fixed by these changes). Based on some previous discussion in #20169 (specifically #20169 (review) and related work in #19909), I believe there might be some future desire to remove this mid-range behavior altogether, or at least to allow the user to have granular control over the column widths at various viewport sizes. cc @jasmussen |
|
I would agree that allowing users to adjust not only the specific amount of columns at mobile tablet and desktop breakpoints, but their individual widths as well, is something we should endeavour to support! However it's a task that is perhaps best accomplished in a way that lets you customize ANY block and those breakpoints, not just the Columns block specifically. #19909 was created to enable such customizations. |
|
@jasmussen #19909 seems very promising. But in the meantime, as it was possible before this change to keep an unequal columns layout on medium devices (with some CSS changes), would you consider creating a class that the user can add to avoid the 50% resizing. For example, replacing the current code by |
|
Such a class would be edgecase CSS to support a feature meant to be replaced. However it would be nice if we could replace that
That way you should be able to customize those breakpoints, no? |
|
Yes, if we could replace the |
|
Any thoughts on whether we can find a way to remove that !important, @aduth? |
|
The
But between the comments of #20597 (comment) and #20597 (comment), it seems to me it should already be possible to override this by assigning an additional class name to the column and using a custom style which both has high specificity (higher than the base style) and |
|
Yes, it's already possible to override style with higher specificity and |
|
I just installed WordPress 5.4 RC1 (and disabled the Gutenberg plugin), and I can confirm that #18416 is fixed! 😄 |
Fixes #18416
This pull request seeks to resolve an issue where columns do not always size correctly at mid-range viewports. Specifically, columns with explicit widths assigned will assign those widths using an inline style attribute which, prior to these changes, would take precedent over the
50%intended width at this viewport range.These changes resolve this by assigning the
flex-basisusing!important. While!importantis typically discouraged, it is the only means by which the inline style can be overridden, which is the intent of these styles (to override the explicit assigned width). An alternative would require that the flex-basis be assigned by some other means, e.g. a combination ofdata-attribute and CSSattr(), which has no browser support and would not be backwards-compatible with existing columns content.Note that there is already prior art for this behavior. The styles which force
100%width in mobile viewports is applied in the exact same fashion:gutenberg/packages/block-library/src/columns/style.scss
Lines 20 to 24 in e01b562
The important caveat is that unless this is used with
max-widthmedia query, then the!importantstyles would override the intended explicit widths at larger viewports. That is the reason for changing from using the mixin to a manually-crafted@mediaquery (same as with the mobile styles).Note about WordPress 5.4 cherry-picking: It was my hope to try to have this fixed in time for WordPress 5.4, despite being a bug which has existed for some time. While it could still be included, it is complicated by the facts that (a) RC1 is scheduled for tomorrow, and since this is not a regression of WordPress 5.4, it may not qualify for cherry-picking during RC, and (b) the patch would not apply cleanly because of refactoring which has occurred in this stylesheet since 5.4-beta1 (#20529).
Testing Instructions:
Make a post including Columns with varying combinations of explicit and flexible-width columns. Specifically, you should observe that when assigning an explicit width to a column, the width is overridden when previewed at viewport sizes between ~600-800px.
Repeat steps to reproduce from #18416