-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add Cover block; Deprecate cover image; #10639
Conversation
@@ -23,6 +23,7 @@ import { | |||
withColors, | |||
getColorClassName, | |||
} from '@wordpress/editor'; | |||
import deprecated from '@wordpress/deprecated'; | |||
|
|||
const validAlignments = [ 'left', 'center', 'right', 'wide', 'full' ]; |
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.
Shouldn't this constant use CAPS_CASE like the constants in edit.js
?
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.
Whoops, I just noticed this is the deprecated block. Well, I guess the point still applies, but it isn't really important. 😛
6a91691
to
22507a4
Compare
I'm guessing you already have this planned, but there should be a transform available to transform existing Cover Image blocks to Cover blocks, similar to what was done for the other deprecated blocks (Subheading and Text Columns), so that users can more easily convert the old blocks to the replacements. Also, what if you could use a background image as a fallback for browsers that didn't support autoplaying videos (e.g. every major mobile browser)? I'm noticing quite a bit of overlap with #10562. A lot of the features of this block could also exist in a Container block. Actually, I think a fully-featured Container block would do everything this block does and more. That is not to say they can't both exist, but I think at least some of the stuff done here could be copied over into #10562. |
The Cover Image block seems like a block that is used a lot. I wonder if it's the right opportunity to introduce the "cross-block" migration to the deprecated block feature. |
@youknowriad That certainly seems like something that should be implemented before WP 5.0, in my opinion. |
I'm closing this PR it was a proof of concept but did too many things making it hard to review. |
Description
This PR adds a Cover block that is based on the cover image block.
The differences between this PR and the cover image is that this new block uses nesting (simplifying a lot the styles) and this PR allows background videos.
The Cover Image block was deprecated, no automatic migration exists but it should not be possible to insert new Cover Images (the same approach followed on the text columns to columns migration).
To do: transforms.
How has this been tested?
Verify it is not possible to insert the cover image block.
Add the cover block verify it contains the same functionality offered in the cover image.
Verify now we can use videos as the background.
Verify the video background functionality works correctly on the editor and on the front end.
Screenshots