diff --git a/.agents/docs/ink-divergences.md b/.agents/docs/ink-divergences.md index 71d8e5a..0e6ab4e 100644 --- a/.agents/docs/ink-divergences.md +++ b/.agents/docs/ink-divergences.md @@ -309,6 +309,42 @@ current-props model, or API conventions. The cost is limited to explicit nullish public bindings; true omission remains Ink-parity. Tests: `prop-reset.test.tsx`. +#### Withdrawing a `margin`/`padding` edge override falls back to the surviving shorthand + +- **Ink:** when a box has both a shorthand and a more-specific override of the same family + (`margin={5} marginTop={8}`, `margin={5} marginX={2}`, padding equivalents) and the override + is later withdrawn, the edge **collapses to 0**, not back to the surviving shorthand + (run-verified vs v7.0.4, both spread-removal and explicit `marginTop={undefined}`: with + `margin:5 marginTop:8` the top margin renders 8 cells, and after removing `marginTop` it + renders **0**, not 5). The cause is yoga edge precedence — a per-edge value (`EDGE_TOP`) + overrides the all-edges shorthand (`EDGE_ALL`) **even when reset to 0** — combined with + Ink's `applyMarginStyles`/`applyPaddingStyles` emitting one yoga setter per prop, so a + withdrawn `marginTop` becomes `setMargin(EDGE_TOP, 0)` that still beats the surviving + `EDGE_ALL=5`. +- **vue-tui:** the withdrawn override falls back to whatever shorthand still applies + (`marginTop` removed from `margin={5} marginTop={8}` → top margin = 5). On any margin/padding + prop change, `reconcileMarginEdges`/`reconcilePaddingEdges` recompute **all four physical + edges** from the box's full current props with most-specific-wins precedence + (`top = marginTop ?? marginY ?? margin ?? 0`, etc.) and zero the composite edges, so no stale + per-edge value can shadow the shorthand. This mirrors the existing `reconcileBorderEdges` + pattern (an edge that depends on several props can't be set correctly by a single + per-prop yoga setter). +- **Spacing value contract:** an edge resolves from a prop only when it is a **finite number** + (matching the `number` prop type + Ink's number-only margin/padding); a numeric **string** + (`margin="5"`) is coerced for Vue **static-template attribute** ergonomics, but any other + non-numeric value (`"50%"`, junk, `""`) is treated as **not-set** and falls through to the + surviving shorthand rather than being forwarded to yoga. So the family recompute drops the + OLD per-setter code's incidental, off-contract string forwarding — `marginTop="50%"` no longer + becomes a yoga percent and `marginTop="foo"` no longer throws. +- **Why:** render = f(current props): with current props `{margin: 5}` the top margin is 5, full + stop — a value that is no longer set must not linger via yoga's edge layering (G19, the same + declarative-reset principle as the `display` and `flexDirection`/`flexWrap` entries above). + This is NOT an Ink-parity item: Ink and pre-fix vue-tui both collapsed to 0 (the identical + bug); the fix diverges from Ink by being declaratively correct. Verified against + yoga-layout@3.2.1 that the recompute produces identical computed edges as the old per-setter + code for the SET path (no layout regression), and the correct fallback on removal. + Tests: `prop-reset.test.tsx`, `unit/yoga-prop-reset.test.ts`. + #### Public composable naming follows Vue conventions - **Ink/React:** public APIs are hooks (`useFocus`, `useInput`, ...), but return-type naming diff --git a/packages/runtime-tests/integration/layout/prop-reset.test.tsx b/packages/runtime-tests/integration/layout/prop-reset.test.tsx index 0721347..95db21e 100644 --- a/packages/runtime-tests/integration/layout/prop-reset.test.tsx +++ b/packages/runtime-tests/integration/layout/prop-reset.test.tsx @@ -283,6 +283,124 @@ test("marginX composes with marginLeft (specific edge wins) (Blocker 1)", async expect(lastFrame({ trimLines: true })).toBe(" X Y"); }); +// Withdrawing a more-specific edge override must fall back to the surviving +// shorthand, NOT collapse to 0. With `margin={5} marginTop={8}`, the per-edge +// EDGE_TOP overrides EDGE_ALL; the old per-setter code reset EDGE_TOP to 0 on +// removal, and EDGE_TOP=0 still overrides EDGE_ALL=5, so the top margin wrongly +// collapsed to 0 instead of falling back to the surviving margin={5}. The +// family-recompute (reconcileMarginEdges) resolves each physical edge from the +// full prop set, so the withdrawn marginTop correctly falls back to 5. + +test("removing marginTop falls back to surviving margin shorthand (G19 family-recompute)", async () => { + // margin={5} marginTop={8}: top=8 while set. Remove marginTop → top must fall + // back to margin=5 (5 blank lines above 'x'), NOT collapse to 0. + const hasTop = shallowRef(true); + + const Dynamic = defineComponent(() => () => ( + + x + + )); + + const { lastFrame } = await render(Dynamic, { columns: 100 }); + // margin=5 left + marginTop=8 → 8 blank rows then " x" + expect(lastFrame({ trimLines: true })).toBe("\n\n\n\n\n\n\n\n x\n\n\n\n\n"); + + hasTop.value = false; + await nextTick(); + // marginTop withdrawn → top falls back to margin=5 (5 blank rows), NOT 0 + expect(lastFrame({ trimLines: true })).toBe("\n\n\n\n\n x\n\n\n\n\n"); +}); + +test("removing marginX falls back to surviving margin shorthand (G19 family-recompute)", async () => { + // margin={5} marginX={2}: left/right=2 while set. Remove marginX → left/right + // must fall back to margin=5, NOT collapse to 0. + const hasX = shallowRef(true); + + const Dynamic = defineComponent(() => () => ( + + + X + + Y + + )); + + const { lastFrame } = await render(Dynamic, { columns: 100 }); + // Inner box has top/bottom margin 5 and left/right margin 2 (marginX). In the + // row, sibling Y has no margin so it sits at row 0 col 5 (after the inner box's + // 2-left + X + 2-right = 5 wide); X sits at row 5 col 2. + expect(lastFrame({ trimLines: true })).toBe(" Y\n\n\n\n\n X\n\n\n\n\n"); + + hasX.value = false; + await nextTick(); + // marginX withdrawn → left/right fall back to margin=5. Inner box is now + // 5 + X + 5 = 11 wide, so Y moves to col 11; X sits at row 5 col 5. NOT col 0. + expect(lastFrame({ trimLines: true })).toBe(" Y\n\n\n\n\n X\n\n\n\n\n"); +}); + +test("removing paddingTop falls back to surviving padding shorthand (G19 family-recompute)", async () => { + // padding={4} paddingTop={8}: top pad=8 while set. Remove paddingTop → top must + // fall back to padding=4, NOT collapse to 0. + const hasTop = shallowRef(true); + + const Dynamic = defineComponent(() => () => ( + + x + + )); + + const { lastFrame } = await render(Dynamic, { columns: 100 }); + // top pad 8 → 8 blank rows then " x" (4 left pad) + expect(lastFrame({ trimLines: true })).toBe("\n\n\n\n\n\n\n\n x\n\n\n\n"); + + hasTop.value = false; + await nextTick(); + // paddingTop withdrawn → top pad falls back to padding=4 (4 blank rows), NOT 0 + expect(lastFrame({ trimLines: true })).toBe("\n\n\n\n x\n\n\n\n"); +}); + +test("removing paddingX falls back to surviving padding shorthand (G19 family-recompute)", async () => { + // padding={4} paddingX={1}: left/right pad=1 while set. Remove paddingX → + // left/right must fall back to padding=4, NOT collapse to 0. + const hasX = shallowRef(true); + + const Dynamic = defineComponent(() => () => ( + + + X + + Y + + )); + + const { lastFrame } = await render(Dynamic, { columns: 100 }); + // Inner box has top/bottom padding 4 and left/right padding 1 (paddingX). The + // box is 1 + X + 1 = 3 wide, so sibling Y sits at row 0 col 3; X at row 4 col 1. + expect(lastFrame({ trimLines: true })).toBe(" Y\n\n\n\n X\n\n\n\n"); + + hasX.value = false; + await nextTick(); + // paddingX withdrawn → left/right pad fall back to padding=4. Inner box is now + // 4 + X + 4 = 9 wide, so Y moves to col 9; X sits at row 4 col 4. NOT col 0. + expect(lastFrame({ trimLines: true })).toBe(" Y\n\n\n\n X\n\n\n\n"); +}); + +test("keeping marginTop over margin shorthand still wins (no-regression)", async () => { + // Control: with BOTH margin={5} and marginTop={8} present and nothing removed, + // the more-specific marginTop=8 must win for the top edge while the other edges + // stay at margin=5. Guards against the family-recompute over-zeroing. + const Dynamic = defineComponent(() => () => ( + + x + + )); + + const { lastFrame } = await render(Dynamic, { columns: 100 }); + // 8 blank rows above, 5 left margin, then trailing 5 below + expect(lastFrame({ trimLines: true })).toBe("\n\n\n\n\n\n\n\n x\n\n\n\n\n"); +}); + // Removing `display` resets to the DEFAULT (visible / DISPLAY_FLEX), not persist // and not hide. This is a DELIBERATE divergence from Ink documented in // .agents/docs/ink-divergences.md ("Removing `display` resets to the default"): diff --git a/packages/runtime-tests/unit/yoga-prop-reset.test.ts b/packages/runtime-tests/unit/yoga-prop-reset.test.ts index eebc413..604d6cd 100644 --- a/packages/runtime-tests/unit/yoga-prop-reset.test.ts +++ b/packages/runtime-tests/unit/yoga-prop-reset.test.ts @@ -5,7 +5,13 @@ import { expect, test } from "vite-plus/test"; // so it cannot be imported here directly. We reference the stable yoga enum // values numerically (EDGE_LEFT=0, EDGE_TOP=1, DIRECTION_LTR=1) and inspect the // computed layout via the node's own getComputedMargin/Padding. -import { applyYogaProp, attachYoga, detachYoga } from "../../runtime/src/host/yoga.ts"; +import { + applyYogaProp, + attachYoga, + detachYoga, + reconcileMarginEdges, + reconcilePaddingEdges, +} from "../../runtime/src/host/yoga.ts"; import { createBox } from "../../runtime/src/host/nodes.ts"; // yoga-layout YGEnums (generated/YGEnums.ts) — stable values. @@ -18,8 +24,16 @@ const DISPLAY_NONE = 1; // Blocker 2: Vue's HOST renderer passes next=null (not undefined) when a key // disappears from a spread props object (e.g. Static spreads `style` into host -// props). applyYogaProp's reset path must treat null the same as undefined so a +// props). The removal reset path must treat null the same as undefined so a // removed yoga key resets to its documented default instead of writing NaN/0. +// +// NOTE on layer: margin/padding edges are reconciled from the FULL el.props by +// reconcileMarginEdges / reconcilePaddingEdges (their per-prop yoga setters are +// no-ops — an edge depends on the specific edge + axis + all-edges shorthands +// together). So these tests drive the reconcilers directly with the el.props +// patchProp would have stored (a removed key is null/undefined → treated as +// absent by the reconciler), mirroring the border reconcile pattern. display (a +// single-prop reset) still goes through applyYogaProp below. function freshBox() { const box = createBox(); @@ -29,12 +43,13 @@ function freshBox() { test("null removal of marginTop resets to default (Blocker 2)", () => { const box = freshBox(); - applyYogaProp(box, "marginTop", 4, undefined); + reconcileMarginEdges(box, { marginTop: 4 }); box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(4); - // Removal arrives as next=null (key removed from a spread props object). - applyYogaProp(box, "marginTop", null, 4); + // Removal arrives as null in el.props (key removed from a spread props object); + // the reconciler treats null/undefined as absent → edge falls back to 0. + reconcileMarginEdges(box, { marginTop: null }); box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(0); @@ -43,11 +58,11 @@ test("null removal of marginTop resets to default (Blocker 2)", () => { test("null removal of paddingLeft resets to default (Blocker 2)", () => { const box = freshBox(); - applyYogaProp(box, "paddingLeft", 5, undefined); + reconcilePaddingEdges(box, { paddingLeft: 5 }); box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(5); - applyYogaProp(box, "paddingLeft", null, 5); + reconcilePaddingEdges(box, { paddingLeft: null }); box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(0); @@ -56,9 +71,9 @@ test("null removal of paddingLeft resets to default (Blocker 2)", () => { test("raw null does not corrupt a yoga dimension to NaN (Blocker 2)", () => { const box = freshBox(); - applyYogaProp(box, "marginTop", 7, undefined); + reconcileMarginEdges(box, { marginTop: 7 }); // Removal arrives as null; must reset to 0, never NaN. - applyYogaProp(box, "marginTop", null, 7); + reconcileMarginEdges(box, { marginTop: null }); box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); const m = box.yoga.getComputedMargin(EDGE_TOP as never); expect(Number.isNaN(m)).toBe(false); @@ -66,6 +81,164 @@ test("raw null does not corrupt a yoga dimension to NaN (Blocker 2)", () => { detachYoga(box); }); +// Family-recompute fallback: a withdrawn more-specific edge must fall back to the +// surviving shorthand, NOT collapse to 0 (the bug). EDGE_TOP overrides EDGE_ALL +// even at 0, so the old per-setter reset to 0 beat a surviving margin={5}. (G19) + +test("withdrawn marginTop falls back to surviving margin shorthand, not 0 (G19)", () => { + const box = freshBox(); + reconcileMarginEdges(box, { margin: 5, marginTop: 8 }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(8); + + // marginTop removed (null in el.props); top must fall back to margin=5, NOT 0. + reconcileMarginEdges(box, { margin: 5, marginTop: null }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + + detachYoga(box); +}); + +test("withdrawn paddingLeft falls back to surviving padding shorthand, not 0 (G19)", () => { + const box = freshBox(); + reconcilePaddingEdges(box, { padding: 4, paddingLeft: 7 }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(7); + + reconcilePaddingEdges(box, { padding: 4, paddingLeft: null }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(4); + + detachYoga(box); +}); + +// Non-finite numeric edge (NaN/±Infinity, e.g. a user calc like 0/0): the OLD +// per-setter code did setMargin(EDGE_TOP, NaN), which yoga treats as unset so the +// edge fell back to the surviving shorthand → top = margin = 5. The reconcile must +// preserve that by treating a present-but-non-finite value as ABSENT and falling +// THROUGH to the next precedence level (axis → all → 0), not resolving it to 0. + +test("non-finite marginTop (NaN) falls through to surviving margin shorthand, not 0 (G19)", () => { + const box = freshBox(); + reconcileMarginEdges(box, { margin: 5, marginTop: NaN }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + + detachYoga(box); +}); + +test("non-finite paddingLeft (NaN) falls through to surviving padding shorthand, not 0 (G19)", () => { + const box = freshBox(); + reconcilePaddingEdges(box, { padding: 5, paddingLeft: NaN }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(5); + + detachYoga(box); +}); + +// Explicit zero is NOT non-finite — Number(0) is finite — so an explicit edge +// override of 0 must STILL win over the shorthand (resolve to 0), distinct from +// the NaN fall-through above. + +test("explicit marginTop=0 overrides the margin shorthand → top is 0, not 5 (G19)", () => { + const box = freshBox(); + reconcileMarginEdges(box, { margin: 5, marginTop: 0 }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(0); + + detachYoga(box); +}); + +// --- spacing value contract (PR #184) ------------------------------------ +// +// Spacing props are typed `number` (box-props.ts) and Ink's margin/padding are +// number-only. The family recompute resolves an edge from a value only when it +// coerces to a FINITE number, with one carve-out for template ergonomics: a Vue +// STATIC template attribute (``) arrives as the numeric STRING +// "5", which must still resolve to 5. Any OTHER non-numeric value ("50%", "foo", +// "") is treated as not-set and falls through to the surviving shorthand — the +// reconcile drops the OLD per-setter code's incidental, off-contract string +// forwarding (setMargin(edge, "50%") → yoga percent; setMargin(edge, "foo") → +// throw). These pin that contract so it can't silently drift. + +test('numeric string margin="5" resolves to 5 (static template attribute ergonomics)', () => { + const box = freshBox(); + // `` reaches the host renderer as the string "5"; the family + // recompute coerces it like the numeric prop margin={5}. + reconcileMarginEdges(box, { margin: "5" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + expect(box.yoga.getComputedMargin(EDGE_LEFT as never)).toBe(5); + + detachYoga(box); +}); + +test('numeric string marginTop="8" resolves to 8 and overrides the shorthand', () => { + const box = freshBox(); + reconcileMarginEdges(box, { margin: 5, marginTop: "8" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(8); + + detachYoga(box); +}); + +test('non-numeric string marginTop="50%" falls through to the surviving margin shorthand (PR #184: no longer a yoga percent)', () => { + const box = freshBox(); + // OLD per-setter code forwarded "50%" raw → yoga read it as a 50% percent + // margin. The typed contract is number-only, so "50%" is now off-contract / + // not-set and the edge falls back to the surviving margin shorthand. + reconcileMarginEdges(box, { margin: 5, marginTop: "50%" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + + detachYoga(box); +}); + +test('non-numeric string paddingLeft="50%" falls through to the surviving padding shorthand (PR #184: no longer a yoga percent)', () => { + const box = freshBox(); + reconcilePaddingEdges(box, { padding: 4, paddingLeft: "50%" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(4); + + detachYoga(box); +}); + +test('junk string marginTop="foo" falls through to the surviving margin shorthand (PR #184: no longer throws)', () => { + const box = freshBox(); + // OLD per-setter code did setMargin(EDGE_TOP, "foo") which threw; now it is + // not-set and falls back to the shorthand without throwing. + expect(() => { + reconcileMarginEdges(box, { margin: 5, marginTop: "foo" }); + }).not.toThrow(); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + + detachYoga(box); +}); + +// Empty-string tweak (PR #184): `Number("") === 0` would otherwise make +// marginTop="" resolve to 0 (overriding the shorthand) while every other +// non-numeric string falls through — `present()` excludes "" so the contract is +// uniform: only numeric strings are coerced, all other strings fall through. + +test('empty string marginTop="" falls through to the surviving margin shorthand, not 0 (PR #184 "" tweak)', () => { + const box = freshBox(); + reconcileMarginEdges(box, { margin: 5, marginTop: "" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedMargin(EDGE_TOP as never)).toBe(5); + + detachYoga(box); +}); + +test('empty string paddingLeft="" falls through to the surviving padding shorthand, not 0 (PR #184 "" tweak)', () => { + const box = freshBox(); + reconcilePaddingEdges(box, { padding: 4, paddingLeft: "" }); + box.yoga.calculateLayout(undefined, undefined, DIRECTION_LTR as never); + expect(box.yoga.getComputedPadding(EDGE_LEFT as never)).toBe(4); + + detachYoga(box); +}); + // display: removing/undefining `display` resets to the DEFAULT (DISPLAY_FLEX = // visible), a DELIBERATE divergence from Ink (which hides on present-undefined). // See .agents/docs/ink-divergences.md ("Removing `display` resets to the diff --git a/packages/runtime/src/host/node-ops.ts b/packages/runtime/src/host/node-ops.ts index 67036e9..79bee16 100644 --- a/packages/runtime/src/host/node-ops.ts +++ b/packages/runtime/src/host/node-ops.ts @@ -21,6 +21,10 @@ import { isYogaProp, BORDER_PROPS, reconcileBorderEdges, + MARGIN_PROPS, + PADDING_PROPS, + reconcileMarginEdges, + reconcilePaddingEdges, bindTextMeasure, markTextDirty, markTransformDirty, @@ -70,6 +74,26 @@ const STYLE_PROPS = new Set([ "overflow", "overflowX", "overflowY", + // Margin/padding families are yoga-only (not visual), but each physical edge + // depends on up to three of these props together, so reconcileMargin/PaddingEdges + // must read the full set from el.props. Storing them here is how they get there; + // no paint-pass consumer reads margin/padding from el.props (verified), so this + // is purely the reconcile's data source — same role STYLE_PROPS plays for the + // border per-edge toggles above. + "margin", + "marginX", + "marginY", + "marginTop", + "marginBottom", + "marginLeft", + "marginRight", + "padding", + "paddingX", + "paddingY", + "paddingTop", + "paddingBottom", + "paddingLeft", + "paddingRight", ]); /** Walk up the DOM tree to find the root node. */ @@ -428,6 +452,19 @@ export function buildNodeOps(options: TtyRendererOptions): RendererOptions }).props); } + // Margin/padding edges depend jointly on the specific edge + axis + all-edges + // shorthands (a yoga setter sees one value, and a more-specific edge overrides + // a shorthand even at 0), so any family-prop change triggers a full recompute + // from el.props — same pattern as border. The per-prop yoga setters are no-ops; + // these reconcilers are the single source of truth. el.props was just updated + // above (margin/padding keys are in STYLE_PROPS), so a withdrawn override + // correctly falls back to the surviving shorthand. (G19) + if (MARGIN_PROPS.has(key)) { + reconcileMarginEdges(el, (el as { props: Record }).props); + } + if (PADDING_PROPS.has(key)) { + reconcilePaddingEdges(el, (el as { props: Record }).props); + } } else if (STYLE_PROPS.has(key)) { (el as { props: Record }).props[key] = next; } else if (key === "aria-role" || key === "ariaRole") { diff --git a/packages/runtime/src/host/yoga.ts b/packages/runtime/src/host/yoga.ts index 9ccf3c9..3317b26 100644 --- a/packages/runtime/src/host/yoga.ts +++ b/packages/runtime/src/host/yoga.ts @@ -278,30 +278,34 @@ const YOGA_PROP_SETTERS: Record void> = { // Ink default: rowGap=0 (yoga default). Reset to 0 on removal. (G19) rowGap: (n, v) => n.setGap(Yoga.GUTTER_ROW, v == null ? 0 : (v as number)), - // Ink default: margin=0 (yoga default). Reset all margin edges to 0 on removal. (G19) - margin: (n, v) => n.setMargin(Yoga.EDGE_ALL, v == null ? 0 : (v as number)), - // marginX/marginY map to the HORIZONTAL/VERTICAL axis edges (matching Ink - // styles.ts applyMarginStyles). These compose with the specific edges per - // yoga precedence (EDGE_START/END/TOP/BOTTOM override the axis), so removing - // the axis resets only the axis edge and never clobbers a surviving - // marginLeft/Right/Top/Bottom. (Blocker 1) - marginX: (n, v) => n.setMargin(Yoga.EDGE_HORIZONTAL, v == null ? 0 : (v as number)), - marginY: (n, v) => n.setMargin(Yoga.EDGE_VERTICAL, v == null ? 0 : (v as number)), - marginTop: (n, v) => n.setMargin(Yoga.EDGE_TOP, v == null ? 0 : (v as number)), - marginBottom: (n, v) => n.setMargin(Yoga.EDGE_BOTTOM, v == null ? 0 : (v as number)), - marginLeft: (n, v) => n.setMargin(Yoga.EDGE_START, v == null ? 0 : (v as number)), - marginRight: (n, v) => n.setMargin(Yoga.EDGE_END, v == null ? 0 : (v as number)), + // margin/padding families do NOT compute their own edge widths here. Each + // PHYSICAL edge depends on up to three props together (the specific edge, the + // axis shorthand, the all-edges shorthand), and per yoga precedence the + // more-specific edge OVERRIDES the shorthand even when set to 0 — so a single + // yoga setter that sees one value can't reconcile the family. In particular, + // withdrawing `marginTop` from `margin={5} marginTop={8}` used to setMargin( + // EDGE_TOP,0), and EDGE_TOP=0 still overrides EDGE_ALL=5, collapsing the top + // margin to 0 instead of falling back to the surviving margin={5}. patchProp + // owns the joint reconciliation via reconcileMarginEdges / reconcilePaddingEdges + // (below), which read the full el.props and resolve each physical edge with + // explicit precedence. These no-op entries exist only so isYogaProp still routes + // margin/padding props through the yoga branch (which also stores them into + // el.props for the reconcile). (G19; mirrors the border reconcile pattern.) + margin: () => {}, + marginX: () => {}, + marginY: () => {}, + marginTop: () => {}, + marginBottom: () => {}, + marginLeft: () => {}, + marginRight: () => {}, - // Ink default: padding=0 (yoga default). Reset all padding edges to 0 on removal. (G19) - padding: (n, v) => n.setPadding(Yoga.EDGE_ALL, v == null ? 0 : (v as number)), - // paddingX/paddingY map to the HORIZONTAL/VERTICAL axis edges (matching Ink - // styles.ts applyPaddingStyles); same composition/reset semantics as margin. - paddingX: (n, v) => n.setPadding(Yoga.EDGE_HORIZONTAL, v == null ? 0 : (v as number)), - paddingY: (n, v) => n.setPadding(Yoga.EDGE_VERTICAL, v == null ? 0 : (v as number)), - paddingTop: (n, v) => n.setPadding(Yoga.EDGE_TOP, v == null ? 0 : (v as number)), - paddingBottom: (n, v) => n.setPadding(Yoga.EDGE_BOTTOM, v == null ? 0 : (v as number)), - paddingLeft: (n, v) => n.setPadding(Yoga.EDGE_LEFT, v == null ? 0 : (v as number)), - paddingRight: (n, v) => n.setPadding(Yoga.EDGE_RIGHT, v == null ? 0 : (v as number)), + padding: () => {}, + paddingX: () => {}, + paddingY: () => {}, + paddingTop: () => {}, + paddingBottom: () => {}, + paddingLeft: () => {}, + paddingRight: () => {}, // borderStyle and the per-edge toggles do NOT compute their own edge widths // here: an edge's width depends on BOTH borderStyle and that edge's per-edge @@ -469,6 +473,115 @@ export function reconcileBorderEdges(node: YogaCarrier, props: Record`) as the string "5". +// +// Three reasons a value FALLS THROUGH (not present → next precedence level +// axis → all → 0, instead of resolving to 0): +// 1. withdrawn prop (null/undefined), +// 2. present-but-non-finite number (NaN/±Infinity from a bad user calc like 0/0) +// — preserves the prior setMargin(EDGE_TOP, NaN) → yoga-treats-as-unset +// fallback, which the now-zeroed composite edges can no longer provide +// implicitly, so it is made explicit here, and +// 3. an off-contract non-numeric string ("50%", "foo", …). The old per-setter +// code forwarded such strings raw to yoga (so "50%" incidentally became a +// yoga percent and "foo" threw); normalizing them to not-set is consistent +// with the typed number contract + Ink. The empty string "" is excluded too, +// since `Number("") === 0` would otherwise make `marginTop=""` resolve to 0 +// while every other non-numeric string falls through — an inconsistency, not +// a contract worth keeping. +// (An explicit 0 is finite, so it still counts as present and correctly overrides +// the shorthand to 0 — distinct from the fall-through cases above.) +function present(props: Record, key: string): boolean { + const v = props[key]; + return v != null && v !== "" && Number.isFinite(Number(v)); +} + +/** + * Recompute all four PHYSICAL margin edges from a box's full prop set. Each edge + * resolves with most-specific-wins precedence (specific edge → axis → all → 0): + * top = marginTop ?? marginY ?? margin ?? 0 (etc.) + * then the four physical edges are set and the composite edges (ALL/HORIZONTAL/ + * VERTICAL) are ZEROED so nothing layers on top of them. + * + * Why this and not the obvious per-setter mapping (margin→EDGE_ALL, marginX→ + * EDGE_HORIZONTAL, marginTop→EDGE_TOP, …): an edge depends on up to THREE props + * together and a single yoga setter sees only one. Per yoga edge precedence a more + * specific edge OVERRIDES a composite EVEN WHEN SET TO 0, so resetting a withdrawn + * `marginTop` to 0 (the old code) still beats a surviving `margin={5}` → + * `getComputedMargin(TOP)` collapsed to 0 instead of 5. Resolving every physical + * edge from el.props and zeroing the composites removes that layering entirely, so + * a withdrawn override falls back to whatever shorthand still applies. Verified + * against yoga-layout@3.2.1 to produce identical getComputedMargin for the SET path + * as the old per-setter code across representative combinations. (G19) + * + * NOTE the EDGE_START/END (not LEFT/RIGHT) mapping for left/right is preserved from + * the prior margin setters — margin uses start/end edges, padding uses left/right. + */ +export function reconcileMarginEdges(node: YogaCarrier, props: Record): void { + const y = node.yoga as YogaNode; + const pick = (specific: string, axis: string): number => { + if (present(props, specific)) return Number(props[specific]); + if (present(props, axis)) return Number(props[axis]); + if (present(props, "margin")) return Number(props["margin"]); + return 0; + }; + y.setMargin(Yoga.EDGE_TOP, pick("marginTop", "marginY")); + y.setMargin(Yoga.EDGE_BOTTOM, pick("marginBottom", "marginY")); + y.setMargin(Yoga.EDGE_START, pick("marginLeft", "marginX")); + y.setMargin(Yoga.EDGE_END, pick("marginRight", "marginX")); + // Zero the composites so the four physical edges above are authoritative. + y.setMargin(Yoga.EDGE_ALL, 0); + y.setMargin(Yoga.EDGE_HORIZONTAL, 0); + y.setMargin(Yoga.EDGE_VERTICAL, 0); +} + +/** + * Padding analogue of {@link reconcileMarginEdges}. Same precedence and composite- + * zeroing, but padding maps left/right to EDGE_LEFT/EDGE_RIGHT (margin uses + * START/END) — preserving the prior padding setters' edge mapping. (G19) + */ +export function reconcilePaddingEdges(node: YogaCarrier, props: Record): void { + const y = node.yoga as YogaNode; + const pick = (specific: string, axis: string): number => { + if (present(props, specific)) return Number(props[specific]); + if (present(props, axis)) return Number(props[axis]); + if (present(props, "padding")) return Number(props["padding"]); + return 0; + }; + y.setPadding(Yoga.EDGE_TOP, pick("paddingTop", "paddingY")); + y.setPadding(Yoga.EDGE_BOTTOM, pick("paddingBottom", "paddingY")); + y.setPadding(Yoga.EDGE_LEFT, pick("paddingLeft", "paddingX")); + y.setPadding(Yoga.EDGE_RIGHT, pick("paddingRight", "paddingX")); + y.setPadding(Yoga.EDGE_ALL, 0); + y.setPadding(Yoga.EDGE_HORIZONTAL, 0); + y.setPadding(Yoga.EDGE_VERTICAL, 0); +} + const RESETTABLE_PROPS = new Set([ // Already handled undefined in their setters (reset to yoga/Ink default on removal): "width",