Add Storybook for developing application components #366
Conversation
emmenko
added
🙏 Status: Dev Review
.storybook/webpack.config.js
Outdated
| loaders: [require.resolve('@storybook/addon-storysource/loader')], | ||
| enforce: 'pre', | ||
| }, | ||
| ...mcWebpackConfig.module.rules, |
There was a problem hiding this comment.
I found it easier to use like this as we need things like postcss, svg loaders etc.
| import styles from './maintenance-page-layout.mod.css'; | ||
|
|
||
| const MaintenancePageLayout = props => ( | ||
| <div className={styles.container}> |
There was a problem hiding this comment.
This renders a css grid. We can't use the ui-kit Grid as we need additional styles (e.g. height)
| const MaintenancePageLayout = props => ( | ||
| <div className={styles.container}> | ||
| <Constraints.Horizontal constraint="l"> | ||
| <Spacings.Stack scale="m"> |
There was a problem hiding this comment.
We now use Constraints and Spacings components 🎉
| </div> | ||
| <div> | ||
| <Text.Headline elementType="h2">{props.title}</Text.Headline> | ||
| </div> |
There was a problem hiding this comment.
The wrapping divs are for the margins 😕
|
I'm wondering if it might actually make sense to move |
My head goes like: no please not more repos 😄 |
What do you mean? 🤔 |
|
Maybe Malcolm means to have that new repo with VRT (percy) for all the common components like the PageNotFound 🤔 I like a lot the idea of having this Nicola 🥇 |
|
I mean if we triggered VRT tests everytime there was a chance in any of the packages, we would be wasting snaps and spending a lot of money, whereas if we had a way of running and requiring VRT when rollup config and or the components themselves changed, that would be more reasonable. |
|
We can run percy only if something in the package changed right? |
|
@emmenko yes, but if we enabled percy for the whole github repo, it would block merging of PRs that didn't run Percy. |
|
We could have a "global" run script called |
| @@ -25,6 +25,8 @@ | |||
| "test:watch": "jest --config jest.test.config.js --watch", | |||
| "playground:start": "yarn --cwd playground start", | |||
| "playground:build": "yarn --cwd playground build", | |||
| "storybook:build": "yarn --cwd packages/application-components storybook:build", | |||
There was a problem hiding this comment.
shouldn't these scripts be part of the application-components package?
There was a problem hiding this comment.
We should be able to use yarn workspace application-components storybook:build here? ref
|
Yeah, sorry for throwing us off track. Let's concentrate on Storybook in this PR of course |
|
I think what Malcom means is the github check for Percy? |
|
@emmenko exactly. |
| import { getDisplayName } from 'recompose'; | ||
| import warning from 'warning'; | ||
|
|
||
| export default function({ message = '' }) { |
There was a problem hiding this comment.
why is this here?
Is the intention then to import from app-kit when using deprecate-component in the future?
| />, | ||
| { route: '/my-project' } | ||
| ); | ||
| await waitForElement(() => |
There was a problem hiding this comment.
This would throw already so the test fails.
emmenko
force-pushed
the
nm-storybook
branch
from
25b33ec to
00049d3
Compare
The setup is similar to the one we have in ui-kit.
Additional changes
I cleaned up a bit the current components (PageNotFound, ServicePageResponseLayout) and used a css grid to center the elements in the page.
I also renamed
ServicePageResponseLayouttoMaintenancePageLayout, as the name was very confusing. However, theServicePageResponseLayoutexport is still available but marked as deprecated (so no breaking change necessary).