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

Design Tools: Enhance Block Gap control to support axial configuration #34529

Closed
jasmussen opened this issue Sep 3, 2021 · 9 comments · Fixed by #37736
Closed

Design Tools: Enhance Block Gap control to support axial configuration #34529

jasmussen opened this issue Sep 3, 2021 · 9 comments · Fixed by #37736
Assignees
Labels
[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

Comments

@jasmussen
Copy link
Contributor

What problem does this address?

#32366 adds a theme.json mechanism to provide a a blockGap value to space out items in a container. Spacing using gap has the benefit of letting you not worry about the first or last elements in a container, or complex nth-child margins in containers that wrap.

#33991 added a basic UI for it, with a single CSS variable to begin with.

What is your proposed solution?

Like how the padding control offers axial controls, a similar interaction could be used to split the single gap value into a column-gap and row-gap:

padding

The control would still be subject to design improvements pending from #27331 work, but the primary interaction would be the same. Sucha control would benefit Navigation, Social Icons and Buttons block, as those get gap support (#34525, #34526, #34527).

@jasmussen jasmussen added 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 labels Sep 3, 2021
@andrewserong
Copy link
Contributor

Thanks for writing up the tracking issue for the next steps on this one, Joen! There's a couple of challenges to exploring how we can split out the values. I'll add some dot-points here to hopefully lay out where things are at now for any of us who might pick up this task!

Note: The thoughts below are a little rambling, and other folks might already have better ideas about how to handle this. In the shorter-term, I think it's a good idea to opt individual blocks into the existing single block gap value, and see if we can add in axial configuration in a follow-up as it could get a little complex trying to come up with a neat solution.


Here's the current state of things:

  • In Global Styles: Consider supporting gap #32366 and Block Support: Add gap block support feature #33991 the block gap support was added that uses a single CSS variable --wp--style--block-gap to store the gap value.
  • This is a single CSS variable, that doesn't account for separate vertical or horizontal values.
  • To opt blocks into the UI control for gap support, their CSS will need to be updated to use the CSS variable, because the UI control updates the variable, and not the gap CSS value directly. A good example is the PR to update the Columns block's CSS: Use blockGap between Columns blocks  #34456
  • To support themes opting out of the CSS variable, styles need to include a fallback value, e.g. gap: var(--wp--style--block-gap, 2em);

For background, the CSS variable approach was chosen so that the blockGap UI can be consistent with how the layout support works (with gap introduced in #33812). Using a CSS variable also gives us the flexibility to have different blocks use different approaches for how they calculate the gap between elements. For example, with the single block gap support, we can have the Buttons block use gap CSS for controlling spacing, whereas the Columns block can use margins, but both will get their value from the one CSS variable. This allows us to continue to refactor each block's styling over time, while still using the same gap block support.

So, we have some flexibility thanks to using the CSS variable, however right now we only have a single CSS variable, which doesn't give us access to separate horizontal and vertical values which we'd use for column-gap and row-gap styling via an axial control. This is where we'll need to do some exploration to come up with a solution. On the top of my mind at the moment, I'm wondering if we can do something like:

  • Continue to use the single CSS variable for block gap everywhere that we're currently using it
  • In the axial controls, if a user sets separate values for vertical / horizontal gap, we generate a couple of additional CSS variables, e.g. --wp--style--block-column-gap and --wp-style--block-row-gap.
  • In the CSS for the blocks that intend to use the axial variables, we might need to use fallback values, like:

row-gap: var( --wp--style--block-row-gap, var( --wp--style--block-gap, 2em ) );

In the above rule, we attempt to use the row gap variable it it's available, and if not fall back to using the single block gap variable, and if that's not available, we fall back to a real CSS value (2em).

The main problem with the above approach is that if an individual block tries to use the grouped variable, but something further up the chain has set separate values, then the wrong CSS variable could be picked up instead. So, in individual block controls, we might need to always set the separate row/column variables. However, if we did that, how do we ensure that theme.json support is intuitive if someone specifies a grouped values?

I'm keen to hear if anyone else has ideas about how we might support separate horizontal / vertical gap values in a neat way 🙂

@paaljoachim
Copy link
Contributor

paaljoachim commented Sep 17, 2021

I am thinking Isabel @tellthemachines Jorge @jorgefilipecosta and Andre @oandregal might have some CSS/Global and theme.json thoughts to share here.

@ramonjd
Copy link
Member

ramonjd commented Oct 6, 2021

I'm also noting down some observations with no particular claim to expertise in this area 😄

In the axial controls, if a user sets separate values for vertical / horizontal gap, we generate a couple of additional CSS variables, e.g. --wp--style--block-column-gap and --wp-style--block-row-gap.

I don't know if this is even possible, but would it be too complex to use the existing variable --wp--style--block-gap but use and check for split shorthand values?

It's possible to set blockGap shorthand values in theme.json, e.g.,

	"styles": {
		"spacing": { "blockGap": "24px 24px" }
	}

Which renders --wp--style--block-gap: 24px 24px;.

Could we not check for and use shorthand values in the block support gap hook and corresponding inline styles as well?

For block supports, we could keep the axial nomenclature in block.json, e.g., "supports.spacing.blockGap": [ "horizontal", "vertical" ], but I think we'd also need to implement a variation in BoxControl, similar to AxialInputControls, but that supports style object properties other than { top, right, bottom, left }. In this case perhaps { row-gap, column-gap }.

Screen Shot 2021-10-06 at 3 22 06 pm

There might be places where the application would have to be aware where a shorthand value is used, e.g., 10px 100% vs. 20px, however it might avoid the grouped vs separate values inheritance if we were to use two extra row/col rules.

Individual blocks could always define their fallbacks like this (I think):

.wp-block-navigation__container {
    gap: var( --wp--style--block-gap, 2em 2em );
}

I'm almost certainly missing crucial information as to why this wouldn't be possible 😆 and I also understand this could be regressive in the case of the Columns Block unless we figured out a way to apply the right value (row) to the margin in the case of the shorthand value 🤔

@ramonjd
Copy link
Member

ramonjd commented Oct 7, 2021

Pasting some interesting notes from a conversation with @andrewserong so we can refer to them as we explore this issue.

It’s definitely worth exploring, so it’ll be a valuable rabbithole I’m sure. I really liked your idea of combining the shorthand values into the one CSS variable… the only other gotcha I could think of is the margin-top used by the Layout support which expects the variable to be a single value, unfortunately

I was hoping we could regex the blockGap value and use the row segment in the case of shorthand and the value if it’s single, if you get me

Interesting! if it’s just the one server-rendered case, parsing the value sounds like it could work. I guess we’d then treat the “public” API of the blockGap as “this is a value you should be using with gap”. We’d then still need to refactor the Columns block styling to use gap instead of margins, of course

@jasmussen
Copy link
Contributor Author

There are very likely learnings to be had for the UI as well, for any other theme.json properties which can be fed shorthand syntax. Margin and padding just to name a few.

@ramonjd
Copy link
Member

ramonjd commented Oct 26, 2021

For block supports, we could keep the axial nomenclature in block.json, e.g., "supports.spacing.blockGap": [ "horizontal", "vertical" ],

Here's another potential use case for splitting the blockGap values: #35778

@apeatling apeatling moved this to ⏳ In Progress in Design Tools Project Nov 2, 2021
@apeatling apeatling moved this from ⏳ In Progress to 🔺 Stale in Design Tools Project Nov 2, 2021
@ramonjd
Copy link
Member

ramonjd commented Jan 5, 2022

I've closed Block spacing: add axial gap block support #35454 for now since we're not using CSS vars for block gap styles. ( See #37360 )

We could experiment with alternative ways to split values and pass them to the layouts/flex styles.

Block-level exploration sans CSS var over here: #37736

@apeatling apeatling moved this from 🔺 Stale to ⏳ In Progress in Design Tools Project Jan 13, 2022
@scruffian
Copy link
Contributor

This would be great, particularly for the navigation block. In that case I want space horizontal space between my items, but not vertical:
Screenshot 2022-02-04 at 14 13 02

@apeatling apeatling removed the [Status] In Progress Tracking issues with work in progress label Apr 25, 2022
@apeatling apeatling moved this from ⏳ In Progress to 🔺 Stale in Design Tools Project Apr 25, 2022
@ramonjd
Copy link
Member

ramonjd commented May 11, 2022

Resolved by #37736

Feel free to reopen if there are outstanding issues

@ramonjd ramonjd closed this as completed May 11, 2022
Repository owner moved this from 🔺 Stale to ✅ Done in Design Tools Project May 11, 2022
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 Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
Status: Done
7 participants