Blocks: Add Ribbon to indicate Paid blocks in block picker by ockham · Pull Request #13490 · Automattic/jetpack

ockham · 2019-09-18T10:37:07Z

WIP, currently untested.

Changes proposed in this Pull Request:

Add a ribbon to visually indicate paid blocks in the block picker

Is this a new feature or does it add/remove features to an existing part of Jetpack?

It does add a feature to an existing part of Jetpack 😬

Testing instructions:

TBD

Proposed changelog entry for your changes:

Blocks: Add Ribbon to indicate Paid blocks in block picker

matticbot · 2019-09-18T10:37:12Z

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

extensions/shared/register-jetpack-block.js

jetpackbot · 2019-09-18T10:39:50Z

	Warnings
⚠️	`pre-commit` hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 73eccab

matticbot · 2019-09-18T19:45:40Z

ockham, Your synced wpcom patch D32892-code has been updated.

ockham · 2019-09-19T20:44:19Z

I've bumped calypso-build to 4.0.1 and added calypso-ui. Upon yarn build, I'm now getting

Error: [BABEL] ~/src/jetpack/gulpfile.babel.js: Cannot find module '@babel/plugin-proposal-class-properties' from '~/src/jetpack'
    at Function.module.exports [as sync] (~/src/jetpack/node_modules/resolve/lib/sync.js:71:15)
    at resolveStandardizedName (~/src/jetpack/node_modules/@babel/core/lib/config/files/plugins.js:101:31)
    at resolvePlugin (~/src/jetpack/node_modules/@babel/core/lib/config/files/plugins.js:54:10)
    at loadPlugin (~/src/jetpack/node_modules/@babel/core/lib/config/files/plugins.js:62:20)
    at createDescriptor (~/src/jetpack/node_modules/@babel/core/lib/config/config-descriptors.js:154:9)
    at items.map (~/src/jetpack/node_modules/@babel/core/lib/config/config-descriptors.js:109:50)
    at Array.map (<anonymous>)
    at createDescriptors (~/src/jetpack/node_modules/@babel/core/lib/config/config-descriptors.js:109:29)
    at createPluginDescriptors (~/src/jetpack/node_modules/@babel/core/lib/config/config-descriptors.js:105:10)
    at plugins (~/src/jetpack/node_modules/@babel/core/lib/config/config-descriptors.js:40:19)

Even though that package is a dependency of calypso-build.

This seems somewhat similar to p1566905517230900-slack-luna, which we hoped to fix with #13343 🤔

Installing that package directly gives the same kind of error for @babel/plugin-syntax-dynamic-import, which is also in calypso-build's dependencies 🤦‍♂️

package.json

ockham · 2019-10-10T08:49:45Z

Rebased, and bumped calypso-ui to 2.0.0-alpha.0. Getting the following error during yarn build:

        ERROR in ./node_modules/@automattic/calypso-ui/dist/esm/card/style.scss (./node_modules/css-loader/dist/cjs.js??ref--5-1!./node_modules/postcss-loader/src??ref--5-2!./node_modules/@automattic/calypso-build/node_modules/sass-loader/lib/loader.js??ref--5-3!./node_modules/@automattic/calypso-ui/dist/esm/card/style.scss)
        Module build failed (from ./node_modules/@automattic/calypso-build/node_modules/sass-loader/lib/loader.js):
        
        
                 ^
              No mixin named clear-fix
              in ~/src/jetpack/node_modules/@automattic/calypso-ui/dist/esm/card/style.scss (line 11, column 11)

This is because of https://github.com/Automattic/wp-calypso/blob/9ebe145354a7e7ab5540ce2ecd861c61ff7f963b/packages/calypso-ui/src/card/style.scss#L10 which needs https://github.com/Automattic/wp-calypso/blob/9ebe145354a7e7ab5540ce2ecd861c61ff7f963b/client/assets/stylesheets/shared/mixins/_clear-fix.scss.

Ad-hoc solution idea: Bundle mixins in a new package (calypso-sass-mixins?) and make them a dep for calypso-ui? Thoughts @sirreal @Automattic/team-calypso ?

simison · 2019-10-10T09:05:45Z

Why not include the mixins (and possible animations) file in calypso-ui package?

sirreal · 2019-10-10T09:21:28Z

Some exploratory work was done here, you should be able to use this published package as the sas loader prelude:

Automattic/wp-calypso#36170

That may fix the issues but isn't really well thought out.

ockham · 2019-10-10T11:10:31Z

Some exploratory work was done here, you should be able to use this published package as the sas loader prelude:

Automattic/wp-calypso#36170

That may fix the issues but isn't really well thought out.

Ah right, had forgotten about that PR.

Right off the bat: We're injecting the prelude at Webpack config level (through the SASS "preset") there, which is something I'd rather avoid for projects using @automattic/calypso-ui. Can we 'simply' change @automattic/calypso-ui itself to use the prelude?

simison · 2019-10-10T11:31:52Z

Similar issues highlighted by Automattic/wp-calypso#36471

jsnajdr · 2019-10-10T12:16:05Z

Can we 'simply' change @automattic/calypso-ui itself to use the prelude?

The prelude is used only when compiling SASS down to CSS, and therefore needs to be included at the place that does that compilation.

If the published calypso-ui package include only compiled CSS files (and not the SCSS ones), then the prelude could be its dev dependency.

But if the compilation is happening outside calypso-ui, then the prelude needs to be used there.

Publishing our Calypso base styles, i.e.,

the CSS reset stylesheet
the CSS stylesheet for form elements (we style even plain input elements that don't have any CSS classes)
shared animations (keyframe definitions)
SCSS variable and mixin definitions of all kinds

as @automattic/calypso-base-styles would be a great idea! 🚀

One of incremental steps is cleanup of @automattic/calypso-color-schemes usage in calypso-build in Automattic/wp-calypso#36471. Please have a look at that PR!

matticbot · 2019-12-11T07:15:20Z

ockham, Your synced wpcom patch D32892-code has been updated.

simison · 2020-02-10T22:05:54Z

extensions/shared/register-jetpack-block.js

 		example: requiredPlan ? undefined : settings.example,
+		icon: requiredPlan ? (
+			<>
+				<Ribbon>{ __( 'Paid' ) }</Ribbon>


We should probably give something little more for screen reader users than "paid", like "block name (requires paid plan)"

jeherve · 2020-03-06T07:21:07Z

closing in favor of #14739

ockham added [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Sep 18, 2019

ockham requested a review from a team September 18, 2019 10:37

ockham self-assigned this Sep 18, 2019

matticbot added Touches WP.com Files labels Sep 18, 2019

ockham commented Sep 18, 2019

View reviewed changes

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

ockham force-pushed the add/paid-blocks-ribbon branch from eb6c0db to b5ad966 Compare September 19, 2019 19:57

simison reviewed Sep 20, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

sirreal mentioned this pull request Sep 23, 2019

Build: Update calypso-build to v4 #13503

Merged

ockham force-pushed the add/paid-blocks-ribbon branch 2 times, most recently from 614232e to fbd5fa2 Compare October 10, 2019 06:52

ockham mentioned this pull request Oct 10, 2019

Update/publish calypso sass Automattic/wp-calypso#36170

Closed

ockham force-pushed the add/paid-blocks-ribbon branch from f78681d to 7cb264a Compare October 28, 2019 08:59

ockham force-pushed the add/paid-blocks-ribbon branch 2 times, most recently from 574031d to 0dfdd67 Compare November 11, 2019 07:14

ockham added 2 commits December 11, 2019 13:13

[not verified] Add @automattic/components dependency

f26bdf8

Add Ribbon to Paid blocks in block picker

73eccab

ockham force-pushed the add/paid-blocks-ribbon branch from 0dfdd67 to 73eccab Compare December 11, 2019 07:15

Automattic deleted a comment from matticbot Dec 11, 2019

simison added [Block] Payment Button aka Recurring Payments Simple Payments labels Feb 4, 2020

simison mentioned this pull request Feb 10, 2020

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

Closed

simison reviewed Feb 10, 2020

View reviewed changes

jeherve mentioned this pull request Feb 19, 2020

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

Merged

jeherve closed this Mar 6, 2020

ockham deleted the add/paid-blocks-ribbon branch July 9, 2020 13:44

jeherve added the [Block] Pay With Paypal aka Simple Payments label Feb 12, 2021

kraftbj removed the [Status] In Progress label Oct 8, 2021

jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023

Conversation

ockham commented Sep 18, 2019

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

matticbot commented Sep 18, 2019

Uh oh!

Uh oh!

jetpackbot commented Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented Sep 18, 2019

Uh oh!

ockham commented Sep 19, 2019

Uh oh!

Uh oh!

ockham commented Oct 10, 2019

Uh oh!

simison commented Oct 10, 2019

Uh oh!

sirreal commented Oct 10, 2019

Uh oh!

ockham commented Oct 10, 2019

Uh oh!

simison commented Oct 10, 2019

Uh oh!

jsnajdr commented Oct 10, 2019 • edited by ockham Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matticbot commented Dec 11, 2019

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeherve commented Mar 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jetpackbot commented Sep 18, 2019 •

edited

Loading

jsnajdr commented Oct 10, 2019 •

edited by ockham

Loading

simison Feb 10, 2020 •

edited

Loading