Skip to content
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

Closed
wants to merge 3 commits into from
Closed

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Oct 15, 2018

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

oct-16-2018 00-42-10

@jorgefilipecosta jorgefilipecosta added [Type] Enhancement A suggestion for improvement. New Block Suggestion for a new block labels Oct 15, 2018
@jorgefilipecosta jorgefilipecosta added this to the 4.1 - UI freeze milestone Oct 15, 2018
@@ -23,6 +23,7 @@ import {
withColors,
getColorClassName,
} from '@wordpress/editor';
import deprecated from '@wordpress/deprecated';

const validAlignments = [ 'left', 'center', 'right', 'wide', 'full' ];
Copy link
Member

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?

Copy link
Member

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. 😛

@ZebulanStanphill
Copy link
Member

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.

@youknowriad
Copy link
Contributor

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.

@ZebulanStanphill
Copy link
Member

@youknowriad That certainly seems like something that should be implemented before WP 5.0, in my opinion.

@jorgefilipecosta
Copy link
Member Author

I'm closing this PR it was a proof of concept but did too many things making it hard to review.
We will go in smaller steps. The first step is just applying a rename to cover image using an approach similar to what happened in core/text -> core/paragraph. And add video support to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Block Suggestion for a new block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants