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

Show preview of Premium Content on frontend for admin #47388

Closed

Conversation

stacimc
Copy link
Contributor

@stacimc stacimc commented Nov 12, 2020

Changes proposed in this Pull Request

Currently, when the Premium Content block is not configured correctly -- either a plan upgrade is necessary, or a Stripe account needs to be connected -- it does not render on the frontend. This PR changes that behavior to instead display a preview of the block (the Logged Out View with the buttons disconnected) if the user viewing it is an admin.
This allows them to preview what the block will look like on the frontend of their site when they finish configuring it.

It is intended to be merged in conjunction with Automattic/jetpack#17793, which enables the same behavior for the Recurring Payments block.

The plan upgrade and Stripe connection nudges should also display in the frontend when relevant; the Stripe nudge will display without the Connect button since the url cannot be fully built in the SSR-rendered component. The user must return to the editor to connect to Stripe.

Testing instructions

Make sure you have Automattic/jetpack#17793 checked out as well.

Should be tested using instructions from Automattic/jetpack#17793.

@matticbot
Copy link
Contributor

@stacimc stacimc self-assigned this Nov 12, 2020
@stacimc stacimc requested a review from apeatling November 12, 2020 19:44
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 12, 2020
@matticbot
Copy link
Contributor

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

D52684-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit 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.

@stacimc stacimc marked this pull request as draft November 16, 2020 06:58
@stacimc stacimc force-pushed the update/show-premium-content-on-frontend-for-admins branch from b631f35 to c5ad608 Compare November 20, 2020 02:00
@sarayourfriend sarayourfriend changed the base branch from master to trunk November 20, 2020 16:10
@stacimc stacimc marked this pull request as ready for review November 20, 2020 22:07
Staci Cooper added 6 commits November 20, 2020 14:19
When the Premium Content block is not configured correctly -- either
a plan upgrade is necessary, or a Stripe account needs to be connected
-- it does not render on the frontend. This commit changes this
behavior to instead display a preview of the block (the Logged Out
View with the buttons disconnected) if the user viewing it is an admin.
This allows them to preview what the block will look like on the
frontend of their site when they finish configuring it.
This is used in the Recurring Payments block to determine whether
to render the button preview.
Updates render callback to directly use render_component instead
of a custom render function for the stripe nudge.
Since the Stripe connect url cannot be fully built on the backend.
we pass null for the url to the nudge component so that no button
will be rendered. The user must return to the editor to connect.
@stacimc stacimc force-pushed the update/show-premium-content-on-frontend-for-admins branch from 9e5b97d to b7f7ed2 Compare November 20, 2020 22:20
Copy link
Member

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

I'm seeing both the visitor and subscriber content rendered on the front end when running locally. I haven't yet connected Stripe, and I see the Stripe nudge displayed in the front end. I'll test this on my sandbox next.

Screen Shot 2020-11-23 at 11 25 55 AM

@apeatling
Copy link
Member

On my sandbox using a free plan, I saw the nudge, but I still see both the subscriber and visitor content when looking as an admin:

Screen Shot 2020-11-23 at 11 42 47 AM

@stacimc
Copy link
Contributor Author

stacimc commented Nov 23, 2020

On my sandbox using a free plan, I saw the nudge, but I still see both the subscriber and visitor content when looking as an admin:

Good catch, thanks -- I expect this would be showing up in all contexts. Fix is up to stop rendering the subscriber content when the 'preview' is being displayed.

@apeatling apeatling self-requested a review November 24, 2020 23:41
Copy link
Member

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

This is looking good now. 👍

This is an edge case that can happen if a user signs up for a
premium (or higher) plan, connects a Stripe account, and then
removes the plan. Since the site requires the upgrade in order
to use the Premium Content block, we should still render the
preview in this scenario.
@apeatling
Copy link
Member

Going to close this one out since the changes are now merged into the move PR and have been tested. Thanks for your work on this @stacimc!

@apeatling apeatling closed this Dec 10, 2020
@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 Dec 10, 2020
apeatling added a commit to Automattic/jetpack that referenced this pull request Dec 16, 2020
stacimc pushed a commit to Automattic/jetpack that referenced this pull request Jan 5, 2021
apeatling added a commit to Automattic/jetpack that referenced this pull request Jan 11, 2021
stacimc pushed a commit to Automattic/jetpack that referenced this pull request Jan 13, 2021
apeatling added a commit to Automattic/jetpack that referenced this pull request Jan 20, 2021
* First pass at moving over the premium content block from editing toolkit to Jetpack.

* Fix include paths.

* Add premium content plan requirement, and allow the block to skip the jetpack/ prefix. This will let existing users transition without interruption.

* Correctly register child blocks.

* Add Premium content child blocks to the paid plan check.

* Move the container block to the root, and register the child blocks via the child block functionality built into the Jetpack register block function.

* Add the editor css styles.

* Include the server side render functions for child blocks.

* Add fix for Automattic/wp-calypso#47944

* Allow login and buttons blocks to appear in the inserter and be inserted independently. This matches the ETK version of the block.

* Update the default template content for the subscriber view.

* Fix icon.js lint errors

* First pass at fixing PHP linting errors.

* Replace all full-site-editing textdomain usage.

* Second pass at PHP lint errors.

* Third pass at PHP lint errors.

* Final pass at php lint issues.

* Remove references to Dashicon component.

* Keep deprectated setup consistent across blocks.

* Incorporate changes from @stacimc's PR Automattic/wp-calypso#47388

* Fix phpcs issues.

* Remove upgrade nudges

New functionality will be added in follow-up PRs.

* Use prefix instead of noPrefix for remvoing the Jetpack prefix.

* Better block descriptions.

* Add missing translation.

* Move to shared icon store.

* Prepend action with jetpack_ and document.

* Convert all indentation to tabs.

* Plan checks for premium content buttons and login button.

* Remove tabs.js since it's no longer used.

* Remove log2logstash because this is not available outside of WordPress.com

* Only load the premium content blocks on WordPress.com.

* Use shared supported currencies data.

* Remove plan check from buttons blocks and instead hide from inserter. Move to experimental blocks.

* Make sure the block loads on Atomic sites that do not use IS_WPCOM

* Add get_current_user() call.

* Fix phpcs issues.

* Consolidate currency functions

* Switch to Gutenberg colors

* Add missing translations

* Ensure CSS gets enqueued on the frontend

* Add inline docs for filters

* Fix reference to Subscription_Service

* Add missing arguments when rendering legacy buttons

* Merge in fix for block transformations menu, to ensure the block preview is disarmed

The fix was originally merged into wp-calypso in: Automattic/wp-calypso#48988

* Try fixing linting issues

* Add our new js files to the lint exclude list for now

* Revert "Add our new js files to the lint exclude list for now"

This reverts commit f1498e9.

* Try to address all linting warnings in new files

I've taken the opportunity to suppress some of those warnings for all extensions, as I do not think they can be realistically all filled for all our blocks at the moment.

* Remove file from excludelist now that it is free of linting errors

Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Co-authored-by: Staci Cooper <staci.cooper@automattic.com>
Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Co-authored-by: Andrew Serong <andrewserong@users.noreply.github.com>
matticbot pushed a commit to Automattic/jetpack-production that referenced this pull request Jan 20, 2021
* First pass at moving over the premium content block from editing toolkit to Jetpack.

* Fix include paths.

* Add premium content plan requirement, and allow the block to skip the jetpack/ prefix. This will let existing users transition without interruption.

* Correctly register child blocks.

* Add Premium content child blocks to the paid plan check.

* Move the container block to the root, and register the child blocks via the child block functionality built into the Jetpack register block function.

* Add the editor css styles.

* Include the server side render functions for child blocks.

* Add fix for Automattic/wp-calypso#47944

* Allow login and buttons blocks to appear in the inserter and be inserted independently. This matches the ETK version of the block.

* Update the default template content for the subscriber view.

* Fix icon.js lint errors

* First pass at fixing PHP linting errors.

* Replace all full-site-editing textdomain usage.

* Second pass at PHP lint errors.

* Third pass at PHP lint errors.

* Final pass at php lint issues.

* Remove references to Dashicon component.

* Keep deprectated setup consistent across blocks.

* Incorporate changes from @stacimc's PR Automattic/wp-calypso#47388

* Fix phpcs issues.

* Remove upgrade nudges

New functionality will be added in follow-up PRs.

* Use prefix instead of noPrefix for remvoing the Jetpack prefix.

* Better block descriptions.

* Add missing translation.

* Move to shared icon store.

* Prepend action with jetpack_ and document.

* Convert all indentation to tabs.

* Plan checks for premium content buttons and login button.

* Remove tabs.js since it's no longer used.

* Remove log2logstash because this is not available outside of WordPress.com

* Only load the premium content blocks on WordPress.com.

* Use shared supported currencies data.

* Remove plan check from buttons blocks and instead hide from inserter. Move to experimental blocks.

* Make sure the block loads on Atomic sites that do not use IS_WPCOM

* Add get_current_user() call.

* Fix phpcs issues.

* Consolidate currency functions

* Switch to Gutenberg colors

* Add missing translations

* Ensure CSS gets enqueued on the frontend

* Add inline docs for filters

* Fix reference to Subscription_Service

* Add missing arguments when rendering legacy buttons

* Merge in fix for block transformations menu, to ensure the block preview is disarmed

The fix was originally merged into wp-calypso in: Automattic/wp-calypso#48988

* Try fixing linting issues

* Add our new js files to the lint exclude list for now

* Revert "Add our new js files to the lint exclude list for now"

This reverts commit f1498e9f551998e1a80e2e9cb2c550b854e54094.

* Try to address all linting warnings in new files

I've taken the opportunity to suppress some of those warnings for all extensions, as I do not think they can be realistically all filled for all our blocks at the moment.

* Remove file from excludelist now that it is free of linting errors

Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Co-authored-by: Staci Cooper <staci.cooper@automattic.com>
Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Co-authored-by: Andrew Serong <andrewserong@users.noreply.github.com>\n\nCommitted via a GitHub action: https://github.com/automattic/jetpack/runs/499322406
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.

3 participants