Global styles: add background to global styles changes output #60229
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
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. |
69 tasks
|
Size Change: +397 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
…es. Updates tests.
ramonjd
force-pushed
the
update/global-styles-changes-with-background
branch
from
de9d5a7
to
b821de7
Compare
packages/block-editor/src/components/global-styles/get-global-styles-changes.js
Outdated
Show resolved
Hide resolved
andrewserong
approved these changes
Mar 27, 2024
…styles-changes.js Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
|
Thanks for the quick test 🙇🏻
You passed the test! |
Thank you!
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…ess#60229) * Adding "background" top-level styles to the possible change list values. Updates tests. Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
|
I'm curious, why doesn't this follow the same UI/UX as the Group block's background controls? I.e. when there's an image, you see the related controls, "cover" is set as the scale initially, and "repeat" is available (on) when contained or fixed size. The focal point picker should also be fullwidth, like it is on the group block's controls. We should follow-up with these; it feels a bit under-baked experience wise. |
|
Thanks for checking out this feature!
As far as I can see the focal point picker is fullwidth in both cases, but only when an image is selected. When there is no image, the picker is narrower in both instances.
The picker's width could be forced to 100% in all scenarios however. Is that what you mean?
As for the background size controls showing by default in the global styles, I think the original thought was to make it apparent to the user that the image can be tweaked, but I also think they could be unified. The background image control might look lonely, but I'm not sure it's useful to show the controls before an image is added anyway. There's also some further discussion about design going on over in #60264 (comment)
In the first iteration, the site and block background sizes were the same, that is, "cover". The switch to remove the background size value for the site background was deliberate. The rationale was to make it more likely for the user to see their image choice in the background of the target element as soon as it is selected. So, taking the same image, here's how it looks side by side:
The above was based on testing several dozen images of various dimensions, but I understand every scenario is different, so there's no one size fits all. I'm open to standardizing the controls across global and block styles - it'd be good to do it soon. I have what probably is an unpopular stance - that we shouldn't add any opinionated defaults, but happy to implement what folks thinks works best for users. Thank you! |
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
…ess#60229) * Adding "background" top-level styles to the possible change list values. Updates tests. Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
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? How??
Follow up to:
Part of:
Adds "background" top-level styles to the possible change list values. Updates tests.
Why?
So that changes to background styles will be reported in the save entity panel and also the global styles revision items.
Testing Instructions
In the site editor, open the Global styles panel.
Select "Background".
Add a background image
Save the template.
Check that "Background styles" appears in the list of changes to be saved.
Now open up the revisions panel, and select the latest revision.
Check that "Background styles" appears in the list of revision changes.
Run the unit tests:
npm run test:unit packages/block-editor/src/components/global-styles/test/get-global-styles-changes.js