fix(runtime): gate per-edge border width on borderStyle (align Ink) (#168)
Per-edge border props (borderTop/Bottom/Left/Right) reserved 1 yoga cell whenever truthy, regardless of borderStyle. Since these props default to `true`, toggling one on an UPDATE while borderStyle stays unset (Vue patches only the changed per-edge prop, not borderStyle) left a spurious 1-cell inset with no border ever drawn — content shifted to "\n HELLO" instead of "HELLO". Mirror Ink's applyBorderStyles: an edge's width is `borderStyle ? 1 : 0`, forced to 0 when that edge is explicitly `false`. A per-edge toggle can only SUBTRACT, never add. The per-edge yoga setters become no-ops; patchProp now recomputes all four edges from el.props on any border-prop change via the new reconcileBorderEdges helper, so borderStyle flipping in EITHER direction (set->unset zeroes, unset->set re-reserves) and per-edge toggles are all handled jointly — the computation a single (n, v) yoga setter cannot do. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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(() => () => (
|
||||||
|
<Box
|
||||||
|
width={20}
|
||||||
|
borderTop={showEdges.value}
|
||||||
|
borderBottom={showEdges.value}
|
||||||
|
borderLeft={showEdges.value}
|
||||||
|
borderRight={showEdges.value}
|
||||||
|
>
|
||||||
|
<Text>HELLO</Text>
|
||||||
|
</Box>
|
||||||
|
)),
|
||||||
|
{ 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(() => () => (
|
||||||
|
<Box borderStyle="round" borderTop={showTop.value} alignSelf="flex-start">
|
||||||
|
<Text>Content</Text>
|
||||||
|
</Box>
|
||||||
|
)),
|
||||||
|
{ 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<string | undefined>(undefined);
|
||||||
|
const { lastFrame } = await render(
|
||||||
|
defineComponent(() => () => (
|
||||||
|
<Box width={20} borderStyle={style.value as never}>
|
||||||
|
<Text>HELLO</Text>
|
||||||
|
</Box>
|
||||||
|
)),
|
||||||
|
{ 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
|
// hide top border
|
||||||
test("hide top border", async ({ expect }) => {
|
test("hide top border", async ({ expect }) => {
|
||||||
const { lastFrame } = await render(
|
const { lastFrame } = await render(
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ import {
|
|||||||
removeYogaChild,
|
removeYogaChild,
|
||||||
applyYogaProp,
|
applyYogaProp,
|
||||||
isYogaProp,
|
isYogaProp,
|
||||||
|
BORDER_PROPS,
|
||||||
|
reconcileBorderEdges,
|
||||||
bindTextMeasure,
|
bindTextMeasure,
|
||||||
markTextDirty,
|
markTextDirty,
|
||||||
markTransformDirty,
|
markTransformDirty,
|
||||||
@@ -392,25 +394,18 @@ export function buildNodeOps(options: TtyRendererOptions): RendererOptions<TuiNo
|
|||||||
if (STYLE_PROPS.has(key)) {
|
if (STYLE_PROPS.has(key)) {
|
||||||
(el as { props: Record<string, unknown> }).props[key] = next;
|
(el as { props: Record<string, unknown> }).props[key] = next;
|
||||||
}
|
}
|
||||||
// Special case: borderStyle resets all four yoga border-edge widths to
|
// Border edge widths depend jointly on borderStyle AND each per-edge prop
|
||||||
// 1 (or 0). If per-edge toggles (borderTop/Bottom/Left/Right) were
|
// (a yoga setter sees only one value), so any border-prop change triggers a
|
||||||
// already applied before this patch, their values were clobbered.
|
// full recompute from el.props — mirroring Ink's applyBorderStyles, which
|
||||||
// Re-apply any per-edge toggles that are stored in el.props so that
|
// rewrites all four edges on any border change. The per-edge yoga setters
|
||||||
// yoga reflects the user's explicit per-edge settings.
|
// are intentional no-ops; this is the single source of truth. Reading
|
||||||
//
|
// el.props (just updated above for STYLE_PROPS) means borderStyle flipping
|
||||||
// Only re-apply when `next` is truthy (i.e. a border style is actually
|
// in EITHER direction is handled: unset→set re-reserves, set→unset zeroes,
|
||||||
// being set). When borderStyle is cleared/undefined, applyYogaProp sets
|
// and a per-edge toggle while borderStyle stays unset reserves nothing (so
|
||||||
// all edges to 0 which is the correct final state — there is nothing to
|
// the borderTop/Left/etc defaults of `true` never inset content with no
|
||||||
// restore, and re-applying per-edge defaults (e.g. borderTop:true from
|
// border drawn).
|
||||||
// Box component defaults) would incorrectly reserve border space even
|
if (BORDER_PROPS.has(key)) {
|
||||||
// though no border is drawn.
|
reconcileBorderEdges(el, (el as { props: Record<string, unknown> }).props);
|
||||||
if (key === "borderStyle" && next) {
|
|
||||||
const props = (el as { props: Record<string, unknown> }).props;
|
|
||||||
for (const edge of ["borderTop", "borderBottom", "borderLeft", "borderRight"] as const) {
|
|
||||||
if (props[edge] !== undefined) {
|
|
||||||
applyYogaProp(el, edge, props[edge]);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} else if (STYLE_PROPS.has(key)) {
|
} else if (STYLE_PROPS.has(key)) {
|
||||||
(el as { props: Record<string, unknown> }).props[key] = next;
|
(el as { props: Record<string, unknown> }).props[key] = next;
|
||||||
|
|||||||
@@ -303,18 +303,18 @@ const YOGA_PROP_SETTERS: Record<string, (n: YogaNode, v: unknown) => void> = {
|
|||||||
paddingLeft: (n, v) => n.setPadding(Yoga.EDGE_LEFT, 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)),
|
paddingRight: (n, v) => n.setPadding(Yoga.EDGE_RIGHT, v == null ? 0 : (v as number)),
|
||||||
|
|
||||||
borderStyle: (n, v) => {
|
// borderStyle and the per-edge toggles do NOT compute their own edge widths
|
||||||
// Border occupies 1 cell on every side when a style is set.
|
// here: an edge's width depends on BOTH borderStyle and that edge's per-edge
|
||||||
const w = v ? 1 : 0;
|
// prop together, and a yoga setter only sees one value. patchProp owns the
|
||||||
n.setBorder(Yoga.EDGE_TOP, w);
|
// joint reconciliation via reconcileBorderEdges (below), which reads the full
|
||||||
n.setBorder(Yoga.EDGE_BOTTOM, w);
|
// el.props and mirrors Ink's applyBorderStyles. These no-op entries exist only
|
||||||
n.setBorder(Yoga.EDGE_LEFT, w);
|
// so isYogaProp still routes border props through the yoga branch (which also
|
||||||
n.setBorder(Yoga.EDGE_RIGHT, w);
|
// stores them into el.props for the paint pass).
|
||||||
},
|
borderStyle: () => {},
|
||||||
borderTop: (n, v) => n.setBorder(Yoga.EDGE_TOP, v ? 1 : 0),
|
borderTop: () => {},
|
||||||
borderBottom: (n, v) => n.setBorder(Yoga.EDGE_BOTTOM, v ? 1 : 0),
|
borderBottom: () => {},
|
||||||
borderLeft: (n, v) => n.setBorder(Yoga.EDGE_LEFT, v ? 1 : 0),
|
borderLeft: () => {},
|
||||||
borderRight: (n, v) => n.setBorder(Yoga.EDGE_RIGHT, v ? 1 : 0),
|
borderRight: () => {},
|
||||||
|
|
||||||
// Ink styles.ts applyDisplayStyles: `display === 'flex' ? DISPLAY_FLEX : DISPLAY_NONE`,
|
// 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
|
// 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);
|
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<string, unknown>): 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([
|
const RESETTABLE_PROPS = new Set([
|
||||||
// Already handled undefined in their setters (reset to yoga/Ink default on removal):
|
// Already handled undefined in their setters (reset to yoga/Ink default on removal):
|
||||||
"width",
|
"width",
|
||||||
|
|||||||
Reference in New Issue
Block a user