SDK: import Calypso UI into a Gutenberg extension and scope styles #26683

simison · 2018-08-14T12:17:21Z

(Based on @dmsnell earlier work in #26541)

Namespacer library

Existing solutions for namespacing (namespace-css, postcss-prefixwrap) seem to lack support for scoping things like @keyframes and @font, so there's a change that we would need to build our own CSS-name-spacer to really scope all CSS that Calypso components have. Fortunately, it's fairly straightforward
as demonstrated in Gutenberg.

I've wrapped the whole Gutenberg block with calypso CSS class but that's not ideal; Gutenberg block should not need to know anything about CSS scopes and this should all be manual. See other explorations for automating that part in #26852 and #26851

Resets and other shared styles

I didn't import shared styles like CSS-resets or typography.

Editor styles leaking in

If you can observe .card CSS class coming from Gutenberg overwriting some styles. This is a problem.

Testing

Build the extension:

npm run sdk:gutenberg -- --editor-script=client/gutenberg/extensions/editor-notes/index.js

Open compiled CSS client/gutenberg/extensions/editor-notes/build/editor-notes-editor.css
- Calypso components are namespaced with .calypso
- Block styles aren't scoped (look for .wp-block-a8c-editor-notes__box)
- Mixins and colour constants worked for both Calypso components and block styles:
  - No z-index: z-index( '.ribbon', '.ribbon__title::before' ); but z-index: -1
  - No $white but white in Calypso components
  - No $alert-red but #d94f4f in block styles
Confirm the block works in Gutenberg outside Calypso
Confirm that this doesn't affect Calypso builds

matticbot · 2018-08-14T12:17:22Z

Test live: https://calypso.live/?branch=try/global-sass-prefix-calypso

simison · 2018-08-17T08:37:00Z

Looks like @import's are failing in Jest → needs some more work. I'd be happy for a half-way review, though.

simison · 2018-08-17T10:42:22Z

Fixed tests: mocked all style imports in Jest with an empty object:
https://github.com/Automattic/wp-calypso/blob/07e57160e122434eb8c58617d3e4340aab946db9/test/test/helpers/styles/README.md

https://jestjs.io/docs/en/webpack#handling-static-assets

dmsnell · 2018-08-17T10:58:13Z

bin/sdk/gutenberg.js

+	const baseConfig = getBaseConfig( {
+		externalizeWordPressPackages: true,
+		stylesNamespacing: 'calypso',
+		stylesNamespacingExclude: path.join( __rootDir, 'client', 'gutenberg' ),


do we need a customizable exclude list? do you think we could get by with a whitelist instead just namespacing Calypso's components directory and maybe some others?

You had the right intuition — it was indeed possible to simplify this and get rid of the custom exclusion-list. Thanks for pointing it out!

simison · 2018-08-17T14:50:35Z

Looks like wp-desktop tests are failing and Webpack config would need sass-loader added there, too.

dmsnell · 2018-08-17T15:35:16Z

webpack.config.js

+ *
+ * @param {object}  env                               environment options
+ * @param {boolean} env.externalizeWordPressPackages  whether to bundle or extern the `@wordpress/` packages
+ * @param {string}  env.stylesNamespacing             prefix Calypso component styles with CSS class or ID


probably singular might work more naturally here…

/** * @param {string} styleNamespace prefix Calypso component styles with this class or ID */

also of not here leading back to my earlier comment - the explanation is that this applies to Calypso components but we're only excluding specific directories. is it more difficult to whitelist directories instead? if the exclude-list is better then we should probably update this comment to make that less confusing

(could even see cssNamespace being clearer - no strong opinion though)

In this patch I'm doing two things at once to reach the goal of using Calypso UI components: pulling in the UI code and pulling in the styles. As you may note I'm bringing in the entirety of the Calypso styles since the framework styles are monolithic. This has the immediate drawback that the stylesheet is over a megabyte in size, but the immediate advantage that we can import Calypso UI and not have to write it all ourselves. If this turns out to be a good path then we will obviously have to optimize the CSS, which is already in the plans. As the SDK and Calypso framework improves we should see that bundled CSS drop down to very small sizes. Currently the framework isn't ready to split apart the CSS though I'm afraid.

Previously we were pulling in the top-level stylesheet from Calypso and we were doing so from the top of the extension code. This pulled in over a megabyte of CSS and required us to be aware of Calypso's styling and style paths. In this change I have updated the `Card` and `Ribbon` components so that they explicitly import their SASS dependencies. This allows us to pull in _only_ the required style when building the extension. The output dropped from a megabyte to three kilobytes.

…omponents

> - configuration.module.rules[2].exclude should be one of these: > RegExp | non-empty string | function | [RegExp | non-empty string | function | (recursive) | object { and?, exclude?, include?, not?, or?, test? }] | object { and?, exclude?, include?, not?, or?, test? } Setting it to `undefined` would work as well.

tyxla

It seems the styles are being loaded as scripts when I run a regular Calypso build locally:

tyxla · 2018-08-24T06:50:04Z

client/components/card/style.scss

@@ -1,3 +1,5 @@
+@import '../../../assets/stylesheets/shared/_utils.scss';


We seem to need only:

assets/stylesheets/shared/_colors.scss

assets/stylesheets/shared/mixins/clear-fix.scss

assets/stylesheets/shared/mixins/breakpoints.scss

How do you feel about being even more specific and listing them specifically? Being granular enough will hopefully result in smaller bundle sizes.

tyxla · 2018-08-24T06:53:25Z

client/components/ribbon/style.scss

@@ -1,3 +1,5 @@
+@import '../../../assets/stylesheets/shared/_utils.scss';


We seem to need only:

assets/stylesheets/shared/_colors.scss

assets/stylesheets/shared/functions/_z-index.scss

How do you feel about being even more specific and listing them specifically? Being granular enough will hopefully result in smaller bundle sizes.

tyxla · 2018-08-24T06:57:07Z

client/gutenberg/extensions/editor-notes/index.js

-		) }
+const edit = ( { attributes: { notes }, className, setAttributes } ) => (
+	<Card highlight="error" className={ `${ className } ${ className }__box calypso` }>
+		<Ribbon>Hidden</Ribbon>


Maybe we should localize this string?

Maybe we should localize this string?

We definitely should give it a try! 😍

Let's wait until we have some more clarity on i18n, before we dip our toes into this :)

tyxla · 2018-08-24T07:04:20Z

client/gutenberg/extensions/editor-notes/index.js

@@ -14,23 +16,16 @@ const attributes = {
 	},
 };

-const edit = ( { attributes: { notes }, className, isSelected, setAttributes } ) => (
-	<div className={ isSelected ? 'is-selected' : '' }>
-		{ ! isSelected && (


Any good reason to be getting rid of this functionality? It was nice to have the notebook icon while editing the block content.

tyxla · 2018-08-24T07:06:25Z

webpack.config.js

+ *
+ * @param {object}  env                               environment options
+ * @param {boolean} env.externalizeWordPressPackages  whether to bundle or extern the `@wordpress/` packages
+ * @param {string}  env.styleNamespace             prefix Calypso component styles with CSS class or ID


Nit: param description seems to be misaligned here

tyxla · 2018-08-24T07:08:48Z

webpack.config.js

- * @param {object}  env                              additional config options
- * @param {boolean} env.externalizeWordPressPackages whether to bundle or extern the `@wordpress/` packages
- * @param {object}  argv                             given by webpack?
+ * `env` params are typically set by files in `bin/sdk/*`


This comment might be confusing for anyone who isn't familiar with the SDK. Would you mind clarifying what can be found in bin/sdk/*? Or is there a chance we're leaking SDK functionality out of the SDK directory too much?

tyxla · 2018-08-24T07:29:27Z

webpack.config.js

+};
+
+// When styles-namespacing is enabled, these are the files we want to namespace
+const styleNamespaceDirectories = [ path.join( __dirname, 'client', 'components' ) ];


From the outside perspective, it feels a little inconsistent to namespace only components, and not namespace the block stylesheets. If I'm an external plugin/theme developer, I'd be confused why some stylesheets have the prefix and others don't. What do you think?

Theory is that this doesn't have any affect.

simison added Framework [Status] In Progress [Project] SDK labels Aug 14, 2018

simison requested review from ockham, ehg, coderkevin, dmsnell and tyxla August 14, 2018 12:18

simison mentioned this pull request Aug 15, 2018

SDK: refactor block CSS name-spacing #26662

Merged

simison force-pushed the try/global-sass-prefix-calypso branch from 70abd52 to 9e55b90 Compare August 17, 2018 07:25

simison added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Aug 17, 2018

simison changed the title ~~SDK: explore importing Calypso UI into a Gutenberg extension, take two~~ SDK: import Calypso UI into a Gutenberg extension and namespace styles Aug 17, 2018

simison added the [Status] In Progress label Aug 17, 2018

simison removed the [Status] In Progress label Aug 17, 2018

dmsnell reviewed Aug 17, 2018

View reviewed changes

simison force-pushed the try/global-sass-prefix-calypso branch from f3b95d2 to 07e5716 Compare August 17, 2018 12:55

dmsnell reviewed Aug 17, 2018

View reviewed changes

simison force-pushed the try/global-sass-prefix-calypso branch from 25955c6 to 92d6293 Compare August 20, 2018 14:53

simison mentioned this pull request Aug 22, 2018

Build CSS assets with Webpack #26820

Merged

dmsnell and others added 5 commits August 22, 2018 12:33

Include sass utilities globally and namespace only imported Calypso c…

920f61d

…omponents

Load shared/utils also in webpack.config.node.js

bbaed35

Remove globals and keep namespacing

99df4ea

simison added 9 commits August 22, 2018 12:39

Remove unrelated scss changes

5e863bc

,

f5853f0

Fix failing tests — don't import scss files in Jest

35541b8

Clean out global import from webpack.config.node.js

a9fc7eb

✂ Remove unnessesary _.compact

d2ab07e

Add moduleNameMapper also to integration tests

88ae7e4

Simplify namespacing inclusion/exclusion

261fc7d

Use singular name

052349f

simison force-pushed the try/global-sass-prefix-calypso branch from c72ff9c to 052349f Compare August 22, 2018 09:43

simison mentioned this pull request Aug 22, 2018

Styles: add Editor Styles support WordPress/gutenberg#9008

Merged

This was referenced Aug 23, 2018

SDK: use Babel to scope styles for Calypso components #26851

Closed

SDK: use higher level component to scope styles for Calypso components #26852

Closed

simison added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 24, 2018

simison changed the title ~~SDK: import Calypso UI into a Gutenberg extension and namespace styles~~ SDK: import Calypso UI into a Gutenberg extension and scope styles Aug 24, 2018

tyxla reviewed Aug 24, 2018

View reviewed changes

Do a test run of integration Jest config in CI

c52e42f

Theory is that this doesn't have any affect.

dmsnell mentioned this pull request Aug 26, 2018

SDK: Explore importing Calypso UI into a Gutenberg extension #26541

Closed

simison closed this Sep 25, 2018

simison deleted the try/global-sass-prefix-calypso branch September 25, 2018 17:21

matticbot removed the [Status] In Progress label Sep 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SDK: import Calypso UI into a Gutenberg extension and scope styles #26683

SDK: import Calypso UI into a Gutenberg extension and scope styles #26683

simison commented Aug 14, 2018 •

edited

Loading

matticbot commented Aug 14, 2018

simison commented Aug 17, 2018 •

edited

Loading

simison commented Aug 17, 2018 •

edited

Loading

dmsnell Aug 17, 2018

simison Aug 17, 2018

simison commented Aug 17, 2018

dmsnell Aug 17, 2018 •

edited

Loading

tyxla left a comment

tyxla Aug 24, 2018

tyxla Aug 24, 2018

tyxla Aug 24, 2018

simison Aug 24, 2018

ehg Aug 27, 2018

tyxla Aug 24, 2018

tyxla Aug 24, 2018

tyxla Aug 24, 2018

tyxla Aug 24, 2018

		@@ -1,3 +1,5 @@
		@import '../../../assets/stylesheets/shared/_utils.scss';

SDK: import Calypso UI into a Gutenberg extension and scope styles #26683

SDK: import Calypso UI into a Gutenberg extension and scope styles #26683

Conversation

simison commented Aug 14, 2018 • edited Loading

Namespacer library

Resets and other shared styles

Editor styles leaking in

Testing

matticbot commented Aug 14, 2018

simison commented Aug 17, 2018 • edited Loading

simison commented Aug 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simison commented Aug 17, 2018

dmsnell Aug 17, 2018 • edited Loading

Choose a reason for hiding this comment

tyxla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simison commented Aug 14, 2018 •

edited

Loading

simison commented Aug 17, 2018 •

edited

Loading

simison commented Aug 17, 2018 •

edited

Loading

dmsnell Aug 17, 2018 •

edited

Loading