mirrored from git://develop.git.wordpress.org/
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Backport background size and repeat block support features from Gutenberg #5828
Closed
andrewserong
wants to merge
9
commits into
WordPress:trunk
from
andrewserong:add/background-size-and-repeat-block-support-features
Closed
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6423a57
Backport background size and repeat block support features from Guten…
andrewserong 977cfaf
Fix linting issue
andrewserong d9f0819
Update reference to trac ticket
andrewserong 5f80467
Fix linting issue
andrewserong 6b4dca4
Update src/wp-includes/style-engine/class-wp-style-engine.php
andrewserong db08860
Update comment for background output
andrewserong bf5bed8
Add has-background classname output
andrewserong 185ee76
Empty commit to re-run tests
andrewserong c2982a3
Return early if there's no source and url
andrewserong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we check if a background image url exists as part of the condition that checks for block support above? That way we could return early if there is no image. Or are there reasons to still go through this logic in the absence of a url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do an early return. It was mostly laid out like it currently is to support subsequent updates where we might have a range of different values for the background image source. I.e.
'featured_image' === $background_image_source
'id' === $background_image_source
(for referencing a media library item directly), etcSo we could do an early return just before we define
$background_size
, something like:Would that be worth doing, do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't those sources still require defining a URL though? We always need the URL in order to output the styles. But yeah I guess in any case, in subsequent updates we can add to the condition if that changes.
Edit: to be clear, I do think it's worth returning as early as possible 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okie-doke! I'll update with an early return 👍
For the
featured_image
case we won't have aurl
in the block attributes, as that would get generated at render time. But that could happen anywhere we like in the function, so we could always put that logic before this$background_position
line and do an early return if we don't have a featured image, either. Let's take a look at that part when we go to implement the feature 🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an early return in c2982a3 — let me know if you'd like me to tweak it!