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

Use supports align in cover block to fix some align related bugs; #12878

Conversation

jorgefilipecosta
Copy link
Member

Fixes: #12333

Description

The cover block was not using the supports align mechanism, replicating its code on the block itself (when the block was created, I think the mechanism did not exist).
The supports align mechanism was improved with time, to for example not show wide and full alignments on the editor if the theme does not supports them.
As the cover block did not use the align mechanism it has not the logic to check wide and full theme support.
This causes two problems:

  • If pasting a block with aligning full support in the editor when the theme does not supports it the block is shown as full with no option to remove this alignment as described by issue Copied wide-aligned blocks keep the alignment even in themes that don't support it #12333.
  • If the site had a theme that supported wide alignments, and some blocks are created using wide alignments, and then the theme is changed to one that does not supports wide alignments, the wide blocks continue to be shown as wide.

How has this been tested?

I copied the following blocks with all the possible alignments to a new post in a theme that does not support wide alignments:

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","align":"left","id":6} -->
<div class="wp-block-cover has-background-dim alignleft" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">Left</p></div>
<!-- /wp:cover -->

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","align":"center","id":6} -->
<div class="wp-block-cover has-background-dim aligncenter" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">Center</p></div>
<!-- /wp:cover -->

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","align":"right","id":6} -->
<div class="wp-block-cover has-background-dim alignright" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">Right</p></div>
<!-- /wp:cover -->

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","align":"wide","id":6} -->
<div class="wp-block-cover has-background-dim alignwide" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">Wide</p></div>
<!-- /wp:cover -->

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","align":"full","id":6} -->
<div class="wp-block-cover has-background-dim alignfull" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">Full</p></div>
<!-- /wp:cover -->

<!-- wp:cover {"url":"https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg","id":6,"overlayColor":"dark-gray"} -->
<div class="wp-block-cover has-dark-gray-background-color has-background-dim" style="background-image:url(https://mere-dark.jurassic.ninja/wp-content/uploads/2018/12/Porto_Panorama_2004.jpg)"><p class="wp-block-cover-text">None</p></div>
<!-- /wp:cover -->

This blocks were created with wordpress 5.0.1, to make sure we are not changing the markup and invalidating previous blocks.

I verified the wide and full blocks appear as standard blocks.

I published the post with all these blocks.

I changed to a theme that supports wide and full alignments, I opened the post again and verified the wide, and full blocks now appear as wide and full.

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Dec 14, 2018
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification too 👍

@ZebulanStanphill
Copy link
Member

I already made a PR doing this, but it seems to have been overlooked: #10758

@@ -429,6 +412,9 @@ export const settings = {
deprecated: [ {
attributes: {
...blockAttributes,
align: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary. Behind the scenes, the supports flag adds the align attribute to the block, so the block still has the same attribute set regardless of whether align is manually defined or the supports flag is used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ZebulanStanphill, here we are adding the attribute to the deprecated versions. The supports flag adds the attribute to the current version but it does not mutate blockAttributes, so now blockAttributes variable does not contain the align attribute and we need to add it to the deprecated versions, so the deprecations continue with the same attributes as before.
This seems to be the only difference between our PR's so I think we can add this change to your PR and merge it.

@jorgefilipecosta
Copy link
Member Author

Closing as #10758 already existed. Sorry, @ZebulanStanphill I missed your PR in the middle of some pings.

@jorgefilipecosta jorgefilipecosta deleted the fix/use-supports-align-in-cover-fix-align-related-bugs branch December 17, 2018 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants