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

Global Styles: Consider supporting gap #32366

Closed
Tracked by #33447
jasmussen opened this issue Jun 1, 2021 · 17 comments
Closed
Tracked by #33447

Global Styles: Consider supporting gap #32366

jasmussen opened this issue Jun 1, 2021 · 17 comments
Assignees
Labels
[Block] Group Affects the Group Block [Block] Navigation Affects the Navigation Block [Block] Social Affects the Social Block - used to display Social Media accounts [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. [Status] In Progress Tracking issues with work in progress

Comments

@jasmussen
Copy link
Contributor

What problem does this address?

Margins are good for spacing out items. However in many cases, gap (see details) provides a much better experience for laying out items.

Instead of spacing items by providing margins on child elements, a gap as applied to the container defines how much space is between the child elements.

This illustration, courtesy of CSS-tricks, showcases very well how it works:

before

While it can accomplish the same, there are a few benefits. Here, the navigation block is shown with a few menu items, spaced using margin, and in various justifications:

image

In this screenshot, the light red items have right-margins applied to space items out, and then uses :last-child to nullify the right-most margin. But this quickly makes for some growing complexity:

  • If the navigation items wrap, then the last-child will no longer be the right-most item.
  • If the navigation has a different justification, margins will need further adjusting.
  • While much of the RTL context is handled by the build system in the block editor, for themes needing to override values, they would have a harder time.

What is your proposed solution?

Using gap (shorthand for column-gap and row-gap) removes all of those headaches, simplifying the CSS and making it more predicactable, yet can still accomplish exactly the same. Only, you no longer have to worry about last-child or RTL considerations, and you have only a single property to consider for spacing out items inside.

Note that the gap needs to be applied to the container instead of the child items, and it need for that container to be a flex or grid item. However as a property supported in global styles for containers such as navigation, buttons or group, it could make for an excellent solution.

It would need some visual explorations for how it need be surfaced in the UI. Since the Buttons block already uses gap to space buttons out, though, it might be worth surfacing the feature in theme.json even before a UI was made available.

@jasmussen jasmussen added Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Block] Group Affects the Group Block [Block] Navigation Affects the Navigation Block [Block] Social Affects the Social Block - used to display Social Media accounts labels Jun 1, 2021
@jasmussen
Copy link
Contributor Author

One way to think about gap is to consider it an easier alternative to margins. Consider how Figma does auto-layout, here are 3 squares in an auto-layout container:

Screenshot 2021-06-08 at 09 56 53

Here are the auto-layout properties:

Screenshot 2021-06-08 at 09 56 27

To translate that:

  • 40px padding inside the gray box
  • 40px gap between items inside the box

It isn't a replacement for margins, but in many cases it saves us headaches around first-child/last-child margins, or those gnarly nth-child margins you have in galleries that wrap.

@andrewserong
Copy link
Contributor

I think this is a great idea @jasmussen, and could be useful across a number of current and future blocks. I thought I'd start having a go at hacking in a Gap block support, so have a draft PR that I'm working from in #32571. It's quite buggy at the moment, but I'll keep chipping away at it. I think this would be a great feature to have as a prerequisite to #32137.

An observation from hacking around inserting it into the proposed dimensions panel — I think the BoxControl would be a good candidate for controlling the gap spacing. We can pull the values for top and left and pass them into rowGap and columnGap respectively to control vertical and horizontal spacing. If that sounds like a good direction, I think a next step might be to extend the BoxControl for better support of grouped top/bottom and left/right values, as these would need to be paired for the two gap attributes we want to change.

When linked this looks good to me When unlinked, this should be two inputs: top/bottom and left/right?
image image

I'll keep hacking away over in #32571.

@jasmussen
Copy link
Contributor Author

When unlinked, this should be two inputs: top/bottom and left/right?

Well, perhaps horizontal gap and vertical gap, but yes — and perhaps it doesn't need to wrap onto a new line like the 4 padding options do, but maybe it can fit between.

@andrewserong
Copy link
Contributor

Excellent, I’ll have a play around tomorrow. Cheers!

@jasmussen
Copy link
Contributor Author

Sweet deal!

@andrewserong
Copy link
Contributor

I've had a go at updating the BoxControl to support grouped vertical / horizontal controls when the BoxControl is unlinked (I'm imagining the Gap support as the main use case at the moment). I think it's ready for review or for feedback at #32610 🙂

@jasmussen
Copy link
Contributor Author

jasmussen commented Jul 1, 2021

#32367 is blocked on this being addressed.

@andrewserong
Copy link
Contributor

Just a quick update on where I'm up to with my exploration for implementing this in #32571 (I've been a bit delayed by working on another project these past couple of weeks, too):

@andrewserong
Copy link
Contributor

Just clarifying that adding in the additional support for gap is being worked on in #32571 (which is currently blocked / waiting on #32392 before we proceed). This will ultimately help us resolve the remaining task from #30753 (thanks for the ping @paaljoachim!)

@andrewserong
Copy link
Contributor

Update: I have a PR to implement gap block support in #33991, which builds on top of the CSS variable introduced in #33812. There is currently an outstanding regression in trunk, but once that's resolved, #33991 should be good to be rebased and then reviewed / tested. The PR introduces the gap block support, and then in follow-ups we could look at switching it on for each block that we want to opt-in to the block-level control.

@andrewserong
Copy link
Contributor

The gap block support feature has been merged in (#33991). There's a minor bug in Safari that I'll look into in a follow-up, but I think this might mean we can close out this issue now @jasmussen, unless you want to track follow-ups here?

@andrewserong andrewserong self-assigned this Sep 3, 2021
@jasmussen
Copy link
Contributor Author

I think this might mean we can close out this issue now @jasmussen, unless you want to track follow-ups here?

Happy to close it. I'd like to see theme.json provided gap land in social icons, buttons, navigation blocks, but that requires refactors to migrate the margins and to ideally use the new theme.json provided flex layout as well. Ideally we also want to surface a UI for it, though hopefully the axial configuration of padding (show a single gap by default, click advanced to split into separate vertical/horizontal options) should likely be the UI for it. But that could be 4 different tickets, perhaps. What do you think?

@andrewserong
Copy link
Contributor

Thanks, Joen. Yes, I think creating separate issues for tracking the next steps would be a good way to handle it.

The gap block support and UI for the single gap value has landed in #33991, so with each block we enable it for, we'll be able to look at what refactoring is required to get it to play nicely, and how we might eventually handle axial configuration. My hope is that we'll be able to land with a single CSS variable to begin with (and the simple UI control), and add the splitting into axial controls as an enhancement in follow-ups.

@jasmussen
Copy link
Contributor Author

Definitely. Given the deeper understanding you have (sounds good to me as well!), want to create those tickets? Otherwise I can take a stab and then feel free to edit.

@andrewserong
Copy link
Contributor

I'm just about to wrap up for my week, but I'm more than happy to create them on Monday. Feel free to get started before then if you'd like, but it's no trouble at all for me to start my week with it (and it'll give me a good chance to review where we're at again with a fresh brain, too! 😁)

@jasmussen
Copy link
Contributor Author

Have a great weekend, and thanks for your work!

@jasmussen
Copy link
Contributor Author

Closing this one in favor of separate per-block tasks:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Block] Navigation Affects the Navigation Block [Block] Social Affects the Social Block - used to display Social Media accounts [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Feedback Needs general design feedback. Needs Design Needs design efforts. [Status] In Progress Tracking issues with work in progress
Projects
Status: Done
Development

No branches or pull requests

2 participants