Premium Content: Remove double banners #46659

apeatling · 2020-10-21T20:16:00Z

Changes proposed in this Pull Request

Update the premium content block to remove the Stripe connect banner, and the subscriber/non subscriber banner. Doing this means there is no persistent block UI in the canvas, improving the design experience for patterns using this block. These two banners have been merged into the block toolbar for the block.

Before	After
Multiple "banners" when selecting the block.	Options from the old banners merged into the block toolbar.

Testing instructions

Check out this branch locally and run yarn dev --sync in the wp-calypso/apps/editing-toolkit directory
Open up a site on your sandbox and insert the premium content block.
Confirm that you see the "After" UI above.
Confirm that you can connect stripe and the "Connect Stripe" button disappears.
Confirm that you can switch back and forth between the Visitor, and Subscriber views.
Confirm that you can edit blocks in both views.

Fixes https://github.com/Automattic/view-design/issues/84
cc @ianstewart

matticbot · 2020-10-21T20:16:03Z

Test live: https://calypso.live/?branch=update/premium-content-block-editing-ui

apeatling · 2020-10-21T20:22:23Z

Adding in serenity for possible review since they've worked or are actively working on this block also.

matticbot · 2020-10-21T20:23:09Z

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.

a8ci18n · 2020-10-21T20:23:38Z

apps/editing-toolkit/editing-toolkit-plugin/premium-content/blocks/container/edit.js

+							} }
+							className="connect-stripe components-tab-button"
+						>
+							{ __( 'Connect Stripe', 'full-site-editing' ) }


ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 28 times:
translate( 'Stripe Connection' ) ES Score: 6

a8ci18n · 2020-10-21T20:23:39Z

