Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .claude/skills/component-audit/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ Check: JSDoc comments on props

### 5. Stories & MDX
- 0: No story, wrong title group, or one story with no argTypes
- 1: Default story + some argTypes but no per-subcomponent stories or no MDX
- 2: Story per exported component, full argTypes, MDX with usage + Controls
- 1: Default story + some argTypes but subcomponents with no story at all or no MDX
- 2: One file per root component and per preset with subcomponent stories in the parent file, full argTypes, MDX with usage + Controls

Check: apps/docs/src/stories/ for *.stories.tsx and *.mdx files

Expand Down
3 changes: 3 additions & 0 deletions .claude/skills/component-docs-stories/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ Rules:

Use shared classes from `apps/docs/index.css`:
- `story-container`, `story-section`, `story-grid`, `story-row`, `story-panel`, `icon-md`
- `story-freeze-animations` on the story root of a snapshotted story: pauses every animation at
frame 0 and drops every transition, document-wide (portals included). Skip it when the
animation is what the story shows.

No Tailwind classes. No ad-hoc inline `style` where shared class exists.

Expand Down
4 changes: 2 additions & 2 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ that apply to this PR and delete the rest. A docs-only or CI-only PR needs none
- [ ] `{Component}Props` exported; only the props the component actually needs are exposed
- [ ] Upstream-owned prop types borrowed by indexed access (`OriginalProps['x']`), never restated by hand
- [ ] Every type named by a public prop is exported from `index.ts`
- [ ] `forwardRef` + `displayName`; `asChild`, `testId` supported
- [ ] `forwardRef` with a named render function (or `displayName`); `testId` supported
- [ ] Defaults in the destructuring; controlled/uncontrolled naming follows Radix

