Skip to content

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 colour 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 changeset must be major, and the note must say what consumers have to do. "Renamed prop" is not enough — "rename type to variant" is.

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.

Proprietary — internal use only.