Size Change: -2.31 kB (0%)

Total Size: 1.04 MB

_{compressed-size-action}

This is excellent:

Note the above has a pretty spacious (intentionally so to test) margin, but it's applied correctly:

Here's the gap highlighted by the inspector:

Since we are employing gap now, I think we can remove the margin that sits around each item, which was previously there just to space things out:

I did notice in the resting state, this "click plus" prompt is subject to the issue as well:

I never really loved that help text in the first place, I think we'll want to do something different. But a small interim fix we can push in the mean time might just be to set the gap to zero when in that setup state.

Another thing I noticed, which we also don't have to address here, is that the Button component which is intended only for use inside editor UI is used for the markup, when an actual button should be used instead. By replacing the button component with the HTML element, we won't have to unstyle all the editor styles attached to the components-button style:

I also tried customizing the gap for this block in particular, so that it has its own rules and not those inherited from the block gap defined in settings. This:

			"core/social-links": {
				"spacing": {
					"blockGap": "0"
				}
			}

works as intended:

— though it does show we still have to remove the margins from the individual items still. It also opens another question: should we provide a block gap rule just for this block, or do we want it to be inherited from the default 24px styles rule? I don't have a strong opinion, but my instinct is that people would want large-ish block gaps for the main content, and a somewhat smaller gap inside social links.

I never really loved that help text in the first place, I think we'll want to do something different. But a small interim fix we can push in the mean time might just be to set the gap to zero when in that setup state.

Can't we wrap the plus and the text in a single element?

Another thing I noticed, which we also don't have to address here, is that the Button component which is intended only for use inside editor UI is used for the markup, when an actual button should be used instead. By replacing the button component with the HTML element, we won't have to unstyle all the editor styles attached to the components-button style:

I agree with this but it means we might need a button primitive to share with mobile, I wonder if that one is there for this reason.

I don't have a strong opinion, but my instinct is that people would want large-ish block gaps for the main content, and a somewhat smaller gap inside social links.

That is a good though but also one that relates more to how block gap works and not specific to this PR. It seems like you're suggesting one of the two

1- blockGap for horizontal blocks should have its own CSS variable different from the global one.
2- blockGap is not inherited at all and should be defined on each container.

For simplicity, I personally prefer the current behavior (consistent blockGap) but if we do want to make the distinction, maybe the first option is better.

It also related to a previous feedback by @andrewserong : Do we need both "column gap" and "row gap" which also raises the question about whether "row gap" in a "flow" container is equivalent to a "row gap" in a "flex" container

I agree with this but it means we might need a button primitive to share with mobile, I wonder if that one is there for this reason.

The search block was recently refactored along the same lines: #33961 — if mobile is an issue, we need to look back to that one as well.

1- blockGap for horizontal blocks should have its own CSS variable different from the global one.
2- blockGap is not inherited at all and should be defined on each container.

I think maybe I'm suggesting a 3rd option, one where there's a global CSS variable that is inherited, and then a default definition in the bundled lib/theme.json that overrides it. Someting like:

		"blocks": {
			"core/social-links": {
				"spacing": {
					"blockGap": "1em"
				}
			},

It's not a fully formed thought — mostly just sharing the thought process at this point.

The search block was recently refactored along the same lines: #33961 — if mobile is an issue, we need to look back to that one as well.

👍 we need to check with mobile folks here @geriux @fluiddot @hypest if this doesn't produce regressions there. (I'm sure it does if the code is shared)

I think maybe I'm suggesting a 3rd option, one where there's a global CSS variable that is inherited, and then a default definition in the bundled lib/theme.json that overrides it.

That works for me 👍

1. Insert Social Links block and in Inspector controls play with the justify content control.
2. Check that previously created Social Links blocks are displayed as before in both editor and front-end.

This seems to only work in block-based themes. I'm generally using non-block based themes for working on the widgets screen, and noticed justification options are no longer available.

It might be on your radar, but thought I'd mention it.

Thanks for the ping!

I think maybe I'm suggesting a 3rd option, one where there's a global CSS variable that is inherited, and then a default definition in the bundled lib/theme.json that overrides it.

That should now work now that #33991 has landed. Adding the block gap value for a specific block in the theme.json will render the CSS variable value for that block in the stylesheet. Or, inline to the rendered block markup if it's set within a block's style attribute, so you can have a separate value for the particular social links block (either via theme.json for the block in general, or once we've opted into the block support, via the individual block's attribute).

This seems to only work in block-based themes. I'm generally using non-block based themes for working on the widgets screen, and noticed justification options are no longer available.

Thanks for testing @talldan! I've made some changes to work on all themes. Also justification options are related to the flex layout, so they now live under the layout panel in Inspector Controls. So any block with flex layout will have this out of the box.

I pushed a small fix to remove the margin on link items, since they are now spaced apart wiht gap. The gap in the following is intentionally crazy to show that it works, but we'd want a sane default, like 1em, as suggested previously:

The gap looks equally crazy in this case, as noted:

However I wasn't able to immediately figure out how to move that prompt next to the appender only in the empty state. The CSS is a bit gnarly there. Since the gap will make sense in most cases, this might not be worth fixing since we are thinking of other options.

I noted the comments about justification changes, but I'm not sure if they have UI consequences. In this branch I'm seeing justification tools moved to the inspector, like so:

The justification tools as part of the toolbar work better, in my opinion. Can we move them back to how they are in trunk?

I noted the comments about justification changes, but I'm not sure if they have UI consequences. In this branch I'm seeing justification tools moved to the inspector, like so:

I don't have strong opinion here but I'm more inclined to the approach of having the controls related to each layout grouped together in one place. This way I think it's clearer for users what controls are coupled with each layout. Also with more use cases/additions to come, the change of layouts through the layout switcher will not cause toolbar items to appear/disappear.

In the near future as we augment layout features, we could explore more things with controls. What do you think?

I don't think that there needs to be any conceptual connection between our more advanced layout tools and justifications, I'd group those with alignments and text alignments.

I agree with Nik for this one here, there's definitely a conceptual link between layout type and these justification controls.

Think of the group block for instance, imagine we enable the control to switch layout type (switch between flow and row) (we already have that it's just hidden), the justification controls will appear when you select "row" but disappear when you select "flow", it's not a great experience for group block to have toolbar buttons appear and disappear when you toggle something on the inspector IMO.

That said, there are blocks that are intrinsically "flex" layouts (or flow layouts) for instance Social Links, Columns, Buttons. For these, it makes sense to have controls for justification in the toolbar for instance (like today).

The problem is in the code architecture, we don't make any difference between these two type of blocks (both use layout). Potentially we could offer two different UIs depending on whether wether the block allow switching layout types. It's not that easy to do and if we can go with the inspector UI while we iterate on these things, I'd be game for it.

Think of the group block for instance, imagine we enable the control to switch layout type (switch between flow and row) (we already have that it's just hidden), the justification controls will appear when you select "row" but disappear when you select "flow", it's not a great experience for group block to have toolbar buttons appear and disappear when you toggle something on the inspector IMO.

Good point, it does make sense for things like group, and I can imagine other containers where justification is important but not a primary interaction. In those cases we might be able to show a dropdown like this, rather than the segmented control, as I imagine something similar could be useful for some alignment properties:

It's not that easy to do and if we can go with the inspector UI while we iterate on these things, I'd be game for it.

I feel somewhat strongly about keeping justification controls in the block toolbar. We could potentially have it sidebar-only for a few plugin releases, but I don't think we'd want to ship 5.9 with justification tools moved away from the block toolbar.

I feel somewhat strongly about keeping justification controls in the block toolbar. We could potentially have it sidebar-only for a few plugin releases, but I don't think we'd want to ship 5.9 with justification tools moved away from the block toolbar.

Makes sense

@ntsekouras I guess we should try to rearchitecture the edit function of layouts a bit. Maybe the responsibility to render the slots (InspectorControl and GroupControl) should be moved to that function instead of leaving it to the wrapper. And the layout itself could receive some options (like allowSwitching: true/false) to switch between toolbar or inspector depending on the block

@ntsekouras I guess we should try to rearchitecture the edit function of layouts a bit.

I'll see to it 😄

In case it's useful, I wouldn't personally mind if justification tools were both in the inspector and block toolbar for something like social links and buttons, but inspector only for something like group.