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: Group "Background image" in "Layout" section #61494

Closed
jameskoster opened this issue May 8, 2024 · 12 comments
Closed

Global styles: Group "Background image" in "Layout" section #61494

jameskoster opened this issue May 8, 2024 · 12 comments
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.

Comments

@jameskoster
Copy link
Contributor

jameskoster commented May 8, 2024

The global styles inspector is growing:

new items shadows and background image

Those categories are all related to presets or design tools, except "Background image", which is a tool. So conceptually it doesn't fit at the top level.

A previous version of this issue put it inside a renamed "Layout" section, but per discussion it's not clear that this is actually where we want to be either, given there's a larger revisit we need to do as part of #47369.

To that end, it would be good to — for 6.6 — simply move the Background image tool inside inside Layout. It can even have a subheading, "Background". This would keep the existing, if slightly unclear, IA in place for 6.6, and not preclude a better structure in 6.7.

Previous version of this issue site

As a pathway to a control for a single property, the newly added "Background image" section in global styles is an outlier.

To improve the iA for 6.6 we could combine it with Layout controls in a new "Site" section. This would also allow the panel title to be consistent across global styles and the block inspector.

@jameskoster jameskoster added [Type] Enhancement A suggestion for improvement. Needs Dev Ready for, and needs developer efforts Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels May 8, 2024
@ramonjd
Copy link
Member

ramonjd commented May 16, 2024

I'll take a look at this soon, with a view to renaming "Layout" to "Site" - hopefully it'll be a single PR. 👍🏻

@jasmussen
Copy link
Contributor

I'll take a look at this soon, with a view to renaming "Layout" to "Site" - hopefully it'll be a single PR. 👍🏻

@jameskoster are we sure this is the right path forward? Does it fit with the larger IA pieces from #47369? Another path would be to keep the Layout option, holding content and wide widths, as well as block spacing. Then "Site" could be a place for both shadows and background, and any other presets we create until the larger pieces in 47369 come to pass.

Alternately, perhaps the Layout icon works better than the globe? 🤔

This is mostly a question of the dev time involved. If it's easy to try "Site" in a plugin cycle, let's do that, with the option of reverting to Plan B, which is to just put background image inside the exiting "Layout", and instead make progress on 47369 for 6.7.

@jameskoster
Copy link
Contributor Author

It's really hard to say because the revised iA isn't fully fleshed out. We need to properly define all the concepts and group accordingly.

The way I look at this particular issue is; there should be a section where you can essentially style the body tag: define the default block spacing, padding, background/image, and so on. Should that be an 'element'? I'm not sure.

Saxon has mentioned Layout becoming a group of presets which makes sense to me in the long run. So I suppose another option could be; keep 'Layout', containing only the 'Dimensions' (content & wide) settings. Then add another section ('Site', 'Body'?) containing background image, spacing, and padding.

It's hard to be confident about any direction here :/

@ramonjd
Copy link
Member

ramonjd commented May 17, 2024

there should be a section where you can essentially style the body tag: define the default block spacing, padding, background/image, and so on

So would that mean styles directly and visibly affecting the body rather than targeting the body for inheritance (as typography and color do).

I agree it's tough to make a call when things are in flux, and this close to 6.6.

This is mostly a question of the dev time involved. If it's easy to try "Site" in a plugin cycle, let's do that, with the option of reverting to Plan B, which is to just put background image inside the exiting "Layout", and instead make progress on 47369 for 6.7.

I'm not confident a larger refactor would make 6.6. Also, Layout doesn't communicate background styles to me, and I'm wondering if it'll cause a bit of head scratching.

What's the minimum we could achieve in 6.6 without causing too much friction? Just thinking out loud here:

  1. Rename "Layout" to"Site" and insert background image controls there. The risk is that the label "Site" might change in future iterations and also the controls within.
  2. We leave background image as it currently stands. It's accessible and easy to comprehend. The risk is that it will be moved around in future iterations, but it still represent fewer changes than point 1, and leaves breathing room for the IA research to take place.

@jasmussen
Copy link
Contributor

IMO the minimum is to just move the background image inside "Layout". It's not semantically the most correct, but Layout has already become a bit of a kitchen sink, and moving it there may be better than either reverting the feature, or making a new taxonomy or even rename, given we already know we want to revisit this again later. I'm increasingly leaning in this direction as the best path forward, unless we can start to make some progress on the larger IA. I know that's not your preference, @jameskoster, but if we address the larger IA for 6.7 maybe it's best? Perhaps CC @richtabor as well for a take.

@jameskoster
Copy link
Contributor Author

Fwiw I don't really have a preference; given the constraints none of the options feel great to me.

@ramonjd
Copy link
Member

ramonjd commented May 21, 2024

It'd be good to have consensus before the end of the week - after that, there's a short window we can exploit before Gutenberg packages are prepped for WordPress 6.6 Beta.

I have a weak opinion, weakly held I guess in that Layout seems a bit too kitchen sink for background images, and the feature would be undiscoverable until the next version of WordPress. However I see the advantage of not creating a new top-level navigation item that will presumably disappear in the next version.

There is also the option of removing it from the nav altogether and just ship theme.json functionality.

Whatever folks think is best, I'll do my best to get it ready for review in time 🤞🏻

Thank you!

@jasmussen
Copy link
Contributor

Outside of any better ideas that emerge between now and then, I'd put it inside Layout. Can even have a subheading, "Background". The main reason for this is: color, typography, layout, those are taxonomies: groups that hold tools. "Background image" is just the tool, as a taxonomy it isn't able to hold anything else.

I don't think this plan B would be the end of the world: it's not the best place for it, but it keeps Global Styles looking as it does today, and very specifically does not preclude revisiting this IA for 6.7.

@jasmussen jasmussen changed the title Global styles: Group "Layout" and "Background image" together in new "Site" section Global styles: Group "Background image" in "Layout" section May 22, 2024
@ramonjd
Copy link
Member

ramonjd commented May 22, 2024

I don't think this plan B would be the end of the world: it's not the best place for it, but it keeps Global Styles looking as it does today, and very specifically does not preclude revisiting this IA for 6.7.

All good. I'll move it inside layout for now. Thanks!

"Background image" is just the tool, as a taxonomy it isn't able to hold anything else.

The top-level label was originally "Background". The rough idea was to move color backgrounds there, especially gradients, which use the CSS background property and currently conflict with all background-* declarations.

It'd also be a place for video backgrounds whenever that happens 😄

@jasmussen
Copy link
Contributor

The top-level label was originally "Background". The rough idea was to move color backgrounds there, especially gradients, which use the CSS background property and currently conflict with all background-* declarations.

Those are still relative to individual blocks or elements, no?

That's the main piece, and part of what the larger IA discussion dives into: each element, perhaps including body, can have its own section, similar to how each block does. The other category is "presets". Gradients, colors, style variatons, type sets, etc. To Jay's point, I'm sure we'll find a good location for where to move this in one of those categories in the future.

@ramonjd
Copy link
Member

ramonjd commented May 24, 2024

Those are still relative to individual blocks or elements, no?

Color and gradient can be applied to the backgrounds of individual blocks and elements - the plan is to also widen background images to other, container-like blocks as well.

That's the main piece, and part of what the larger IA discussion dives into: each element, perhaps including body, can have its own section, similar to how each block does. The other category is "presets". Gradients, colors, style variatons, type sets, etc. To Jay's point, I'm sure we'll find a good location for where to move this in one of those categories in the future.

👍🏻

@jasmussen
Copy link
Contributor

Thanks for the work. Let's keep going from here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
Development

No branches or pull requests

3 participants