From 0c113a30637fe15b22f155d630bd31a71acb0a11 Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Fri, 29 May 2026 23:14:25 +0800 Subject: [PATCH] fix(runtime): don't paint Box backgroundColor onto border glyphs (Ink parity, G04) (#30) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ink's render-border.ts computes each border edge's background from borderBackgroundColor ?? borderBackgroundColor only — it never falls back to the Box's own backgroundColor. vue-tui's colorizeEdge had an extra `?? bgColor` fallback, so a Box with backgroundColor but no explicit border background painted its background onto the border glyphs too. Drop the fallback. Background still fills the inner content area; border glyphs are now uncolored unless an explicit border background is set. Tests rewritten to match Ink (per maintainer's align-to-Ink policy; see .agents/docs/parity-ledger.md Decisions log): - add failing-first repro "Box backgroundColor does not bleed onto border glyphs (Ink parity)" - "wrapped text preserves backgroundColor on every content line": assert inner rows carry bg, border rows don't (height 4->5 so text fits) - "Box background with border fills content area": snapshot updated so border rows have no bg Co-authored-by: Claude Opus 4.8 --- .agents/docs/parity-ledger.md | 66 ++++++++++++++----- .../components/background-color.test.tsx | 45 ++++++++++--- packages/runtime/src/paint/paint.ts | 6 +- 3 files changed, 90 insertions(+), 27 deletions(-) diff --git a/.agents/docs/parity-ledger.md b/.agents/docs/parity-ledger.md index dd7c8bc..fe45fdd 100644 --- a/.agents/docs/parity-ledger.md +++ b/.agents/docs/parity-ledger.md @@ -12,26 +12,40 @@ Refuted (NOT gaps — kept for the record): `exit()` second-wins — vue-tui is already guarded, not last-wins as earlier suspected; kitty key-release printable-text suppression — Ink behaves the same. +## Decisions log + +Non-obvious calls made while fixing gaps, recorded for review in the final report. + +- **2026-05-29 — Conflict policy (set by maintainer):** when an audit gap conflicts with an existing vue-tui test that deliberately asserts non-Ink behavior, align to Ink and rewrite the conflicting test — and record the decision here so it can be reviewed against expectations later. +- **2026-05-29 — Sequencing:** the P1 gaps G01 (Static unmount), G02 (useAnimation throttle coalescing), G03 (screen-reader render path) are architecturally invasive — they hinge on timing between Vue's reactivity and the scheduler's commit, where a mistake silently drops output. Deferred until the end-to-end pipeline (PR → codex → CI → auto-merge) is validated on a low-risk fix. Started with G04. +- **2026-05-29 — G04 (border background):** maintainer confirmed Ink behavior — border glyphs carry NO background unless `borderBackgroundColor` (or a per-edge variant) is set. Removed the `?? bgColor` fallback in `paint.ts` `colorizeEdge`. Tests rewritten to match Ink: + - _"wrapped text preserves backgroundColor on every line"_ → _"…on every content line"_: now asserts inner content rows carry the bg while the pure-border first/last rows do not (box height 4→5 so the wrapped text fits without overflowing onto the bottom border). + - _"Box background with border fills content area"_: inline snapshot updated — border rows (`╭───╮`/`╰───╯`) no longer carry bg; inner rows keep it. + - _"Box backgroundColor survives border rendering"_: unchanged, still passes (the inner fill keeps the bg). + - Added _"Box backgroundColor does not bleed onto border glyphs (Ink parity)"_ as the failing-first reproduction. + ## Confirmed gaps `status` ∈ `todo · in-progress · pr-open · merged · blocked`. Priority: correctness/behavior first, omissions next. -| id | area | summary | priority | status | branch | PR | -| --- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | -------- | ------ | ------ | --- | -| G01 | static-newline-spacer | Static keeps every already-written item permanently mounted instead of unmounting it | P1 | todo | — | — | -| G02 | app-exit-instances-animation-sr | useAnimation does not coalesce ticks within the render-throttle window — delta does not 'account for throttled renders' | P1 | todo | — | — | -| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | todo | — | — | -| G04 | box-layout-border | Border edges incorrectly inherit the Box backgroundColor | P2 | todo | — | — | -| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | todo | — | — | -| G06 | text-wrap-transform | Nested / transform fn receives hardcoded index 0 instead of childNode index | P2 | todo | — | — | -| G07 | input-keypress-kitty-paste | Kitty-protocol Ctrl+C triggers app exit in vue-tui but only suppresses the handler in Ink | P2 | todo | — | — | -| G08 | focus | useFocus does not react to changes in the id prop | P2 | todo | — | — | -| G09 | stdout-stderr-stdin-size-cursor | External stdout/stderr writes are not wrapped in synchronized-update (BSU/ESU) markers | P2 | todo | — | — | -| G10 | stdout-stderr-stdin-size-cursor | setRawMode silently no-ops in unsupported environments instead of throwing a descriptive error | P2 | todo | — | — | -| G11 | render-lifecycle-reconciler | Resize handler does not clear+reset on terminal-width decrease | P2 | todo | — | — | -| G12 | render-lifecycle-reconciler | Renderer frame width/rows lack terminal-size fallback (only ?? defaults) | P2 | todo | — | — | -| G13 | box-layout-border | Custom border style objects (BoxStyle) not supported | P3 | todo | — | — | -| G14 | app-exit-instances-animation-sr | No per-stdout instance reuse/guard — two concurrent renderers can compete for the same stdout | P3 | todo | — | — | +| id | area | summary | priority | status | branch | PR | +| --- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | -------- | ------- | ---------------------- | --- | +| G01 | static-newline-spacer | Static keeps every already-written item permanently mounted instead of unmounting it | P1 | todo | — | — | +| G02 | app-exit-instances-animation-sr | useAnimation does not coalesce ticks within the render-throttle window — delta does not 'account for throttled renders' | P1 | todo | — | — | +| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | todo | — | — | +| G04 | box-layout-border | Border edges incorrectly inherit the Box backgroundColor | P2 | pr-open | `fix/parity-border-bg` | #30 | +| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | todo | — | — | +| G06 | text-wrap-transform | Nested / transform fn receives hardcoded index 0 instead of childNode index | P2 | todo | — | — | +| G07 | input-keypress-kitty-paste | Kitty-protocol Ctrl+C triggers app exit in vue-tui but only suppresses the handler in Ink | P2 | todo | — | — | +| G08 | focus | useFocus does not react to changes in the id prop | P2 | todo | — | — | +| G09 | stdout-stderr-stdin-size-cursor | External stdout/stderr writes are not wrapped in synchronized-update (BSU/ESU) markers | P2 | todo | — | — | +| G10 | stdout-stderr-stdin-size-cursor | setRawMode silently no-ops in unsupported environments instead of throwing a descriptive error | P2 | todo | — | — | +| G11 | render-lifecycle-reconciler | Resize handler does not clear+reset on terminal-width decrease | P2 | todo | — | — | +| G12 | render-lifecycle-reconciler | Renderer frame width/rows lack terminal-size fallback (only ?? defaults) | P2 | todo | — | — | +| G13 | box-layout-border | Custom border style objects (BoxStyle) not supported | P3 | todo | — | — | +| G14 | app-exit-instances-animation-sr | No per-stdout instance reuse/guard — two concurrent renderers can compete for the same stdout | P3 | todo | — | — | +| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | todo | — | — | +| G16 | box-layout-border | Per-edge borderDimColor=false cannot override general borderDimColor (`\|\| dimAll` vs Ink's `??`) | P3 | todo | — | — | ## Gap details @@ -268,3 +282,23 @@ Not already covered: only public-api.test.ts references createApp, and merely to Severity low: this is a real gap that survives refutation, but it requires a user to deliberately mount two trees on one stdout, an uncommon path; the original claim also self-rated lower confidence. The guard is genuinely absent, so confirmed=true. - **Fix sketch:** Add an instances Map in render.ts; in mount() check for an existing live instance on the resolved stdout and, if found, write Ink's warning to process.stderr and return/reuse rather than wiring a second renderer; delete the entry on unmount/exit. + +### G15 — Vertical border sides not shifted up when borderTop=false + +_area:_ `box-layout-border` · _kind:_ behavior · _severity:_ low · _priority:_ P2 + +_Surfaced by codex during the G04 review (2026-05-29), not the sweep-1 audit._ + +- **Ink:** `/tmp/ink-40b3a75/src/render-border.ts:133` — vertical sides start at `y + offsetY` where `offsetY = showTopBorder ? 1 : 0`. With `borderTop={false}` the left/right rails begin on the first row. +- **vue-tui:** `packages/runtime/src/paint/paint.ts:378` — vertical sides always start at `i = 1`, so when the top border is hidden the left/right rails are shifted down by one row (and the first content row has no rails). +- **Fix sketch:** compute `offsetY = top ? 1 : 0` and start the vertical-side loop at `offsetY`, clamping the run length to the visible inner height; mirror Ink's per-edge offset for the bottom too. + +### G16 — Per-edge borderDimColor=false cannot override general borderDimColor + +_area:_ `box-layout-border` · _kind:_ behavior · _severity:_ low · _priority:_ P3 + +_Surfaced by codex during the G04 review (2026-05-29), not the sweep-1 audit._ + +- **Ink:** `/tmp/ink-40b3a75/src/render-border.ts:54` — `borderTopDimColor ?? borderDimColor`, so an explicit per-edge `false` overrides the general `borderDimColor`. +- **vue-tui:** `packages/runtime/src/paint/paint.ts:351` — `(props[\`border${capEdge}DimColor\`] as boolean | undefined) || dimAll`, so a per-edge `false`cannot turn dim off once`borderDimColor` is set. +- **Fix sketch:** use a nullish fallback `?? dimAll` (treating only `undefined` as "inherit"), matching Ink. diff --git a/packages/runtime-tests/integration/components/background-color.test.tsx b/packages/runtime-tests/integration/components/background-color.test.tsx index ddcac8c..e35fb23 100644 --- a/packages/runtime-tests/integration/components/background-color.test.tsx +++ b/packages/runtime-tests/integration/components/background-color.test.tsx @@ -4,6 +4,29 @@ import { render } from "@vue-tui/testing"; import { Box, Text } from "@vue-tui/runtime"; const BG_BLUE = "\x1b[44m"; +const BG_CYAN = "\x1b[46m"; + +// Ink parity (render-border.ts:35-52): a border edge's background comes only from +// borderBackgroundColor ?? borderBackgroundColor — it never falls back to the +// Box's own backgroundColor. So a Box with backgroundColor but no explicit border +// background must draw plain (uncolored-bg) border glyphs; the bg fills the inner +// content area only. +test("Box backgroundColor does not bleed onto border glyphs (Ink parity)", async ({ expect }) => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + Hi + + )), + { columns: 100 }, + ); + const lines = lastFrame()!.split("\n"); + // Top and bottom border rows carry no background. + expect(lines[0]).not.toContain(BG_CYAN); + expect(lines.at(-1)).not.toContain(BG_CYAN); + // Inner content rows still get the background fill. + expect(lines[1]).toContain(BG_CYAN); +}); test("Box backgroundColor produces ANSI background codes", async ({ expect }) => { const { frames } = await render(() => , { @@ -36,10 +59,10 @@ test("child Text inherits backgroundColor from parent Box", async ({ expect }) = expect(raw).toContain(BG_BLUE); }); -test("wrapped text preserves backgroundColor on every line", async ({ expect }) => { +test("wrapped text preserves backgroundColor on every content line", async ({ expect }) => { const { frames } = await render( () => ( - + long text here ), @@ -47,9 +70,15 @@ test("wrapped text preserves backgroundColor on every line", async ({ expect }) ); const raw = frames.at(-1)!; const lines = raw.split("\n").filter(Boolean); - for (const line of lines) { + // Ink parity: the inner content rows carry the background on every line; the + // first/last rows are pure border glyphs and carry no background. + const contentLines = lines.slice(1, -1); + expect(contentLines.length).toBeGreaterThan(0); + for (const line of contentLines) { expect(line).toContain(BG_BLUE); } + expect(lines[0]).not.toContain(BG_BLUE); + expect(lines.at(-1)).not.toContain(BG_BLUE); }); // --- Ink background tests --- @@ -301,11 +330,11 @@ test("Box background with border fills content area", async ({ expect }) => { { columns: 100 }, ); expect(lastFrame()).toMatchInlineSnapshot(` - "╭────────╮ - │Hi │ - │ │ - │ │ - ╰────────╯" + "╭────────╮ + │Hi │ + │ │ + │ │ + ╰────────╯" `); }); diff --git a/packages/runtime/src/paint/paint.ts b/packages/runtime/src/paint/paint.ts index 56bf6f0..3c896e0 100644 --- a/packages/runtime/src/paint/paint.ts +++ b/packages/runtime/src/paint/paint.ts @@ -343,17 +343,17 @@ function drawBorder( const right = props["borderRight"] !== false; const borderColor = props["borderColor"] as string | undefined; - const bgColor = props["backgroundColor"] as string | undefined; const dimAll = !!props["borderDimColor"]; function colorizeEdge(s: string, edge: "top" | "bottom" | "left" | "right"): string { const capEdge = edge.charAt(0).toUpperCase() + edge.slice(1); const edgeColor = (props[`border${capEdge}Color`] as string | undefined) ?? borderColor; const edgeDim = (props[`border${capEdge}DimColor`] as boolean | undefined) || dimAll; + // Ink parity (render-border.ts:44-52): an edge's background comes only from the + // per-edge or general border background — never from the Box's own backgroundColor. const edgeBg = (props[`border${capEdge}BackgroundColor`] as string | undefined) ?? - (props["borderBackgroundColor"] as string | undefined) ?? - bgColor; + (props["borderBackgroundColor"] as string | undefined); const p: TextProps = {}; if (edgeColor) p.color = edgeColor; if (edgeBg) p.backgroundColor = edgeBg;