feat(docs): add F0ButtonDropdown Storybook documentation#3916
feat(docs): add F0ButtonDropdown Storybook documentation#3916desiree-np wants to merge 16 commits intomainfrom
Conversation
- Remove DocsNav and manual Overview/Guidelines/Code/Examples menu - Remove Purpose section (description under title serves this role) - Remove Code section (Controls in Anatomy is the interactive props table) - Add Modes section with stacked Canvas (Modes story) + table - Add Variants section with stacked Canvas (Variants story) + table - Add Sizes section with stacked Canvas (Sizes story) + table - Move Controls to Anatomy as interactive props reference - Add Modes, Variants, Sizes stories to stories file - Usage section retains named scenarios without tsx snippets
…n to use - Merge top-level Usage section into Guidelines as ### Usage examples - When to use now introduces split vs dropdown with prose + Canvas before the table - Add SplitModeExample and DropdownModeExample stories with realistic Factorial copy - Add CriticalItems story to illustrate critical action placement - Improve split vs dropdown explanation throughout
Headings inside Usage examples generate right-side nav anchors and clutter the menu. Replace #### with **bold** for scenario titles.
Modes, Variants and Sizes describe the structural forms of the component, not usage guidance — they belong under Anatomy as ###, not as ## top-level sections. This cleans up the right-side nav to: Anatomy · Guidelines · Accessibility
- Remove 'When to use' column from Modes table (Mode | Description only) - Replace generic 'Item 1/2/3' copy in Default story with realistic copy - Simplify Sizes table to Size | Description with component-specific notes - Remove redundant 'Dropdown mode with custom trigger label' usage example - Add generic type T and onClick signature documentation in Behavior section
…tem defaults - Drop ### Sizes from MDX: sm/md/lg behavior is identical across components, repeating the table adds noise without component-specific value - Update skill: ### Sizes is now opt-in, only when a component has size-specific behavior not covered by system defaults
When to use / When not to use / Do's and don'ts were #### headings, generating anchors in the right-side nav and cluttering it. Converted to **bold** in F0ButtonDropdown, F0Alert, and the f0-docs skill template.
Second DoDont (critical: true usage) now renders the component in both do/dont cards — the red color contrast makes the rule self-evident. First DoDont (verb-first labels) stays text-only — the difference is not visually distinguishable from the component alone.
…ine JSX Inline JSX fails because F0ButtonDropdown needs I18nProvider which is only available in story decorators. Use Canvas of dedicated DoDontCriticalDo/Dont stories instead — these pass through the decorator chain correctly.
…ion in cards - Add DoDontLabelsDo/Dont stories for verb-first label examples - Add Canvas children to first DoDont (labels) - Strip Canvas wrapper styles inside DoDonts to avoid double-card collision
…c to this component The rule belongs to button labeling in general, not ButtonDropdown specifically. The Content best practices section already covers it. Will live in F0Button docs.
- Remove redundant split/dropdown rows from When to use table (already in prose+Canvas) - Remove redundant prose from Critical items usage example (covered by DoDont) - Fix ambiguous When not to use row: add F0Button + F0Dialog as concrete alternatives - Clarify Modes table: split mode label/action come from selected item (value prop) - Update WithDescription story with realistic employee management copy
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
📦 Alpha Package Version PublishedUse Use |
🔍 Visual review for your branch is published 🔍Here are the links to: |
There was a problem hiding this comment.
Pull request overview
Adds manual Storybook MDX documentation for F0ButtonDropdown (moving away from autodocs), and updates Storybook utility/styles plus internal “skills” documentation to support the MDX workflow.
Changes:
- Introduces
F0ButtonDropdown.mdxand expandsF0ButtonDropdownstories to provide dedicated reference stories for MDX sections (modes/variants/guidelines/do-don’ts). - Updates
DoDontsStorybook utility styling to better renderCanvascontent inside do/don’t cards. - Updates internal skills docs (
f0-storybook-stories,f0-quality-gate,f0-docsreferences) to reflect the new MDX documentation workflow and formatting expectations.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/lib/storybook-utils/do-donts.tsx | Adjusts do/don’t rendering to avoid Storybook Canvas “double card” styling conflicts. |
| packages/react/src/components/F0ButtonDropdown/stories/F0ButtonDropdown.stories.tsx | Adds/updates reference stories used by the new MDX documentation and tweaks meta/story tags. |
| packages/react/src/components/F0ButtonDropdown/stories/F0ButtonDropdown.mdx | Adds full manual docs page for F0ButtonDropdown (anatomy, guidelines, usage, accessibility). |
| packages/react/.skills/f0-storybook-stories/SKILL.md | Updates stories authoring guidance and adds a new rule section. |
| packages/react/.skills/f0-quality-gate/SKILL.md | Updates the quality gate to include explicit formatting steps and MDX-related notes/checks. |
| packages/react/.skills/f0-docs/references/mdx-authoring.md | Adds a comprehensive MDX authoring reference for the f0-docs workflow. |
| import { FC, ReactNode } from "react"; | ||
|
|
||
| import { F0TagStatus } from "@/components/tags/F0TagStatus" | ||
| import { F0TagStatus } from "@/components/tags/F0TagStatus"; |
There was a problem hiding this comment.
This file now uses semicolons, but packages/react is formatted by oxfmt with semi:false (no semicolons). Please run pnpm format and keep the file consistent with the repo formatting rules so format:check passes.
| import type { Meta, StoryObj } from "@storybook/react-vite"; | ||
|
|
||
| import { expect, within } from "storybook/test" | ||
| import { expect, within } from "storybook/test"; | ||
|
|
||
| import { Add, Delete, Pencil, Replace, Save } from "@/icons/app/index.ts" | ||
| import { dataTestIdArgs } from "@/lib/data-testid/__stories__/args" | ||
| import { withSnapshot } from "@/lib/storybook-utils/parameters.ts" | ||
| import { Add, Delete, Pencil, Replace, Save } from "@/icons/app/index.ts"; | ||
| import { dataTestIdArgs } from "@/lib/data-testid/__stories__/args"; | ||
| import { withSnapshot } from "@/lib/storybook-utils/parameters.ts"; |
There was a problem hiding this comment.
This stories file is using semicolons throughout, which conflicts with the repo’s oxfmt config (semi:false) used by pnpm format / pnpm format:check. Please reformat (or remove semicolons) to match the established style so formatting checks don’t fail.
| import { Canvas, Meta, Controls, Unstyled } from "@storybook/addon-docs/blocks"; | ||
| import * as Stories from "./F0ButtonDropdown.stories"; | ||
| import { DoDonts } from "@/lib/storybook-utils/do-donts"; | ||
|
|
||
| <Meta of={Stories} /> | ||
|
|
There was a problem hiding this comment.
The repo already contains a __stories__/controls.mdx stub for this component. Adding F0ButtonDropdown.mdx without removing the stub will likely create duplicate/extra docs entries in the sidebar. Please delete the old controls.mdx (or otherwise ensure there is only one documentation entry).
| guidelines: [ | ||
| '"Delete employee" with critical: true — cannot be undone', | ||
| '"Archive record" with critical: true — requires deliberate intent', | ||
| ], | ||
| children: <Canvas of={Stories.DoDontCriticalDo} sourceState="none" />, | ||
| }} | ||
| dont={{ | ||
| description: | ||
| "Don't mark non-destructive actions as critical just to draw attention.", | ||
| guidelines: [ | ||
| '"Export report" with critical: true — exports are reversible', | ||
| '"Send notification" with critical: true — not destructive; trains users to ignore the warning', | ||
| ], | ||
| children: <Canvas of={Stories.DoDontCriticalDont} sourceState="none" />, |
There was a problem hiding this comment.
In the Do’s section, the guideline text says ""Archive record" with critical: true", but the referenced story (DoDontCriticalDo) does not mark Archive as critical. This makes the docs internally inconsistent—please align the guideline bullets with the actual examples (or update the story).
| - In dropdown mode all items appear in the menu. The `trigger` prop sets an independent label for the trigger button. | ||
| - The `value` prop (split mode) controls which item is currently selected. When omitted, defaults to the first item's value. | ||
| - `disabled` and `loading` apply to the entire control — both the main button and the chevron are affected. | ||
| - The component is generic: `F0ButtonDropdown<T>` where `T` defaults to `string`. The `value` field of each `ButtonDropdownItem<T>` and the first argument of `onClick: (value: T, item: ButtonDropdownItem<T>) => void` share the same type. Pass a union, enum, or any literal type as `T` to get fully typed item values. |
There was a problem hiding this comment.
The Behavior section claims the component is generic (F0ButtonDropdown<T>), but the exported component is not declared as a generic React component (it currently takes F0ButtonDropdownProps with the default T=string). As written, consumers can’t pass a type parameter in JSX, so this statement is misleading—please either adjust the docs to match current typing or update the component typing to actually be generic.
| - The component is generic: `F0ButtonDropdown<T>` where `T` defaults to `string`. The `value` field of each `ButtonDropdownItem<T>` and the first argument of `onClick: (value: T, item: ButtonDropdownItem<T>) => void` share the same type. Pass a union, enum, or any literal type as `T` to get fully typed item values. | |
| - Item values and the first argument passed to `onClick` are the same value. In split mode, `value` should match one of the item `value` fields. |
| <Meta title="F0ComponentName/Overview" /> | ||
| ``` | ||
|
|
||
| > Semicolons in imports are fine in MDX — `oxfmt` only applies to `.ts`/`.tsx` files. |
There was a problem hiding this comment.
This reference states that oxfmt only applies to .ts/.tsx and that semicolons are fine in MDX, but f0-quality-gate was updated to say .mdx files go through oxfmt formatting and must pass format:check. Please reconcile these instructions so authors don’t get conflicting guidance about MDX formatting.
| > Semicolons in imports are fine in MDX — `oxfmt` only applies to `.ts`/`.tsx` files. | |
| > MDX files also go through formatting checks, so follow the output required by `format:check` for import style and semicolons instead of assuming MDX is exempt from `oxfmt`. |
| ### Render-only stories must include `args` | ||
|
|
||
| Stories that use a custom `render` function without inheriting args from the meta | ||
| **must** include an explicit `args` property to satisfy the `Story` type constraint. | ||
| Omitting `args` causes a TypeScript error when the meta has required props. | ||
|
|
||
| ```tsx | ||
| // WRONG — TS error: Property 'args' is missing | ||
| export const Variants: Story = { | ||
| tags: ["!dev"], | ||
| render: () => <div>...</div>, | ||
| }; | ||
|
|
||
| // CORRECT — provide required args even if render ignores them |
There was a problem hiding this comment.
The new rule "Render-only stories must include args" appears incorrect/inconsistent with the existing codebase: many StoryObj<typeof meta> stories use render: () => ... without providing args and compile (e.g. F0BigNumber’s Skeleton story). Please clarify the actual condition under which args is required, or remove the rule to avoid misleading guidance.
| ### Render-only stories must include `args` | |
| Stories that use a custom `render` function without inheriting args from the meta | |
| **must** include an explicit `args` property to satisfy the `Story` type constraint. | |
| Omitting `args` causes a TypeScript error when the meta has required props. | |
| ```tsx | |
| // WRONG — TS error: Property 'args' is missing | |
| export const Variants: Story = { | |
| tags: ["!dev"], | |
| render: () => <div>...</div>, | |
| }; | |
| // CORRECT — provide required args even if render ignores them | |
| ### `args` in render-only stories | |
| A custom `render` function does **not** automatically require an `args` property. | |
| Add story-level `args` only when the `StoryObj<typeof meta>` type still needs required | |
| component props to be satisfied, and those props are not already provided via `meta.args`. | |
| Omitting `args` can cause a TypeScript error when the meta's component has required props. | |
| But if there are no required props to satisfy (or they are already covered by `meta.args`), | |
| a render-only story without `args` is valid. | |
| ```tsx | |
| // VALID — no required args to satisfy for this story | |
| export const Skeleton: Story = { | |
| tags: ["!dev"], | |
| render: () => <div>...</div>, | |
| }; | |
| // ALSO VALID — provide required args when the Story type needs them |
| @@ -35,10 +35,10 @@ const meta = { | |||
| component: F0Example, | |||
| tags: ["autodocs", "stable"], // or "experimental" | "internal" | "deprecated" | |||
| // ... | |||
| } satisfies Meta<typeof F0Example> | |||
| } satisfies Meta<typeof F0Example>; | |||
|
|
|||
| export default meta | |||
| type Story = StoryObj<typeof meta> | |||
| export default meta; | |||
| type Story = StoryObj<typeof meta>; | |||
There was a problem hiding this comment.
The Storybook stories skill now shows semicolons in the example imports/exports, but packages/react is formatted by oxfmt with semi:false (no semicolons). To avoid teaching a style that will immediately be reformatted (and cause noisy diffs), please update the code examples to match the enforced formatting.
| import { Canvas, Meta, Controls, Unstyled } from "@storybook/addon-docs/blocks"; | ||
| import * as Stories from "./F0ButtonDropdown.stories"; | ||
| import { DoDonts } from "@/lib/storybook-utils/do-donts"; |
There was a problem hiding this comment.
This new MDX file uses semicolons in the import lines. Given the repo’s formatting rules (oxfmt semi:false) and the updated quality gate stating MDX should pass format:check, please format MDX imports consistently (no semicolons) to avoid formatting-check failures/noisy diffs.
| import { Canvas, Meta, Controls, Unstyled } from "@storybook/addon-docs/blocks"; | |
| import * as Stories from "./F0ButtonDropdown.stories"; | |
| import { DoDonts } from "@/lib/storybook-utils/do-donts"; | |
| import { Canvas, Meta, Controls, Unstyled } from "@storybook/addon-docs/blocks" | |
| import * as Stories from "./F0ButtonDropdown.stories" | |
| import { DoDonts } from "@/lib/storybook-utils/do-donts" |
…ies and apply oxfmt - Stories with render-only pattern need args to satisfy TypeScript Story type - oxfmt formatting applied to stories and MDX Implemented-with: factorial-f0
Coverage Report for packages/react
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Implemented-with: factorial-f0
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
packages/react/.skills/f0-quality-gate/SKILL.md:85
- The quality gate instructions use
pnpm lint-fix, butpackages/react/package.jsondefines the script aslint:fix(with a colon). As written, the command will fail; please update the doc to the correct script name.
### 1c. Lint
```bash
pnpm lint-fix
pnpm lint
</details>
| }, | ||
| }, | ||
| tags: ["autodocs"], | ||
| tags: ["stable"], |
There was a problem hiding this comment.
Storybook has global tags: ["autodocs"] in .storybook/preview.tsx, so setting tags: ["stable"] here does not opt this component out of autodocs. If the goal is to replace autodocs with the manual MDX page, add the opt-out tag (e.g. include "!autodocs") to prevent an extra autogenerated docs entry.
| tags: ["stable"], | |
| tags: ["stable", "!autodocs"], |
| "Mark only genuinely destructive or irreversible actions as critical.", | ||
| guidelines: [ | ||
| '"Delete employee" with critical: true — cannot be undone', | ||
| '"Archive record" with critical: true — requires deliberate intent', |
There was a problem hiding this comment.
The “Do’s” guidelines mention "Archive record" with critical: true, but the referenced DoDontCriticalDo story does not mark the archive action as critical (only delete is critical). Update the guideline text (or the story) so the written rule matches the visual example.
| '"Archive record" with critical: true — requires deliberate intent', | |
| '"Archive record" without critical: true — secondary but not destructive', |
| - [ ] Components rendered as JSX in MDX (e.g. inside `<DoDonts children>`) are imported via relative path (`from "../F0Component"`), never from `@factorialco/f0-react` (causes module fetch error in dev) | ||
| - [ ] `## Accessibility` included only when the component type requires it — complex interactive (required), simple interactive (only if non-obvious), static display (only for live region or icon meaning), layout/typography (omit entirely) | ||
| - [ ] Optional function props use `control: "boolean"` in `argTypes` + `render` function to interpret boolean → `fn()` or `undefined` (never `control: "function"`) | ||
| - [ ] No semicolons in `.tsx` import statements (oxfmt rule — does not apply to `.mdx` files) |
There was a problem hiding this comment.
The checklist states the no-semicolons rule “does not apply to .mdx files”, which contradicts the formatting guidance elsewhere in this repo and the existing MDX style (imports are consistently written without semicolons). Update this checklist item so it doesn’t instruct authors to write MDX in a different style than what pnpm format/the repo expects.
| - [ ] No semicolons in `.tsx` import statements (oxfmt rule — does not apply to `.mdx` files) | |
| - [ ] No semicolons in import statements, including in `.tsx` and `.mdx` files |
| ```tsx | ||
| import type { Meta, StoryObj } from "@storybook/react-vite" | ||
| import { fn, expect, within } from "storybook/test" | ||
| import { withSnapshot, withSkipA11y } from "@/lib/storybook-utils/parameters" | ||
| import type { Meta, StoryObj } from "@storybook/react-vite"; | ||
| import { fn, expect, within } from "storybook/test"; | ||
| import { withSnapshot, withSkipA11y } from "@/lib/storybook-utils/parameters"; | ||
| // Icons: "@/icons/app" | "@/icons/modules" | "@/icons/ai" |
There was a problem hiding this comment.
The TSX examples in this skill file use semicolons, but packages/react is formatted with semi:false (and the quality gate explicitly calls out “no semicolons”). Updating these snippets to match the repo’s actual output will avoid copy/paste introducing formatting diffs or failing pnpm format:check.
| ```tsx | ||
| const meta = { | ||
| title: "ComponentName", | ||
| component: F0Example, | ||
| tags: ["autodocs", "stable"], // or "experimental" | "internal" | "deprecated" | ||
| // ... | ||
| } satisfies Meta<typeof F0Example> | ||
| } satisfies Meta<typeof F0Example>; | ||
|
|
||
| export default meta | ||
| type Story = StoryObj<typeof meta> | ||
| export default meta; | ||
| type Story = StoryObj<typeof meta>; | ||
| ``` |
There was a problem hiding this comment.
The Meta example adds semicolons after satisfies Meta and export default meta, which doesn’t match packages/react formatting (semi:false). Consider removing semicolons in these snippets so they reflect what developers will actually see after pnpm format.
Summary
F0ButtonDropdownreplacing autodocsT), and Usage examplesCanvas sourceState="none"as childrendo-donts.tsxto neutralize double-card CSS collision when Canvas is used inside cardsRelated
Implemented-with: factorial-f0