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.

Sign up for GitHub

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

Jump to bottom

WP.com Block Editor: Add front-end styles for text justify. #36401

Merged
merged 2 commits into from
Sep 30, 2019
Merged

WP.com Block Editor: Add front-end styles for text justify. #36401

merged 2 commits into from
Sep 30, 2019

Conversation

kwight
Copy link
Contributor

@kwight kwight commented Sep 27, 2019 •
edited
Loading

In Gutenberg 6.3, text align styles for blocks were moved from inline to classes, breaking our Justify text styles for Rich Text blocks. This PR updates our integration to use the new class-based method.

Testing Instructions

See D33289-code.

Fixes #36391

@kwight kwight requested review from a team as code owners September 27, 2019 19:52
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=fix/36391-justify

@kwight kwight self-assigned this Sep 27, 2019
@kwight kwight added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Post/Page Editor The editor for editing posts and pages. labels Sep 27, 2019
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@simison
Copy link
Member

simison commented Sep 30, 2019 •
edited
Loading

Since this file is served to Jetpack (it's in /common/*), will this still work with WP 5.2 and 5.1 after the change?

@gwwar
Copy link
Contributor

gwwar commented Sep 30, 2019

Since this file is served to Jetpack (it's in /common/*), will this still work with WP 5.2 and 5.1 after the change?

@kwight is it right that we should be serving an older JS file with the related hash or do we only serve latest?

If it's the latter, I'm still inclined on accepting this for now with follow up, since this has been half-broken for folks for a while.

gwwar
gwwar approved these changes Sep 30, 2019
View reviewed changes
Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks @kwight! This tests well for me

@kwight
Copy link
Contributor Author

kwight commented Sep 30, 2019

@simison the one line I've removed wasn't doing anything anyways in my my initial debugging of the problem, so I'm confident we're fine with older versions too. I'll merge this, confirm I'm right, and if not, do a followup 👍

@kwight kwight merged commit fbf7da7 into master Sep 30, 2019
@kwight kwight deleted the fix/36391-justify branch September 30, 2019 17:49
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 30, 2019
@kwight
Copy link
Contributor Author

kwight commented Sep 30, 2019

@simison Yep, we're good; this patch applied over the older 6.2 still successfully creates and displays the old inline format (which I'm assuming would be the WP 5.2 and 5.1 formats, since I don't see any other earlier, related format changes).

@simison
Copy link
Member

simison commented Oct 1, 2019

Nice, thanks for confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gwwar gwwar gwwar approved these changes

Assignees

@kwight kwight

Labels
[Feature] Post/Page Editor The editor for editing posts and pages.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Editor: Justify text option not working on Simple Sites
4 participants
@kwight @matticbot @simison @gwwar