Components: Added a Storybook and some example stories for automattic/components #38496

jameslnewell · 2019-12-18T00:45:42Z

Changes proposed in this Pull Request

This PR adds a Storybook for @automattic/components.

/devdocs already exists with a lot of overlap of the features provided by Storybook. The primary motivation behind adding a new thing in this PR is to make it easy to develop and test the @automattic/components in isolation from Calypso and Calypso's global styles in order to ensure that @automattic/components will work anywhere.

I've intentionally kept this PR pretty minimal and will expand on this in future PRs e.g. build + deploy on CI, getting component styles working in isolation etc etc

Testing instructions

Run npm run components:storybook:start

matticbot · 2019-12-18T00:45:43Z

Test live: https://calypso.live/?branch=add/storybook

matticbot · 2019-12-18T10:57:30Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~21 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-login                +30 B  (+0.0%)       +4 B  (+0.0%)
entry-main                 +27 B  (+0.0%)       +0 B
entry-gutenboarding        +22 B  (+0.0%)      +17 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~208 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
woocommerce                 +171 B  (+0.0%)      +81 B  (+0.0%)
domains                      +64 B  (+0.0%)      +18 B  (+0.0%)
email                        +56 B  (+0.0%)      +19 B  (+0.0%)
checkout                     +32 B  (+0.0%)      +15 B  (+0.0%)
wp-super-cache               +24 B  (+0.0%)       +8 B  (+0.0%)
hosting                      +24 B  (+0.0%)      +11 B  (+0.0%)
purchases                    +20 B  (+0.0%)       +7 B  (+0.0%)
site-blocks                  +16 B  (+0.0%)       +5 B  (+0.0%)
security                     +16 B  (+0.0%)       +3 B  (+0.0%)
privacy                      +16 B  (+0.0%)       +5 B  (+0.0%)
plans                        +16 B  (+0.0%)       +6 B  (+0.0%)
notification-settings        +16 B  (+0.0%)       +5 B  (+0.0%)
me                           +16 B  (+0.0%)       +5 B  (+0.0%)
help                         +16 B  (+0.0%)       +5 B  (+0.0%)
checklist                    +16 B  (+0.0%)       +5 B  (+0.0%)
account-close                +16 B  (+0.0%)       +5 B  (+0.0%)
account                      +16 B  (+0.0%)       +5 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~37 bytes added 📈 [gzipped])

name                                                         parsed_size           gzip_size
async-load-my-sites-current-site-domain-warnings                   +16 B  (+0.0%)       +5 B  (+0.0%)
async-load-my-sites-checklist-wpcom-checklist-component-jsx        +16 B  (+0.0%)       +5 B  (+0.0%)
async-load-design-blocks                                           +16 B  (+0.0%)      +14 B  (+0.0%)
async-load-design                                                  +16 B  (+0.0%)       +8 B  (+0.0%)
async-load-reader-sidebar                                          +12 B  (+0.0%)       +5 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

jsnajdr · 2019-12-18T13:29:17Z

packages/components/package.json

+	"devDependencies": {
+		"@storybook/addon-actions": "^5.2.8",
+		"@storybook/preset-scss": "^1.0.2",
+		"@storybook/react": "^5.2.8"


Packages in monorepos managed by Lerna should never have devDependencies. They should be all hoisted to the top-level package.json. See also the Gutenberg setup which does it the right way.

Also, please pin the @storybook/* dependencies to exact versions and define a Storybook monorepo in the project's renovate.json. That tells Renovate to always file only one PR to do all @storybook/* updates at once.

See also the Gutenberg setup which does it the right way.

Gutenberg has setup Storybook across all of their packages. In contrast in this PR I've only setup storybook for the @automattic/components package because I want to test the components in isolation from the rest of Calypso.

I put the dependencies in packages/components/package.json because, following the above reasoning, the Storybook dependencies were only relevant for the @automattic/components package.

I can think of cases where devDependencies should be used and avoiding hoisting is necessary, but this isn't one of them and I've updated the PR as requested.

Lerna docs recommend hoisting dev dependencies to the top, it's not a Calypso-only convention.

The NPM script to run Storybook (npm run components:storybook:start) is also in the top-level package.json and relies on the start-storybook binary being present in node_modules/.bin. If you added the dependencies to the package and didn't duplicate them in the root, too, as you did, the binary would be in packages/components/node_modules/.bin/start-storybook and the top-level NPM wouldn't find it.

jsnajdr · 2019-12-18T13:33:59Z

packages/components/src/button/index.stories.jsx

+	<Button primary onClick={ handleClick }>
+		{ helloWorld }
+	</Button>
+);


When clicking on the "Is Primary" option in the storybook, the expected styles are not applied. Can Storybook deal with styles imported as import './style.scss'? Does the webpack config that it uses support the scss-loader and style-loader?

I've already added support for SCSS and you can see the black outline styles being applied in the screenshot above.

I (purposely) haven't imported the Calypso base styles where the CSS variables and other styles are being applied - the reason I haven't done this is that I want to develop and test these components as other A12s would find them - without the Calypso base styles.

I see, the button styles are present, but the theme CSS that turns CSS variables into actual colors is not.

Yep, that's intentional in order to demonstrate the isolated use of @automattic/components.

package.json

jsnajdr · 2019-12-19T07:54:28Z

packages/components/src/button/index.stories.jsx

+	<Button primary onClick={ handleClick }>
+		{ helloWorld }
+	</Button>
+);


I see, the button styles are present, but the theme CSS that turns CSS variables into actual colors is not.

jsnajdr

Looks good 👍 Thanks for addressing the review feedback 🤝

jameslnewell requested a review from a team as a code owner December 18, 2019 00:45

jameslnewell added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 18, 2019

jsnajdr reviewed Dec 18, 2019

View reviewed changes

jameslnewell force-pushed the add/storybook branch from 4870f12 to cafb069 Compare December 18, 2019 22:39

jameslnewell requested a review from jsnajdr December 18, 2019 22:50

jameslnewell force-pushed the add/storybook branch from c0ce668 to e099f62 Compare December 18, 2019 23:18

jsnajdr reviewed Dec 19, 2019

View reviewed changes

James Newell added 4 commits December 19, 2019 19:24

added storybook and some example stories

44e55fd

address feedback on dependencies

281b639

ignore linting storybook setup

237e18f

move deps to devDeps

61e9d20

jameslnewell force-pushed the add/storybook branch from c97d833 to 61e9d20 Compare December 19, 2019 08:25

jsnajdr approved these changes Dec 19, 2019

View reviewed changes

jameslnewell merged commit cf46a34 into master Dec 19, 2019

jameslnewell deleted the add/storybook branch December 19, 2019 21:23

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 19, 2019

griffbrad mentioned this pull request Dec 19, 2019

try: add storybook to showcase progressbar from calypso-ui #33166

Closed

andrewserong mentioned this pull request Jan 9, 2020

Add storybook instructions for @automattic/components #38732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Components: Added a Storybook and some example stories for automattic/components #38496

Components: Added a Storybook and some example stories for automattic/components #38496

jameslnewell commented Dec 18, 2019

matticbot commented Dec 18, 2019

matticbot commented Dec 18, 2019 •

edited

Loading

jsnajdr Dec 18, 2019

jsnajdr Dec 18, 2019

jameslnewell Dec 18, 2019 •

edited

Loading

jsnajdr Dec 19, 2019

jsnajdr Dec 18, 2019

jameslnewell Dec 18, 2019 •

edited

Loading

jsnajdr Dec 19, 2019

jameslnewell Dec 19, 2019

jsnajdr Dec 19, 2019

jsnajdr left a comment

Components: Added a Storybook and some example stories for automattic/components #38496

Components: Added a Storybook and some example stories for automattic/components #38496

Conversation

jameslnewell commented Dec 18, 2019

Changes proposed in this Pull Request

Testing instructions

matticbot commented Dec 18, 2019

matticbot commented Dec 18, 2019 • edited Loading

jsnajdr Dec 18, 2019

Choose a reason for hiding this comment

jsnajdr Dec 18, 2019

Choose a reason for hiding this comment

jameslnewell Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr Dec 18, 2019

Choose a reason for hiding this comment

jameslnewell Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

jameslnewell Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr Dec 19, 2019

Choose a reason for hiding this comment

jsnajdr left a comment

Choose a reason for hiding this comment

matticbot commented Dec 18, 2019 •

edited

Loading

jameslnewell Dec 18, 2019 •

edited

Loading

jameslnewell Dec 18, 2019 •

edited

Loading