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

Blocks: Remove is- prefix from embed alignment class #5293

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 27, 2018

Related: #4118 (specifically #4118 (comment))

This pull request seeks to partially revert changes introduced in #4118, specifically with regards to the alignment class. Prior to #4118, the alignment class applied to embed blocks had intentionally targeted compatibility with common theme styling for .alignleft, .alignright, etc . These changes restore that intended class name.

Testing instructions:

Verify there are no invalid blocks in demo content (as there is on master).

Verify that aligning a block is styled correctly when viewed on the front-end. Depending on your theme, this may not have much of an impact, if any at all.

cc @Luehrsen

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Feb 27, 2018
@aduth aduth added this to the 2.3 milestone Feb 27, 2018
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

This looks good to me, before the align in my tests was not viewable in the frontend and now the align works (Gutenberg starter theme).
We are changing save function, so blocks created before this change will become invalid.
If the behaviour that we are partially reverting was merged before the last release, I think we should add the old save to the deprecated version so we don't see invalid block warning. If the change is recent I think it is ok to not add to the deprecated versions as the probability of someone being affected and not understanding the error is lower.

@aduth
Copy link
Member Author

aduth commented Feb 28, 2018

If the behaviour that we are partially reverting was merged before the last release, I think we should add the old save to the deprecated version so we don't see invalid block warning. If the change is recent I think it is ok to not add to the deprecated versions as the probability of someone being affected and not understanding the error is lower.

Correct, the class name change has never landed in a public release. The changes here reflect what already exists in 2.2.

@aduth aduth merged commit 52fa016 into master Feb 28, 2018
@aduth aduth deleted the revert/4118-embed-align branch February 28, 2018 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants