-
Notifications
You must be signed in to change notification settings - Fork 0
Review 4590
#4590 · jiunshinn · aged-PR cleanup Step 3, batch 3, 2026-08-23
Verdict: changes requested + ruling
ListItem insets row content by a density-dependent 8/12px, so a list under a section heading reads as misaligned and consumers reach for negative-margin CSS (#2626).
A List.isEdgeAligned boolean applying a matching negative marginInline to the <ul>, mirroring Item's own --spacing-2 / --spacing-3.
-
List.isEdgeAligned?: boolean = false(public, core barrel)
no new theme targets
Class exists — Divider has isFullBleed, Table has an edge adjustment, and #3938 wants one on TabList. But every existing member is container-aware (calc(-1 * var(--container-padding-inline-start, 0px)) plus a width compensation, Divider.tsx:116-129) and spelled isFullBleed. This member picks a different name and a different mechanism, deliberately and with a written argument.
API no (new opt-in prop, default false) · Visual no for existing consumers; for opt-in the <ul> grows 8px past its parent on each side (measured 884 vs 868) · Theme no new targets, but see judgement
Chromium, core-list--edge-aligned at 900px on the PR head. Default theme: heading text and row text both at x=16 — the feature works. Inject .astryx-item{padding-inline:var(--spacing-3)} (the value butter already writes on eight targets): row text moves to x=20, heading stays at 16. Frames /tmp/b3shots/4590__A-default.png and 4590__B-theme-item-spacing3.png, both opened and looked at.
request changes. [BLOCKS] the cancel margin mirrors a value Item owns and themes routinely change, so it is off by the difference under such a theme — the same off-by-4 the PR's own description flags in the detail-page template, one level up (R3d, same shape one step sideways). [ruling] naming and mechanism: Cindy's own comment on #2626 says isFullBleed is 'not quite accurate' and that this 'depends on container awareness'. The PR argues against the container-padding mechanism, well. That call is hers, not mine, and I said so on the PR rather than picking.
Needs a Rulings entry: does the edge-adjustment family take a second mechanism (cancel the item's own inset) alongside the container-padding one, and under what name?
See the review on #4590. Attribution: [Reviewed by Robohands].
(posted directly by the loop under the aged-PR brief; nothing edited)