Playbook: review a component

What to look for when reviewing a change to Habitus. The bar is higher than for an application PR: every app inherits whatever ships here, and a bad API is far more expensive to undo once several apps depend on it.

The public API is the expensive part

Implementation can be rewritten silently; a prop name cannot. Spend most of the review here.

• Is every prop necessary? A prop that exists because one caller needed it once is a permanent maintenance obligation. Could a slot cover it?
• Do names match existing components? If buttons use variant and size, a new component must not introduce kind and scale. Inconsistent vocabulary is exactly the incoherence this library exists to prevent.
• Are booleans the right shape? Three mutually exclusive booleans should be one union prop.
• Is anything leaking? Exposing internals via defineExpose or emitting implementation details makes them part of the contract.

Accessibility

• Does it wrap a reka-ui primitive, or reimplement one badly? Hand-rolled focus traps, dropdowns and dialogs are almost always wrong in ways that only show up with a screen reader.
• Is the semantic element right — a real <button>, a real <a href>?
• Are aria-* attributes accurate, and do they update with state?
• Does the spec assert expectNoA11yViolations for the default state and for each state that changes roles or labels?
• Is the focus ring visible on keyboard focus and not suppressed?

Styling

• Tokens only. Any literal color or spacing value is a finding. If no token fits, the token set needs extending — that is a design conversation, not a quiet inline value.
• Does it work in dark mode? Ask whether the author actually looked.
• Does it use --spacing-control-* for height, so it lines up with other controls?
• Does it restyle anything outside itself? A component must not reach out of its own subtree.

Structure

• Component directory layout per docs/guide/architecture.md.
• No import reaching past a sibling's index.ts (lint catches this, but check the shape of the dependency — needing a sibling's internals often means something belongs in composables/ or utils/).
• Shared logic extracted rather than copied between components.

Documentation

• Docs page exists, with demos that show the states a consumer will actually hit — including the awkward ones (long labels, empty states, loading).
• API table is the @include, never hand-written.
• Does the Accessibility section say what the consumer is still responsible for? That is the part people miss.

Breaking changes

Habitus is consumed by apps we do not control. A change is breaking if it renames or removes a prop, slot, event or exported type; changes a default; changes rendered markup others may be selecting on; or changes a token's meaning.

If it is breaking, the note must say what consumers have to do. "Renamed prop" is not enough — "rename type to variant" is.

On the bump level, follow release.md, not your instinct. While the package is 0.x a breaking change ships as minor, because Changesets reads major as "cut 1.0" and that is a decision, not a side effect of a bug fix. The consequence for a reviewer is that the level tells you nothing — you cannot spot a breaking change by looking for major, so read the note and decide for yourself whether it describes one, and whether it says what to do about it.

Prefer deprecating over removing where it is cheap: keep the old prop working, document it as deprecated, remove it in a later major.

Tests

• Do they test behaviour, or restate the implementation? A test asserting a class name is present is weak; one asserting the button is not clickable while loading is real.
• Are the failure modes covered, not just the happy path?

Finally

Pull the branch and run npm run docs:dev. Read the rendered demos in light and dark mode. Most design problems are visible in ten seconds there and invisible in a diff.