Blocks: add paid label next to block title when it requires a plan by jeherve · Pull Request #14739 · Automattic/jetpack

jeherve · 2020-02-19T21:43:25Z

Changes proposed in this Pull Request:

Related: #11875, #13490

This adds a (paid) label next to block titles whenever a block requires a plan.

A few notes:

I have chosen to have (beta) take priority whenever a block is both paid and in Beta. I am not sure that's the best approach.
(beta) is not translatable. I don't know if it should be.

@jeryj Do you think you could take a look and take over?

Thank you!

Testing instructions:

Start from a site using the free version of Jetpack, and connected to WordPress.com.
Add the following filter to enable paid blocks on your site: add_filter( 'jetpack_block_editor_enable_upgrade_nudge', '__return_true' );
Open the block picker.

Proposed changelog entry for your changes:

Blocks: better differentiate paid blocks from free ones.

matticbot · 2020-02-19T21:43:31Z

Caution: This PR has changes that must be merged to WordPress.com
Hello jeherve! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D39107-code before merging this PR. Thank you!

jetpackbot · 2020-02-19T21:45:29Z

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.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 25129a6

jeryj · 2020-02-19T22:57:47Z

Thank you for getting this started and providing direction, @jeherve! I have a questions that is likely just related to my newness to Jetpack. The only paid plan block I'm seeing is Recurring Payments, and it's not showing the (paid) label.

This is for both my local docker ngrok'd site and the PR-generated site. They're both connected to my WP account via a free Jetpack plan. I've looked in both wp-admin and WordPress.com's editor for more beta and upgrade-required blocks. Am I looking in the wrong area, or is there some other set-up I need to do to see more beta and paid blocks like in your screenshot?

jeherve · 2020-02-20T07:27:37Z

I've added a note to the PR description: to see paid blocks when you don't have a plan in Jetpack, you need to use this filter:
add_filter( 'jetpack_block_editor_enable_upgrade_nudge', '__return_true' );

Beta blocks, on the other hand, can be enabled by adding the following to your site's wp-config.php file:
define( 'JETPACK_BETA_BLOCKS', true );

WunderBart · 2020-02-20T13:01:09Z

Thanks for opening this PR and for the detailed description, @jeherve! 🙌

I have chosen to have (beta) take priority whenever a block is both paid and in Beta. I am not sure that's the best approach.

How about joining those tags/badges/labels? We'd get something like:

(not sure about the order here ☝️)

(beta) is not translatable. I don't know if it should be.

I'm not sure here either, but when I drop it in Google Translate, e.g. the Russian version is "бета". I guess the question is if we want to make it translatable?

For the implementation of the above, how about creating blockTags object that would contain (translated?) strings, e.g.:

const blockTags = {
  paid: __( 'paid' ),
  beta: __( 'beta' ),
};

...and append (& join) them accordingly to the block's name when needed. What do you think?
cc @davemart-in

davemart-in · 2020-02-20T14:18:43Z

Thanks for kicking this off @jeherve! Combining beta and paid when appropriate sounds fine.

matticbot · 2020-02-20T16:47:28Z

jeherve, Your synced wpcom patch D39107-code has been updated.

extensions/shared/register-jetpack-block.js

…ation

matticbot · 2020-03-03T19:58:48Z

jeherve, Your synced wpcom patch D39107-code has been updated.

…adable.

…BlockTags.paid The availableBlockTags object has the property paid, and not plan. The check to add the (paid) label was looking for the availableBlockTags.plan property, which was undefined. I updated this to look for availableBlockTags.paid.

matticbot · 2020-03-03T20:25:04Z

jeherve, Your synced wpcom patch D39107-code has been updated.

jeryj · 2020-03-03T20:29:26Z

I made the requested changes to the code. I think this is ready for another round of review 👍

getdave

Nice job. Looks good from a code perspective. I haven't been able to test manually yet. Still working on env setup.

getdave · 2020-03-04T09:26:45Z

extensions/shared/register-jetpack-block.js

 	}

+	let blockTitle = settings.title;
+	const blockTags = [];


Minor: it might seem obvious when you've been working on it for a while, but I wonder whether it could be helpful to add a comment here detailing what we're doing here. It was immediately obvious to me even though I had some context. For others it could be even less clear.

For example:

Some Blocks should be "tagged" to highlight various attributes such as being part of a paid plan or being in a beta programme. We add these to the Block's visible title in order that they show up in the picker.

Alternatively, perhaps it would be better to handle adding "tags" in a dedicated method. This might remove the need to document with a comment as the code would be self describing. Perhaps:

buildBlockTags and buildBlockTitle?

I've refactored the implementation here: 25129a6

This adds buildBlockTags and buildBlockTitle functions and builds the title with buildBlockTitle( settings.title, buildBlockTags( name, requiredPlan ) )

extensions/shared/register-jetpack-block.js

…solated build functions. - Adds buildBlockTags( name, requiredPlan) - Adds buildBlockTitle( blockTitle, blockTags ) - Builds the title in registerJetpackBlock with buildBlockTitle( settings.title, buildBlockTags( name, requiredPlan ) )

matticbot · 2020-03-04T15:18:31Z

jeherve, Your synced wpcom patch D39107-code has been updated.

getdave

Looks much better. Thanks for the refactor 👍

I'm still setting up my env for testing so please can you source another manual review?

I'm sorry about this.

getdave · 2020-03-04T15:57:46Z

extensions/shared/register-jetpack-block.js

