SDK: refactor block CSS name-spacing #26662
Conversation
- Rename block namespace from `jetpack` to `a8c`
simison
changed the title
| .wp-block-a8c-markdown__preview p { | ||
| min-height: 1.8em; | ||
| white-space: pre-wrap; | ||
| } |
There was a problem hiding this comment.
I took this CSS from the original Jetpack-Markdown PR: https://github.com/Automattic/jetpack/pull/9705/files#diff-3e61a3fdc3d89ac00c7baef9a6d68d6c (kudos @Ferdev)
I left this bit out because I didn't find it being used anywhere:
.wp-block-jetpack-markdown-block__live-preview__token {
/* $dark-gray-300 from Gutenberg _colors.scss */
color: #6c7781;
}...although it would've been a nice example of importing a Gutenberg variable in.
simison
added
the
[Status] Needs Review
bin/sdk-cli.js
Outdated
| @@ -72,6 +72,10 @@ yargs | |||
| description: 'Whether to watch for changes and automatically rebuild.', | |||
| type: 'boolean', | |||
| }, | |||
| 'namespace': { | |||
| description: 'Wheater CSS be namespaced.', | |||
There was a problem hiding this comment.
Typo here: should be whether. Also, I think it's missing a "should" right before "be".
bin/sdk/gutenberg.js
Outdated
| @@ -72,7 +72,9 @@ exports.compile = args => { | |||
| }; | |||
|
|
|||
| const name = path.basename( path.dirname( options.editorScript ).replace( /\/$/, '' ) ); | |||
| const baseConfig = getBaseConfig( { extensionName: name } ); | |||
| const baseConfig = getBaseConfig( { | |||
| extensionName: options.namespace ? name : false | |||
There was a problem hiding this comment.
With --namespace, this would result in:
.tiled-gallery .wp-block-a8c-tiled-gallery .tiled-gallery-square
and
.jetpack .wp-block-a8c-tiled-gallery .tiled-gallery-square
is that expected?
If so, wouldn't it be more robust if we have the namespace option as a string instead, which would allow us to specifically set the namespace as we use the SDK from the CLI?
There was a problem hiding this comment.
I hid the namespacer tool behind the flag instead of removing it because I think it might still be useful; I'm experimenting with it in #26683
Since it's broken like you describe, it should be considered only for development testing.
Do you reckon I should completely remove the namespacer in this PR even if we might add it back later on?
| import JetpackMarkdownBlockEditor from './jetpack-markdown-block-editor'; | ||
| import JetpackMarkdownBlockSave from './jetpack-markdown-block-save'; | ||
|
|
||
| registerBlockType( 'jetpack/markdown-block', { | ||
| registerBlockType( 'a8c/markdown', { |
There was a problem hiding this comment.
Do we also want to apply that to any other extensions? Publicize still uses jetpack-publicize, and we might want to be consistent with that.
There was a problem hiding this comment.
Seems like this introduced a bug 😕
See #26726
matticbot
removed
the
[Status] Needs Review
Refactor block CSS namespacing.
a8c(instead ofjetpack). This could be coming from a variable and customizable via cli to be different for each environment.--namespaceCLI arg.wp-block-a8c-blocknamealready in stylesheets. Could be done in Webpack but this way we have more flexibility:wp-block-a8c-markdown__placeholder-format.wp-block-a8c-tiled-gallery .tiled-gallery-image-format.Testing
Previously
If you build Jetpack bundle:
...you would get CSS:
If you build Tiled gallery block:
...you would get CSS:
After
With this PR you should always end up with: