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

Extend theme.json to provide spacing size presets #39371

Open
14 of 18 tasks
Tracked by #33447
glendaviesnz opened this issue Mar 11, 2022 · 124 comments
Open
14 of 18 tasks
Tracked by #33447

Extend theme.json to provide spacing size presets #39371

glendaviesnz opened this issue Mar 11, 2022 · 124 comments
Assignees
Labels
Developer Experience Ideas about improving block and theme developer experience [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Design Feedback Needs general design feedback. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Mar 11, 2022

What problem does this address?

Background discussion.

On the above issue, there was some consensus that there would be value in extending theme.json to allow spacing size presets to be added, which gives users a set of preset size tokens to select from as well as/or instead of just adding custom spacing values.

This blog post provides a good introduction to some of the reasons for this.

This is intended to be a top-level issue for some of the initial discussions, as well as tracking the ongoing work needed to implement this.

The block UI will need to allow people to select from the presets (e.g. padding value Medium) in addition to the "absolute value" settings (e.g typing "23px"), and also allow for theme authors to disable the option to add absolute values.

Update

Summary of discussion and suggested way forward

Todo

Done

@glendaviesnz glendaviesnz added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Developer Experience Ideas about improving block and theme developer experience labels Mar 11, 2022
@eric-michel

This comment was marked as outdated.

@cbirdsong

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@mrwweb

This comment was marked as outdated.

@cbirdsong

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@cbirdsong

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@cbirdsong

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@glendaviesnz glendaviesnz self-assigned this Mar 24, 2022
@bradhogan

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@jameskoster

This comment was marked as outdated.

@fabiankaegy

This comment was marked as outdated.

@eric-michel

This comment was marked as outdated.

@jameskoster

This comment was marked as outdated.

@glendaviesnz

This comment was marked as outdated.

@glendaviesnz
Copy link
Contributor Author

By way of an update on this work, we have now:

We will hopefully also get presets added to gap in the next week or so ready to get that in WP 6.1 along with the above.

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Sep 8, 2022

@cbirdsong, @mrwweb, @eric-michel, @bradley2083, @richtabor - just pinging a few people that have commented on various spacing presets related issues - apologies if I missed someone, please feel free to comment about the below if you weren't pinged.

With the code freeze for 6.1 features coming up next week we are at a point where we probably won't get any additional spacing preset features added in time to make it into 6.1, so it is probably a good point to check what we currently have, and that it is fully featured and stable enough for going into Core WP 6.1. The current state is:

✅ Spacing presets can be added to theme.json either as a calculated scale or as a manual fixed array of sizes
✅ Spacing preset selections are now the default for padding, margin and gap in both post editor block controls and global styles block controls
✅ Custom spacing sizes can be disabled in theme.json and doing so makes the selection of a preset the only option for padding, margin and gap

Would be good to hear if you think there is anything critical missing still in your view that would prevent this from being feature complete enough to go into Core.

One aspect of the current implementation that would be good to review is the way that presets from the Core theme.json are merged with any presets added to a themes theme.json spacingSizes array when the CSS vars are generated, eg. if a theme adds a single size of {"name": "Smallish", "slug": "45", "size": "1.40rem"} in the spacingSizes array, the following CSS vars will be added to the doc:

--wp--preset--spacing--20: 0.44rem;
--wp--preset--spacing--30: 0.67rem;
--wp--preset--spacing--40: 1rem;
--wp--preset--spacing--50: 1.5rem;
--wp--preset--spacing--60: 2.25rem;
--wp--preset--spacing--70: 3.38rem;
--wp--preset--spacing--80: 5.06rem;
--wp--preset--spacing--45: 1.40rem

... but, in the Editor UI, only the single "Smallish" size appears for selection, the theme presets list overwrites the Core ones rather than extending them at this point.

This code for the spacing presets largely mirrored the font sizes code which behaves the same way. It appears that the intention was that any theme wanting to add different fonts would include all of the fonts they wanted in the theme.json, rather than just adding fonts to supplement the Core ones. I am not sure why it instead extends at the CSS var level, but maybe that is useful for making sure any presets in Core patterns, etc. are covered.

Does it make sense from a theme/plugin author's point of view that spacing preset sizes behaves the same way as font size presets, ie. adding a spacingSizes array in theme.json overwrites all the Core defaults in terms of the size selection UI?

We could also merge the values for the UI, but then it would diverge from the font sizes behaviour so may cause confusion.

Something to note is that the discussion around the slugs, and the decision to go with the format of 10,20,30 ... was based on the idea of themes merging additional values into the Core scale, so as in the above example an item can be slotted between 50 and 40 easily without having to invent a slug. Then, if a theme that block content is copied to doesn't have a 45 defined it is easy to fall back to 40 or 50. This discussion happened while working on the CSS var generation code which does merge these values ... without the understanding that by default from the UI perspective the theme has to define all the values.

I still think there is value in this slug format, eg. a theme might add a much wider static scale than Core, and doing so in say a 10,15,20,25 ... slug format makes it more likely that we can fall back to closer values when block content is copied between that theme and one with less presets.

Just to clarify, we don't currently have the functionality to find fallback values, this will be a 6.2 feature at this point 🤞

@bradhogan
Copy link

bradhogan commented Sep 8, 2022

@glendaviesnz Thanks for all of your work on this! From what you're saying, I would say from a theme author point of view, these features seem extensive and stable enough to push to 6.1. One question I have on the current spacing presets and I assume it's possible, but can we change the label for spacing from "2X-Small", "X-Small", "Small", etc. to something custom e.g. "10", "20", "30"?

And I know I'm repeating myself here, but I do think mobile spacing has to be a top priority in 6.2 if it won't make 6.1. Something so the site author can adjust spacing on mobile like:

Screen Shot 2022-09-08 at 10 23 31 AM

@jasmussen
Copy link
Contributor

There's a separate goal outlined in #43412 of replacing those with 1x, 2x, 3x, etc.

@eric-michel
Copy link

eric-michel commented Sep 8, 2022

@glendaviesnz thank you so much for dedicating so much time to getting this ready for 6.1!

...check what we currently have, and that it is fully featured and stable enough for going into Core WP 6.1.

I'm currently working on a 200+ page site for one of our larger clients, and have fully transitioned our parent theme to adopt theme.json primarily to take advantage of your new spacing features! I'm developing the site on the Gutenberg plugin so that I can create pages with these features ahead of 6.1, so I have a good amount of experience using these controls in a real-life scenario.

I'd say it's absolutely ready for a 6.1 deployment in its current state. I'm finding the controls very intuitive to use, and though our content editors have not gotten their hands on the site yet, I'm sure they will find it easier to use than utility classes.

There's one usability request I have (which is definitely not needed before 6.1, but could theoretically be done pretty quickly if others agree): right now, the padding control links all 4 sides, and when unlinked, makes all 4 sides independent. We very rarely use left/right padding, but use top/bottom padding all the time, and having to unlink the 4 sides then select padding values for both top and bottom separately feels cumbersome compared to my old method of adding a utility class like py-xl to the block, which added padding to the top and bottom.

I would love to see one of two options:

  1. an intermediate step that links top/bottom and left/right after unlinking all 4 sides
  2. don't even have all 4 sides linked at all, and always break out top/bottom and left/right (which would require both to be set to the same value if the user wants all 4 sides to be the same value) like so:
    image

My preference would be number 2 since that's how we build sites (in the rare circumstance that we want all 4 sides to have the same padding, we'll just set both sliders to the same value), but I also realize that there might be tons of people commonly use padding equally on all 4 sides. I'd be curious what others on this board think.

edit: I guess in a perfect world, these could be options set in theme.json (how to link or not link various sides for various settings) to offer maximum flexibility. I would love to be able to set padding so that it does not link all 4 sides at all, but does link top/bottom and left/right.

Does it make sense from a theme/plugin author's point of view that spacing preset sizes behaves the same way as font size presets, ie. adding a spacingSizes array in theme.json overwrites all the Core defaults in terms of the size selection UI?

I tend to prefer consistent behavior whenever possible, so I'd say yes. Merging the values could also result in some pretty weird outcomes, depending on the scale that the theme creator uses for their spacing presets.

@cbirdsong
Copy link

It would be great if the spacer block could be converted to use it in time since that's the only other core block that uses custom sizes of this type, but I'm guessing the timing is too tight?

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Sep 9, 2022

but can we change the label for spacing from "2X-Small", "X-Small", "Small", etc. to something custom e.g. "10", "20", "30"?

@bradley2083 you can set these to whatever you want by adding a settings.spacing.spacingSizes array to theme.json and set the name property, eg. {"name": "10, "slug:" "10", "size":"0.8rem"}.

Instead of a specific mobile setting would it work if there was the option to add fluid values in the interfaces that compile down to a clamp() var?

@cbirdsong I have a draft PR up for adding spacing presets to the Spacer block, but there are a couple of complications, so not sure if we will get agreement and sign off on it before the 6.1 code freeze. One question is whether when customSpaceSizes is disabled should the resizing drag handle also be disabled? It would be nice if when custom sizes are disabled it was still there but could only drag between the presets, but that would take a bit more work to sort.

@eric-michel I have added your idea about multi-side editing as a task on this tracking issue so it doesn't get forgotten.

@bradhogan
Copy link

@glendaviesnz re: mobile clamp() option, I think that should work, just need to test it a bit. And that's not currently available right? It'll be pushed out with the updates you've been working on?

@glendaviesnz
Copy link
Contributor Author

And that's not currently available right? It'll be pushed out with the updates you've been working on?

@bradley2083 You can manually set clamp values for the spacing presets in the theme.json, but you can't currently add or edit them in the editor UI - not sure what the timeframe it for getting adding/editing of these into the UI.

@abhansnuk
Copy link

Hello @glendaviesnz really useful dev note. I am one of the reviewers. Could you please check that an introductory text on the dev note, as below, would be in line with the solution on this issue please. An adaptation of this will also be used for the excerpt. Thank you.
"The introduction of pre-set sizes to be shared across padding, margin, and block gap. This goes into WordPress core in 6.1 for the block editor.

@glendaviesnz
Copy link
Contributor Author

thanks, @abhansnuk, that looks good to me - FYI, I just made a couple of edits to the dev note as I noticed it was missing a couple of last-minute updates that were merged into 6.1.

@Azragh
Copy link

Azragh commented Oct 4, 2022

I just started testing out the new spacing presets and noticed the generated CSS in the frontend looks like this:

.wp-block-group.wp-container-25.wp-block-group.wp-container-25 > * + *, 
.wp-block-group.wp-container-28.wp-block-group.wp-container-28 > * + *  {
    margin-block-start: var:preset|spacing|small; // !?
    margin-block-end: 0;
}

In the editor it works:

.editor-styles-wrapper .wp-block-group.wp-container-70 > * + * {
    margin-block-start: var(--wp--preset--spacing--small);
    margin-block-end: 0;
}

In my theme.json I have:

      "spacingSizes": [
       {
         "name": "0.5em",
         "slug": "tiny",
         "size": "0.5em"
       },
       {
         "name": "1em",
         "slug": "small",
         "size": "1em"
       },
       {
         "name": "BlockGap",
         "slug": "blockgap",
         "size": "var(--wp--style--block-gap)"
       },
       {
         "name": "Modular Padding",
         "slug": "modpadd",
         "size": "var(--modpadd)"
       }

Am I doing something wrong, am I missing something or is this a bug? The difference in the class numbers and the double assignment of them in the frontends CSS is kinda confusing. Is there a reason behind this?

@glendaviesnz
Copy link
Contributor Author

Thanks for reporting this @Azragh - I was not able to replicate the output you are seeing, what versions of Gutenberg and WP are you using?

@glendaviesnz
Copy link
Contributor Author

Actually @Azragh it looks like it might be caused by this bug which is resolved in 14.3

@Azragh
Copy link

Azragh commented Oct 5, 2022

@glendaviesnz I'm using Gutenberg v14.2.0 and WordPress v6.0.2. Even without any plugins except Gutenberg I get this output. But I just tested with twentytwentytwo (made a child theme, defined variables in style.css, copied theme.json and added my spacing presets) and here it works.. I still get the classname assigned 2 times but the variable is defined. So it has to do something with my theme obviously..

Which bug do you mean?

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Oct 5, 2022

Which bug do you mean?

Sorry @Azragh, I forgot to add the link in the previous comment #44435 - 🤞 the fix here will resolve this for you. You could try the 14.3 release candidate to confirm that.

@Azragh
Copy link

Azragh commented Oct 7, 2022

@glendaviesnz Sorry for the late response - works as it should. 😁

@wnolfm
Copy link

wnolfm commented Feb 13, 2023

Do we have a general consensus of how margin/padding are going to work in the future for different screen sizes?

clamp() seems to work OK, but I think it would be better to have greater control of spacingSizes at different breakpoints. Maybe a minimum width or a range?

"spacingSizes": [
  {
	  "name": "X-Small",
	  "size": "20px",
	  "slug": "x-small"
  },
  {
	  "name": "Small",
	  "size": "clamp(30px, 4vw, 40px)",
	  "slug": "small"
  },
  {
	  "name": "Medium",
	  "size": "clamp(40px, 6vw, 60px)",
	  "slug": "medium"
  },
  {
	  "name": "Large",
	  "size": "clamp(50px, 8vw, 80px)",
	  "slug": "large"
  },
  {
	  "name": "X-Large",
	  "size": "clamp(60px, 10vw, 100px)",
	  "slug": "x-large"
  }
]

Given these spacing sizes, On mobile (<400px) I might want 20px of padding on something (X-Small) and Desktop (>=1024) I might want 100px of padding (X-Large).

@annezazu
Copy link
Contributor

Removing from 6.4 -- still lots to design and figure out with only 4 weeks remaining ahead of 6.4 beta 1.

@richtabor
Copy link
Member

I'd like to get more eyes on this, and font size presets. Theme designers (and users of their themes) should be able to configure all presets of a theme from Global Styles.

@cbirdsong
Copy link

There's currently a major issue with the way presets work inside the spacer block: #54084

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Needs Design Feedback Needs general design feedback. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests