Try removing default align-items styles from flex layout #47099
Conversation
tellthemachines
requested review from
spacedmonkey,
ellatrix,
geriux and
ajitbohra
as code owners
tellthemachines
added
[Type] Bug
|
Size Change: -44 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
tellthemachines
requested review from
ndiego,
andrewserong,
glendaviesnz and
aristath
|
Oh, what an interesting problem, thanks for diving in! At first glance, I'm not sure how feasible it'll be to remove some of the default values, because the Stack block launched with the idea that by default blocks have the width of their content (a bit like the social icons block). Here's what I noticed on an existing Stack block:
Now that the layout features have been out in the wild for a while, if we do remove things from the default values, I think we'll need to be quite careful to see what sorts of existing patterns might have been depending on these values 🤔
Rather than changing the defaults, is there a way that we could add a control that would support stretching children to fill the container, and treat this an enhancement? |
|
Now that #47134 has landed, shall we close out this one? Or do you think it's still worth pursuing changing the default options 🤔 |
|
I think it's still good to remove defaults that aren't useful, just need to work out if it won't break anything. Ideally those styles would never have been added in the first place 😅 |
|
Okie-doke!
Ah, the perennial backwards compatibility challenge! 😄 |
|
I was curious: TIL it turns out the |
|
I can second this. |
|
I'm not terribly familiar with the context, but I've been testing this and can confirm that the default Inserting a group block (Stack) in trunk, I get:
In this PR I get: Playing around with justification controls I see that The social links block has the following styles too: .wp-block-social-links.is-vertical {
align-items: flex-start;
}All looks good on the frontend for me. Are there some other patterns that need testing? |
Sorry, but "playing around with justification" is not whats the problem here. It is about the default value. And it is set, like the OP described and like I describeb in my linked issue. Individual Styles from blocks like the social links block you described is not the problem too. Please read the initial post carefully. It is about the default css "align-items: center" that is added but should not! |
Would you be able to test this PR and offer your expertise? I think it'd help move this issue along if so, especially if there are things that I've missed after testing it for the first time. Hence my question:
The PR description specifically mentions the following:
|
Thanks for taking a look! I've meaning to come back to this PR and do some more testing on it 😅 Might be worth taking a look at the WP Directory to see if there are third party blocks using this layout type (probably) and check those, but I'm more than happy to do that myself. |
With "we are adding" the PR means Wordpress (Gutenberg) is adding those on default. See the Screenshot in my last post and the linked issue. When you add a block and don't change anything to the alignment, you get those classes. In my case in a custom block with "__experimental" activated. |
That's what I understood too, and formed the basis of my quick test. This PR is not marked as resolving your issue. If you think it should cover it, it might be more helpful to the PR author to test it and provide feedback as to where changes or improvements could be made. |
tellthemachines
force-pushed
the
try/removing-flex-align-styles
branch
from
3824c35 to
6a91fe5
Compare
|
Flaky tests detected in ae30c09. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4782542866
|
|
I rebased and updated the default alignment control values to the new defaults. I've also added a default In testing the core blocks, there are a few discrepancies with trunk in specific cases, namely:
Trunk:
This branch:
For Row and Stack blocks at least, I don't believe we should add a default alignment value such as I added to Social Icons, as that would mostly defeat the purpose of this PR. We want the default to change. However, this may bring about breaking changes to websites relying on the current defaults. We could conceivably work around this by adding a default alignment that matches the current one and then adding an override in the Row/Stack block variation settings to make all new Row and Stack blocks have the new default. It's a bit messy, and in that scenario I think that programatically added blocks will still get the legacy defaults unless explicitly specified, but I can't think of any way to write a deprecation for this as there are no changes to the actual blocks, so that may be our only alternative to breaking changes. For Buttons, adding a default center alignment might work well. Further feedback and ideas welcome! |
Why?
We're increasingly running into issues caused by the
align-itemsproperty that gets added by default to flex layouts. In horizontal layouts, we're addingalign-items: centerand in vertical onesalign-items: flex-start. For most blocks, it's not useful to set these properties to these particular values, and in some cases it's actively working against their layout potential.I've only found one case where setting this value by default is needed: the Social Icons block in its vertical orientation, as described in #38069.
#46133 and #45117 describe layout scenarios that should be fairly straightforward to build and become difficult or impossible with the current
align-itemsdefaults.How?
align-itemsfrom the default flex layout.It's still possible to manipulate the value to some extent in horizontal orientations (such as Row block) with the block alignment settings, and the justification controls for vertical orientations (such as Stack block).
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
A Stack inside a Row should now by default take up the full height of the tallest block inside the Row: