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

WP.com block editor: Exclude WP.com features from Jetpack sites #14230

Merged
merged 5 commits into from
Dec 20, 2019

Conversation

mmtr
Copy link
Member

@mmtr mmtr commented Dec 13, 2019

Changes proposed in this Pull Request:

In order to don't expose some @automattic/wpcom-block-editor features to Jetpack sites that are specific of WP.com sites, the common bundle has been split into two new bundles default and wpcom in Automattic/wp-calypso#38248.

This PRs changes the WordPress.com block editor module to replace the common bundle with the new default and to load wpcom only when the site is a WP.com site (Atomic).

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Changes the existing WordPress.com block editor module.

Testing instructions:

  • Spin up a new JN site running this branch.
  • Verify the following features still work in both Calypso and WP Admin block editors:
    • Justify and underline format.
  • Verify the following features still work only in the Calypso block editor:
    • Switch to classic (from the "More tools" menu).
    • Core media modal replaced by Calypso media modal.
    • Posts are previewed with the Calypso preview modal.
    • Revisions are managed with the Calypso revisions modal.
    • Media images in classic blocks are added with the Calypso media modal.
  • Verify the following features are no longer available in both Calypso and WP Admin block editors:
    • Disable welcome guide tour on new sites (users should see the welcome guide the first time they use the block editor).
    • Reorder block categories (if CoBlocks is installed, it should be at the top).
    • Disable experimental blocks (those blocks should be available).
    • Attempt block recovery on editor load (invalid blocks should remain invalid).

Proposed changelog entry for your changes:

  • WP.com block editor: Exclude WP.com features from Jetpack sites

@mmtr mmtr added [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Feature] WordPress.com Block Editor labels Dec 13, 2019
@mmtr mmtr requested review from simison and a team December 13, 2019 14:56
@mmtr mmtr requested a review from a team as a code owner December 13, 2019 14:56
@mmtr mmtr self-assigned this Dec 13, 2019
@jetpackbot
Copy link

jetpackbot commented Dec 13, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 14, 2020.
Scheduled code freeze: January 7, 2020

Generated by 🚫 dangerJS against b035deb

@jeherve jeherve added this to the 8.1 milestone Dec 13, 2019
jeherve
jeherve previously approved these changes Dec 13, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Other than my question here, this seems to work well in my tests.

I have not tested on Atomic though

@mmtr mmtr added the DO NOT MERGE don't merge it! label Dec 16, 2019
@kwight
Copy link
Contributor

kwight commented Dec 16, 2019

All tests great for me, wonderful 👍

kwight
kwight previously approved these changes Dec 16, 2019
@kwight kwight self-requested a review December 16, 2019 22:10
@kwight kwight dismissed their stale review December 16, 2019 22:11

can't see experimental blocks

@kwight
Copy link
Contributor

kwight commented Dec 16, 2019

Oh, hm, I'm not seeing experimental blocks at all (at least, I can't pull up GradientPicker, for example).

@mmtr
Copy link
Member Author

mmtr commented Dec 17, 2019

Oh, hm, I'm not seeing experimental blocks at all (at least, I can't pull up GradientPicker, for example).

That's a component, not a block 🙂 . AFAIK, the only experimental block that is available in Gutenberg right now is the Legacy Widget block. To see it, you need to install the Gutenberg plugin and enable the Widgets experiment:

Screenshot 2019-12-17 at 11 16 34

public function enqueue_block_assets() {
// These styles are manually copied from //widgets.wp.com/wpcom-block-editor/default.view.css in order to
// improve the performance by avoiding an extra network request to download the CSS file on every page.
wp_add_inline_style( 'wp-block-library', '.has-text-align-justify{text-align:justify;}' );
Copy link
Contributor

@gwwar gwwar Dec 17, 2019

Choose a reason for hiding this comment

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

As a follow up we might be able to script out an autogenerated diff later. One tradeoff here for inlining is that we can't update published styles outside of Jetpack release cycles for any other integration changes. Probably okay here to avoid an extra request on published views

Screen Shot 2019-12-17 at 10 00 57 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

One tradeoff here for inlining is that we can't update published styles outside of Jetpack release cycles for any other integration changes.

We could, by just adding another inline style that overrides what's in Jetpack, until the new JP release comes out – certainly not a fun solution, though 🙃

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.

Tests well for me ✨

@mmtr mmtr removed the DO NOT MERGE don't merge it! label Dec 18, 2019
@mmtr
Copy link
Member Author

mmtr commented Dec 18, 2019

Just landed D36841-code so this could be finally merged. @jeherve I dismissed your review with my latest changes using the new bundle names, so this needs your approval again.

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Looks good and tests well for me. 🚢

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Dec 19, 2019
@mmtr mmtr merged commit b3651ff into master Dec 20, 2019
@mmtr mmtr deleted the update/wpcom-block-editor-exclude-features-jetpack branch December 20, 2019 08:31
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Dec 20, 2019
jeherve added a commit that referenced this pull request Dec 20, 2019
zinigor added a commit that referenced this pull request Dec 30, 2019
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
zinigor added a commit that referenced this pull request Dec 30, 2019
* Changelog: 8.1 additions

* Changelog: add #13858

* Changelog: add #13963

* Changelog: add #14174

* Changelog: add #14178

* Changelog: add #14175

* Changelog: add #14192

* Changelog: add #14196

* Changelog: add #14182

* Changelog: add #14218

* Changelog: add #14214

* Changelog: add #13757

* Changelog: add #14190

* Changelog: add #14131

* Changelog: add #14101

* Changelog: add #14203

* Changelog: add #14211

* Changelog: add #14224

* Changelog: add #14230

* Changelog: add #14241

* Changelog: add #14249

* Changelog: add #14264

* Changelog: add #14263

* Changelog: add #14256

* Changelog: add #10189

* Changelog: add #14240

* Changelog: add #14239

Also added some new entries to the testing file.

Co-authored-by: Igor Zinovyev <zinigor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] WordPress.com Block Editor [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants