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

Premium Content: Handle plans in the container block #43684

Merged
merged 3 commits into from
Jun 30, 2020

Conversation

mmtr
Copy link
Member

@mmtr mmtr commented Jun 25, 2020

Changes proposed in this Pull Request

For some reason I couldn't find, the Recurring Payments block acting as a subscribe button for the Premium Content block is auto-selected when the Premium Content block component is mounted.

This was causing some confusion since the Recurring Payments block displays a plan selector in the sidebar making users to assume that selector can change the plan of the Premium Content block. But that plan selector is actually useless since the Premium Content block forces its selected plan to any nested Recurring Payments block.

To help solving that problem this PR ensures the Premium Content block is auto-selected when mounted, so users can use the right plan selector .

It also hides the plan selector of the Recurring Payments block if the block is nested in a Premium Content block (requires Automattic/jetpack#16313).

This PR also fixes a couple of CSS styles so the subscribe/login buttons are separated by the same margin used in the core buttons.

Before After
Jun-25-2020 16-05-22 Jun-29-2020 14-37-20

Testing instructions

  • cd apps/full-site-editing; yarn dev --sync.
  • Apply D45624-code.
  • Sandbox the API and the store. More info in PCYsg-lW4-p2 #sandbox-method.
  • Switch to a site with a paid plan.
  • Go to Earn > Collect recurring payments > Recurring payments plans.
  • Create a few plans.
  • Go into the block editor and insert a Premium Content block.
  • Make sure the Premium Content block is auto-selected.
  • Make sure you can switch plans from the plan selector that is displayed in the block toolbar.
  • Select the subscribe button.
  • Make sure the sidebar does not display any selector for changing the plan.

Fixes #43450.

@mmtr mmtr added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Premium Content Controlling specific content for paying site visitors. labels Jun 25, 2020
@mmtr mmtr requested a review from a team June 25, 2020 14:22
@mmtr mmtr self-assigned this Jun 25, 2020
@matticbot
Copy link
Contributor

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

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

D45491-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

@mmtr mmtr changed the title Premium Content: Ensure container block is selected on mount Premium Content: Handle plans in the container block Jun 29, 2020
//
// Execution delayed with setTimeout to ensure it runs after any block auto-selection performed by inner blocks
// (such as the Recurring Payments block). @see https://github.com/Automattic/wp-calypso/issues/43450
setTimeout( selectContainerBlock, 0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can cause a flash of the block title in the sidebar from "Recurring Payments" to "Premium Content", but it doesn't sound like anything to be dealt with better here 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now. As a follow up we could have the child not dispatch the selection event when we detect we're nested within particular containers.

If we see tricky behavior like this in other spots, it's also possible to create a custom store to better manage shared state between payment blocks.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's interesting with the serialized blocks here is that it's possible for a mismatch to occur between the premium content block and subscription button (say we edit this manually, or a save is botched for some reason).

Just thinking out loud, it'd eventually be more ideal to not be possible to save this sort of nonsense state, though offhand I'm not sure what the best serialized shape for that would be. Maybe compose a block that isn't rendered that represents a plan? Allow planId of inherit? No need to follow up on this, but wanted to leave a note in case we run into similar situations elsewhere.

Notice the two instance of planId "164".

<!-- wp:premium-content/container {"selectedPlanId":164} -->
<div class="wp-block-premium-content-container">
	<!-- wp:premium-content/subscriber-view -->
	<div class="wp-block-premium-content-subscriber-view">
		<!-- wp:paragraph {"placeholder":"Insert the piece of content you want your visitors to see after they subscribe."} -->
			<p></p>
		<!-- /wp:paragraph --></div>
	<!-- /wp:premium-content/subscriber-view -->

	<!-- wp:premium-content/logged-out-view -->
	<div class="wp-block-premium-content-logged-out-view">
		<!-- wp:heading {"level":3} -->
		<h3>Subscribe to get access</h3>
		<!-- /wp:heading -->

		<!-- wp:paragraph -->
		<p>Read more of this content when you subscribe today.</p>
		<!-- /wp:paragraph -->

		<!-- wp:premium-content/buttons -->
		<div class="wp-block-premium-content-buttons wp-block-buttons">
			<!-- wp:jetpack/recurring-payments {"planId":164,"submitButtonText":"Subscribe"} /-->
		
			<!-- wp:premium-content/login-button -->
			<div class="wp-block-premium-content-login-button wp-block-button"><a class="wp-block-button__link">Log in</a></div>
			<!-- /wp:premium-content/login-button --></div>
		<!-- /wp:premium-content/buttons -->

	<!-- /wp:premium-content/logged-out-view --></div>
<!-- /wp:premium-content/container -->

@kwight
Copy link
Contributor

kwight commented Jun 29, 2020

Works great for me, paired with D45624-code ✅

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 @mmtr ! This tests well for me 💖

@mmtr mmtr merged commit c99c030 into master Jun 30, 2020
@mmtr mmtr deleted the update/premium-content-select-container branch June 30, 2020 10:05
@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 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Premium Content Controlling specific content for paying site visitors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Premium Content: Switching from NZD plan throws a JS error
4 participants