Skip to content

[BpkButton,BpkBadge,BpkCard] Colocate stories with components and convert to CSF3#4331

Open
Gert-Jan Vercauteren (gert-janvercauteren) wants to merge 3 commits intomainfrom
gert-janvercauteren/colocate-storybook
Open

[BpkButton,BpkBadge,BpkCard] Colocate stories with components and convert to CSF3#4331
Gert-Jan Vercauteren (gert-janvercauteren) wants to merge 3 commits intomainfrom
gert-janvercauteren/colocate-storybook

Conversation

@gert-janvercauteren

Summary

Pilot migration to colocate Storybook stories with component source files, preparing for NX monorepo migration. Migrates Button, Badge, and Card as proof of concept.

Changes:

  • Moved stories from examples/ into packages/*/src/*.stories.tsx for 3 pilot components
  • Converted all migrated stories from CSF2 (function exports) to CSF3 (satisfies Meta + StoryObj)
  • Merged the two-file pattern (examples.tsx + stories.tsx) into single colocated story files
  • Added build safeguards to prevent stories from shipping in the npm package:
    • babel.config.js: Added .stories to the ignore list
    • packages/.npmignore: Added *.stories.* and *.test.* patterns
    • scripts/scss/styles-prod.js: Skip .stories. SCSS files during compilation
  • Updated .storybook/main.ts with dual-glob discovery to support incremental migration

Validated:

  • Storybook builds successfully with colocated stories
  • npm pack --dry-run confirms zero story files in published output

Remember to include the following changes:

  • Ensure the PR title includes the name of the component you are changing so it's clear in the release notes for consumers of the changes in the version e.g [Clover-123][BpkButton] Updating the colour
  • README.md (If you have created a new component)
  • Component README.md
  • Tests
  • Accessibility tests
    • The following checks were performed:
      • Ability to navigate using a keyboard only
      • Zoom functionality (Deque University explanation):
        • The page SHOULD be functional AND readable when only the text is magnified to 200% of its initial size
        • Pages must reflow as zoom increases up to 400% so that content continues to be presented in only one column i.e. Content MUST NOT require scrolling in two directions (both vertically and horizontally)
      • Ability to navigate using a screen reader only
  • Storybook examples created/updated
  • For breaking changes or deprecating components/properties, migration guides added to the description of the PR. If the guide has large changes, consider creating a new Markdown page inside the component's docs folder and link it here

…vert to CSF3

Migrate stories from examples/ to packages/*/src/ for Button, Badge, and Card
as a pilot for full story colocation. Convert from CSF2 to CSF3 format. Add
build safeguards (babel ignore, .npmignore, SCSS skip) to prevent stories
from shipping in the npm package. Update storybook config with dual-glob
discovery to support incremental migration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Pilot migration to colocate Storybook stories alongside component source (Button, Badge, Card) and convert them to CSF3, while ensuring these story assets don’t ship in published packages.

