-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Comments
CC: @ramonjd in case some of these are easy! |
Thanks a lot! I'll check them out next week and get back to you 🙇🏻 |
Thanks for raising these issues!
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 For individual blocks on the other hand, |
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.
👍🏻 Done.
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.
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.
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 For for site-wide backgrounds however should the default value also be Let me know. In tried to change the order of the units so that |
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. 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? |
Good points.
My main fear is that in the process, a user might set 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. |
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: ![]()
What do you think? |
Definitely agreed. We're choosing between tradeoffs.
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. |
Nicely captured description of the problem here 👍 I wonder if your idea of defaulting to Tile and with |
Thanks for the continued discussion.
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 👍🏻
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. |
Update:
I've got a PR which does only that going over here: 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:
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. |
Just coming back to this, this morning, with extra coffee, and first off thanks for all the work. Re-reading this one:
What I meant to say was that I was leaning towards cover by default, not tile, because cover solves more issues by default. Apologies 😅 |
So basically the new Background image tool is in the Global Styles impacting all the pages. 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.
Attached to the featured image control, or near it perhaps. ![]() 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. |
There's a neat new background image tool in global styles, allowing you to place a site wide background:
The tool is similar to that of Group or Cover background images:
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:
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":
Figma.
The text was updated successfully, but these errors were encountered: