-
Notifications
You must be signed in to change notification settings - Fork 814
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
Blocks: Add Ribbon to indicate Paid blocks in block picker #13490
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
This is an automated check which relies on |
ockham, Your synced wpcom patch D32892-code has been updated. |
eb6c0db
to
b5ad966
Compare
I've bumped
Even though that package is a dependency of 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 |
614232e
to
fbd5fa2
Compare
Rebased, and bumped
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 ( |
Why not include the mixins (and possible animations) file in |
Some exploratory work was done here, you should be able to use this published package as the sas loader prelude: 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 |
Similar issues highlighted by Automattic/wp-calypso#36471 |
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 But if the compilation is happening outside Publishing our Calypso base styles, i.e.,
as One of incremental steps is cleanup of |
f78681d
to
7cb264a
Compare
574031d
to
0dfdd67
Compare
0dfdd67
to
73eccab
Compare
ockham, Your synced wpcom patch D32892-code has been updated. |
@@ -49,6 +51,14 @@ export default function registerJetpackBlock( name, settings, childBlocks = [] ) | |||
title: betaExtensions.includes( name ) ? `${ settings.title } (beta)` : settings.title, | |||
edit: requiredPlan ? wrapPaidBlock( { requiredPlan } )( settings.edit ) : settings.edit, | |||
example: requiredPlan ? undefined : settings.example, | |||
icon: requiredPlan ? ( | |||
<> | |||
<Ribbon>{ __( 'Paid' ) }</Ribbon> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably give something little more for screen reader users than "paid", like "block name (requires paid plan)"
closing in favor of #14739 |
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