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
variantandsize, a new component must not introducekindandscale. 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
defineExposeor 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
expectNoA11yViolationsfor 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 incomposables/orutils/). - 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.