**Docs** ([guidelines](https://github.com/SigNoz/components/blob/main/COMPONENT_GUIDELINES.md#4-how-to-document-props))

- [ ] JSDoc on **every** public prop, with `@default` where applicable
- [ ] Story file per exported component, correct `title` group
- [ ] Story file per root component and per preset, subcomponent stories in the parent's file (`@access private` ones exempt), correct `title` group
- [ ] `argTypes` complete with `category`, `type.summary`, `defaultValue.summary`
- [ ] Stories for every meaningful state, not just the happy path
- [ ] `{component}.mdx` with a usage snippet and a `<Controls>` per exported piece
Expand Down
4 changes: 2 additions & 2 deletions COMPONENT_AUDIT_RUBRIC.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ is for auditing what already shipped.
| --- | --- | --- | --- | --- |
| 1 | **Structure & exports** | Logic in `index.ts`, or a flat file dump, or missing from one of the four lists | Correct layout, but naming drift (`components/` instead of `subcomponents/`) or shared helpers inlined | Kebab-case dir, clean `index.ts`, `subcomponents/` + `presets/` split, all lists in sync |
| 2 | **CSS & tokens** | Hardcoded values, primitive (`--bg-*` / `--text-*`) colours, Tailwind remnants, global selectors, or a `--x: var(--x)` bug | Tokenized, but still carries literal fallbacks on design tokens, or some literals / class-based variants left | Every value a `--{component}-*` var resolving to a **semantic** design token with no literal fallback, `data-*` variants, `-internal-` used for non-overridable values, token region current, all interaction states styled |
| 3 | **Props & types** | Upstream signatures restated by hand, unexported types a prop needs, or no `forwardRef` | Correct `forwardRef`/`testId`, but some upstream-owned prop types still hand-written instead of borrowed | Every prop declared explicitly with JSDoc, upstream-owned types borrowed via `OriginalProps['x']`, `Pick` reserved for undocumented pass-throughs, all referenced types exported, `asChild` where sensible, Radix-style controlled/uncontrolled naming |
| 3 | **Props & types** | Upstream signatures restated by hand, unexported types a prop needs, or no `forwardRef` | Correct `forwardRef`/`testId`, but some upstream-owned prop types still hand-written instead of borrowed | Every prop declared explicitly with JSDoc, upstream-owned types borrowed via `OriginalProps['x']`, `Pick` reserved for undocumented pass-throughs, all referenced types exported, Radix-style controlled/uncontrolled naming |
| 4 | **Prop documentation** | No JSDoc | Partial JSDoc, or `@default` tags that disagree with the implementation | Every public prop documented with its constraints and interactions; `@default` matches implementation *and* `argTypes` |
| 5 | **Stories & MDX** | No story, or wrong title group, or one story with no `argTypes` | `Default` story + some `argTypes`; no per-subcomponent stories or no MDX | Story per exported component, fully categorized `argTypes`, a story per meaningful state, MDX with usage snippet + correct per-piece `<Controls>` |
| 5 | **Stories & MDX** | No story, or wrong title group, or one story with no `argTypes` | `Default` story + some `argTypes`; subcomponents with no story at all or no MDX | One file per root component and per preset, subcomponent stories in the parent's file (`@access private` ones exempt), fully categorized `argTypes`, a story per meaningful state, MDX with usage snippet + correct per-piece `<Controls>` |
| 6 | **Tests** | None | Render smoke test only | Behaviour test per interactive prop, `forward-ref` test, interaction story exercised by `test-storybook` |

## Bands
Expand Down
60 changes: 48 additions & 12 deletions COMPONENT_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ Five principles behind every component. Read when building; refer back when maki
4. Props are **picked deliberately**, exposing only what the component actually needs, and
**every prop carries JSDoc**, so a human or an agent reading the type declaration
understands it without opening the implementation.
5. **One story file per exported component**, subcomponents included.
5. **One story file per exported component**, subcomponents included. Symbols tagged
`@access private` are exempt.

Reference implementations to copy from:

Expand Down Expand Up @@ -93,11 +94,14 @@ packages/ui/src/dialog/

Rules:

- **`subcomponents/`** holds the composable primitives. One file per exported component, named
after it in kebab-case (`dialog-close-button.tsx` for `DialogCloseButton`). `select/` uses
`components/` for this. That is drift, not an alternative. Use `subcomponents/`.
- **`presets/`** holds the batteries-included versions. A preset must be buildable from the
exported primitives. If it needs something the primitives don't expose, expose it.
- **`subcomponents/`** holds the parts a component is composed from. One file per component,
named after it in kebab-case (`dialog-close-button.tsx` for `DialogCloseButton`). `select/`
uses `components/` for this. That is drift, not an alternative. Use `subcomponents/`.
- **`presets/`** holds the batteries-included versions, built from the subcomponents.
- A composed component exports only the composed surface. Its subcomponents stay out of
`index.ts` and carry `@access private` (`tooltip/` exports `Tooltip`, not `TooltipTrigger` or
`TooltipContent`). Another component that needs one imports it by relative path, and a change
to that subcomponent has to keep those imports working.
- Shared non-component logic goes in `utils.ts` (see `pagination/utils.ts`) or a `lib/`
subfolder (`table/lib/`). Cross-component helpers go in `src/lib/`.
- One style file per component directory is the norm; add `{subcomponent}.module.scss` only
Expand Down Expand Up @@ -129,6 +133,33 @@ export { Badge } from './badge.js';
- Every type referenced by a public prop must be exported here. A prop typed with something a
consumer can't import is a bug: they cannot declare their own handler or hold the value in a
typed variable.
- `@access private` in a symbol's JSDoc marks it as not public API: the subcomponents a
composed component is built from, its contexts and hooks. Nothing outside the package may
rely on it, it needs no story or MDX section, and it can change without a major version. A
symbol can carry the tag and still be exported when another component in this repo needs it
(`TooltipProviderIfMissing`).
- Applying it is mechanical: **every `export` in the component directory that `index.ts` does
not re-export carries the tag**, props types as much as components (`TooltipTriggerProps`
next to `TooltipTrigger`), contexts, providers and hooks included. A symbol that is never
exported from its own file needs nothing, it is already unreachable.
- A symbol listed in `index.ts` must **not** carry the tag. Public and private at once is a
bug: it exempts a real part of the surface from its story, MDX section and prop docs.
- The tag goes last in the JSDoc block, after the prose and the other tags, separated by a
blank ` *` line. A symbol with no prose gets a block holding only the tag:

```tsx
/**
* Where the tooltip content is portalled to.
*
* @access private
*/
export type TooltipPortalProps = ...;

/**
* @access private
*/
export type TooltipRootProps = ...;
```

### Import hygiene

Expand Down Expand Up @@ -345,7 +376,6 @@ export interface BadgeProps extends Pick<
testId?: string;
variant?: BadgeVariant;
color?: BadgeColor;
asChild?: boolean;
}
```

Expand Down Expand Up @@ -458,8 +488,8 @@ the lowercase string that lands in `data-*`, and never a TS `enum`.

| Convention | Rule |
| --- | --- |
| `forwardRef` | Every component forwards its ref to the real DOM node, and sets `Component.displayName = 'Component'` |
| `asChild` | Support it (via `@radix-ui/react-slot`) wherever a consumer might want to swap the element. Document what it disables: `Badge` ignores `closable` under `asChild`; `Button` doesn't support `loading`/`prefix`/`suffix` |
| `forwardRef` | Every component forwards its ref to the real DOM node. Name the render function, `forwardRef(function Badge(props, ref) { ... })`, so DevTools and stack traces show the name. An explicit `Component.displayName` does the same and stays valid where it already exists |
| Providers | A component that needs a provider wraps itself in `XProviderIfMissing` (see `TooltipProviderIfMissing`), never in `XProvider`. It adds the provider when none is above and reuses the existing one otherwise, so the component never fails for want of a provider and never shadows what the app configured. Apps place `XProvider` once near the root |
| `testId` | Always present; forwarded as `data-testid`. Don't make consumers use `className` for test hooks |
| Defaults | Set in the destructuring (`variant = 'default'`), and mirrored in an `@default` JSDoc tag |
| Controlled/uncontrolled | Follow Radix naming: `value`/`defaultValue`/`onChange`, `open`/`defaultOpen`/`onOpenChange` |
Expand Down Expand Up @@ -542,7 +572,8 @@ Top-level groups are fixed by `storySort.order` in `apps/docs/.storybook/preview
- Don't invent a new top-level group.

**One story file per exported component**, subcomponents included. That is what makes
per-component Controls tables possible. Bind `Meta` to the actual symbol:
per-component Controls tables possible. Symbols tagged `@access private` are exempt: they are
not public API, so they get no story and no Controls table. Bind `Meta` to the actual symbol:

```tsx
const meta: Meta<typeof DialogContent> = {
Expand Down Expand Up @@ -617,9 +648,14 @@ Layout: use the shared classes in `apps/docs/index.css` (`story-container`, `sto
`story-grid`, `story-row`, `story-panel`, `icon-md`) or a `{name}.stories.module.css`. No
Tailwind classes, and no ad-hoc inline `style` where a shared class exists.

Snapshotted stories add `story-freeze-animations` to the story root: it pauses every animation
at its first frame and drops every transition, document-wide, so a Chromatic capture cannot
land mid-spinner or mid-marching-border. Leave it off the stories whose animation is the point.

### The MDX page

One `{component}.mdx` per component, wiring Controls per exported piece:
One `{component}.mdx` per component, wiring Controls per exported piece (`@access private`
ones excluded):

```mdx
import { Meta, Controls, Primary } from '@storybook/addon-docs/blocks';
Expand Down Expand Up @@ -669,7 +705,7 @@ Visual changes need the `run-visual-testing` label on the PR to get Chromatic sn

| File | Covers |
| --- | --- |
| `{name}.test.tsx` | Behaviour: each variant renders, callbacks fire, controlled + uncontrolled, keyboard interaction, `asChild` composition |
| `{name}.test.tsx` | Behaviour: each variant renders, callbacks fire, controlled + uncontrolled, keyboard interaction |
| `{name}.forward-ref.test.tsx` | `ref.current` is the expected element instance and carries `data-slot` |
| `{name}.test-utils.tsx` | Shared render helpers, when several test files need them |
| `apps/docs/stories/*.stories.tsx` | Render + interaction in a real browser via `@storybook/addon-vitest` |
Expand Down
8 changes: 8 additions & 0 deletions apps/docs/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ button,
background-color: var(--muted);
}

body:has(.story-freeze-animations),
body:has(.story-freeze-animations) *,
body:has(.story-freeze-animations) *::before,
body:has(.story-freeze-animations) *::after {
animation-play-state: paused !important;
transition: none !important;
}

.story-resizable {
width: 100%;
height: 400px;
Expand Down
3 changes: 2 additions & 1 deletion apps/docs/stories/combobox-simple.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ const meta: Meta<typeof ComboboxSimple> = {
},
disableTooltipProvider: {
control: 'boolean',
description: 'Disable internal TooltipProvider when already inside one.',
description:
'Deprecated. An outer TooltipProvider is detected on its own, so this no longer has to be set.',
table: {
category: 'Behavior',
type: { summary: 'boolean' },
Expand Down
2 changes: 1 addition & 1 deletion apps/docs/stories/tabs-root.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default function MyComponent() {
}
```

> Note: Disabled tab tooltips and automatic lock-icon replacement for disabled states require the `Tabs` composed component. Primitive composition does not wrap `TabsTrigger` with `TooltipSimple`, so `disabledReason` has no effect.
> Note: Disabled tab tooltips and automatic lock-icon replacement for disabled states require the `Tabs` composed component. Primitive composition does not wrap `TabsTrigger` with `Tooltip`, so `disabledReason` has no effect.

## TabsRoot Props

Expand Down
3 changes: 0 additions & 3 deletions apps/docs/stories/tooltip-content.stories.module.css

This file was deleted.

126 changes: 0 additions & 126 deletions apps/docs/stories/tooltip-content.stories.tsx

This file was deleted.

Loading
Loading