-
Notifications
You must be signed in to change notification settings - Fork 0
Review 4847
AKnassa · community · MERGED 78e8e1c · view on GitHub
Verdict: approve — posted, second round, after kentonquatman's asks were satisfied.
55 components describe their anatomy — 236 named parts — and authors have been
writing that content all along. The CLI (astryx component <Name>) showed all
of it; the doc site showed none of it. Anatomy.tsx arrived with the doc site
and was never imported, so a builder reading a component page could not see the
parts the component is made of.
1. the Overview tab renders the Anatomy table when a component describes
its parts, above Best practices, matching the CLI's order [the fix]
2. a wiring test fails if any piece of the component-detail page becomes
disconnected again [the guard]
Plus the Anatomy heading resized to match Usage and Best practices. No change to any published package and no change to what authors write — this switches on writing that already existed.
Every reader of a component page with anatomy content: 55 components, 236 parts, visible where they were invisible. No consumer of any published package is touched.
No API change. Docsite-internal only.
No new theme targets.
- API — no. Visual — yes, and it is the point: a table appears on 55 component pages where nothing was. Theme — no.
Nothing at runtime beyond rendering one more table per page.
The author's own — Avatar and AvatarGroup, embedded in the PR body. Under R15e those are the claim under review rather than evidence for it; the loop's own frames came later, on #5391, which is where the two defects this page's review surfaced were fixed.
Needs judgement — content appears on 55 pages that had none.
approve.
1. [not blocking] the description still shows the badge column
→ a reader sees the "required" marker twice, once as a badge and once
inline, on any component with required parts
2. [not blocking] red Vercel is the DerivedVar error the author flagged
themselves, since fixed on main
→ not theirs; a merge with main clears it
Thanks — both of Kenton's asks are in; approve. Description still shows the badge column. Red Vercel looks like the
DerivedVarerror you flagged, since fixed on main — try a merge.[Reviewed by Robohands]
R1e worked here, and it is the clean example. kentonquatman had reviewed this PR already, asking for two things: wrap the table in a card to match the other tables on the page, and turn the oddly-positioned "required" badge into inline bold text in the description. The second pass read that review before starting, named him, said what he had concluded, and said the asks were satisfied — it extended his review rather than re-deriving findings from scratch or contradicting him by accident.
That is the whole of R1e's gate: a sentence naming the prior reviewer and what they concluded. It costs one line and it is the difference between two reviewers agreeing in public and two reviewers appearing to disagree over nothing.
And reviewing it produced two defects of our own, both fixed in #5391:
- the wiring guard this PR added counted a unit test as an importer, so the next unwired renderer would have shipped invisible behind a green check — the same failure the guard exists to prevent
- the anatomy table broke part names mid-word on Field and SideNav, which is visible on the page this PR switched on
Neither is the author's debt (R13). The guard was the right idea and its blind spot was ours to close; the mid-word break is a property of the table it now renders into.