Repository navigation
Conversation
Reviewer's GuideUpgrades Storybook and related tooling to v10, reworks CSS/SCSS handling into a custom Webpack config, updates the custom theme addon and theme metadata sourcing to align with Storybook v10 APIs, adjusts preview/a11y/viewport configuration, and applies minor formatting/logic cleanups across components and stories. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Preview is ready. |
|
🎭 Component Tests Report is ready. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new global Webpack rule for
/\.(css|scss)$/in.storybook/main.tswill also process styles fromnode_modules; consider adding aninclude(e.g. your source directory) orexclude: /node_modules/to avoid unexpected loader interactions and build slowdowns. - The theme addon now infers dark mode via a hardcoded
DARK_THEMESset instead ofgetThemeType; double-check that all existing and future theme values (including any brand or HC variants) are still mapped correctly to light/dark manager themes to avoid regressions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new global Webpack rule for `/\.(css|scss)$/` in `.storybook/main.ts` will also process styles from `node_modules`; consider adding an `include` (e.g. your source directory) or `exclude: /node_modules/` to avoid unexpected loader interactions and build slowdowns.
- The theme addon now infers dark mode via a hardcoded `DARK_THEMES` set instead of `getThemeType`; double-check that all existing and future theme values (including any brand or HC variants) are still mapped correctly to light/dark manager themes to avoid regressions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }, | ||
| jsx: {showFunctions: true}, // To show functions in sources | ||
| backgrounds: {disable: true}, | ||
| backgrounds: {disabled: true}, |
| 'qa' | 'className' | 'value' | 'defaultValue' | 'multiple' | 'ariaLabel' | ||
| > | ||
| > { | ||
| interface AccordionProviderProps<Multiple extends boolean> extends Required< |
There was a problem hiding this comment.
Why formating changed? New Prettier version?
There was a problem hiding this comment.
Yes
The new version of @storybook/cli has a dependency with version prettier@3.8.3
There was a problem hiding this comment.
Let's remove cli dependency. We dont use it anyway
| const [{theme}] = useGlobals(); | ||
| React.useEffect(() => { | ||
| api.setOptions({theme: themes[getThemeType(theme)]}); | ||
| api.setOptions({theme: themes[DARK_THEMES.has(theme) ? 'dark' : 'light']}); |
There was a problem hiding this comment.
What's wrong with the getThemeType utility?
There was a problem hiding this comment.
I checked and returned it as it was. Everything is working
| webpackConfig.resolve = webpackConfig.resolve ?? {}; | ||
| webpackConfig.resolve.alias = { | ||
| ...(webpackConfig.resolve.alias ?? {}), | ||
| '@storybook/addon-docs$': '@storybook/addon-docs/blocks', |
There was a problem hiding this comment.
Why we add this alias if we use normal path anyway ('@storybook/addon-docs/blocks)?
There was a problem hiding this comment.
Not everywhere. I replaced all the imports and removed this alias.
| }; | ||
|
|
||
| webpackConfig.module = webpackConfig.module ?? {rules: []}; | ||
| webpackConfig.module.rules = webpackConfig.module.rules ?? []; |
There was a problem hiding this comment.
Why we need these fallbacks for config properties?
There was a problem hiding this comment.
Because webpackConfig.module.rules is possibly undefined
There was a problem hiding this comment.
How it's different from what's the @storybook/addon-styling-webpack is doing?
| title: 'Theme', | ||
| render: () => { | ||
| return <Tool api={api} />; | ||
| return React.createElement(Tool, {api}); |
There was a problem hiding this comment.
What's the purpose of this change?
There was a problem hiding this comment.
I replaced JSX with React.createElement because Storybook 10 loads the preset without TSX-loader.
| }; | ||
|
|
||
| webpackConfig.module = webpackConfig.module ?? {rules: []}; | ||
| webpackConfig.module.rules = webpackConfig.module.rules ?? []; |
There was a problem hiding this comment.
How it's different from what's the @storybook/addon-styling-webpack is doing?
| }, | ||
| }, | ||
| globalTypes: { | ||
| theme: { |
There was a problem hiding this comment.
Let's capitalize the parameter names in the toolbar, Storybook now shows a tooltip with that name
| "npm-run-all": "^4.1.5", | ||
| "postcss": "^8.5.6", | ||
| "prettier": "^3.5.3", | ||
| "prettier": "3.5.3", |
There was a problem hiding this comment.
returned it as it was
# Conflicts: # package-lock.json # src/components/layout/LayoutProvider/__stories__/Layout.mdx # src/stories/Branding/Overview.mdx
# Conflicts: # package-lock.json

Summary by Sourcery
Upgrade Storybook tooling to v10 and update related configuration and theming to maintain compatibility.
Enhancements:
Build: