From 6203ef3a23b2da07995e233818e8998f7692c365 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Sat, 22 Aug 2026 23:53:51 -0400 Subject: [PATCH 1/2] fix: bordered boxes now reserve layout space for their border sides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clay doesn't account for border widths in layout — borders were drawn as visual overlays, so any element with border width > padding had its content collapsed behind the border glyphs (or, for fit-height boxes, collapsed to a single row with top and bottom glyphs overlapping and children invisible). Fix: at pack time, compute effective padding per side as max(userPadding, borderWidth). Border glyphs draw in the same cells as before; only the Clay layout values change so the engine reserves those cells. Semantics of the max rule: - No explicit padding: border width becomes the effective padding; content lands inside the border, not behind it. - padding == borderWidth (prior workaround): max evaluates to the same value, no double-reservation; these elements render identically. - padding > borderWidth: extra padding provides breathing room inside the border, measured from the border edge inward. Callers who set padding == borderWidth as a workaround are unaffected. Downstream compensators (e.g. lgtm.shop Panel) will render identically until they drop the manual compensation on their next pin bump. Resolves Open Decision #4 in specs/renderer-spec.md. --- ops.ts | 8 +- specs/renderer-spec.md | 35 ++++++--- test/border.test.ts | 171 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 200 insertions(+), 14 deletions(-) diff --git a/ops.ts b/ops.ts index 3fd1d04..91100e2 100644 --- a/ops.ts +++ b/ops.ts @@ -161,10 +161,14 @@ export function pack( o = packAxis(view, o, l.height ?? { type: "fit" }); let p = l.padding ?? {}; + let bw = op.border; + let padLeft = Math.max(p.left ?? 0, sideWidth(bw?.left)); + let padRight = Math.max(p.right ?? 0, sideWidth(bw?.right)); + let padTop = Math.max(p.top ?? 0, sideWidth(bw?.top)); + let padBottom = Math.max(p.bottom ?? 0, sideWidth(bw?.bottom)); view.setUint32( o, - (p.left ?? 0) | ((p.right ?? 0) << 8) | ((p.top ?? 0) << 16) | - ((p.bottom ?? 0) << 24), + padLeft | (padRight << 8) | (padTop << 16) | (padBottom << 24), true, ); o += 4; diff --git a/specs/renderer-spec.md b/specs/renderer-spec.md index 398e78a..18caa09 100644 --- a/specs/renderer-spec.md +++ b/specs/renderer-spec.md @@ -797,12 +797,26 @@ excluding joined corner cells. Per-side attributes affect only the styling of corner cells; corner glyph shape selection (including rounded corners via `cornerRadius`) is unchanged. -**Border width and layout interaction.** In the underlying layout engine (Clay), -border configuration does not affect layout computation. This is Clay's intended -behavior. Borders are drawn as visual overlays within the element's bounding -box. A bordered element with zero padding will have its borders drawn over its -content. Callers must add padding equal to or greater than the border width to -prevent overlap. +**Border width and layout interaction.** The renderer automatically reserves +space for each enabled border side at pack time. For each side, the effective +padding passed to the layout engine is `max(userPadding, borderWidth)`. Border +glyphs are drawn at the same positions as before; the change is purely in how +much layout space Clay allocates for the element. + +Semantics of the `max` rule: + +- **No explicit padding.** The border width itself becomes the effective + padding, so content is placed inside the border rather than behind it. +- **User padding equal to border width (prior workaround pattern).** The `max` + evaluates to the same value, so these elements render identically — no + double-reservation, no migration required. +- **User padding greater than border width.** The extra padding beyond the + border width provides additional breathing room inside the border. Padding is + effectively measured from the border edge inward. + +This is a breaking change for callers who set padding _less than_ the border +width intending the overlap: those elements now have the overlap removed. +Callers who compensated by setting `padding == borderWidth` are unaffected. ### 12.3 Render return type @@ -1037,11 +1051,10 @@ resolution. 3. **Is `pack()` public API?** `pack()` is currently exported but is an internal implementation detail, not public API. `validate()` is public API. -4. **How should border widths interact with layout?** The current behavior - (borders do not affect layout) is inherited from the underlying layout - engine. The project has questioned whether this is the right design. This - specification describes the current behavior in Section 12.2 without - committing to it. +4. **How should border widths interact with layout?** RESOLVED. Border widths + are now accounted for in layout via `max(padding, borderWidth)` per side at + pack time (TypeScript layer). See Section 12.2 for the full semantics + including the no-double-reservation guarantee for prior compensators. 5. **What are the specific transfer encoding details?** The encoding structure is described in Section 12.1 as current implementation surface. Locking down diff --git a/test/border.test.ts b/test/border.test.ts index 3b78100..ddf2cae 100644 --- a/test/border.test.ts +++ b/test/border.test.ts @@ -1,6 +1,15 @@ -import { close, fixed, open, type OpenElement, rgba } from "../ops.ts"; +import { + close, + fixed, + grow, + open, + type OpenElement, + rgba, + text, +} from "../ops.ts"; import { createTerm } from "../term.ts"; import { describe, expect, it } from "./suite.ts"; +import { print } from "./print.ts"; const decode = (b: Uint8Array) => new TextDecoder().decode(b); @@ -477,3 +486,163 @@ describe("instances", () => { expect(again).not.toContain(FG.cyan); }); }); + +const trim = (s: string) => s.split("\n").map((l) => l.trimEnd()).join("\n"); + +describe("box model", () => { + it("full border with no padding reserves space: children visible, box is 3 rows", async () => { + let term = await createTerm({ width: 20, height: 10 }); + let result = term.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { width: fixed(14), direction: "ttb" }, + border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 }, + }), + text("CASE A"), + close(), + close(), + ]); + + expect(result.info.get("box")?.bounds.height).toBe(3); + expect(decode(result.output)).toContain("CASE A"); + }); + + it("partial borders (top+left) reserve only their sides", async () => { + let term = await createTerm({ width: 20, height: 10 }); + let result = term.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { width: fixed(14), direction: "ttb" }, + border: { color: WHITE, top: 1, left: 1 }, + }), + text("CASE B"), + close(), + close(), + ]); + + // top border reserves 1 row, no bottom border so no bottom reservation + expect(result.info.get("box")?.bounds.height).toBe(2); + expect(decode(result.output)).toContain("CASE B"); + }); + + it("padding == border renders identically to no padding (max semantics, no double-reservation)", async () => { + let nopad = await createTerm({ width: 20, height: 10 }); + let r1 = nopad.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { width: fixed(14), direction: "ttb" }, + border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 }, + }), + text("CONTENT"), + close(), + close(), + ]); + + let withpad = await createTerm({ width: 20, height: 10 }); + let r2 = withpad.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { + width: fixed(14), + direction: "ttb", + padding: { top: 1, right: 1, bottom: 1, left: 1 }, + }, + border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 }, + }), + text("CONTENT"), + close(), + close(), + ]); + + // Both produce the same box height: border reserves 1 per side, no double-reservation + expect(r1.info.get("box")?.bounds.height).toBe(3); + expect(r2.info.get("box")?.bounds.height).toBe(3); + }); + + it("explicit padding > border width adds breathing room inside the border", async () => { + let term = await createTerm({ width: 20, height: 10 }); + let result = term.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { + width: fixed(14), + direction: "ttb", + padding: { top: 2, bottom: 2 }, + }, + border: { color: WHITE, top: 1, bottom: 1 }, + }), + text("CONTENT"), + close(), + close(), + ]); + + // effective_top = max(2, 1) = 2, effective_bottom = max(2, 1) = 2 + // height = 2 + 1 text + 2 = 5 + expect(result.info.get("box")?.bounds.height).toBe(5); + }); + + it("nested two-tone bevel lays out without manual padding compensation", async () => { + let term = await createTerm({ width: 20, height: 10 }); + let result = term.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("outer", { + layout: { width: fixed(16), direction: "ttb" }, + border: { color: WHITE, top: 1, left: 1 }, + }), + open("inner", { + layout: { width: grow(), direction: "ttb" }, + border: { color: WHITE, bottom: 1, right: 1 }, + }), + text("NESTED"), + close(), + close(), + close(), + ]); + + // inner: effective_bottom=1, effective_right=1 → height = 0 + text(1) + 1 = 2 + // outer: effective_top=1, effective_left=1 → height = 1 + inner(2) + 0 = 3 + expect(result.info.get("outer")?.bounds.height).toBe(3); + expect(decode(result.output)).toContain("NESTED"); + }); + + it("visual: full border renders border glyphs around content", async () => { + let term = await createTerm({ width: 20, height: 10 }); + let out = trim( + print( + decode( + term.render([ + open("root", { + layout: { width: grow(), height: grow(), direction: "ttb" }, + }), + open("box", { + layout: { width: fixed(14), direction: "ttb" }, + border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 }, + }), + text("CASE A"), + close(), + close(), + ]).output, + ), + 20, + 10, + ), + ); + + let lines = out.split("\n"); + expect(lines[0]).toBe("┌────────────┐"); + expect(lines[1]).toBe("│CASE A │"); + expect(lines[2]).toBe("└────────────┘"); + }); +}); From def6883cdcb0b1d64efb9b80ffe6ecc6a3c5e4b0 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Tue, 25 Aug 2026 22:42:05 -0400 Subject: [PATCH 2/2] fix!: border padding is now additive, not max MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Border presence implies padding on that side equal to the border width. Callers who compensated by setting `padding == borderWidth` now receive double-reservation and must remove the workaround padding. `padding: 1` with `border: 1` → effective 2; `border: 1` alone → effective 1. --- ops.ts | 8 ++++---- specs/renderer-spec.md | 33 +++++++++++++++++---------------- test/border.test.ts | 13 +++++++------ test/clip.test.ts | 2 -- test/term.test.ts | 6 +----- 5 files changed, 29 insertions(+), 33 deletions(-) diff --git a/ops.ts b/ops.ts index 91100e2..9d48684 100644 --- a/ops.ts +++ b/ops.ts @@ -162,10 +162,10 @@ export function pack( let p = l.padding ?? {}; let bw = op.border; - let padLeft = Math.max(p.left ?? 0, sideWidth(bw?.left)); - let padRight = Math.max(p.right ?? 0, sideWidth(bw?.right)); - let padTop = Math.max(p.top ?? 0, sideWidth(bw?.top)); - let padBottom = Math.max(p.bottom ?? 0, sideWidth(bw?.bottom)); + let padLeft = (p.left ?? 0) + sideWidth(bw?.left); + let padRight = (p.right ?? 0) + sideWidth(bw?.right); + let padTop = (p.top ?? 0) + sideWidth(bw?.top); + let padBottom = (p.bottom ?? 0) + sideWidth(bw?.bottom); view.setUint32( o, padLeft | (padRight << 8) | (padTop << 16) | (padBottom << 24), diff --git a/specs/renderer-spec.md b/specs/renderer-spec.md index 18caa09..3d409da 100644 --- a/specs/renderer-spec.md +++ b/specs/renderer-spec.md @@ -799,24 +799,25 @@ corner cells; corner glyph shape selection (including rounded corners via **Border width and layout interaction.** The renderer automatically reserves space for each enabled border side at pack time. For each side, the effective -padding passed to the layout engine is `max(userPadding, borderWidth)`. Border +padding passed to the layout engine is `userPadding + borderWidth`. Border glyphs are drawn at the same positions as before; the change is purely in how much layout space Clay allocates for the element. -Semantics of the `max` rule: +Semantics of the additive rule: - **No explicit padding.** The border width itself becomes the effective - padding, so content is placed inside the border rather than behind it. -- **User padding equal to border width (prior workaround pattern).** The `max` - evaluates to the same value, so these elements render identically — no - double-reservation, no migration required. -- **User padding greater than border width.** The extra padding beyond the - border width provides additional breathing room inside the border. Padding is - effectively measured from the border edge inward. - -This is a breaking change for callers who set padding _less than_ the border -width intending the overlap: those elements now have the overlap removed. -Callers who compensated by setting `padding == borderWidth` are unaffected. + padding, so content is placed immediately inside the border. +- **Explicit user padding.** Adds breathing room _beyond_ the border edge. + `padding: 1` with `border: 1` places content 2 cells from the element edge — 1 + for the border glyph, 1 for the padding inset. +- **Prior workaround pattern (`padding == borderWidth`).** These callers now + receive double-reservation (effective = 2 × borderWidth). This is a breaking + change: remove the workaround padding to restore the original visual. + +This is a breaking change for callers who compensated for the old border-layout +bug by setting `padding >= borderWidth`. Those callers should remove the +compensating padding; border presence now implies the necessary layout +reservation. ### 12.3 Render return type @@ -1052,9 +1053,9 @@ resolution. implementation detail, not public API. `validate()` is public API. 4. **How should border widths interact with layout?** RESOLVED. Border widths - are now accounted for in layout via `max(padding, borderWidth)` per side at - pack time (TypeScript layer). See Section 12.2 for the full semantics - including the no-double-reservation guarantee for prior compensators. + are now accounted for in layout additively (`padding + borderWidth`) per side + at pack time (TypeScript layer). See Section 12.2 for the full semantics. + This is a breaking change: prior workaround padding must be removed. 5. **What are the specific transfer encoding details?** The encoding structure is described in Section 12.1 as current implementation surface. Locking down diff --git a/test/border.test.ts b/test/border.test.ts index ddf2cae..2e62f13 100644 --- a/test/border.test.ts +++ b/test/border.test.ts @@ -529,7 +529,7 @@ describe("box model", () => { expect(decode(result.output)).toContain("CASE B"); }); - it("padding == border renders identically to no padding (max semantics, no double-reservation)", async () => { + it("padding is additive: border=1 alone gives height 3; border=1 plus padding=1 gives height 5", async () => { let nopad = await createTerm({ width: 20, height: 10 }); let r1 = nopad.render([ open("root", { @@ -562,9 +562,10 @@ describe("box model", () => { close(), ]); - // Both produce the same box height: border reserves 1 per side, no double-reservation + // border=1, no padding: effective = 0+1 = 1 per side → height = 1+text+1 = 3 expect(r1.info.get("box")?.bounds.height).toBe(3); - expect(r2.info.get("box")?.bounds.height).toBe(3); + // border=1, padding=1: effective = 1+1 = 2 per side → height = 2+text+2 = 5 + expect(r2.info.get("box")?.bounds.height).toBe(5); }); it("explicit padding > border width adds breathing room inside the border", async () => { @@ -586,9 +587,9 @@ describe("box model", () => { close(), ]); - // effective_top = max(2, 1) = 2, effective_bottom = max(2, 1) = 2 - // height = 2 + 1 text + 2 = 5 - expect(result.info.get("box")?.bounds.height).toBe(5); + // effective_top = 2+1 = 3, effective_bottom = 2+1 = 3 + // height = 3 + 1 text + 3 = 7 + expect(result.info.get("box")?.bounds.height).toBe(7); }); it("nested two-tone bevel lays out without manual padding compensation", async () => { diff --git a/test/clip.test.ts b/test/clip.test.ts index cadf0e4..648a343 100644 --- a/test/clip.test.ts +++ b/test/clip.test.ts @@ -91,7 +91,6 @@ describe("clip", () => { width: fixed(10), height: fixed(3), direction: "ttb", - padding: pad, }, border, }), @@ -150,7 +149,6 @@ describe("clip", () => { width: fixed(10), height: fixed(3), direction: "ttb", - padding: pad, }, border, }), diff --git a/test/term.test.ts b/test/term.test.ts index 121ece2..dd6e51f 100644 --- a/test/term.test.ts +++ b/test/term.test.ts @@ -92,8 +92,8 @@ describe("term", () => { │ │ │ │ │ │ -│ padded │ │ │ +│ padded │ │ │ │ │ ╰──────────────────────────────────────╯`.trim()); @@ -109,7 +109,6 @@ describe("term", () => { width: grow(), height: grow(), direction: "ttb", - padding: { left: 1, top: 1 }, }, border: { color: rgba(255, 255, 255), @@ -212,7 +211,6 @@ describe("term", () => { width: fixed(12), height: fixed(5), direction: "ttb", - padding: { left: 1, top: 1 }, }, border: { color: rgba(255, 255, 255), @@ -297,7 +295,6 @@ describe("term", () => { width: grow(), height: grow(), direction: "ttb", - padding: { left: 1, top: 1 }, }, border: { color: rgba(255, 255, 255), @@ -349,7 +346,6 @@ describe("term", () => { width: grow(), height: grow(), direction: "ttb", - padding: { left: 1, top: 1 }, }, border: { color: rgba(255, 255, 255),