diff --git a/packages/runtime-tests/integration/components/borders.test.tsx b/packages/runtime-tests/integration/components/borders.test.tsx index 8f285b5..6b9e7fd 100644 --- a/packages/runtime-tests/integration/components/borders.test.tsx +++ b/packages/runtime-tests/integration/components/borders.test.tsx @@ -692,6 +692,115 @@ test("render border edge changes after update when borderStyle is unchanged", as `); }); +// Per-edge border props with NO borderStyle must reserve NO layout space. +// box-props.ts defaults borderTop/Bottom/Left/Right to `true`, and box.vue +// forwards them via v-bind="props". On an UPDATE that toggles a per-edge prop +// while borderStyle stays unset, Vue patches ONLY the changed per-edge prop — +// borderStyle is not re-patched — so a per-edge setter that reserves 1 cell +// regardless of borderStyle would survive and inset content by 1 cell with no +// border ever painted. Ink's applyBorderStyles gates edge width on borderStyle +// (borderWidth = currentStyle.borderStyle ? 1 : 0; edge === false ? 0 : +// borderWidth) — a per-edge toggle can only SUBTRACT, never add. Match that. +test("per-edge border toggle with no borderStyle reserves no space", async ({ expect }) => { + const showEdges = shallowRef(false); + const { lastFrame } = await render( + defineComponent(() => () => ( + + HELLO + + )), + { columns: 100 }, + ); + // No borderStyle → no inset regardless of per-edge flags. + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(`"HELLO"`); + + // Toggling the per-edge flags on (with borderStyle still unset) must NOT add + // any layout inset — frame stays exactly "HELLO", no spurious blank row / + // leading space. + showEdges.value = true; + await nextTick(); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(`"HELLO"`); + + // And toggling back off stays clean too. + showEdges.value = false; + await nextTick(); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(`"HELLO"`); +}); + +// Regression guard for the gate-on-borderStyle fix: a per-edge `false` toggled +// AFTER borderStyle is already set must still SUBTRACT that edge (Ink parity), +// and toggling it back on must restore it. borderStyle never changes here, so +// only the per-edge prop is re-patched on update — exercising the gated setter's +// "borderStyle present → edge reflects v" branch. +test("per-edge false with borderStyle set still subtracts on update", async ({ expect }) => { + const showTop = shallowRef(true); + const { lastFrame } = await render( + defineComponent(() => () => ( + + Content + + )), + { columns: 100 }, + ); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(` + "╭───────╮ + │Content│ + ╰───────╯" + `); + + showTop.value = false; + await nextTick(); + // borderTop=false subtracts the top edge → no top row, side rails start at row 0. + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(` + "│Content│ + ╰───────╯" + `); + + showTop.value = true; + await nextTick(); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(` + "╭───────╮ + │Content│ + ╰───────╯" + `); +}); + +// Regression guard: setting borderStyle on an EXISTING box (unset → set) must +// reserve all four edges. With the fix re-applying edges on borderStyle change +// in EITHER direction, an unset→set transition must add the border back even +// though the per-edge props never changed. +test("setting borderStyle after mount reserves all edges", async ({ expect }) => { + const style = shallowRef(undefined); + const { lastFrame } = await render( + defineComponent(() => () => ( + + HELLO + + )), + { columns: 100 }, + ); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(`"HELLO"`); + + style.value = "round"; + await nextTick(); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(` + "╭──────────────────╮ + │HELLO │ + ╰──────────────────╯" + `); + + // Clearing it again removes the border and the inset. + style.value = undefined; + await nextTick(); + expect(stripAnsi(lastFrame()!)).toMatchInlineSnapshot(`"HELLO"`); +}); + // hide top border test("hide top border", async ({ expect }) => { const { lastFrame } = await render( diff --git a/packages/runtime/src/host/node-ops.ts b/packages/runtime/src/host/node-ops.ts index 33be098..9a0db96 100644 --- a/packages/runtime/src/host/node-ops.ts +++ b/packages/runtime/src/host/node-ops.ts @@ -19,6 +19,8 @@ import { removeYogaChild, applyYogaProp, isYogaProp, + BORDER_PROPS, + reconcileBorderEdges, bindTextMeasure, markTextDirty, markTransformDirty, @@ -392,25 +394,18 @@ export function buildNodeOps(options: TtyRendererOptions): RendererOptions }).props[key] = next; } - // Special case: borderStyle resets all four yoga border-edge widths to - // 1 (or 0). If per-edge toggles (borderTop/Bottom/Left/Right) were - // already applied before this patch, their values were clobbered. - // Re-apply any per-edge toggles that are stored in el.props so that - // yoga reflects the user's explicit per-edge settings. - // - // Only re-apply when `next` is truthy (i.e. a border style is actually - // being set). When borderStyle is cleared/undefined, applyYogaProp sets - // all edges to 0 which is the correct final state — there is nothing to - // restore, and re-applying per-edge defaults (e.g. borderTop:true from - // Box component defaults) would incorrectly reserve border space even - // though no border is drawn. - if (key === "borderStyle" && next) { - const props = (el as { props: Record }).props; - for (const edge of ["borderTop", "borderBottom", "borderLeft", "borderRight"] as const) { - if (props[edge] !== undefined) { - applyYogaProp(el, edge, props[edge]); - } - } + // Border edge widths depend jointly on borderStyle AND each per-edge prop + // (a yoga setter sees only one value), so any border-prop change triggers a + // full recompute from el.props — mirroring Ink's applyBorderStyles, which + // rewrites all four edges on any border change. The per-edge yoga setters + // are intentional no-ops; this is the single source of truth. Reading + // el.props (just updated above for STYLE_PROPS) means borderStyle flipping + // in EITHER direction is handled: unset→set re-reserves, set→unset zeroes, + // and a per-edge toggle while borderStyle stays unset reserves nothing (so + // the borderTop/Left/etc defaults of `true` never inset content with no + // border drawn). + if (BORDER_PROPS.has(key)) { + reconcileBorderEdges(el, (el as { props: Record }).props); } } else if (STYLE_PROPS.has(key)) { (el as { props: Record }).props[key] = next; diff --git a/packages/runtime/src/host/yoga.ts b/packages/runtime/src/host/yoga.ts index 4632f30..9ccf3c9 100644 --- a/packages/runtime/src/host/yoga.ts +++ b/packages/runtime/src/host/yoga.ts @@ -303,18 +303,18 @@ const YOGA_PROP_SETTERS: Record void> = { 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)), - borderStyle: (n, v) => { - // Border occupies 1 cell on every side when a style is set. - const w = v ? 1 : 0; - n.setBorder(Yoga.EDGE_TOP, w); - n.setBorder(Yoga.EDGE_BOTTOM, w); - n.setBorder(Yoga.EDGE_LEFT, w); - n.setBorder(Yoga.EDGE_RIGHT, w); - }, - borderTop: (n, v) => n.setBorder(Yoga.EDGE_TOP, v ? 1 : 0), - borderBottom: (n, v) => n.setBorder(Yoga.EDGE_BOTTOM, v ? 1 : 0), - borderLeft: (n, v) => n.setBorder(Yoga.EDGE_LEFT, v ? 1 : 0), - borderRight: (n, v) => n.setBorder(Yoga.EDGE_RIGHT, v ? 1 : 0), + // 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 + // prop together, and a yoga setter only sees one value. patchProp owns the + // joint reconciliation via reconcileBorderEdges (below), which reads the full + // el.props and mirrors Ink's applyBorderStyles. These no-op entries exist only + // so isYogaProp still routes border props through the yoga branch (which also + // stores them into el.props for the paint pass). + borderStyle: () => {}, + borderTop: () => {}, + borderBottom: () => {}, + borderLeft: () => {}, + borderRight: () => {}, // Ink styles.ts applyDisplayStyles: `display === 'flex' ? DISPLAY_FLEX : DISPLAY_NONE`, // so ANY present value that isn't 'flex' (incl. off-spec strings reachable via a TS @@ -439,6 +439,36 @@ export function isYogaProp(key: string): boolean { return Object.hasOwn(YOGA_PROP_SETTERS, key); } +/** Props whose change requires recomputing the yoga border-edge widths. */ +export const BORDER_PROPS = new Set([ + "borderStyle", + "borderTop", + "borderBottom", + "borderLeft", + "borderRight", +]); + +/** + * Recompute all four yoga border-edge widths from a box's full prop set, mirroring + * Ink's applyBorderStyles (styles.ts:729-763): the per-side width is + * `borderStyle ? 1 : 0`, then each edge is forced to 0 when that edge's per-edge + * prop is explicitly `false`. So a per-edge toggle can only SUBTRACT an edge — it + * can NEVER add width without a borderStyle. This is the joint computation a + * single yoga setter cannot do (it sees only one value), and it must run on ANY + * border-prop change, including borderStyle flipping in EITHER direction + * (set→unset re-zeroes, unset→set re-reserves) — otherwise a per-edge toggle made + * while borderStyle stays unset would leave a spurious 1-cell inset with no border + * drawn (the per-edge props default to `true`). + */ +export function reconcileBorderEdges(node: YogaCarrier, props: Record): void { + const y = node.yoga as YogaNode; + const borderWidth = props["borderStyle"] ? 1 : 0; + y.setBorder(Yoga.EDGE_TOP, props["borderTop"] === false ? 0 : borderWidth); + y.setBorder(Yoga.EDGE_BOTTOM, props["borderBottom"] === false ? 0 : borderWidth); + y.setBorder(Yoga.EDGE_LEFT, props["borderLeft"] === false ? 0 : borderWidth); + y.setBorder(Yoga.EDGE_RIGHT, props["borderRight"] === false ? 0 : borderWidth); +} + const RESETTABLE_PROPS = new Set([ // Already handled undefined in their setters (reset to yoga/Ink default on removal): "width",