Skip to content

feat(chip): promote OneChip to stable F0Chip component#3817

Open
eliseo-juan wants to merge 3 commits intomainfrom
feat/f0chip-stable-migration
Open

feat(chip): promote OneChip to stable F0Chip component#3817
eliseo-juan wants to merge 3 commits intomainfrom
feat/f0chip-stable-migration

Conversation

@eliseo-juan
Copy link
Copy Markdown
Contributor

🚪 Why?

Problem

The Chip component lived in src/experimental/OneChip/ but was used by multiple stable components (F0ChipList, F0Select, OneFilterPicker). It had no public export from the experimental barrel, its accessible semantics were broken (clickable <div> with nested <button>, Close button label was always "Close" regardless of which chip), and had 0% test coverage.

🔑 What?

Changes

  • Add src/components/F0Chip/ with F0Chip component, F0ChipProps type, chipVariants CVA config, and chipVariantValues array
  • Replace <div role="button"> with a native <button> for clickable chips — eliminates nested interactive element a11y violation
  • Fix close button aria-label from generic "Close" to contextual `Remove ${label}` so AT users know which chip they're removing
  • Add aria-disabled support for deactivated+clickable chips
  • Replace F0Icon className= usage with the color prop
  • Export F0Chip from src/components/exports.ts (stable public API)
  • Update all 4 internal consumers to import from @/components/F0Chip
  • Deprecate experimental/OneChip with aliased re-exports to F0Chip for backwards compatibility
  • Remove old experimental OneChip stories/MDX (replaced by stable __stories__/F0Chip.stories.tsx)

Migration Type

SINGLE — OneChipF0Chip in one PR; internal consumers updated in the same change.

✅ Verification

Tests

  • 20 unit tests added in src/components/F0Chip/__tests__/F0Chip.test.tsx
  • Tests cover: rendering, variants (default/selected), icon, avatar (person/non-person), deactivated state, aria-disabled, click/Enter/Space interactions, onClose, stopPropagation
  • TypeScript: ✅ pass (pnpm tsc --noEmit)
  • Lint: ✅ pass (pnpm lint:fix && pnpm lint, 0 warnings, 0 errors)
  • All pre-commit hooks pass (cycle-dependencies, format, lint)

Integrate @storybook/addon-mcp so AI agents can access F0 component
documentation, preview stories, and run tests via the MCP protocol.

- Upgrade Storybook from 10.1.11 to 10.3.3
- Install and configure @storybook/addon-mcp (v0.4.2)
- Enable all toolsets (docs, dev, test) for local dev; docs-only for public builds
- Add CORS headers for MCP POST requests in staticwebapp.config.json
- Add .mcp.json for agent auto-discovery of local MCP server
- Document MCP server usage in AGENTS.md
Move experimental OneChip to stable src/components/F0Chip with F0 naming
conventions, native button semantics for a11y, and full test coverage.

- Add F0Chip component with types, stories (7 stories + Snapshot), and 20 unit tests
- Use native <button> element for clickable chips (no more div role=button)
- Fix close button: contextual aria-label (Remove {label}), no redundant tabIndex
- Fix nested interactive element issue (button inside button) by sibling layout
- Add aria-disabled support for deactivated+clickable chips
- Replace F0Icon className with color prop
- Update all internal consumers (F0ChipList, F0Select, OneFilterPicker) to import from stable path
- Deprecate experimental/OneChip with aliased re-exports pointing to F0Chip
- Remove old experimental OneChip stories and MDX docs
@eliseo-juan eliseo-juan requested a review from a team as a code owner March 31, 2026 16:38
Copilot AI review requested due to automatic review settings March 31, 2026 16:38
@github-actions github-actions bot added feat react Changes affect packages/react labels Mar 31, 2026
@github-actions
Copy link
Copy Markdown
Contributor

✅ No New Circular Dependencies

No new circular dependencies detected. Current count: 0

@github-actions
Copy link
Copy Markdown
Contributor

📦 Alpha Package Version Published

Use pnpm i github:factorialco/f0#npm/alpha-pr-3817 to install the package

Use pnpm i github:factorialco/f0#9a27ec96992e49f269973aa7651efa9ac7142547 to install this specific commit

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Visual review for your branch is published 🔍

Here are the links to:

Copy link
Copy Markdown
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

This PR promotes the previously experimental OneChip into a stable, publicly exported F0Chip component in packages/react, updates internal consumers to use the stable import path, and adds Storybook/testing support. It also introduces Storybook MCP server configuration/docs and bumps several Storybook-related dev dependencies.

