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

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

Closed
jasmussen opened this issue May 24, 2024 · 14 comments · Fixed by #62000
Closed

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

jasmussen opened this issue May 24, 2024 · 14 comments · Fixed by #62000
Assignees
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@jasmussen
Copy link
Contributor

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:

  • In both cases, the tool should be called "Background image". It's currently just "Background" on the site wide option.
  • 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".

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
  • "Cover" would remain as is, and default. Same with "Contain".
  • Tile would still reveal width and repeat controls, but would these would be output as new row, with a "width" label, and instead of the help text saying "Specify a fixed width." it would say "Image has a fixed width."
  • 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.

Figma.

@jasmussen jasmussen added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels May 24, 2024
@jasmussen jasmussen changed the title Backgroud image tool: Improvents, find alternative to "fixed" Background image tool: Improvements, bugfixes, find alternative to "fixed" May 24, 2024
@jasmussen
Copy link
Contributor Author

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

@ramonjd
Copy link
Member

ramonjd commented May 24, 2024

n case some of these are easy!

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

@andrewserong
Copy link
Contributor

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
Copy link
Member

ramonjd commented May 27, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
  • 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.

What do you think?

@jasmussen
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

ramonjd commented May 27, 2024

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 #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
Copy link
Member

ramonjd commented May 28, 2024

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:

#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:

  • Should the background size controls always default to "50%" when toggling on "Tile"? For both the group block and global styles? I agree we should restrict it to images that are uploaded/selected via the media library. We can test for these by checking for an id.
  • When the user wants 'auto', that is, they clear the value of the input field, we'll need to preserve that and not default to "50%"
  • Probably some others I forgot

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
Copy link
Contributor Author

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
Copy link
Contributor

paaljoachim commented Jul 9, 2024

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
Copy link
Member

ramonjd commented Jul 9, 2024

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.

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 Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants