WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.19k forks source link

Background image tool: Improvements, bugfixes, find alternative to "fixed" #61928

Closed jasmussen closed 5 months ago

jasmussen commented 5 months ago

There's a neat new background image tool in global styles, allowing you to place a site wide background:

site wide bg

The tool is similar to that of Group or Cover background images:

Site Group
site group

It diverges in a few places, and both can benefit from a few improvements:

Finally, the "Fixed" option is easy to confuse with the "Fixed background" property of the Cover block:

fixed background

One is a fixed size, another is a fixed background attachment. In both cases, the word "Fixed" is the source of confusion. What's a good option there?

One option is to rephrase the option to "tile":

tile option

Figma.

jasmussen commented 5 months ago

CC: @ramonjd in case some of these are easy!

ramonjd commented 5 months ago

n case some of these are easy!

Thanks a lot! I'll check them out next week and get back to you 🙇🏻

andrewserong commented 5 months ago

Thanks for raising these issues!

Both should default to "Cover".

I disagree with this one, though. My understanding is that in most cases, a site wide background image will be more pleasing as a default "fixed" than cover, as each page of the site will have a different height, so with cover as a default, as users navigate between pages, the background image will appear to change size. For site wide background images, wouldn't folks more likely be using either repeating images or setting a fixed size for the image?

For individual blocks on the other hand, cover makes a nice default as it ensures the block is always covered by the background image (similar to the Cover block).

ramonjd commented 5 months ago

I've got a PR started over at:

I also found a bug where, if you set a value + unit value for a tiled background, and then cleared the input the default would revert to "cover" for the group block. It should be "auto". Fixed in the PR.

In both cases, the tool should be called "Background image". It's currently just "Background" on the site wide option.

👍🏻 Done.

At the moment adding a background image to either have different defaults. Site wide it defaults to "Fixed". This is likely a bug, because it holds helptext that suggests of "Cover". Both should default to "Cover".

There was a bug with the title, good spotting! Also fixed in the PR.

With the default values, as @andrewserong hints at above, there was great deal of deliberation about what they should be, and it went back and forth until we landed on the current state. My initial preference was to never set any defaults and let the user define them.

I personally tested multiple image sizes and found that, with cover, larger images were cut off and often unrecognizable.

One is a fixed size, another is a fixed background attachment. In both cases, the word "Fixed" is the source of confusion. What's a good option there? One option is to rephrase the option to "tile":

100% - the plan is to add background attachment so this is very timely 😄

"Tile" seems as good as any label for now, especially since background repeat is the default for this setting. Done.

A default width of 50% would be assigned. This would also immediately have a visual effect in the canvas, whereas now, you have to enter a number first. Percent may also be a neater default unit, than px.

Just checking whether this should apply both to the group block and the site-wide background image in global styles?

So, for the group block we'd set the value to 50% whenever a user clicks on Tile.

For for site-wide backgrounds however should the default value also be 50%? This would apply whenever a background image is set in theme.json. That would require a PHP change as well.

Let me know.

In tried to change the order of the units so that % appears by default, but there seems to be no straight forward way to do this. useCustomUnits always places px at the head of the array as well. It might be a good follow up.

jasmussen commented 5 months ago

I disagree with this one, though. My understanding is that in most cases, a site wide background image will be more pleasing as a default "fixed" than cover, as each page of the site will have a different height, so with cover as a default, as users navigate between pages, the background image will appear to change size. For site wide background images, wouldn't folks more likely be using either repeating images or setting a fixed size for the image?

I don't disagree with your viewpoint, it's definitely valid. But there was a bug (wrong help text, appears fixed by #62000) which was important to fix. There's also a curiosity here, which is that if you choose the "tiling" née "fixed" option, and do not provide a specific width, then the image's intrinsic dimensions are used. I.e. if I use a 1200x1200 bg image, it's shown as 1200x1200. But it's also 1x resolution (for modern screens it should be shown as 600x600), and it's positioned top left by default. Arguably a better experience would be to have it top centered, so that if you resize the viewport, the center of the motif stays in the center.

bg image

In other words, the lack of background-size and background-position tools makes "tile" not the ideal choice.

By defaulting to cover, it would both be consistent with other instances of the background image tool, but it would side-step both the positioning and the resolution issue.

I do expect we'll want to look into those tools at some point in the future, and at that point, would we be able to change the default at that time?

andrewserong commented 5 months ago

Good points.

By defaulting to cover, it would both be consistent with other instances of the background image tool, but it would side-step both the positioning and the resolution issue.

My main fear is that in the process, a user might set cover, not realising how it works across pages, and only find out too late when the background image is inconsistent. In site wide background images, I think it makes for a misleading default, unfortunately.

I don't mind too much which way we go with, but my preference would be to go with the option that results in the least likelihood of confusing or unexpected styles for the user, even if it means they need to set some things after selecting an image. I do agree that the current state means that it isn't just "set an image and forget" for site-wide background images, though. Right now there's more configuration a user needs to do before it feels just right.

jasmussen commented 5 months ago

Following up, I approved #62000, thanks all for the work and conversation.

Another aspect I wanted to surface, and resurface with an addition, is changing the default unit to percent. But also, showing "Auto", until you've added an explicit width:

Auto

What do you think?

jasmussen commented 5 months ago

I don't mind too much which way we go with, but my preference would be to go with the option that results in the least likelihood of confusing or unexpected styles for the user, even if it means they need to set some things after selecting an image. I do agree that the current state means that it isn't just "set an image and forget" for site-wide background images, though. Right now there's more configuration a user needs to do before it feels just right.

Definitely agreed. We're choosing between tradeoffs.

  1. If we go with tile by default, the image will be huge on mobile, low resolution, and anchored to the top left corner. It will be less huge on desktop, same anchor, and depending on your monitor, either acceptable resolution or also low resolution.
  2. If we go with cover by default, a user might design aspects of the content to visually map to details of the background image, and then see that break as soon as they resize the viewport, or even close the inspector.

I still lean towards 1 (though not strongly, and a quick CC @WordPress/gutenberg-design for a gut-check) because it solves more of those issues. Arguably, the problem you're describing for Cover is also, arguably, the case for option the Fixed/Tile option, unless you're making a left-aligned desktop-only design.

andrewserong commented 5 months ago

Nicely captured description of the problem here 👍

I wonder if your idea of defaulting to Tile and with 50% size would get us most of the way there? In that case, I'd imagine if a user uploads or adds an image, at that stage we'd automatically set a default value for the size. If someone is setting the image in theme.json, we wouldn't provide any defaults as the theme developer can set them to whatever they like?

ramonjd commented 5 months ago

Thanks for the continued discussion.

In that case, I'd imagine if a user uploads or adds an image, at that stage we'd automatically set a default value for the size. If someone is setting the image in theme.json, we wouldn't provide any defaults as the theme developer can set them to whatever they like?

This sounds reasonable — only default the background size to 50% for images that the user sets in the editor — I'd be wary of introducing "silent defaults" in the case of theme.json, it might come back to bite us 😄

I'll merge https://github.com/WordPress/gutenberg/pull/62000 and go for another iteration 👍🏻

Percent is arguably a more intuitive way to tile and it has some responsive properties. And so long as you can change to px, it may be a better default. "Auto" is meant to provide some feedback that there's actually a width here, it's just intrinsic. We do the same for the image aspect ratio tool, and it would be a nice enhancement.

This very much rests on the UnitControl component, which has some limitations I believe, however I think such features would make for decent enhancements. I'll play around with it.

ramonjd commented 5 months ago

Update:

"Auto" is meant to provide some feedback that there's actually a width here, it's just intrinsic. We do the same for the image aspect ratio tool, and it would be a nice enhancement.

I've got a PR which does only that going over here:

https://github.com/WordPress/gutenberg/pull/62046

It's not a large change yet 😄 I'm still experimenting with setting the default value of "Tile" to "50%". There were a few UX questions I had to work out however so I rolled those changes back for now - the commits are still in the history however, so I'll come back to them.

The questions were:

Percent is arguably a more intuitive way to tile and it has some responsive properties. And so long as you can change to px, it may be a better default.

I've been looking into extending the the UnitControl component so that it honors the order of units it is given. Neither it nor useCustomUnits does this - in my opinion, it should.

I'm just not sure what the right approach is, so the plan is to get a hacky PR up and ping the components team.

jasmussen commented 5 months ago

Just coming back to this, this morning, with extra coffee, and first off thanks for all the work. Re-reading this one:

I still lean towards 1 (though not strongly, and a quick [...]) because it solves more of those issues. Arguably, the problem you're describing for Cover is also, arguably, the case for option the Fixed/Tile option, unless you're making a left-aligned desktop-only design.

What I meant to say was that I was leaning towards cover by default, not tile, because cover solves more issues by default. Apologies 😅

paaljoachim commented 3 months ago

So basically the new Background image tool is in the Global Styles impacting all the pages. Perhaps similar to this older plugin: https://wordpress.org/plugins/fully-background-manager/

What if I want to do the same thing for only one page? Should there be a similar setting inside a panel for each page?

ramonjd commented 3 months ago

What if I want to do the same thing for only one page?

Interesting idea. I think it'd be useful.

It's not possible through the editor right now, yes - with CSS, sure.

Or, you could create a custom page template with a group block container + background image, and then apply that template to a single page.

Should there be a similar setting inside a panel for each page?

Attached to the featured image control, or near it perhaps.

Screenshot 2024-07-10 at 8 11 40 AM

I do wonder if this is functionality for a plugin as it extend a page's base features.

Regardless, I've added this to the follow up tasks tracking issue so we don't lose it.