Add justification controls to constrained layout #44065
Conversation
tellthemachines
added
[Block] Group
tellthemachines
requested review from
jameskoster,
noisysocks and
andrewserong
|
Size Change: +1.32 kB (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
This mostly tests well for me. When the post content block is placed inside a group with "Inner blocks use content width" enabled, the justification options for the post content block doesn't seem to do anything. I believe the UI may need some small changes, because it may look like the "Customize the width for all...." text is related to the justification option: |
|
Thanks for testing @carolinan !
Yeah, this will always happen if we nest two blocks with the same content width. Not sure what would be best to do - maybe show a warning that the parent is already constrained? That would require adding some sort of context mechanism so we could know the parent's layout settings.
Yes, I just threw it in there, it should probably get some design feedback 😅 cc. @WordPress/gutenberg-design |
|
Thanks for working on this! Here's a GIF:
Here's a "before" shot for reference:
This is an important feature, thank you for working on it (and thanks for pinging the design group!) The big challenge to address here is that the Layout panel is already both big, and complex, so we need to simplify and reduce if possible. Here's a few minimal things we can do:
Letter case, for reference: A followup could be to move this panel to use the ToolsPanel component. This would allow us to hide the justification control by default. The control is useful when you need it, but probably not always?
|
This is a tricky one, since in this in case we'd like to apply one help text paragraph to two separate controls ("content" and "wide"), and I don't think that's possible with out of the box components. It may be possible to achieve something similar with Not sure if it's a good idea, but we could consider creating some sort of "help text" component ? @mirka do you have any ideas? |
|
Just for the purposes of keeping things simple, if we can just get the right color and spacing, that would go a long way. |
|
This is great, thanks for implementing it quickly. |
This just highlights how awkward the current layout controls are, and how bad things can get when you start nesting containers. A similar issue exists on trunk when child containers set different content-width values to their parents. On balance it is probably an issue to explore separately? In terms of the work in this PR, Joen's suggestions look good. I wonder if it would be possible to reflect the justification in the post editor? Here's my current setup:
The Post Editor is letting the team down by still justifying everything centrally :) |
|
@tellthemachines What do you think about implementing this using ToggleGroupControl? We recently added support for this exact use case (i.e. icon buttons, borderless, |
@ciampo It's probably a good idea! We can expose it as a subcomponent of BaseControl, similar to <fieldset>
<SomeControl />
<AnotherControl />
<BaseControl.VisualHelpText as="legend">
The help text.
</BaseControl.VisualHelpText>
</fieldset> |
|
This is testing nicely for me, too! The only other tiny issue I ran into that I didn't see mentioned above is being able to toggle off the justification (e.g. clicking the center button explicitly sets
I think this would be great to investigate separately — if we think of this as effectively a bug in the post editor styling then perhaps we can argue that we can improve this beyond the feature freeze? One potential way to explore addressing this might to be see if we can extract the |
|
Thanks for the feedback folks! @jasmussen I think I've addressed all your comments except for the spacing between the toggle and the inputs, because it's coming from base control styles:
Changing that value will probably impact other areas of the UI, not sure if we want that? Alternatively I can add a local override. @mirka I was able to use The remaining problem with the help text for the input fields is that it's not semantically connected to them in any way, so essentially useless for a screen reader user. I could wrap them in a
I created #42911 a while ago; it's specifically about knowing the layout type of Post Content, but we can use it to track the whole thing. Comments and ideas welcome! |
There was a problem hiding this comment.
Nice work on the quick follow-ups, @tellthemachines! The changes looks nice in the editor for me, and it's testing well:
With the ToggleGroupControl it might be worth us also updating the Flex layout controls and labels in a separate PR for consistency in the UI? Definitely not a blocker for this PR.
This LGTM from my perspective and looks like it implements the design feedback so far 🎉 (looks much nicer to me without the reset button 👍). I agree with Joen, that it'd be good for us to also look at implementing the ToolsPanel for layout controls one day, particularly as we add more controls to the UI.
I created #42911 a while ago; it's specifically about knowing the layout type of Post Content, but we can use it to track the whole thing. Comments and ideas welcome!
Ah, perfect, I knew you'd written something up already. I'll add some thoughts / ideas to that issue 🙂
|
Are you meant to be able to remove the justification? Unfortunately |
Good question! We're not doing it with the flex layout controls, which these are based on. I can't think of any real world use case for it, as the default state will always be one of the possible settings. It might even be less confusing to make sure that it's always clear which setting is applied by default? |
|
The icons in the justification control appear to be the wrong color (they are a lighter grey than all other icon buttons). Is that something that occurred in this PR, or something relating to the underlying component? Either way it would be good to fix it because at the moment the buttons look disabled 🙈 |
|
Testing. Using the PR Gutenberg build.
Ok.
I can see these.
Changing to left.
Changing to center.
Changing to right.
Frontend.Left. Center. Right. What I see...There are hardly any differences in the backend between using left/center/right justification. I expected it to be related to the boundry it is in. I also expected even bigger differences on the frontend. (For whatever reason when I left the site editor and came back in the Justification controls did not work....Skipping this part.) Can we change the color of the grey controls to become black similar to the icons for Content and Wide?
|
@paaljoachim thanks for testing! If the editor canvas width is not much more than the content width, it might be hard to tell the difference between the three settings. On the front end it will also depend on viewport width. As I mentioned in the PR description, this control doesn't align text content, only the blocks themselves. So depending on the width of the content vs the viewport, plus any root padding, the difference might be quite subtle. |
andrewserong
added
the
[Feature] Layout
Why?
Fixes #43650.
How?
Adds justification controls to the "constrained" layout type. These change the
marginvalues on the block children so block content can be aligned left or right.Note that setting justification to "right" does not right-align any text content, only the child blocks themselves.
Note also that when setting "left" or "right" justification on a Post Content block, the post editor view won't reflect that layout because we don't yet have a way to access the Post Content block settings from the post editor.
Testing Instructions
Screenshots or screencast
Before (content centred):
After (content left-aligned):
