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

Fix: Cover Block: Remove focal point attributes when they are not needed #14746

Conversation

jorgefilipecosta
Copy link
Member

Description

Currently, on the cover block, the focal points attributes are not unset even when they are not needed/used e.g: when the background is a video or when the background is fixed.

This makes the markup more complex with attributes that are irrelevant.

How has this been tested?

Add a cover block with an image as background.
Change the focal points.
Enable the fixed background option.
Check on the code editor that there are no references to the focalPoint attribute.

Add a cover block with an image as background.
Enable the fixed background option.
Change the block to use a video background (using the media edit button + the media library).
Check on the code editor that there are no references to the hasParallax attribute.

Add a cover block with an image as background.
Change the focal points.
Change the block to use a video background (using the media edit button + the media library).
Check on the code editor that there are no references to the focalPoint attribute.

@gziolo gziolo added the Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code label Apr 1, 2019
@gziolo gziolo requested a review from getdave April 1, 2019 14:18
@gziolo gziolo added the [Block] Cover Affects the Cover Block - used to display content laid over a background image label Apr 1, 2019
Copy link
Contributor

@frontdevde frontdevde left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! I tested all three test cases as described and can confirm that the behavior was as expected. 👍

@jorgefilipecosta jorgefilipecosta merged commit 6c9a9b2 into master Apr 2, 2019
@jorgefilipecosta jorgefilipecosta deleted the fix/cover-block-remove-focalpoints-on-video-and-on-fixed-background branch April 2, 2019 07:04
@youknowriad youknowriad added this to the 5.5 (Gutenberg) milestone Apr 12, 2019
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 Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants