Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise and condense unlinked padding / margin controls #49264

Closed
jameskoster opened this issue Mar 22, 2023 · 26 comments · Fixed by #50660
Closed

Optimise and condense unlinked padding / margin controls #49264

jameskoster opened this issue Mar 22, 2023 · 26 comments · Fixed by #50660
Assignees
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi

Comments

@jameskoster
Copy link
Contributor

jameskoster commented Mar 22, 2023

What problem does this address?

When padding / margin controls are unlinked (for per-side settings) they occupy an enormous amount of real estate. It's particularly bad when you unlink both. On my 16" MacBook they fill the entire Inspector 🙈 :

Screenshot 2023-03-22 at 11 26 20

This feels especially bad when you only want to apply padding / margin to a single side which is not uncommon.

What is your proposed solution?

Use dynamic two-tone icons to let you select sides:

  1. All sides (split into separate axial controls) by default
  2. Individual sides
  3. Entirely custom per-side borders

Screenshot 2023-04-13 at 11 40 40


Issue updated Apr 13.

See initial proposal

I think we can optimise for single-side value application by allowing folks to choose where the padding / margin is applied in a popover menu containing the following options:

  • All sides (default)
  • Top
  • Right
  • Bottom
  • Left
  • Custom

The custom option would effectively be the same as the current 'Unlink' option and provide a dedicated control for each sides. In this case I think we can condense by arranging the controls into columns.

Here's a visual which (hopefully) describes things a bit better:

Screenshot 2023-03-22 at 12 16 32

As you can see the footprint of both single-side and unlinked control instances is significantly reduced.

@jameskoster jameskoster added Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Mar 22, 2023
@paaljoachim
Copy link
Contributor

Hey James.

That is a very good idea! I like the compact view in the last mockup!
The only thing that comes up is if the controls become too small to handle.
It would be interesting to try out!

@jameskoster jameskoster moved this to Has issue in 6.3 Design Mar 22, 2023
@jameskoster jameskoster moved this from Needs design to Needs feedback in 6.3 Design Mar 22, 2023
@annezazu annezazu moved this to 🎨 Needs design in Gutenberg Phase 2: Customization Mar 22, 2023
@richtabor
Copy link
Member

I'm curious. What's the thinking on the chunkier tracks?

@richtabor
Copy link
Member

And for the columns arrangement, what're you thinking for when a custom value is used?

@jameskoster

This comment was marked as resolved.

@cbirdsong
Copy link

It would really be good if a new design for the slider could solve the unset/inherit/default problem described in #43082 (comment). Would this design also look identical to "none" before it's been modified?

@cbirdsong
Copy link

cbirdsong commented Mar 23, 2023

Separately and more focused on this specific issue, have you considered arranging them in a + shape to make it very clear what slider is associated with each side?

Padding sliders arranged with top in the center, left and right next to each other, and bottom centered below that
(Pardon my very quick rearrangement of the screenshot above, the spacing and alignment is definitely sloppy)

@jasmussen

This comment was marked as resolved.

@jameskoster

This comment was marked as resolved.

@cbirdsong
Copy link

I would really appreciate being able to set top/bottom and left/right simultaneously since I generally want left/right and top/bottom to be balanced. What about something like this?

A popover menu showing options for "all sides", "top/bottom", "left/right" and "custom"
(Very quick rough mockup)


I'm not sure the additional row this layout requires is worth it. It might work better if we could remove the labels, but we still need to accommodate the 'custom' icon which means you end up with something like this:

What if the actual range control was in a popover? Again, very rough quick mockup:

A popover floating over the spacing controls arranged in a + shape. It contains the range slider and a reset button and custom value toggle

This might also allow the range control to be used when there are more than 7 options since it could grow wider than the sidebar width?

@jasmussen
Copy link
Contributor

How do you go from separate horizontal / vertical padding controls, to totally unlinked?

Consecutive clicks.

How would you specify a custom value for one side, but use presets for the others?

Yeah. Indeed. I do think that could happen inside the popover as Cory suggests. In terms of compression, and in light of advanced being advanced, I do think popovers may be needed to achieve a good default compression.

@jameskoster

This comment was marked as resolved.

@jasmussen

This comment was marked as resolved.

@jameskoster

This comment was marked as resolved.

@jasmussen

This comment was marked as resolved.

@jameskoster

This comment was marked as resolved.

@jasmussen
Copy link
Contributor

jasmussen commented Apr 13, 2023

WFM. Let's update the ticket with that. Happy to do so unless you're up for it?

@jasmussen jasmussen added Needs Dev Ready for, and needs developer efforts and removed Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. labels Apr 13, 2023
@jasmussen
Copy link
Contributor

I went ahead and updated this issue with the new proposed designs. They are a good step forward, the two-tone iconography will be useful for indicating sides regardless, and they don't preclude further enhancements in the future.

@jasmussen jasmussen moved this from Needs feedback to Needs dev in 6.3 Design Apr 13, 2023
@mtias mtias added this to Polish May 1, 2023
@priethor priethor moved this to Needs development in Polish May 2, 2023
@aaronrobertshaw aaronrobertshaw self-assigned this May 5, 2023
@aaronrobertshaw
Copy link
Contributor

👋 I've started working through the proposed UI updates. It's coming along although I have a few questions.

  1. Is it possible to get the proposed icons in the mockup as SVGs or a Figma link?
    • I can export the BoxControlIcon from the components package and reuse that for the individual sides & custom
    • The "Horizontal & Vertical" option icon is new though I believe
    • I'd also like to be able to confirm the new layout matches the desired grid spacing
  2. Don't we still need the "linked" view for this control so all sides can be edited at once?
    • It doesn't appear to be an option in the mockup's dropdown.
  3. Should the dropdown auto-close when selecting an option?
    • It seems lately that's been avoided due to a11y concerns.
  4. Currently, on trunk, the spacing control will display a hint, do we still need that feature with the proposed UI changes?
    • We might be able to do this for the linked view & individual sides
    • Those views that have multiple controls and only a single label don't really have a place to render the hint

The following is a quick demo of the very rough PoC I have so far. It reuses the BoxControlIcon as far as possible.

Screen.Recording.2023-05-09.at.5.48.39.pm.mp4

@jasmussen
Copy link
Contributor

Is it possible to get the proposed icons in the mockup as SVGs or a Figma link?

All 4:

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M3.5 17H5V7H3.5v10zM7 20.5h10V19H7v1.5zM19 7v10h1.5V7H19zM7 5h10V3.5H7V5z" style="fill:#1e1e1e"/></svg>

All 4 connected (axial):

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M8.2 5.3h8V3.8h-8v1.5zm0 14.5h8v-1.5h-8v1.5zm3.5-6.5h1v-1h-1v1zm1-6.5h-1v.5h1v-.5zm-1 4.5h1v-1h-1v1zm0-2h1v-1h-1v1zm0 7.5h1v-.5h-1v.5zm1-2.5h-1v1h1v-1zm-8.5 1.5h1.5v-8H4.2v8zm14.5-8v8h1.5v-8h-1.5zm-5 4.5v-1h-1v1h1zm-6.5 0h.5v-1h-.5v1zm3.5-1v1h1v-1h-1zm6 1h.5v-1h-.5v1zm-8-1v1h1v-1h-1zm6 0v1h1v-1h-1z" style="fill-rule:evenodd;clip-rule:evenodd;fill:#1e1e1e"/></svg>

Left/right

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M3.5 17H5V7H3.5v10zM19 7v10h1.5V7H19z" style="fill:#1e1e1e"/><path d="M7 20.5h10V19H7v1.5zm0-17V5h10V3.5H7z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/></svg>

Top/bottom

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M3.5 17H5V7H3.5v10zM19 7v10h1.5V7H19z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/><path d="M7 20.5h10V19H7v1.5zm0-17V5h10V3.5H7z" style="fill:#1e1e1e"/></svg>

Left:

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M5 17H3.5V7H5v10z" style="fill:#1e1e1e"/><path d="M7 20.5h10V19H7v1.5zM19 7v10h1.5V7H19zM7 5h10V3.5H7V5z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/></svg>

Bottom

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M7 20.5h10V19H7v1.5z" style="fill:#1e1e1e"/><path d="M3.5 17H5V7H3.5v10zM19 7v10h1.5V7H19zM7 5h10V3.5H7V5z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/></svg>

Right

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M20.5 7H19v10h1.5V7z" style="fill:#1e1e1e"/><path d="M3.5 17H5V7H3.5v10zM7 20.5h10V19H7v1.5zm0-17V5h10V3.5H7z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/></svg>

Top

<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" style="enable-background:new 0 0 24 24" viewBox="0 0 24 24"><path d="M3.5 17H5V7H3.5v10zM7 20.5h10V19H7v1.5zM19 7v10h1.5V7H19z" style="opacity:.1;fill:#1e1e1e;enable-background:new"/><path d="M7 5h10V3.5H7V5z" style="fill:#1e1e1e"/></svg>

Don't we still need the "linked" view for this control so all sides can be edited at once?

@jameskoster I wonder if this was an oversight for us? I forgot what we discussed here.

Should the dropdown auto-close when selecting an option?

I think either can work here. Mostly I'm team auto-close, and need to understand the accessibility feedback more. I think a larger conversation around consistency can be had separately.

Looks good!

@jameskoster
Copy link
Contributor Author

Iirc we settled on separate controls for horizontal + vertical being an appropriate default, with no need for an all-sides-linked state. The design can probably accommodate one though, if needed.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 16, 2023
@aaronrobertshaw
Copy link
Contributor

aaronrobertshaw commented May 16, 2023

Thanks for the icons and feedback 👍

I have a draft PR up in #50660. There's still a lot left to polish so take it with a grain of salt.

It also still retains the linked/all sides view for the control. Mainly due to covering the potential for odd side support configurations on custom blocks such as ["top", "right", "bottom" ]. It also keeps the initial footprint to a minimum which seemed in the spirit of this issue but I'm happy to iterate from here.

Other than that, I have two further questions;

  1. Do we still want to retain the hints similar to the old version of the control?
    • In the old control we would render a hint beside the label.
    • With the new design we don't necessarily have a label to render a hint beside.
  2. If we were to keep the linked/all sides view, should we omit the axial view if only one axis is supported?
    • If we have a sides configuration of ["top", "bottom"], the axial view ("Vertical") is almost the same as the all sides view.
    • The main difference is that the axial view has a duplicate vertical icon

No doubt, I'll have further questions as we iterate but that's about all my brain can handle for today 🙂

@jameskoster
Copy link
Contributor Author

For me, the hints aren't super useful and just add noise. So I'd be fine with losing them.

If we were to keep the linked/all sides view, should we omit the axial view if only one axis is supported?

Is the sides configuration already built into the control? It seems a bit strange to limit sides for which a user can add/edit padding 🤔 If it is, then yes it probably doesn't make sense to show the option(s) for disabled configs. Apologies if I am misunderstanding the question. FWIW I still lean towards omitting the linked/all sides view. It's quite rare to need equal vertical & horizontal padding.

@aaronrobertshaw
Copy link
Contributor

For me, the hints aren't super useful and just add noise. So I'd be fine with losing them.

👍 Sounds like a plan. I'll leave them out.

Is the sides configuration already built into the control?

That is correct. The ability to configure any supported sides from top, right, bottom, left, was added as far back as April 2021. It was extended shortly after that to allow axial sides (horizontal & vertical) to be supported in the block.json sides config.

It seems a bit strange to limit sides for which a user can add/edit padding

I agree. There are a couple of blocks that restrict padding to only the axial sides i.e. horizontal & vertical, no support for individual sides. The button block is the primary example there.

It is possible for 3rd party blocks to have configured their sides however they wanted. So any subset of [ top, right, bottom, left, horizontal, vertical ]

The other thing worth noting here is that this same approach to sides configuration applies to all spacing block supports and they all (margin, block gap etc) share this same spacing control.