+ */
+function buildBlockTitle( blockTitle, blockTags = [] ) {
+	if ( blockTags.length ) {
+		blockTitle = `${ blockTitle } (${ blockTags.join( ', ' ) })`;


Suggested change

blockTitle = `${ blockTitle } (${ blockTags.join( ', ' ) })`;

blockTitle = `${ blockTitle } (${ blockTags.join() })`;

If omitted, the array elements are separated with a comma

True, but there's no space between them, so it would end up being (paid,beta) instead of (paid, beta)

WunderBart

Tested locally, everything looks great!

jeryj · 2020-03-04T16:19:24Z

I refactored and I believe I addressed all the raised comments. Could I please get another review? 🙏

jeherve · 2020-03-06T07:20:24Z

r203949-wpcom

* Initial changelog entry * Changelog: add #14904 * Changelog: add #14910 * Changelog: add #14913 * Changelog: add #14916 * Changelog: add #14922 * Changelog: add #14924 * Changelog: add #14925 * Changelog: add #14928 * Changelog: add #14840 * Changelog: add #14841 * Changelog: add #14842 * Changelog: add #14826 * Changelog: add #14835 * Changelog: add #14859 * Changelog: add #14884 * Changelog: add #14888 * Changelog: add #14817 * Changelog: add #14814 * Changelog: add #14819 * Changelog;: add #14797 * Changelog: add #14798 * Changelog: add #14802 * Changelog: add #13676 * Changelog: add #13744 * Changelog: add #13777 * Changelog: add #14446 * Changelog: add #14739 * Changelog: add #14770 * Changelog: add #14784 * Changelog: add #14897 * Changelog: add #14898 * Changelog: add #14968 * Changelog: add #14985 * Changelog: add #15044 * Changelog: add #15052 * Update to remove Podcast since it remains in Beta * Changelog: add #14803 * Changelog: add #15028 * Changelog: add #15065 * Changelog:add #14886 * Changelog: add #15118 * Changelog: add #14990 * Changelog: add #14528 * Changelog: add #15120 * Changelog: add #15126 * Changelog: add #15049 * Chanegelog: add #14852 * Changelog: add #15090 * Changelog: add #15138 * Changelog: add #15124 * Changelog:add #15055 * Changelog: add #15017 * Changelog: add #15109 * Changelog: add #15145 * Changelog:add #15096 * Changelog:add #15153 * Changelog: add #15133 * Changelog: add #14960 * Changelog: add #15127 * Changelog: add #15056 * Copy current changelog to changelog archive. * Clarify changelog description

Blocks: add paid label next to block title when it requires a plan

b2f471c

jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Feb 19, 2020

jeherve requested a review from a team February 19, 2020 21:43

matticbot added the Touches WP.com Files label Feb 19, 2020

jeryj assigned jeryj and WunderBart Feb 19, 2020

WunderBart added 2 commits February 20, 2020 15:55

Rename the plan checker func & add comments

1135ebe

Join block tags and make 'beta' translatable

3126d2d

jeherve commented Feb 20, 2020

View reviewed changes

extensions/shared/register-jetpack-block.js Outdated Show resolved Hide resolved

extensions/shared/register-jetpack-block.js Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2020

WunderBart mentioned this pull request Feb 24, 2020

Blocks: differentiate premium blocks from free ones when customer doesn't have paid plan #11875

Closed

simison reviewed Feb 24, 2020

View reviewed changes

extensions/shared/register-jetpack-block.js Show resolved Hide resolved

Using _x to translate 'paid' string to provide context for the transl…

6ef33cd

…ation

jeryj added 2 commits March 3, 2020 14:04

Use template literal for building block tags to make the code more re…

2269416

…adable.

jeryj added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 3, 2020

getdave reviewed Mar 4, 2020

View reviewed changes

jeherve commented Mar 4, 2020

View reviewed changes

extensions/shared/register-jetpack-block.js Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 4, 2020

getdave reviewed Mar 4, 2020

View reviewed changes

WunderBart approved these changes Mar 4, 2020

View reviewed changes

jeryj added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 4, 2020

jeherve added this to the 8.4 milestone Mar 4, 2020

kraftbj approved these changes Mar 4, 2020

View reviewed changes

kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 4, 2020

jeryj merged commit 0fd1874 into master Mar 4, 2020

jeryj deleted the add/paid-mention-blocks branch March 4, 2020 17:31

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 4, 2020

jeherve mentioned this pull request Mar 6, 2020

Blocks: Add Ribbon to indicate Paid blocks in block picker #13490

Closed

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Mar 23, 2020

jeherve added a commit that referenced this pull request Mar 23, 2020

Changelog: add #14739

506fb46

obenland mentioned this pull request Jul 29, 2020

(8P) Premium Blocks: Replace (paid) label with upgrade indicator #16628

Closed

	blockTitle = `${ blockTitle } (${ blockTags.join( ', ' ) })`;
	blockTitle = `${ blockTitle } (${ blockTags.join() })`;

Conversation

jeherve commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

matticbot commented Feb 19, 2020

Uh oh!

jetpackbot commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeryj commented Feb 19, 2020

Uh oh!

jeherve commented Feb 20, 2020

Uh oh!

WunderBart commented Feb 20, 2020

Uh oh!

davemart-in commented Feb 20, 2020

Uh oh!

matticbot commented Feb 20, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matticbot commented Mar 3, 2020

Uh oh!

matticbot commented Mar 3, 2020

Uh oh!

jeryj commented Mar 3, 2020

Uh oh!

getdave left a comment

Choose a reason for hiding this comment

Uh oh!

getdave Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

jeryj Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matticbot commented Mar 4, 2020

Uh oh!

getdave left a comment

Choose a reason for hiding this comment

Uh oh!

getdave Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

jeryj Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

WunderBart left a comment

Choose a reason for hiding this comment

Uh oh!

jeryj commented Mar 4, 2020

Uh oh!

jeherve commented Mar 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

jeherve commented Feb 19, 2020 •

edited

Loading

jetpackbot commented Feb 19, 2020 •

edited

Loading