feat: upgrade Storybook to v7 #6499

nineteen88 · 2024-01-02T10:17:44Z

Proposed behaviour

Upgrade Storybook to v7
Upgrade ESLint to support MDX files
Convert all stories to be CSF3 compatible

Current behaviour

Storybook is on v6

Checklist

Commits follow our style guide
Related issues linked in commit messages if required
Storybook added or updated if required
Translations added or updated (including creating or amending translation keys table in storybook) if required
Typescript d.ts file added or updated if required
Related docs have been updated if required

QA

Tested in CodeSandbox/storybook
Add new Cypress test coverage if required
Carbon implementation matches Design System/designs
UI Tests GitHub check reviewed if required

codesandbox-ci · 2024-01-04T17:10:00Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c3a4371:

Sandbox	Source
carbon-quickstart	Configuration
carbon-quickstart-typescript	Configuration

tsconfig.json

robinzigmond · 2024-01-08T15:01:16Z

.eslintrc

@@ -93,7 +93,17 @@
          }
        ]
      }
-    ]
+    ],
+    /* These rules have been added to allow us to upgrade during Storybook upgrae


Suggested change

/* These rules have been added to allow us to upgrade during Storybook upgrae

/* These rules have been added to allow us to upgrade during Storybook upgrade

suggestion: typo

Suggested change

/* These rules have been added to allow us to upgrade during Storybook upgrae

/* These rules have been added to allow us to upgrade Storybook

.eslintrc

robinzigmond · 2024-01-08T15:32:13Z

.eslintrc

+    {
+      "files": ["*.mdx"],
+      "extends": "plugin:mdx/recommended",
+      "rules": {


comment (non-blocking): unlike the rules above, I'm really not keen on turning these off because they're about accessibility, not just code style. I've just played with it a bit and I'm not sure why we're getting flagged for these two - a lot of the failures seem to be in the same places for both and relate to anchor tags that have perfectly clear text content, so I don't know why that's failing.

As for self-closing-comp, this matters less (imo) but it should be an easy thing to fix where we have problems. Again though this is coming up in the same places, where it really shouldn't - so I think eslint is somehow misreading our code and this is giving rise to all 3 rule failures.

Happy to leave for now and open a new ticket to investigate - but I don't think we should just brush this under the carpet, and in that new ticket we should make a note to turn these rules back on when we've fixed whatever is going on.

I'll double check these rules as per your comment when I go through all the suggestions etc but for the record we've never had linting on mdx files and as I recall when I've added it these weren't possible to "fix" as there was nothing actually wrong. As I say, I will double check this though and you're right we could maybe do some investigation but there is a ticket created to go through all of our linting rules and strip it back as there's a lot of rules that we're putting in just because it's in airbnb's ruleset and I believe it would better to simplify this a fair bit and have more control for the team

ah yes, I'd missed that we didn't have any linting on mdx before.

As I said, these seem to be failing for no good reason as they're on code that absolutely shouldn't fail these rules. Happy to have a new ticket to investigate so this PR doesn't get bogged down with it, but I don't want to just disable important accessibility-related checks and forget about it, even if we end up going with this as a temporary fix for now.

.eslintrc

.storybook/main.js

robinzigmond · 2024-01-08T16:39:43Z

.storybook/main.js

@@ -31,6 +35,7 @@ module.exports = {
    "@storybook/addon-google-analytics",
    "@storybook/addon-links",
    "@storybook/addon-toolbars",
+    "@storybook/addon-mdx-gfm",


comment (non-blocking): I'm sure you're aware, but according to the docs for this addon it's not going to be around for long and we should really be doing this "properly". I've no idea how big a job this would be - but we definitely should add a new task for this if we can't easily do it now, or we'll have another large-ish task for that in the near future!

Again, this is very likely just an oversight. Storybook automatically add it when you run their upgrade wizard to 7 to assist in the update. I don't think it should be needed for us now as I think I've updated all MDX files accordingly but I will check to make sure before removing it otherwise I'll have to raise a ticket to ensure we complete that conversion

.storybook/main.js

.storybook/preview.js

.storybook/utils/styled-system-props.ts

.storybook/utils/translation-keys-table.tsx

.vscode/settings.json

docs/dev-environment-setup.mdx

robinzigmond · 2024-01-09T11:54:10Z

docs/testing-guide.mdx

@@ -1,3 +1,7 @@
+import { Meta } from "@storybook/addon-docs";


comment as with the dev-environment-setup file above, I think this is supposed to be a guide on Github and shouldn't be in our public-facing docs on storybook

docs/validations.stories.tsx

robinzigmond · 2024-01-10T09:52:31Z

src/components/accordion/accordion-group/accordion-group.stories.tsx

comment (non-blocking): I don't particularly like calling this file .stories.tsx when we're not actually using any "stories" from it, and are purely using it to generate the props table. Only an opinion of course, maybe there's a good reason for keeping this name. Although I also wonder if by changing the naming convention, so they won't be found by looking for all the files with this ending, we can remove the need for the isHidden tag, which would be another advantage to renaming (although I appreciate that it would be a lot of effort to change now, so maybe not worth worrying over)?

Obviously the same applies to the many similar files across all components.

src/components/action-popover/action-popover.mdx

src/components/action-popover/action-popover.stories.tsx

src/components/box/box.mdx

src/components/box/box.stories.tsx

docs/codebase-overview.mdx

robinzigmond · 2024-01-10T16:23:25Z

docs/codebase-overview.mdx

comment (non-blocking): I also notice that this file has extensive documentation on how we use Storybook, some of which will certainly need updating as it's not correct with the changes you've made for Storybook v7 (eg argsTable has been renamed, and there is no noHeader prop on it). Happy for this to be a separate task though rather than done as this PR (we might even decide to remove this file entirely, if we think the documentation isn't useful and/or no-one reads it!)

src/components/carbon-provider/carbon-provider.mdx

src/components/card/card.stories.tsx

src/components/checkbox/checkbox-validations.mdx

src/components/checkbox/checkbox-validations.stories.tsx

src/components/content/content-test.stories.mdxt

src/components/content/content-test.stories.tsxt

src/components/decimal/decimal.stories.tsx

src/components/fieldset/fieldset-examples.mdx

src/components/file-input/file-input.mdx

src/components/radio-button/components.test-pw.tsx

robinzigmond · 2024-01-12T16:46:17Z

src/components/radio-button/radio-button-test.stories.tsx

-        />
-      </div>
-    </>
+    <div


suggestion (non-blocking): another div that could be replaced with a Box

robinzigmond

praise: great work to get this huge update over the line. I especially admire the attention to detail, including fixing various little details (eg wording fixes) that came up along the way. Great job 👍

src/components/text-editor/text-editor.mdx

src/components/text-editor/text-editor.stories.tsx

tsconfig.json

Parsium · 2024-03-18T15:50:29Z

src/components/anchor-navigation/anchor-navigation.mdx

@@ -104,22 +96,22 @@ const MyComponent = () => (

 `AnchorNavigation` can be placed anywhere on the page to help with navigating long scrollable sections.

-<Story name="default">{DefaultStory.bind({})}</Story>
+<Story of={AnchorNavigationStories.DefaultStory} />


comment(non-blocking): realise QA had noted this as a change from production, but we will want to at a later date put this story in a canvas since there is no way to view the story source code otherwise.

This is the same on master currently. To be sorted as part of the Story reviews I guess

Parsium · 2024-03-19T10:21:22Z

src/components/step-sequence/step-sequence.mdx

+<DeprecationWarning>
+  StepSequence has been deprecated in Carbon and the Design System.
+  <br />
+  It is recommended to use the StepFlow component - which is an accessible
+  alternative.
+</DeprecationWarning>


issue(non-blocking): I noticed a small styling change to the rendered DeprecationWarning component compared with production. It seems the inner text is now rendered as a paragraph tag inside the component, meaning certain style rules are not being overridden.

New style changes

Current styles in production

Parsium · 2024-03-19T10:32:57Z

src/components/global-header/global-header.stories.tsx

+  decorators: [
+    (Story) => (
+      <div style={{ position: "relative", height: "250px" }}>
+        <Story />
+      </div>
+    ),
+  ],


praise: the docs page loads much more quickly now 👍🏼

Parsium · 2024-03-19T11:14:27Z

src/components/global-header/global-header.stories.tsx

 GlobalLocalNavBarLayout.parameters = {
  docs: { inlineStories: false, iframeHeight: 250 },
 };


question(non-blocking): do we need to make this an inline story given we are wrapping the component in a containing block via the decorator on line 29?

nineteen88 requested review from a team as code owners January 2, 2024 10:17

nineteen88 force-pushed the FE-5749-storybook-upgrade branch 2 times, most recently from 95c3392 to 29ac72c Compare January 2, 2024 10:39

nineteen88 marked this pull request as draft January 2, 2024 10:39

nineteen88 added Pending Review Pending QA No UX QA Required Squad Front End labels Jan 2, 2024

nineteen88 force-pushed the FE-5749-storybook-upgrade branch 5 times, most recently from 5131f82 to 26e97b3 Compare January 4, 2024 16:03

nineteen88 force-pushed the FE-5749-storybook-upgrade branch 2 times, most recently from e34613b to 00afd6b Compare January 4, 2024 17:24

robinzigmond self-requested a review January 8, 2024 14:45

nineteen88 commented Jan 8, 2024

View reviewed changes

tsconfig.json Outdated Show resolved Hide resolved

robinzigmond reviewed Jan 8, 2024

View reviewed changes

robinzigmond reviewed Jan 9, 2024

View reviewed changes

Parsium self-requested a review January 9, 2024 14:49

robinzigmond reviewed Jan 10, 2024

View reviewed changes

nineteen88 force-pushed the FE-5749-storybook-upgrade branch from 00afd6b to 0616196 Compare January 11, 2024 10:49

robinzigmond reviewed Jan 11, 2024

View reviewed changes

src/components/fieldset/fieldset-examples.mdx Show resolved Hide resolved

src/components/file-input/file-input.mdx Show resolved Hide resolved

robinzigmond reviewed Jan 12, 2024

View reviewed changes

robinzigmond reviewed Jan 15, 2024

View reviewed changes

src/components/text-editor/text-editor.mdx Outdated Show resolved Hide resolved

src/components/text-editor/text-editor.stories.tsx Show resolved Hide resolved

tsconfig.json Outdated Show resolved Hide resolved

nineteen88 force-pushed the FE-5749-storybook-upgrade branch from 0616196 to 2de70ee Compare January 18, 2024 16:23

robinzigmond previously approved these changes Mar 18, 2024

View reviewed changes

Parsium reviewed Mar 19, 2024

View reviewed changes

Parsium previously approved these changes Mar 19, 2024

View reviewed changes

Parsium dismissed stale reviews from robinzigmond and themself via 37fdeb7 March 20, 2024 16:04

Parsium force-pushed the FE-5749-storybook-upgrade branch from dc7fc70 to 601ef83 Compare March 20, 2024 16:30

nineteen88 force-pushed the FE-5749-storybook-upgrade branch 5 times, most recently from 48b32a9 to 35c1b0b Compare March 25, 2024 16:20

edleeks87 assigned nineteen88 Mar 25, 2024

nineteen88 force-pushed the FE-5749-storybook-upgrade branch 8 times, most recently from bd4936a to 361a323 Compare March 28, 2024 14:27

DipperTheDan mentioned this pull request Mar 28, 2024

chore(deps): bump the npm_and_yarn group group with 2 updates #6652

Closed

nineteen88 force-pushed the FE-5749-storybook-upgrade branch from 361a323 to 7cb4558 Compare March 28, 2024 16:04

nineteen88 added Passed QA and removed Pending QA labels Mar 28, 2024

docs(storybook): upgrade storybook to v7

750af75

nineteen88 force-pushed the FE-5749-storybook-upgrade branch from 7cb4558 to 750af75 Compare March 28, 2024 16:11

edleeks87 approved these changes Mar 28, 2024

View reviewed changes

stephenogorman approved these changes Mar 28, 2024

View reviewed changes

nineteen88 merged commit 427ee40 into master Mar 28, 2024
21 checks passed

nineteen88 deleted the FE-5749-storybook-upgrade branch March 28, 2024 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade Storybook to v7 #6499

feat: upgrade Storybook to v7 #6499

nineteen88 commented Jan 2, 2024 •

edited

Loading

codesandbox-ci bot commented Jan 4, 2024 •

edited

Loading

robinzigmond Jan 8, 2024

edleeks87 Jan 9, 2024

robinzigmond Jan 8, 2024

nineteen88 Jan 8, 2024

robinzigmond Jan 9, 2024

robinzigmond Jan 8, 2024

nineteen88 Jan 8, 2024

robinzigmond Jan 9, 2024

robinzigmond Jan 10, 2024

robinzigmond Jan 10, 2024

robinzigmond Jan 12, 2024

robinzigmond left a comment

Parsium Mar 18, 2024

nineteen88 Mar 19, 2024

Parsium Mar 19, 2024

Parsium Mar 19, 2024

Parsium Mar 19, 2024

	/* These rules have been added to allow us to upgrade during Storybook upgrae
	/* These rules have been added to allow us to upgrade during Storybook upgrade

		@@ -1,3 +1,7 @@
		import { Meta } from "@storybook/addon-docs";

feat: upgrade Storybook to v7 #6499

feat: upgrade Storybook to v7 #6499

Conversation

nineteen88 commented Jan 2, 2024 • edited Loading

Proposed behaviour

Current behaviour

Checklist

QA

codesandbox-ci bot commented Jan 4, 2024 • edited Loading

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

Choose a reason for hiding this comment

robinzigmond 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

New style changes

Current styles in production

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nineteen88 commented Jan 2, 2024 •

edited

Loading

codesandbox-ci bot commented Jan 4, 2024 •

edited

Loading