Changes:

  • Moved Button/Badge/Card stories into packages/*/src/*.stories.tsx and converted from CSF2 → CSF3 (satisfies Meta + StoryObj).
  • Added/updated build + publish safeguards to exclude story files/styles from transpilation, packaging, and production SCSS compilation.
  • Updated Storybook config to discover both legacy examples/ stories and new colocated stories during the incremental migration.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
scripts/scss/styles-prod.js Skips compiling .stories. SCSS files in production styles build.
packages/bpk-component-card/src/BpkCard.stories.tsx New colocated CSF3 stories for Card.
packages/bpk-component-card/src/BpkCard.stories.module.scss Story-only SCSS module for Card stories.
packages/bpk-component-button/src/BpkButton.stories.tsx New colocated CSF3 stories for Button.
packages/bpk-component-button/src/BpkButton.stories.module.scss Story-only SCSS module for Button stories.
packages/bpk-component-badge/src/BpkBadge.stories.tsx New colocated CSF3 stories for Badge.
packages/bpk-component-badge/src/BpkBadge.stories.module.scss Story-only SCSS module for Badge stories.
packages/.npmignore Ensures story/test files (and story style modules) are excluded from published packages.
examples/bpk-component-card/stories.js Removes legacy examples-based Card story entrypoint.
examples/bpk-component-button/stories.tsx Removes legacy examples-based Button story entrypoint.
examples/bpk-component-badge/stories.tsx Removes legacy examples-based Badge story entrypoint.
babel.config.js Prevents .stories files from being transpiled as part of distribution build.
.storybook/main.ts Adds a new glob to discover colocated package stories alongside existing examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

import { cssModules } from '../../bpk-react-utils';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkThemeProvider from '../../bpk-theming';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an orphaned // @ts-expect-error directive (it is not directly followed by a statement that produces a TS error). With tsc running on the repo, this will fail typechecking with "Unused 'ts-expect-error' directive". Remove this extra directive or attach it to the line that actually needs the suppression.

Suggested change
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.

Copilot uses AI. Check for mistakes.
import { cssModules } from '../../bpk-react-utils';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
import BpkThemeProvider from '../../bpk-theming';
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an orphaned // @ts-expect-error directive (not directly followed by an error-producing statement). This will cause tsc to fail with "Unused 'ts-expect-error' directive". Remove the extra directive or place it immediately above the specific import/line that needs it.

Suggested change
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`.

Copilot uses AI. Check for mistakes.
Comment on lines +366 to +370
render: () => <DockedLeadingExample />,
};

export const DockedLeft: Story = {
render: () => <DockedTrailingExample />,
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DockedRight renders DockedLeadingExample (which uses docked="left"), and DockedLeft renders DockedTrailingExample (which uses docked="right"). This appears swapped and will make the story names misleading. Swap the rendered examples (or rename the exports) so the story name matches the docked prop value being demonstrated.

Suggested change
render: () => <DockedLeadingExample />,
};
export const DockedLeft: Story = {
render: () => <DockedTrailingExample />,
render: () => <DockedTrailingExample />,
};
export const DockedLeft: Story = {
render: () => <DockedLeadingExample />,

Copilot uses AI. Check for mistakes.
<BpkDividedCard
primaryContent={longMessage}
secondaryContent={shortContent}
href="http://www.skyscanner.net/"
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This story uses an http:// URL. Even in Storybook examples it's better to use https:// to avoid unnecessary redirects and mixed-content/security warnings in some environments.

Suggested change
href="http://www.skyscanner.net/"
href="https://www.skyscanner.net/"

Copilot uses AI. Check for mistakes.
);
};

ButtonStory.defaultProps = { className: null };
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ButtonStory.defaultProps = { className: null } relies on the legacy defaultProps pattern and sets className to null even though the prop is typed as string | undefined. Prefer using a default value in the function parameter (or normalizing with className ?? '') and drop defaultProps to keep typings and runtime behavior consistent.

Copilot uses AI. Check for mistakes.
…ed components

Clean up the remaining examples/ files (examples.tsx, SCSS, CSS) for the
three components whose stories were colocated in the previous commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@skyscanner-backpack-bot
Copy link

Visit https://backpack.github.io/storybook-prs/4331 to see this build running in a browser.

Remove Story type annotations from render-only stories to avoid TS errors
where StoryObj requires args for required component props. Remove unused
StoryObj imports. Add @ts-ignore for untyped bpk-storybook-utils module.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@skyscanner-backpack-bot
Copy link

Visit https://backpack.github.io/storybook-prs/4331 to see this build running in a browser.

@skyscanner-backpack-bot
Copy link

Warnings
⚠️

Package source files (e.g. packages/package-name/src/Component.js) were updated, but snapshots weren't. Have you checked that the tests still pass?

Browser support

If this is a visual change, make sure you've tested it in multiple browsers.

Generated by 🚫 dangerJS against 4121684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants