Skip to content

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

Merged
jorgefilipecosta merged 1 commit intomasterfrom
fix/cover-block-remove-focalpoints-on-video-and-on-fixed-background
Apr 2, 2019
Merged

Fix: Cover Block: Remove focal point attributes when they are not needed#14746
jorgefilipecosta merged 1 commit intomasterfrom
fix/cover-block-remove-focalpoints-on-video-and-on-fixed-background

Conversation

@jorgefilipecosta
Copy link
Copy Markdown
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
Copy Markdown
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