apps/editing-toolkit/editing-toolkit-plugin/premium-content/blocks/container/stripe-nudge.js

 				</span>
 				<span className="premium-content-block-nudge__message">
 					{ __(
-						'This block will be hidden from your visitors until you connect to Stripe.',
+						'Premium content will be hidden from your visitors until you connect to Stripe.',


🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

matticbot · 2020-10-21T20:25:30Z

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

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

mmtr

I'm so excited to see this coming!

I quickly tested this and noticed that the block toolbar is not visible after inserting the block, making impossible to switch views. I had to select the block using the block navigation menu from the header in order to see the block toolbar. Is this expected?

blackjackkent · 2020-10-22T20:37:44Z

Oh, man, this is a great UI improvement overall!

I am seeing the same thing Miguel sees...it definitely makes for a slightly confusing user experience. 🤔 I also am only seeing "Connect" instead of "Connect Stripe" on the block-level controls; not sure if this is the desired text or not. (I think "Connect Stripe" is probably clearer.)

apeatling · 2020-10-22T21:22:35Z

Thanks for the reviews! It should definitely show the the parent block toolbar on first insert, I will check that and see what's going on. As a follow up to this I'll also be working on persisting the parent navigation bar in some way (see https://github.com/Automattic/view-design/issues/94). Right now the user would need to hover over the child block icon, and select the parent block icon that pops up.

apeatling · 2020-10-26T19:05:52Z

@mmtr I made some tweaks. Can you try the block focus again? This is what I get on my sandbox:

apeatling · 2020-10-26T19:07:36Z

@blackjackkent Did you have the latest version? I'm not able to reproduce the "Connect" vs "Connect Stripe" text. It should always be "Connect Stripe".

kwight · 2020-10-27T19:53:45Z

Can you try the block focus again?

Works well for me!

sixhours · 2020-10-27T20:04:40Z

This is working for me in testing, but I have concerns about the subtlety of the Stripe connection component with this change.

It's not obvious that the toolbar is a call to action and a required first step for making this block work. A user can publish a post with a Premium Content block that has not been connected to Stripe and there's no warning or messaging; the block simply does not appear on the front end. I worry this makes it look broken and will generate more support requests for an already confusing block.

apeatling · 2020-10-27T21:01:34Z

@sixhours I totally agree re: front end. I have a follow up issue for this. https://github.com/Automattic/view-design/issues/95 What I will do is add a message in the front end about connecting Stripe if this has not been done.

There's a few issues that I'm coming straight back to this sprint, I'm just trying to break them out into separate PRs.

blackjackkent · 2020-10-27T21:07:26Z

@apeatling No idea why I was just seeing "Connect" before but it looks fine now!

Re @sixhours 's comment -- is there a way to give the button a color highlight? (maybe red text or something like that?) That way it would draw the eye a little more.

mmtr

Awesome, it works great now!

It's not obvious that the toolbar is a call to action and a required first step for making this block work. A user can publish a post with a Premium Content block that has not been connected to Stripe and there's no warning or messaging

I share this concern too. Do we have any other block that includes a required action in the block toolbar? I have the feeling that the block toolbar has always being used for customizing the block, so I'm not sure if users will be aware that they have to click on that action in order to make the block work.

Anyway, this dramatically improves the UX in other aspects so I don't want to block changes here due to that. But if we notice there is a spike if published posts containing Premium Content blocks without a Stripe connection, we should reconsider bringing back the Stripe nudge.

mmtr · 2020-10-29T11:09:38Z

apps/editing-toolkit/editing-toolkit-plugin/premium-content/blocks/container/edit.js

+	const shouldShowConnectButton = () => {
+		if ( ! shouldUpgrade && apiState !== API_STATE_CONNECTED && connectURL ) {
+			return true;
+		}

-		stripeNudge = <StripeNudge { ...props } stripeConnectUrl={ stripeConnectUrl } />;
-	}
+		return false;
+	};


Nitpick and non-blocking: This can be simplified to

const shouldShowConnectButton = () => ! shouldUpgrade && apiState !== API_STATE_CONNECTED && connectURL;

Thanks, I'll fix this in a followup PR.

apeatling · 2020-10-29T14:35:13Z

Thanks for the reviews. I'm going to work on the call to action, and communicating that Stripe connection is required in follow up PRs. 👍

…pacing.

apeatling · 2020-10-29T14:40:50Z

✅ Rebased.

a8ci18n · 2020-10-29T17:16:36Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/5067016

Thank you @apeatling for including a screenshot in the description! This is really helpful for our translators.

Release 2.8.3: * Editor close button should deal with situation where experimental GB feature is unavailable (#46666) * Premium Content: Remove double banners (#46659) * Premium Content: Add missing migrations for margin fix (#46924) * Editing Toolkit: Add a workaround to prevent slider blocks breaking the editor at full width (#46951) * Focused Launch: added WIP Focused Launch modal behind a flag (#46686)

a8ci18n · 2020-11-09T12:33:33Z

Translation for this Pull Request has now been finished.

apeatling added Premium Content Controlling specific content for paying site visitors. Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin labels Oct 21, 2020

apeatling requested a review from a team October 21, 2020 20:16

apeatling self-assigned this Oct 21, 2020

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 21, 2020

apeatling requested a review from a team October 21, 2020 20:21

a8ci18n reviewed Oct 21, 2020

View reviewed changes

mmtr reviewed Oct 22, 2020

View reviewed changes

mmtr approved these changes Oct 29, 2020

View reviewed changes

apeatling added 5 commits October 29, 2020 07:38

Move the tabs and stripe connect button to the parent toolbar.

f9183fb

Move the tabs and stripe connect button to the parent toolbar x2.

3bfd6bb

Fix connect stripe button by adding stripe term, and adjusting icon s…

3fa17b1

…pacing.

Add spacing.

a383d63

Don't update selected focus when adding child blocks from template.

85b700b

apeatling force-pushed the update/premium-content-block-editing-ui branch from ab2a951 to 85b700b Compare October 29, 2020 14:40

apeatling merged commit dcca967 into master Oct 29, 2020

apeatling deleted the update/premium-content-block-editing-ui branch October 29, 2020 16:36

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 29, 2020

Copons mentioned this pull request Oct 30, 2020

Editing Toolkit: Update to 2.8.3 #46963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Premium Content: Remove double banners #46659

Premium Content: Remove double banners #46659

apeatling commented Oct 21, 2020 •

edited

Loading

matticbot commented Oct 21, 2020

apeatling commented Oct 21, 2020

matticbot commented Oct 21, 2020

a8ci18n Oct 21, 2020

a8ci18n Oct 21, 2020

matticbot commented Oct 21, 2020

mmtr left a comment •

edited

Loading

blackjackkent commented Oct 22, 2020

apeatling commented Oct 22, 2020 •

edited

Loading

apeatling commented Oct 26, 2020

apeatling commented Oct 26, 2020

kwight commented Oct 27, 2020

sixhours commented Oct 27, 2020

apeatling commented Oct 27, 2020

blackjackkent commented Oct 27, 2020

mmtr left a comment

mmtr Oct 29, 2020

apeatling Oct 29, 2020

apeatling commented Oct 29, 2020

apeatling commented Oct 29, 2020

a8ci18n commented Oct 29, 2020

a8ci18n commented Nov 9, 2020

Premium Content: Remove double banners #46659

Premium Content: Remove double banners #46659

Conversation

apeatling commented Oct 21, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Oct 21, 2020

apeatling commented Oct 21, 2020

matticbot commented Oct 21, 2020

a8ci18n Oct 21, 2020

Choose a reason for hiding this comment

a8ci18n Oct 21, 2020

Choose a reason for hiding this comment

matticbot commented Oct 21, 2020

mmtr left a comment • edited Loading

Choose a reason for hiding this comment

blackjackkent commented Oct 22, 2020

apeatling commented Oct 22, 2020 • edited Loading

apeatling commented Oct 26, 2020

apeatling commented Oct 26, 2020

kwight commented Oct 27, 2020

sixhours commented Oct 27, 2020

apeatling commented Oct 27, 2020

blackjackkent commented Oct 27, 2020

mmtr left a comment

Choose a reason for hiding this comment

mmtr Oct 29, 2020

Choose a reason for hiding this comment

apeatling Oct 29, 2020

Choose a reason for hiding this comment

apeatling commented Oct 29, 2020

apeatling commented Oct 29, 2020

a8ci18n commented Oct 29, 2020

a8ci18n commented Nov 9, 2020

apeatling commented Oct 21, 2020 •

edited

Loading

mmtr left a comment •

edited

Loading

apeatling commented Oct 22, 2020 •

edited

Loading