Changes:

  • Added new stable src/components/F0Chip/ implementation + types, with Storybook stories and unit tests.
  • Deprecated experimental/OneChip by re-exporting F0Chip/types/variants, and updated internal consumers to import F0Chip.
  • Updated Storybook tooling/config (addon versions, @storybook/addon-mcp, CORS headers) and documented MCP usage (root AGENTS.md, .mcp.json).

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/react/src/experimental/OneChip/index.tsx Replaces experimental implementation with deprecated aliases to F0Chip and its types/variants.
packages/react/src/experimental/OneChip/index.stories.tsx Removes old experimental stories (replaced by stable stories).
packages/react/src/experimental/OneChip/index.mdx Removes old experimental MDX docs (migrated to stable stories/docs flow).
packages/react/src/experimental/exports.ts Re-exports deprecated OneChip from experimental barrel for backwards compatibility.
packages/react/src/components/OneFilterPicker/components/FilterChipButton.tsx Updates consumer import to stable F0Chip.
packages/react/src/components/F0Select/components/ActiveFiltersChips.tsx Updates consumer import to stable F0Chip.
packages/react/src/components/F0ChipList/index.tsx Updates consumer import/types to stable F0Chip/F0ChipProps.
packages/react/src/components/F0ChipList/ChipCounter.tsx Updates consumer import/types to stable F0Chip/F0ChipProps.
packages/react/src/components/F0Chip/types.ts Introduces chipVariants, chipVariantValues, and public F0ChipProps typing.
packages/react/src/components/F0Chip/index.tsx Public barrel export for F0Chip + types/variants.
packages/react/src/components/F0Chip/F0Chip.tsx New F0Chip implementation (button semantics, close button, aria-label changes).
packages/react/src/components/F0Chip/tests/F0Chip.test.tsx Adds unit tests for rendering, variants, and interactions.
packages/react/src/components/F0Chip/stories/F0Chip.stories.tsx Adds stable Storybook stories including Snapshot coverage.
packages/react/src/components/exports.ts Exports F0Chip from the stable components barrel.
packages/react/package.json Updates Storybook-related dependencies and adds @storybook/addon-mcp.
packages/react/.storybook/static/staticwebapp.config.json Adds additional CORS headers for the Storybook static deployment.
packages/react/.storybook/main.ts Enables @storybook/addon-mcp with public-build toolset limiting.
package.json Updates eslint-plugin-storybook and pins React via pnpm overrides.
AGENTS.md Documents Storybook MCP server usage and toolsets.
.mcp.json Adds MCP server auto-discovery config for local Storybook MCP endpoint.

Comment on lines +31 to +56
return (
<div
className={cn(
chipVariants({ variant }),
onClose && "pr-1.5",
avatar && "pl-0.5",
avatar && avatar.type !== "person" && "rounded-sm",
icon && !avatar && "pl-1.5"
)}
>
{onClick ? (
<button
type="button"
className={cn(
"flex items-center gap-1 bg-transparent p-0 font-[inherit]",
"cursor-pointer",
focusRing()
)}
onClick={onClick}
aria-disabled={deactivated || undefined}
>
{labelContent}
</button>
) : (
<div className="flex items-center gap-1">{labelContent}</div>
)}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

When onClick is provided, only the inner <button> is clickable, but the chip’s padding/border live on the outer <div>. This leaves a non-interactive “dead zone” around the label and also means the focus ring is applied to the inner content rather than the visible chip shape. Consider making the clickable element cover the full chip area (e.g. render the root as a <button> when there’s no onClose, or move the padding/border classes onto the button and make it stretch).

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +49
"cursor-pointer",
focusRing()
)}
onClick={onClick}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

aria-disabled is set when deactivated is true, but the onClick handler will still fire and the button still presents as interactive (cursor-pointer). If the chip is meant to be “disabled” for assistive tech, it should also be non-actionable (e.g. guard onClick when deactivated, and/or add disabled-like styling/behavior such as pointer-events/tabIndex adjustments consistent with other aria-disabled usages in the codebase).

Suggested change
"cursor-pointer",
focusRing()
)}
onClick={onClick}
deactivated ? "cursor-default" : "cursor-pointer",
focusRing()
)}
onClick={(event) => {
if (deactivated) {
event.preventDefault()
event.stopPropagation()
return
}
onClick(event)
}}

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +9
export { chipVariants }

Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

This module re-exports chipVariants, but the public barrel already re-exports chipVariants from ./types. Keeping this extra re-export in the implementation file makes it unclear which import path should be used and is easy to forget during refactors. Consider removing this re-export and keeping chipVariants exported only from types.ts/index.tsx.

Suggested change
export { chipVariants }

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +9
import { F0Chip } from "../index"
import { chipVariantValues } from "../types"

Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

The story imports chipVariantValues from the internal types module even though it’s already exported from the component’s public barrel (../index). Importing from the barrel reduces coupling to internal file structure and keeps stories aligned with the public API surface.

Suggested change
import { F0Chip } from "../index"
import { chipVariantValues } from "../types"
import { F0Chip, chipVariantValues } from "../index"

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for packages/react

Status Category Percentage Covered / Total
🔵 Lines 44.99% 10723 / 23832
🔵 Statements 44.26% 11048 / 24956
🔵 Functions 37.07% 2433 / 6563
🔵 Branches 36.51% 6872 / 18820
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/react/src/components/exports.ts 100% 100% 100% 100%
packages/react/src/components/F0Chip/F0Chip.tsx 100% 100% 100% 100%
packages/react/src/components/F0Chip/index.tsx 100% 100% 100% 100%
packages/react/src/components/F0Chip/types.ts 100% 100% 100% 100%
packages/react/src/components/F0ChipList/ChipCounter.tsx 0% 0% 0% 0% 17-54
packages/react/src/components/F0ChipList/index.tsx 0% 0% 0% 0% 37-94
packages/react/src/components/F0Select/components/ActiveFiltersChips.tsx 5.55% 0% 0% 6.06% 35-136
packages/react/src/components/OneFilterPicker/components/FilterChipButton.tsx 93.75% 66.66% 100% 93.75% 43
packages/react/src/experimental/exports.ts 0% 100% 100% 0% 56-64
packages/react/src/experimental/OneChip/index.tsx 0% 100% 100% 0% 6
Generated in workflow #12426 for commit 1dd9933 by the Vitest Coverage Report Action

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

Labels

feat react Changes affect packages/react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants