Review 3938
LOOP VERSION: 1.4.0 AUDIT RUBRIC: 1.12 AUTHOR CAN PROCEED: yes — see round 3b
Author: jiunshinn · branch feat/2622-tablist-bleed · issue #2622
Read round 3b. Rounds 2a and 3 asked the author to keep the template's negative margins; round 3a corrected that criterion and proved the LayoutHeader arm. Cindy has now ratified the ownership split: TabList owns inline full bleed; LayoutHeader owns the block-end dock through
paddingBlockEnd={0}. No design choice remains. The current ask is two implementation changes.
| Round | Head | Date | Outcome |
|---|---|---|---|
| 1 | b765a250fe1 |
2026-08-24 | Changes requested — block-axis gating |
| 2 | 24893c04872 |
2026-08-26 | Changes requested — edge stops do not compensate |
| 2a | 24893c04872 |
2026-08-26 | Correction — asked for the wrong shape in round 2; see below |
| 3 | 8a18544aaed |
2026-08-26 |
|
| 3a | 8a18544aaed |
2026-08-26 | Correction — round 3 graded against round 2a's obsolete criterion; see below |
| 3b | 8a18544aaed |
2026-08-26 | Ruling ratified — ownership settled; two implementation changes remain |
Superseded in part by round 3a: item 4 of this round's ask ("leave
marginBottom: -16/marginTop: 12… exactly as they are now") is withdrawn.
Round 1 blocked on the block-axis gating: :first-child/:last-child matched the
strip's own parent rather than the padded container, so the rule meant to prevent
the strip being pulled up was what pulled it up 16px into the metadata row.
The author fixed it in 24893c04872 and fixed it correctly. isFullBleed is now
inline-only, the docblock says so and says why, and the template keeps its
block-axis dock with an honest comment explaining that no component prop can
reach the header's padding edge from inside StackItem > HStack. That blocker
is resolved and is not re-raised.
The strip's box bleeds, but the first and last stops do not pad back in, so their labels land 4px short of the container's content inset. Everything else on the row — the metadata line above, the section heading below — sits on that inset, so the tab labels are the only thing off it.
This is the second half of the prop's own promise. The docblock says the strip extends "out to the container's content edges"; the box does, the content does not.
Real detail-page template rendered by apps/template-viewer (which globs
packages/cli/assets/templates/** and renders page.tsx itself, with
@astryxdesign/core aliased to source). Not a repro story.
| arm | build | first tab label vs content inset | ghost centre / tab centre |
|---|---|---|---|
| A — main today | 58f95421362 |
0.00px | 133.98 / 133.48 (Δ 0.50) |
| B — this branch | edbc7d8c8ce |
−4.00px | 133.98 / 133.48 (Δ 0.50) |
| D — + block-end rule as well | b9e95feb00b |
0.00px | 131.98 / 139.48 (Δ −7.50) |
| D2 — + edge-stop compensation only | b747a345b03 |
0.00px | 133.98 / 133.48 (Δ 0.50) |
D2 is the shape to ask for. D is recorded because it is the trap: adding a
block-end cancel as well double-docks against the template's own
marginBottom: -16, pushing the strip 9px below the divider and the tab centre
6px down. The inline-only version leaves the block axis bit-identical to main.
An earlier arm that deleted the whole tabsRow object and flipped
vAlign="center" to "end" lifted the header's ghost panel-toggle 14px out of
the row (ghost centre 119.98 vs tab centre 133.48). That regression is
attributable to the block-axis deletion, not to isFullBleed; it is the
reason the template's block margins should stay until a LayoutHeader docking
slot exists.
All four frames carry <shot>.sensors.json. Identical on every sensor except
Build:
route=/packages/cli/assets/templates/pages/detail-page
theme=neutral · mode=light dark · dir=ltr · 1440x1000@3
media: forcedColors=false reducedMotion=false coarsePointer=false hover=true
targets=1 · state={tabs:4, active:"Details", ghost:"present"}
fonts=loaded · runningAnimations=0 · pageErrors=none
Builds: A 58f95421362 · B edbc7d8c8ce · D b9e95feb00b · D2 b747a345b03.
Surface luminance reads unknown on all four — the header paints no background,
so expected.surfaceClass was omitted rather than loosened.
Frames and raw receipts: assets/pr-3938/armd/.
Arm isolation. A receipt proves a frame is of the surface and conditions it
claims; it does not prove the build is the change it claims. Arms D and D2 were
hand-built, so the exact diffs are recorded beside the frames:
A-to-B.diff,
B-to-D2.diff,
D2-to-D.diff.
B→D2 is functionally three things and nothing else: TabList publishes
--_tab-edge-inset-{start,end} from the container padding, clamped
max(…, --spacing-2); Tab and TabMenu consume them at :first-child /
:last-child. The rest of that diff is comment churn from the two arms having
been cut from different bases — inert, and inspected to confirm it.
D2→D adds exactly one rule, fullBleedBlockEnd. That is the whole cost of the
6px push in arm D.
Round-1 evidence set (33 frames, isolated repros and full pages):
assets/pr-3938/.
RTL measured identically to LTR in every arm.
The three added tests assert className contains the string fullBleed. StyleX
emits that class from the object's key, so all three pass against an empty style
object — they cannot fail for the reason they exist. The third is weaker still:
it asserts only that the class count grew.
This is inherited, not invented: Divider's isFullBleed test says so in its own
comment (// we verify component renders without error). Worth not copying
forward.
jsdom has no layout, so the geometry belongs in a browser test: set
--container-padding-inline-start on a wrapper, assert the strip's box escapes it
and the first stop's label lands back on it.
- Dropping
marginInline: -12moves the ghost panel-toggle inline as well:ghost_box_end_inset−12 → 0. It now lands on the content inset like everything else. An improvement, but a visible change to that button, and it should be called out rather than discovered. - The template moved on main:
packages/cli/templates/→packages/cli/assets/templates/. The branch is ~904 commits behind, so the rebase will not apply cleanly here. - Unrelated formatting churn in the template diff (
Icon,MetadataListItem,Collapsiblere-wrapped). Harmless, but it is noise in a review. - Not this PR's:
TabListrenders notablist/tabroles at all — plainnavplus buttons. Separate issue.
- Keep the prop and the name. Both are right.
- Add the edge-stop compensation, inline only — the first and last stops pad
back in by the amount the box bled out, clamped so a stop never insets less
than its normal padding. This is three files, not one:
TabListpublishes the amount, and bothTabandTabMenuconsume it. In this template the last stop is theTabMenu("More ⌄"), so the end edge does not move without it. - Do not add a block-end rule. Arm D shows what it costs.
- Leave
marginBottom: -16/marginTop: 12andvAlign="center"in the template exactly as they are now. - Replace the three class-name tests with browser geometry assertions.
- Rebase.
Itself corrected by round 3a: arm E stands, but item 4 of this round's revised ask — "Template keeps
marginBottom: -16,marginTop: 12,vAlign="center"" — was wrong and is withdrawn.
Round 2 asked for the wrong shape. The diagnosis holds (the branch is a 4px regression against main); the remedy does not. Arm E supersedes D2 as the ask.
One rule, one file. TabList keeps the negative margins on the nav and pads
the strip back in by whatever the bleed exceeds a stop's own inline padding:
const STOP_INLINE_PADDING = spacingVars['--spacing-3'];
const EDGE_PAD_START = `max(var(--container-padding-inline-start, 0px) - ${STOP_INLINE_PADDING}, 0px)`;
const EDGE_PAD_END = `max(var(--container-padding-inline-end, 0px) - ${STOP_INLINE_PADDING}, 0px)`;Tab and TabMenu are untouched. No private var protocol, no child selectors.
Built as 0584db591b2 on vis-3938-e; diffs
main-to-E-core.diff,
D2-to-E.diff.
Measured across 13 rigs per arm
(matrix-D2.json,
matrix-E.json):
-
D2's
TabMenurule never fires.TabMenureturns a fragment — the trigger<button>plus the popover's<template>and a zero-size<div>, all strip siblings — so the trigger is never:last-child. Inlayout="fill"D2's chevron lands 4px outside the inset: the exact regression this PR exists to fix, at the other edge. E lands 0.00 and needs noTabMenuchange. -
Widens stops that are not at an edge.
:last-childfires on the last Tab even inhug, where it is nowhere near the end — a 4px-wider hover pill on one tab for no reason. Single-tab strips get both rules and grow 8px. -
Regresses
padding={0}. The property is published as0px, somax(0px, --spacing-2)resolves to 8px and the first stop's padding shrinks 12 → 8, in a container with no padding to bleed. E is a no-op there.
At rest on this template E and D2 are pixel-identical (0 differing pixels); the only on-screen difference is a 13px band at the first hover pill, where E matches main's −12 and D2 sits at −16.
-
Container padding below 12px (
padding={2}→ 8px): E's label lands 4px inside the inset; D2 is exact. Structural — the strip can add padding, never remove it. Main has the same flaw today (its hardcoded-12over-bleeds an 8px container), so E is no worse than the status quo, and D2 is strictly better on this one axis. -
A restated constant. E duplicates
--spacing-3inTabListto subtract the stop's own padding; the import would be circular. If a stop's inline padding changes, edge alignment drifts silently. Wants a unit test. -
layout="fill"distributable area is 8px narrower (~1.3px per stop). Not visible.
Everything else measured the same in both: RTL, unpadded containers, scrolling
strips, overflow="none", and the entire block axis.
Full write-up: arm-E-report.md.
- Keep the prop and the name.
- Add the strip-padding compensation (arm E). One file —
TabandTabMenustay untouched. - No block-end rule.
- Template keeps
marginBottom: -16,marginTop: 12,vAlign="center". - Browser geometry tests, not class-name assertions. Include a stop-padding coupling test.
- Rebase.
Withdrawn in part by round 3a. This round graded the head against round 2a's criterion, which ruling 4 had already superseded. Its finding about the unenforced geometry assertion holds; its judgement that "the implementation and intended layout are correct" does not, and neither does its
AUTHOR CAN PROCEEDreasoning.
LOOP VERSION: 1.4.0
AUDIT RUBRIC: 1.12
HEAD REVIEWED: 8a18544aaed20fe7760f16fb96eda7e87b6ffcf9
AUTHOR CAN PROCEED: yes
Every code/layout finding from rounds 1, 2, and 2a is resolved at this head.
The block-axis cancel is gone; arm E's one-file strip compensation is present;
Tab and TabMenu remain untouched; the template removes only its inline
margin and keeps the measured block dock. This round extends 2a with one finding
about whether the replacement browser assertion is actually enforced.
A builder placing TabList at the bottom of a padded header otherwise needs a fixed negative margin to make the strip meet the header edge. That workaround does not follow container padding and drags the adjacent panel-toggle button outside the content inset.
-
isFullBleedexpands TabList through the logical container padding and pads the strip back only by the amount a tab stop does not already provide. -
detail-pageadopts the prop and removesmarginInline; its measuredmarginTop,marginBottom, andvAlign="center"block dock stay intact.
TabList owns the opt-in inline behavior; Layout publishes the padding values. The implementation uses the existing logical container vars and spacing token, with no child selector or per-stop protocol.
The ruling's Toolbar alternative was also driven on the real template and is
not a smaller replacement: without isFullBleed it shrank the strip
968→316px, left the first label 4px inside, put the ghost action 8px outside,
and left the rail 10px above the header edge; adding the prop over-pulled the
label 8px. Current Toolbar would need new slot-fill and docking behavior.
Existing callers do not move because the prop defaults false. On detail-page,
the label remains aligned, the strip reaches the header box edge, the panel
toggle returns from 12px outside to the content inset, and the block axis is
unchanged.
The accepted arm-E boundary remains: 16/24px padding is exact; at 8px the normal 12px tab-stop inset leaves content 4px inside; 0px and unset are matching no-ops.
<TabList value={activeTab} onChange={setActiveTab} isFullBleed>
<Tab value="details" label="Details" />
<TabMenu label="More" options={moreTabs} />
</TabList>| change | public? | class | documented | verdict |
|---|---|---|---|---|
+ TabList.isFullBleed?: boolean = false |
core subpath + root barrel | existing full-bleed family; ruling 4 settles TabList | JSDoc + typed docs + usage | accepted |
No new theme targets, vars, or tokens.
- API — no; additive and default false.
- Visual — existing TabList callers do not move. The template intentionally moves its panel toggle to the content inset and extends the strip 4px to the header box edge.
- Theme — no; existing targets and overrides still resolve.
- Behaviour — no interaction/state/default change.
No effect, listener, observer, dependency, or O(N) work is added. Exact-head unit, build, a11y, RTL, and visual jobs are green; a fresh Chromium matrix also matched logical LTR/RTL geometry.
Manual capture was required because pixels intentionally change. Fresh sensor-backed Chromium frames on the real template showed only the intended panel-toggle shift; labels, underline, type, spacing, and block alignment were visually coherent. Geometry measured:
| case | result |
|---|---|
| real template, current main → exact head | label 0→0px vs inset; nav box 4→0px from edge; ghost −12→0px; tab/ghost centres and header height unchanged |
| exact head 16/24px | both logical edges exact |
| exact head 8px | both logical edges +4px inside, accepted clamp |
| exact head 0px / unset | matching no-ops with normal 12px stop inset |
| RTL, hug, fill + trailing TabMenu | match LTR and pass |
The old class-name assertions are gone and FullBleedGeometry.play contains
real 16px hug/fill/TabMenu geometry assertions. But no CI test target enforces
that play result: Vitest collects only *.test.*, and the visual/a11y loaders
wait for rendered DOM without observing and failing on play outcomes. A broken
geometry assertion can therefore leave required checks green.
Request changes. The implementation and intended layout are correct; the remaining blocker is regression protection, not another API or layout redesign.
- Wire the existing assertions into a CI-enforced browser check.
- Include the accepted far side: 8px preserves the 12px stop inset; 0px/unset is a no-op.
AUTHOR CAN PROCEED: yes — the acceptance criterion is fully specified and no human-owned API/design choice remains.
Thanks — the one-file strip compensation is the right shape, and this head now holds the intended geometry for hug/fill, RTL, and a trailing
TabMenu.One blocker remains:
FullBleedGeometry.play(apps/storybook/stories/TabList.stories.tsx:103) is not an enforced test. Vitest never collects stories, and the visual/a11y jobs load this story without waiting for or failing on its play result, so a broken geometry assertion can stay green.Could you wire these assertions into a CI-enforced browser check and include the clamp's far side (8px preserves the 12px stop inset; 0/unset is a no-op)? If you'd rather talk it through with someone, we're in Discord.
[Reviewed by Robohands]
Not posted — this was a read-only re-review; Cindy has not judged the draft.
Superseded by round 3b. The held API question below is resolved: Cindy accepted
LayoutHeader.paddingBlockEnd={0}.
LOOP VERSION: 1.4.0
AUDIT RUBRIC: 1.12
HEAD REVIEWED: 8a18544aaed20fe7760f16fb96eda7e87b6ffcf9 (same head as round 3)
AUTHOR CAN PROCEED: yes
Round 3 graded this head against the wrong criterion, and round 2a is where the
wrong criterion came from. Round 2a told the author to keep marginBottom: -16,
marginTop: 12 and vAlign="center" and to remove only marginInline. That
contradicted ruling 4, which had already settled the acceptance
test in as many words:
The acceptance test is the template.
detail-page'stabsRowmust lose ALL its hand-written CSS — not shrink, disappear. … Everything is computed from the container system vars. No hardcoded amount anywhere — not in the prop, not in the template.
So round 3's verdict — "the implementation and intended layout are correct", with the unenforced play function as the only blocker — is withdrawn. The block it found is real and survives as the second finding below. The criterion it passed the layout against does not.
tabsRow still exists (detail-page/page.tsx:56) and is still applied
(:263). And the constant it carries is not merely untidy — it is correct at exactly
one header padding:
header padding
|
head 8a18544, underline → divider |
|---|---|
{4} (16px) |
−1.00px — docked |
{6} (24px) |
+7.00px — 8px short of the divider |
marginBottom: -16 cancels a 16px padding by coincidence of value. The container
already publishes that number as --container-padding-block-end.
Driven, not argued — six arms on the real detail-page route through
apps/template-viewer, one probe, one page. "5/5" means all five targets the
criterion names: inline bleed + label compensation from container vars; underline
docked on the header divider; ghost panel toggle centred on the tab row; prop-false
defaults unchanged; RTL and padding variants.
| arm | template CSS | new API | @ padding={4}
|
@ padding={6}
|
|---|---|---|---|---|
head 8a18544
|
marginTop:12; marginBottom:-16 |
— | 5/5 | underline +7.00 |
Z — delete tabsRow, nothing else |
none | — | underline +15.00 | — |
A — LayoutHeader paddingBlockEnd (chosen) |
none | 1 optional prop | 5/5 | 5/5 |
A2 — LayoutHeader bottom docking slot |
none | new slot + its own gap rule | 5/5 | not driven |
| B — TabList owns block docking | none | 1 prop, wrong owner | underline +5.00, ghost −8.00 | — |
S — existing Section bleed seam |
none | none | 5/5 | label −8.00, ghost −8.00 |
(a) beats (b) on measurement, not on taste. LayoutHeader publishes the padding
and paints the divider; TabList can see neither through StackItem > HStack > VStack. Arm B's negative block-end margin shrinks only the nav's outer box inside
the HStack, so the row stops sizing to the strip: the header keeps 11px of its
padding and vAlign="center" re-centres the ghost 8px above the tab row.
The existing seam was tried before any API was invented. Section already cancels
container padding on all four edges, so arm S wraps the row in
<Section variant="transparent" padding={0} paddingInline={4}> and adds nothing to
the public surface. It passes at padding={4} and puts both the label and the ghost
8px outside the content inset at padding={6} — paddingInline={4} is the same
restated 16 in prop clothing. It trades a magic number for a magic prop.
A2 works and costs more. The literal docking slot hits 5/5, but adds a slot
concept, needs its own block-start gap rule (nothing supplies one outside children),
and restructures the template's JSX. Arm A reaches the same geometry by naming one
edge in a vocabulary that already exists. A2 was driven at padding={4} only; its dock
reads the same container var, so it should generalise, but that is stated rather than
measured.
armA-layoutheader-dock.diff —
+22/−16 across two files.
// LayoutHeader.tsx — per-edge override last, so it wins on its own edge only
paddingBlockEnd != null && paddingBlockEndStyles[paddingBlockEnd],
paddingBlockEnd != null && containerPaddingBlockEndVarStyles[paddingBlockEnd],// detail-page/page.tsx — the whole tabsRow object and style={tabsRow} are gone
<LayoutHeader hasDivider padding={4} paddingBlockEnd={0}>
…
<HStack vAlign="center">Section already publishes all six per-edge padding overrides
(Section.tsx:191-226), backed by padding.stylex.ts helpers LayoutHeader already
imports. This is that class's second member, not a new concept, and the name is
Section's — so there is no naming decision here. The container var moves with the
padding so bleed children compensate against the padding actually applied, which is
the rule Section.tsx:335 states for the same reason.
The other arms are banked beside it:
armA2-layoutheader-slot.diff ·
armS-section-seam.diff ·
matrix-*.json.
Magenta = header content inset, red = header divider, blue = the two vertical centres. One clip per composition, guides drawn from live geometry.
The pair, at the template's own padding={4} — every measured target identical:
The boundary, at padding={6} — this is the frame the block rests on. The head's
underline sits visibly above the red divider guide; arm A stays docked:
| frame | build | (a) first label vs inset | (b) underline → divider | (c) ghost vs tab-row centre |
|---|---|---|---|---|
head-8a18544-header.png |
8a18544 |
0.00px | −1.00px | 0.00px |
armA-dock-header.png |
4fe97cd |
0.00px | −1.00px | 0.00px |
head-pad6-header.png |
217409e |
0.00px | +7.00px | 0.00px |
boundary-pad6-header.png |
b5c822e |
0.00px | −1.00px | 0.00px |
All four carry <shot>.sensors.json and match on every sensor except Build. The
build sensor was proved live rather than assumed: capturing the head frame from the
arm checkout failed closed with expected 8a18544…, observed 4fe97cd….
tabrow-3938-r16c-tests.cjs.
Expectations are authored from the contract before observation
(expectedLabelDelta(pad) = max(12 − pad, 0)), never loosened to match output.
| build | result |
|---|---|
| arm A | 78/78 pass |
head 8a18544
|
78/78 pass — geometry parity; the head's failure is the retained CSS, not the pixels |
| Z, delete-only | 73/78 — the 5 failures are exactly the dock, at every padding and both directions |
Axes: five targets × real template at 0/8/16/24 × LTR/RTL; the story fixture's hug
and fill at 0/8/16/24 × LTR/RTL with a real fragment-rendering TabMenu as the
fill strip's last stop; and prop-absent defaults. On arm A the repo's own targets are
green too: vitest --project ui Layout + TabList + Section 195/195, tsc --noEmit
clean, eslint/prettier and all ten check:* scripts.
FullBleedGeometry.play (apps/storybook/stories/TabList.stories.tsx:103) is observed
by nothing that can fail:
-
vitest.config.tscollects*.test.{ts,tsx,mjs}in both projects; no include pattern reaches*.stories.tsx. - There is no
@storybook/test-runnerin the repo. - The visual gate loads each story and waits only for
#storybook-root > *anddocument.fonts.ready(.github/scripts/visual-gate/gate.mjs:381-382). It never awaitsplay, never inspects a story error, and itscatchswallows a story that will not render.
| round | finding | state at 8a18544
|
|---|---|---|
| 1 | block-axis :first-child/:last-child gating pulled the strip 16px up |
fixed — deleted |
| 2 | edge stops do not compensate; label 4px short | fixed — label 0.00 |
| 2 | three tests assert only that className contains fullBleed
|
fixed — replaced by real geometry assertions |
| 2 | D2's TabMenu rule can never fire |
moot — no child selectors at this head |
| 2 | dropping marginInline moves the ghost to the inset; call it out |
fixed — the PR body states it |
| 2 | branch ~904 commits behind; template path moved | open — still needs a rebase |
| 2 | unrelated formatting churn in the template diff | open — still noise |
| 2 | TabList renders no tablist/tab roles |
not this PR — separate issue |
| 2a | keep marginBottom:-16, marginTop:12, vAlign="center"
|
superseded — see above |
| 2a | an arm deleting tabsRow "lifted the ghost 14px out of the row" |
corrected — not reproducible. Deleting tabsRow alone leaves the ghost-vs-tab-row centre at 0.00px (matrix-armZ-delete-only.json); the only thing it breaks is the dock (+15.00px). The 14px belonged to that arm's vAlign="center"→"end" flip, which this round does not make. The real ghost shift is −8.00px and belongs to arm B |
| 3 |
FullBleedGeometry.play is not CI-enforced |
open — re-verified above |
| 3 | "the implementation and intended layout are correct" | withdrawn |
With the hand-tuned marginTop: 12 gone, the tab row sits on the header stack's own
gap={3} (12px, not 24px), so the header is 140.98px tall instead of 152.98px.
That is the price of "no replacement magic numbers" — the 24px was never a token, and
re-spelling it would re-fail the acceptance. Recorded rather than restored.
The accepted arm-E clamp is unchanged: 16/24px exact, 8px leaves the label 4px inside, 0px/unset are matching no-ops.
- Keep
isFullBleed, its name and its implementation. The inline half is right. - Delete the whole
tabsRowobject andstyle={tabsRow}. No replacement constant. - Add
paddingBlockEnd?: SpacingSteptoLayoutHeaderinSection's existing spelling, and passpaddingBlockEnd={0}on the header — held for ruling 12; this is new public surface on a second component, so the shape is Cindy's before it is built. Recommendation and the driven arm are on that entry. - If the prop lands, give it a
LayoutHeader.doc.mjsrow. (paddingis undocumented there already — that half is pre-existing debt, not this PR's.) - Put the geometry assertions — now including the dock — in a target CI fails on, and
keep the clamp's far side (8px preserves the 12px stop inset; 0/unset is a no-op).
There is a harness rather than one to invent:
pr-visualalready walks every story in a real Chromium viagate.mjs, andpr-a11yalready drives@axe-core/playwright. Which one is the author's call. - Update the changeset: its last sentence still says the template "keeps only its
hand-tuned block-dock margins in
tabsRow." - Rebase, and drop the unrelated
Icon/MetadataListItem/Collapsiblere-wrapping from the template diff.
Request changes, with one ruling held alongside. Two blocks, each forcing the
verdict independently: the template still carries hand-written tab-row CSS whose
constant is padding-specific, and the replacement geometry guard is enforced by no CI
target. A BLOCKS outranks a [Needs human judgement] in the brief's order, so the
GitHub state is CHANGES_REQUESTED whichever way the ruling goes.
AUTHOR CAN PROCEED: partly, and the split is the honest part.
| ask | proceedable now? |
|---|---|
Delete tabsRow and style={tabsRow}
|
no — deletion alone leaves the underline 15.00px off the divider; it needs whatever replaces the dock, and that is the ruling |
Add LayoutHeader.paddingBlockEnd
|
no — new public surface on a second component; ruling 12, recommendation and driven arm attached |
| Put the geometry assertions in an enforced target | yes — no decision left; two existing Chromium-in-CI harnesses named |
| Update the changeset's last sentence | yes |
| Rebase; drop the unrelated formatting churn | yes |
Round 3 said "no human-owned API/design choice remains." That was the other half of its error: the remedy it implied needs one.
Thanks — the inline half is right, and the strip compensation holds for hug/fill, RTL and a trailing
TabMenu.An apology first: round 2a asked you to keep
marginTop: 12andmarginBottom: -16. That was our mistake — the ruling behind #2622 says the template must need no hand-written CSS at all. You built what we asked for.
-16only cancels a 16px padding by coincidence. Atpadding={6}the underline lands 7px off the divider:
The header owns that edge, and
Sectionalready spells it. I droveLayoutHeader paddingBlockEnd—tabsRowdisappears and the geometry holds at both paddings. It's new public API though, so Cindy picks the shape before you build it. I'm asking her now.Separately:
FullBleedGeometry.play(TabList.stories.tsx:103) is enforced by nothing — Vitest never collects stories and the visual job never awaitsplay.[Reviewed by Robohands]
Not posted — read-only re-review, no PR action taken. Cindy has not judged the draft.
One independent critic pass, blind to the code, graded the first draft FAIL and
this record answers all of it: AUTHOR CAN PROCEED: yes was not earned (the remedy
adds public surface on a second component — now ruling 12, with the
approval split per ask); a block forced by a +7px pixel claim carried no frame (now
head-pad6-header.png and the composed boundary above); "dock at any padding" was
four driven values (now "at both paddings"); "every alternative was driven" contradicted
the record's own table (the verdict now names what was not driven); "Second, smaller:"
demoted a finding marked BLOCKS; the comment never told jiunshinn the loop had asked
for the opposite two rounds ago (it now opens with the retraction); and the rebase and
the formatting churn were listed open and then dropped from the ask (both are back).
LOOP VERSION: 1.4.0
AUDIT RUBRIC: 1.12
HEAD REVIEWED: 8a18544aaed20fe7760f16fb96eda7e87b6ffcf9
AUTHOR CAN PROCEED: yes
Cindy ratified ruling 12: TabList owns inline full bleed;
LayoutHeader owns the block-end dock. The existing isFullBleed implementation is
accepted as an improvement over current behavior. The only API addition is
LayoutHeader.paddingBlockEnd?: SpacingStep, using the per-edge family Section
already publishes. There is no remaining choice about TabList, a docking slot, or
whether to retain template CSS.
- Delete the whole
tabsRowobject andstyle={tabsRow}; add the LayoutHeader per-edge prop and passpaddingBlockEnd={0}. Arm A is the accepted implementation: 78/78 sensor assertions, 195/195 targeted tests, typecheck and all checks green. - Put the geometry assertion in required Chromium CI.
FullBleedGeometry.playis still unobserved: Vitest does not collect stories and the visual gate does not awaitplay.
The redline remains the decisive visual evidence. At padding={6}, current head is
+7.00px from the divider; the LayoutHeader arm is −1.00px (docked).
Request changes. Both remaining items are proceedable; no human judgement is held.
Thanks — Cindy accepted the ownership split: TabList owns inline full bleed, and LayoutHeader owns the block-end dock.
isFullBleedis an improvement and needs no further design change.Two changes remain:
- Delete
tabsRowandstyle={tabsRow}; addLayoutHeader.paddingBlockEnd?: SpacingStepinSection's existing per-edge family and usepaddingBlockEnd={0}.
At
padding={6}, current head leaves the underline +7.00px above the divider; the LayoutHeader arm is docked at −1.00px.
- Move
FullBleedGeometry.playinto required Chromium CI. Vitest never collects stories and the visual job never awaitsplay, so the current guard cannot fail CI.Sorry for the contradictory round-2a direction; you built exactly what we asked for. Full review and measurements.
[Reviewed by Robohands]