fix(#1216): storybook config #2310

ofhope · 2023-07-17T11:20:16Z

Related to #1216

It looks like Storybook has been set up previously but is currently in a broken state.

I was attempting to explore the project via Storybook and notice the commands weren't working.

The changes in package.json were due to running this command npx storybook@latest init which took me through an install wizard. These are the packages Storybook recommends.

addDecorator, addParameters and withKnobs are no longer supported. Controls are created for you through prop introspection with the ability to override with argTypes.

I'm not familiar with the work prior to these changes so I can't offer garuntee that thats how these stories behaved prior to this upgrade.

But I can confirm all stories are now rendering.

I notice one of the initial motivations were "help reduce bugs and make contributing easier" which I absolutely agree with. It makes debugging hard to reach aspect a breeze. It also helps seperate concerns between stylistic/ux concerns and business logic.

Changes:

I have verified that this pull request:

has no linting errors (npm run lint)
has no test errors (npm run test)
is from a uniquely-named feature branch and is up to date with the develop branch.
is descriptively named and links to an issue number, i.e. Fixes #123

welcome · 2023-07-17T11:20:19Z

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already.

lindapaiste · 2023-07-17T17:25:31Z

Thank you so much for looking into this! I've never been able to get Storybook to work so I'm uncertain of what it looked like before. It does seem from you screenshots that the components which use styled-components are rendering properly, but the ones that use .scss files aren't getting their styles loaded. Do you have any ideas about loading .scss files? No pressure if you don't, this is still a huge improvement.

lindapaiste · 2023-07-17T17:28:31Z

.storybook/main.js

  stories: ['../client/**/*.stories.(jsx|mdx)'],
  addons: [
-    '@storybook/addon-actions',
-    '@storybook/addon-docs',
-    '@storybook/addon-knobs',


I noticed that you removed some packages from the addons array here but kept them in the package.json. Do they need to be kept as dependencies or should they be removed?

They should be removed now. They've been deprecated https://www.npmjs.com/package/@storybook/addon-knobs

Actions are built in by default. And knobs have been replaced with args and argTypes where you can configure controls to play with. I've updated Buttons and Icons stories to use these rather than the knobs that were there.

lindapaiste · 2023-07-17T17:33:05Z

.storybook/preview.js

-}));
+export const decorators = [
+  (Story) => (
+      <Provider store={store}>


If we are connecting to Redux here then maybe we want a way to modify the initial state on a per-story basis? Just a thought. That's something for a future PR.

I'm not sure what Redux is used for but all components with stories needed it. It could be good to try and remove it as a dependancy from Storybook/Design System components 🤷🏽

Yeah I actually agree. I think of Storybook as being for pure UI components which are not connected to Redux. But the current FileNode and the SocialAuthButton components are connected and they have stories so that's why we need it for now.

lindapaiste

Looks great overall! We do need to fix the eslint setup before merging.

package.json

ofhope · 2023-07-18T01:50:47Z

@lindapaiste It looks like the css is already built so it was just a matter of including the css file 🎉

import '../client/styles/build/css/main.css'

These screen shots look better

ofhope · 2023-07-18T23:23:57Z

@lindapaiste so it looks like including the storybook eslint rules in .eslintrc works. It took an eslint restart for it to start prompting with misconfigured stories.

"extends": ["airbnb", "prettier", "plugin:storybook/recommended"],

As a test commenting out the icon stories and looking at the warnings we can see eslint(storybook/default)

The file should have a default export.

And if you fix that you get another error about having at least one story.

lindapaiste · 2023-07-22T21:26:34Z

Right now my only concern is the changes in unrelated files due to Prettier. Ideally we want it to pass the linting checks without changing anything. I noticed in your package-lock.json file that there's a minor version bump of prettier and this could be the cause.

Can you try the following steps?

git revert 1c3748d7d349040793ef6c74c168b998f7098262 To rollback the linting changes to FileNode, etc. from this commit.
npm install [email protected] to install the specific version of prettier that we had previously.
npm run lint -- does it pass?

I definitely want to get this merged in ASAP so that we can keep building on it. Now that you've got the basics of Storybook working I'm seeing problems and areas for improvement in the stories and in the components themselves. I've been reading some docs to figure out how to make the most out of Storybook and I've figured out theme switching, selecting icons from a dropdown on the Button story, etc.

lindapaiste · 2023-07-22T21:37:44Z

client/common/Button.stories.jsx

-    label={text('label', 'submit')}
-  >
-    {text('children', 'this is the button')}
+export const AllFeatures = (args) => (


The controls on this story don't seem to do anything because we aren't passing down all of the args. Looks like we can write the story as an object with the new "Component Story Format 3". Very cool.

export default { title: 'Common/Button', component: Button } export const AllFeatures = { args: { children: 'this is the button' } };

Thanks for investigating linting... I mistakenly thought these were linting issues on the main branch. I wasn't giving them much thought. I'll try your suggestion later today and see if I can revert them.

Yeah with stories you can set some defaults for all stories. Customise default args per story. Or present them with fixed values too if you like.

Previously this AllFeatures story had dynamic args as knobs and all the reset were fixed.

This is a button story from a personal project where args were overridden.

const Template: Story<ButtonProps> = (args) => <Button {...args} />; export const FullWidth = Template.bind({}); FullWidth.args = { primary: true, children: 'Button', fullWidth: true };

This reverts commit 1c3748d.

ofhope · 2023-07-23T06:59:39Z

One other addition. I ran npm uninstalled @storybook/addons-knobs this is deprecated.

lindapaiste

🚀 Thank you for all of your work on this!

I'm going to go ahead and merge this. Feel free to keep adding other improvements in additional PRs.

lindapaiste · 2023-07-23T16:45:29Z

Ah 💩, one last thing. Can you please commit the package-lock.json file after running npm install [email protected]?

lindapaiste

Thank you!

fix: storybook config

a3ffa62

lindapaiste reviewed Jul 17, 2023

View reviewed changes

lindapaiste requested changes Jul 17, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

fix: include built css in storybook preview

76aca03

Alexis and others added 5 commits July 18, 2023 11:51

fix: undefined arg value

ae0a1f7

fix: linting

1c3748d

Merge branch 'develop' into upgrade-storybook

602505e

fix: revert eslint rule in package.json

abab128

fix: include storybook linting in eslint

7eccdd8

ofhope requested a review from lindapaiste July 19, 2023 23:32

lindapaiste reviewed Jul 22, 2023

View reviewed changes

ofhope and others added 2 commits July 23, 2023 13:05

Revert "fix: linting"

87df250

This reverts commit 1c3748d.

chore: remove depreacted knobs

26c0102

lindapaiste previously approved these changes Jul 23, 2023

View reviewed changes

chore: reinstall prettier to refresh lockfile

6b73804

ofhope dismissed lindapaiste’s stale review via 6b73804 July 23, 2023 20:06

lindapaiste approved these changes Jul 24, 2023

View reviewed changes

lindapaiste merged commit c9b0c4d into processing:develop Jul 24, 2023
1 check passed

ofhope mentioned this pull request Jul 24, 2023

fix: storybook i18n strings #2324

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#1216): storybook config #2310

fix(#1216): storybook config #2310

ofhope commented Jul 17, 2023 •

edited

Loading

welcome bot commented Jul 17, 2023

lindapaiste commented Jul 17, 2023

lindapaiste Jul 17, 2023

ofhope Jul 17, 2023

lindapaiste Jul 17, 2023

ofhope Jul 18, 2023

lindapaiste Jul 18, 2023

lindapaiste left a comment

ofhope commented Jul 18, 2023

ofhope commented Jul 18, 2023

lindapaiste commented Jul 22, 2023

lindapaiste Jul 22, 2023

ofhope Jul 22, 2023

ofhope Jul 23, 2023

ofhope commented Jul 23, 2023

lindapaiste left a comment

lindapaiste commented Jul 23, 2023

lindapaiste left a comment

fix(#1216): storybook config #2310

fix(#1216): storybook config #2310

Conversation

ofhope commented Jul 17, 2023 • edited Loading

welcome bot commented Jul 17, 2023

lindapaiste commented Jul 17, 2023

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

lindapaiste left a comment

Choose a reason for hiding this comment

ofhope commented Jul 18, 2023

ofhope commented Jul 18, 2023

lindapaiste commented Jul 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofhope commented Jul 23, 2023

lindapaiste left a comment

Choose a reason for hiding this comment

lindapaiste commented Jul 23, 2023

lindapaiste left a comment

Choose a reason for hiding this comment

ofhope commented Jul 17, 2023 •

edited

Loading