If it is, then yes it probably doesn't make sense to show the option(s) for disabled configs

The current state of #50660 doesn't show any controls for disabled sides. The issue is when only one axial side is supported via ["top", "bottom"], the "linked" view is effectively the same as the "axial" view which would only be showing a single control as well.

Screen.Recording.2023-05-17.at.7.08.36.pm.mp4

Regarding the inclusion of the "linked" view, I kept it for the moment as noted to cover a sides configuration that doesn't neatly align with the horizontal and vertical axial views.

A contrived example might be a 3rd party block, using an inner blocks template with a custom child block. It might have that custom child block styled to appear in the top-right corner and they wish to allow the configuration of margin for only the left and bottom sides. (I did say contrived 🙂). In such a situation, I don't think it makes sense to default to the "axial" view.

We could default to the "custom" view containing all the separated controls. Would that be undermining the effort to reduce the footprint of this control? Or is it an edge case we can safely ignore for the time being?

Left, Bottom Top, Bottom, Left
Screen.Recording.2023-05-17.at.7.10.53.pm.mp4
Screen.Recording.2023-05-17.at.7.11.56.pm.mp4

I'm happy to iterate on #50660 however you'd like it. I can certainly remove the "linked" view, or we could only include it for those edge case configurations like [top, bottom, left] etc. where it might be more useful.

Thanks for the guidance and direction on this one 🙇

@jameskoster
Copy link
Contributor Author

We could default to the "custom" view containing all the separated controls.

In your example, this makes the most sense to me. I don't think it undermines the "reduce the footprint" objective because only the editable sides would be represented, so in this case two controls. It does also feel a bit of an edge case that we can perhaps circle back to.

Thanks for the diligence here, it's a tricky one!

@ciampo
Copy link
Contributor

ciampo commented May 17, 2023

I have a question that is tangential to this specific PR, but it came to mind while observing the current WIP: for languages with different writing directions (ie. RTL, or even top to bottom), how do we handle these margins? Ie. is "margin left" always applied to the left of the container, regardless of writing direction?

Have we considered using a model similar to logical CSS properties so that our layouts are writing direction agnostic? This will interesting especially in the context of multi-language sites.

(excuse my intermission, definitely not blocking for this issue).

@priethor priethor moved this from 🎨 Needs design to 🏗 In progress in Gutenberg Phase 2: Customization May 17, 2023
@priethor priethor moved this from Needs development to In progress in Polish May 17, 2023
@aaronrobertshaw
Copy link
Contributor

In your example, this makes the most sense to me.

I've removed the linked view and the control now defaults to the axial view when available, single side view if that's all that's supported, and the custom view for edge case configurations or when individual sides have mixed values.

The PR still needs some polishing but it is in a state where you can give it a run and get a feel for the new UX.

Quick demo
Screen.Recording.2023-05-18.at.5.23.20.pm.mp4

how do we handle these margins? Ie. is "margin left" always applied to the left of the container, regardless of writing direction?

At this stage, that's how the style engine maps the values to CSS styles, which follows the approach the block supports took originally.

Have we considered using a model similar to logical CSS properties so that our layouts are writing direction agnostic? This will interesting especially in the context of multi-language sites.

Good question. Long term, I'm expecting we will need to move to something like that.

Updating the block supports and style engine to produce the correct styles with logical CSS properties should be pretty straightforward. Manipulating the control's UI to reflect the physical spacing might be a little more involved. I'd expect the style object to remain fairly consistent so with luck the trick would be applying appropriate labels and icons to the individual spacing sizes controls.

This would be a great follow-up to explore so it can be stable before its need becomes more pressing.

@priethor priethor removed the Needs Dev Ready for, and needs developer efforts label May 24, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Gutenberg Phase 2: Customization May 29, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 31, 2023
@priethor priethor moved this from In progress to Done in Polish May 31, 2023
@jameskoster jameskoster moved this from Needs dev to Done in 6.3 Design Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi
Projects
Status: Done
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants