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

Earn: do not register premium content blocks when gutenberg features are not available #44191

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

gwwar
Copy link
Contributor

@gwwar gwwar commented Jul 15, 2020

Fixes #44160 this PR simply opts to not register the premium content block when our desired gutenberg features are not available.

Previously in #42715 we added the ability to decouple the premium content buttons from the container block. This depended on the provides context block feature that landed in Gutenberg plugin ~8.0. Latest WP 5.4.2 has a mix of Gutenberg plugin versions of 6.6-7.5 https://developer.wordpress.org/block-editor/principles/versions-in-wordpress meaning that a fallback approach isn't viable in this case (we'd have to guess the passed planId).

FSE + WP 5.4.2 (no plugin) FSE + Gutenberg 8.3+
Screen Shot 2020-07-16 at 5 36 21 PM Screen Shot 2020-07-16 at 5 38 52 PM

Testing Instructions

  • Setup a WP instance of your choosing
  • Install Jetpack and connect, paying for a personal plan or above
  • Download and activate the FSE plugin from FSE plugin / Build FSE plugin (pull_request) In this PR's GH workflow
  • Try to insert the premium content block with Gutenberg disabled (should not be available in picker)
  • Activate Gutenberg
  • Try to insert the premium content block (verify that it works as expected)
  • Deactivate Gutenberg
  • Reload the edited page, and see that we have a block not registered warning.
FSE build Download Artifact
screen-shot-2020-07-14-at-4 41 45-pm screen-shot-2020-07-14-at-4 41 08-pm

@gwwar gwwar added [Status] In Progress [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jul 15, 2020
@gwwar gwwar requested a review from a team July 15, 2020 23:25
@gwwar gwwar self-assigned this Jul 15, 2020
@matticbot
Copy link
Contributor

{ /* eslint-disable-next-line wpcalypso/jsx-classname-namespace */ }
<Block.div className="wp-block-button">
<RichText
placeholder={ __( 'Add text…', 'full-site-editing' ) }
Copy link

Choose a reason for hiding this comment

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

ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 103 times:
translate( 'Add text…' ) ES Score: 9

@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D46452-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

@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.

@gwwar
Copy link
Contributor Author

gwwar commented Jul 16, 2020

So latest WP 5.4.2 has a mix of Gutenberg plugin versions of 6.6-7.5 https://developer.wordpress.org/block-editor/principles/versions-in-wordpress/ so I'm pretty sure provides context isn't available, meaning that decoupled buttons won't work. I believe this originally landed in WordPress/gutenberg#21467 (landed in 8.0)

Chatting through some options with Kirk, when we detect that we don't have our required features available:

  • Not register the button blocks in this case
  • Show a warning in the edit view
  • No-op the premium content block save

@gwwar gwwar force-pushed the fix/premium-content-g-disabled branch from 54df03c to ffe1b0f Compare July 17, 2020 00:41
@gwwar gwwar changed the title Earn: premium content fallback when gutenberg plugin is disabled Earn: do not register premium content blocks when gutenberg features are not available Jul 17, 2020
@gwwar
Copy link
Contributor Author

gwwar commented Jul 17, 2020

If the default block not registered message is too confusing we can add specific instructions, but we'll need to be careful to not be destructive on save.

Screen Shot 2020-07-16 at 5 30 36 PM

@gwwar gwwar requested review from fullofcaffeine, a team and deBhal and removed request for fullofcaffeine July 17, 2020 01:04
@gwwar
Copy link
Contributor Author

gwwar commented Jul 17, 2020

Unrelated E2E failure for Gutenberg media upload looks like it's timing out on multiple branch runs. Manual testing looks okay so likely that test needs tuning.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

I followed the instructions to test on Atomic ephemeral and this worked as described. I used Gutenberg 8.5.1 for my testing.

Thanks!

Comment on lines +26 to +28
const supportsDecoupledBlocks = !! (
__experimentalAlignmentHookSettingsProvider && __experimentalBlock
);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I was starting to investigate experimental feature usage (#44251), I can tick these off.

@gwwar
Copy link
Contributor Author

gwwar commented Jul 17, 2020

Thanks for the review @sirreal! I'll go ahead and land this to unblock the next FSE release

@gwwar gwwar merged commit 6c73f50 into master Jul 17, 2020
@gwwar gwwar deleted the fix/premium-content-g-disabled branch July 17, 2020 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 Jul 17, 2020
@deBhal deBhal mentioned this pull request Jul 20, 2020
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Premium Content: Block cannot be inserted on Atomic sites when Gutenberg Plugin is deactivated
4 participants