Background image: display default background size value in global styles #60490
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ensure that the default background size is displayed in the control
ramonjd
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ramonjd
commented
Apr 5, 2024
| @@ -8,6 +8,11 @@ import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; | |||
| */ | |||
| import { unlock } from '../../lock-unlock'; | |||
|
|
|||
| // Initial control values where no block style is set. | |||
| const BACKGROUND_DEFAULT_VALUES = { | |||
| backgroundSize: 'auto', | |||
There was a problem hiding this comment.
This PR is useful regardless of the defaults we choose.
See:
We want to display the default value in the control.
background-size is technically not set at all, but the effect is the same as "Fixed" which is the label for "background-size: auto"
|
Size Change: -24 B (0%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
|
Following up on our conversation from This PR is a small one to display the current default value of top-level background image sizes, mentioned in #60264 (comment) Something like this will be required no matter the default value. There's an exploratory PR in the works to standardize all default values here (background size to "cover" / background position to "center" if background size is "contain"): I was chatting with @andrewserong and, no matter which one we go with and when, it's something that can be changed later. Assume 18.1 (next week, April 10) goes out and the site background size is valueless, which is the current state of trunk. This is what users will have if they upload an image and do nothing else. If it's decided that we merge #60405 or some other PR that changes that default value later, we can ensure that these default values are set at the time the image is uploaded/selected. This means that users who have already used the feature and not set any values won't experience any regressions. Thanks for the continued guidance on this. 🙇🏻 |
andrewserong
reviewed
Apr 5, 2024
There was a problem hiding this comment.
Thanks for the follow-up! If we go with this as the default, do we also need to update the logic that shows the fixed width input so that it's visible? Currently once you upload an image this isn't showing (you have to toggle to another size and back to get it to appear):
Oh dang, then yes we do 😄 Thanks for pointing that out |
…e to determine if the size control should be displayed
ramonjd
commented
Apr 5, 2024
| { sizeValue !== undefined && | ||
| sizeValue !== 'cover' && | ||
| sizeValue !== 'contain' ? ( | ||
| { currentValueForToggle !== undefined && |
There was a problem hiding this comment.
Testing if currentValueForToggle is a good substitute for sizeValue to determine if the size control should be displayed
There was a problem hiding this comment.
That's working nicely for me!
andrewserong
reviewed
Apr 8, 2024
| { sizeValue !== undefined && | ||
| sizeValue !== 'cover' && | ||
| sizeValue !== 'contain' ? ( | ||
| { currentValueForToggle !== undefined && |
There was a problem hiding this comment.
That's working nicely for me!
Thanks again for testing @andrewserong! |
|
This is working for me 👍 Follow-up territory; it would be good to do something with the unit input when you select a fixed background size to indicate the default behavior ( 
|
jameskoster
approved these changes
Apr 8, 2024
andrewserong
approved these changes
Apr 8, 2024
ramonjd
deleted the
update/site-background-image-show-default-size-value-control
branch
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
…les (WordPress#60490) * rename constant to BACKGROUND_DEFAULT_VALUES ensure that the default background size is displayed in the control * Tesating if `currentValueForToggle` is a good substitute for sizeValue to determine if the size control should be displayed Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: jameskoster <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Part of:
Follow up to:
This PR:
BACKGROUND_DEFAULT_VALUESWhy?
The default background size is not displayed on the control.
How?
Passing the default background size.
Testing Instructions
Using a block theme like 2024, head over to the site editor.
Select "Background" in global styles.
Ensure that "Fixed" is selected in the background size toggle control.
Screenshots or screencast