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: deprecate the server-side rendered nudges #18054

Closed
wants to merge 6 commits into from

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 10, 2020

Changes proposed in this Pull Request:

We've updated the way we display nudges in #16611, so it may make sense to move away from our custom implementation to simplify things a bit with our build.

This PR's counterpart was #13070, where most of this was introduced.

I'm now trying to:

  • deprecate the upgrade_nudge method in the Gutenberg class. It is not used anymore since Premium Blocks: Implement the new design #16611, where we moved to a new upgrade nudge design for the blocks.
  • switch back to simple notice for the Pay with Paypal shortcode. Since the block does not rely on shortcodes anymore, and since the Calypso classic editor has been deprecated, I can't really think of many ways ways a site owner can end up inserting a Pay With PayPal shortcode on their site today. We may be able to simplify some things by moving back to a simple notice here.
  • Deprecate methods that are not in use anymore.
  • Remove server-side rendered upgrade nudge

Jetpack product discussion

  • Internal reference: p1607628394297200-slack-CBG1CP4EN

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

This will require some testing of:

  • the Pay With PayPal block (with a free plan and a paid plan)
  • the Pay With PayPal shortcode,
  • and all the blocks that use the upgrade nudge (Pay with PayPal, Donations, Social Previews, ...). You can display that nudge by adding this filter on a site with a free plan, connected to WordPress.com: add_filter( 'jetpack_block_editor_enable_upgrade_nudge', '__return_true' );

Proposed changelog entry for your changes:

  • N/A

@jeherve jeherve added [Status] In Progress [Type] Janitorial [Pri] Normal [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Build labels Dec 10, 2020
@jeherve jeherve requested a review from ockham December 10, 2020 19:26
@jeherve jeherve self-assigned this Dec 10, 2020
Copy link

@test-case-reminder test-case-reminder bot left a comment

Choose a reason for hiding this comment

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

Here are some suggested test cases for this PR.

Gutenberg extensions

  • Use Core's block editor
  • Use latest stable Gutenberg plugin

Blocks

  • Tiled Gallery
  • Business Hours
  • Calendly
  • Form
  • Contact Info
  • Eventbrite
  • Google calendar
  • Mailchimp
  • Map
  • OpenTable
  • Pinterest
  • Podcast player
  • Star rating
  • Recurring Payments
  • Repeat Visitor
  • Revue
  • Simple Payments
  • Slideshow

Extensions

  • Publicize
  • Likes

If you think that suggestions should be improved please edit the configuration file here. You can also modify/add test-suites to be used in the configuration file.

@jetpackbot
Copy link

jetpackbot commented Dec 10, 2020

Scheduled Jetpack release: January 12, 2021.
Scheduled code freeze: January 4, 2021

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.

Generated by 🚫 dangerJS against 8c35c98

@ockham
Copy link
Contributor

ockham commented Dec 10, 2020

Nice, this would indeed allow us to get rid of a ton of code, and unblock the wordpress monorepo upgrade (#16763), without even needing the preparatory steps (#18024) which were all related to the upgrade nudge's server-side stuff. cc/ @anomiex @brbrr

I don't know exactly how the upgrade nudge has changed since I implemented it, so I'm not totally familiar how (and under what circumstances) it works now -- meaning I can't really approve that aspect of the PR comfortably. I'll just note that the main reason for #16611 was to have the upgrade nudge rendered on the frontend (for logged-in users). It might be worth testing that nothing is broken there per this PR.

LMK if you'd like me to review the technical removal bits 🙂

It is not used anymore since #16611, where we moved to a new upgrade nudge design for the blocks.
Since the block does not rely on shortcodes anymore, and since the Calypso classic editor has been deprecated, there are very few ways a site owner can end up inserting a Pay With PayPal shortcode on their site today. We may be able to simplify some things by moving back to a simple notice here.
We've now moved to new nudges in #16611.
@jeherve
Copy link
Member Author

jeherve commented Dec 17, 2020

Putting this on hold for now, as per pcjTuq-8U-p2

@jeherve jeherve closed this Feb 11, 2021
@jeherve jeherve deleted the deprecate/render-nudge branch February 11, 2021 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Pri] Normal [Status] Blocked / Hold Touches WP.com Files [Type] Janitorial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants