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.

Sign up for GitHub

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: add paid label next to block title when it requires a plan #14739

Merged
merged 7 commits into from
Mar 4, 2020
Merged

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

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b2f471c
Blocks: add paid label next to block title when it requires a plan
jeherve Feb 19, 2020
1135ebe
Rename the plan checker func & add comments
WunderBart Feb 20, 2020
3126d2d
Join block tags and make 'beta' translatable
WunderBart Feb 20, 2020
6ef33cd
Using _x to translate 'paid' string to provide context for the transl…
jeryj Mar 3, 2020
2269416
Use template literal for building block tags to make the code more re…
jeryj Mar 3, 2020
0a325fb
Changed the object property from availableBlockTags.plan to available…
jeryj Mar 3, 2020
25129a6
Refactors block title and block tags implementation to be into more i…
jeryj Mar 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions extensions/shared/register-jetpack-block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* External dependencies
*/
import { __, _x } from '@wordpress/i18n';
import { addFilter } from '@wordpress/hooks';
import { registerBlockType } from '@wordpress/blocks';

Expand All @@ -12,9 +13,21 @@ import getJetpackExtensionAvailability from './get-jetpack-extension-availabilit
import withHasWarningIsInteractiveClassNames from './with-has-warning-is-interactive-class-names';
import wrapPaidBlock from './wrap-paid-block';

const availableBlockTags = {
paid: _x( 'paid', 'Short label appearing near a block requiring a paid plan', 'jetpack' ),
beta: __( 'beta', 'jetpack' ),
};

const betaExtensions = extensionList.beta || [];

function requiresPlan( unavailableReason, details ) {
/**
* Checks whether the block requires a paid plan or not.
*
* @param {string} unavailableReason The reason why block is unavailable
* @param {Object} details The block details
* @returns {string|boolean} Either false if the block doesn't require a paid plan, or the actual plan name it requires.
*/
function requiresPaidPlan( unavailableReason, details ) {
if ( unavailableReason === 'missing_plan' ) {
return details.required_plan;
}
Expand All @@ -32,7 +45,7 @@ function requiresPlan( unavailableReason, details ) {
export default function registerJetpackBlock( name, settings, childBlocks = [] ) {
const { available, details, unavailableReason } = getJetpackExtensionAvailability( name );

const requiredPlan = requiresPlan( unavailableReason, details );
const requiredPlan = requiresPaidPlan( unavailableReason, details );

if ( ! available && ! requiredPlan ) {
if ( 'production' !== process.env.NODE_ENV ) {
Expand All @@ -44,9 +57,23 @@ export default function registerJetpackBlock( name, settings, childBlocks = [] )
return false;
}

let blockTitle = settings.title;
const blockTags = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've refactored the implementation here: 25129a6

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


if ( requiredPlan ) {
blockTags.push( availableBlockTags.paid );
}
if ( betaExtensions.includes( name ) ) {
blockTags.push( availableBlockTags.beta );
}

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

const result = registerBlockType( `jetpack/${ name }`, {
...settings,
title: betaExtensions.includes( name ) ? `${ settings.title } (beta)` : settings.title,
title: blockTitle,
edit: requiredPlan ? wrapPaidBlock( { requiredPlan } )( settings.edit ) : settings.edit,
example: requiredPlan ? undefined : settings.example,
} );
Expand Down