Skip to content

Commit 31fc1ae

Browse files
authored
fix(breaking): bordered elements should reserve layout space for border sides (#116)
* fix: bordered boxes now reserve layout space for their border sides 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. * fix!: border padding is now additive, not max 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. * refactor: move border padding reservation into the wasm renderer The additive effective-padding rule (userPadding + borderWidth per side) was applied in pack() on the TypeScript side, so the packed layout word carried a pre-computed value. It now happens in clayterm.c when the PROP_BORDER block decodes border widths: decl is zero-initialized and PROP_LAYOUT decodes first, so border-without-layout and layout-without-border both fall out naturally. The wire format's padding field now carries raw user padding; the renderer owns the reservation. Behavior is unchanged — all existing border tests pass as-is.
1 parent 2ebc906 commit 31fc1ae

5 files changed

Lines changed: 205 additions & 19 deletions

File tree

specs/renderer-spec.md

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -867,12 +867,28 @@ excluding joined corner cells. Per-side attributes affect only the styling of
867867
corner cells; corner glyph shape selection (including rounded corners via
868868
`cornerRadius`) is unchanged.
869869

870-
**Border width and layout interaction.** In the underlying layout engine (Clay),
871-
border configuration does not affect layout computation. This is Clay's intended
872-
behavior. Borders are drawn as visual overlays within the element's bounding
873-
box. A bordered element with zero padding will have its borders drawn over its
874-
content. Callers must add padding equal to or greater than the border width to
875-
prevent overlap.
870+
**Border width and layout interaction.** The renderer automatically reserves
871+
space for each enabled border side. For each side, the effective padding passed
872+
to the layout engine is `userPadding + borderWidth`; the WASM renderer applies
873+
this adjustment when decoding the element declaration. Border glyphs are drawn
874+
at the same positions as before; the change is purely in how much layout space
875+
Clay allocates for the element.
876+
877+
Semantics of the additive rule:
878+
879+
- **No explicit padding.** The border width itself becomes the effective
880+
padding, so content is placed immediately inside the border.
881+
- **Explicit user padding.** Adds breathing room _beyond_ the border edge.
882+
`padding: 1` with `border: 1` places content 2 cells from the element edge — 1
883+
for the border glyph, 1 for the padding inset.
884+
- **Prior workaround pattern (`padding == borderWidth`).** These callers now
885+
receive double-reservation (effective = 2 × borderWidth). This is a breaking
886+
change: remove the workaround padding to restore the original visual.
887+
888+
This is a breaking change for callers who compensated for the old border-layout
889+
bug by setting `padding >= borderWidth`. Those callers should remove the
890+
compensating padding; border presence now implies the necessary layout
891+
reservation.
876892

877893
### 12.3 Render return type
878894

@@ -1107,11 +1123,10 @@ resolution.
11071123
3. **Is `pack()` public API?** `pack()` is currently exported but is an internal
11081124
implementation detail, not public API. `validate()` is public API.
11091125

1110-
4. **How should border widths interact with layout?** The current behavior
1111-
(borders do not affect layout) is inherited from the underlying layout
1112-
engine. The project has questioned whether this is the right design. This
1113-
specification describes the current behavior in Section 12.2 without
1114-
committing to it.
1126+
4. **How should border widths interact with layout?** RESOLVED. Border widths
1127+
are now accounted for in layout additively (`padding + borderWidth`) per side
1128+
in the WASM renderer's decode step. See Section 12.2 for the full semantics.
1129+
This is a breaking change: prior workaround padding must be removed.
11151130

11161131
5. **What are the specific transfer encoding details?** The encoding structure
11171132
is described in Section 12.1 as current implementation surface. Locking down

src/clayterm.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,13 @@ void reduce(struct Clayterm *ct, uint32_t *buf, int len, int mode, int row,
726726
decl.border.width.top = (bw >> 16) & 0xff;
727727
decl.border.width.bottom = (bw >> 24) & 0xff;
728728

729+
/* Border sides reserve layout space additively: effective padding
730+
* is userPadding + borderWidth per side (renderer-spec 12.2). */
731+
decl.layout.padding.left += decl.border.width.left;
732+
decl.layout.padding.right += decl.border.width.right;
733+
decl.layout.padding.top += decl.border.width.top;
734+
decl.layout.padding.bottom += decl.border.width.bottom;
735+
729736
/* Resolved per-side fg/bg attribute words (top, right, bottom,
730737
* left). Routed to render_border via userData; the command buffer
731738
* remains valid for the whole render pass. */

test/border.test.ts

Lines changed: 171 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
import { close, fixed, open, type OpenElement, rgba } from "../ops.ts";
1+
import {
2+
close,
3+
fixed,
4+
grow,
5+
open,
6+
type OpenElement,
7+
rgba,
8+
text,
9+
} from "../ops.ts";
210
import { createTerm } from "../term.ts";
311
import { describe, expect, it } from "./suite.ts";
12+
import { print } from "./print.ts";
413

514
const decode = (b: Uint8Array) => new TextDecoder().decode(b);
615

@@ -477,3 +486,164 @@ describe("instances", () => {
477486
expect(again).not.toContain(FG.cyan);
478487
});
479488
});
489+
490+
const trim = (s: string) => s.split("\n").map((l) => l.trimEnd()).join("\n");
491+
492+
describe("box model", () => {
493+
it("full border with no padding reserves space: children visible, box is 3 rows", async () => {
494+
let term = await createTerm({ width: 20, height: 10 });
495+
let result = term.render([
496+
open("root", {
497+
layout: { width: grow(), height: grow(), direction: "ttb" },
498+
}),
499+
open("box", {
500+
layout: { width: fixed(14), direction: "ttb" },
501+
border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 },
502+
}),
503+
text("CASE A"),
504+
close(),
505+
close(),
506+
]);
507+
508+
expect(result.info.get("box")?.bounds.height).toBe(3);
509+
expect(decode(result.output)).toContain("CASE A");
510+
});
511+
512+
it("partial borders (top+left) reserve only their sides", async () => {
513+
let term = await createTerm({ width: 20, height: 10 });
514+
let result = term.render([
515+
open("root", {
516+
layout: { width: grow(), height: grow(), direction: "ttb" },
517+
}),
518+
open("box", {
519+
layout: { width: fixed(14), direction: "ttb" },
520+
border: { color: WHITE, top: 1, left: 1 },
521+
}),
522+
text("CASE B"),
523+
close(),
524+
close(),
525+
]);
526+
527+
// top border reserves 1 row, no bottom border so no bottom reservation
528+
expect(result.info.get("box")?.bounds.height).toBe(2);
529+
expect(decode(result.output)).toContain("CASE B");
530+
});
531+
532+
it("padding is additive: border=1 alone gives height 3; border=1 plus padding=1 gives height 5", async () => {
533+
let nopad = await createTerm({ width: 20, height: 10 });
534+
let r1 = nopad.render([
535+
open("root", {
536+
layout: { width: grow(), height: grow(), direction: "ttb" },
537+
}),
538+
open("box", {
539+
layout: { width: fixed(14), direction: "ttb" },
540+
border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 },
541+
}),
542+
text("CONTENT"),
543+
close(),
544+
close(),
545+
]);
546+
547+
let withpad = await createTerm({ width: 20, height: 10 });
548+
let r2 = withpad.render([
549+
open("root", {
550+
layout: { width: grow(), height: grow(), direction: "ttb" },
551+
}),
552+
open("box", {
553+
layout: {
554+
width: fixed(14),
555+
direction: "ttb",
556+
padding: { top: 1, right: 1, bottom: 1, left: 1 },
557+
},
558+
border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 },
559+
}),
560+
text("CONTENT"),
561+
close(),
562+
close(),
563+
]);
564+
565+
// border=1, no padding: effective = 0+1 = 1 per side → height = 1+text+1 = 3
566+
expect(r1.info.get("box")?.bounds.height).toBe(3);
567+
// border=1, padding=1: effective = 1+1 = 2 per side → height = 2+text+2 = 5
568+
expect(r2.info.get("box")?.bounds.height).toBe(5);
569+
});
570+
571+
it("explicit padding > border width adds breathing room inside the border", async () => {
572+
let term = await createTerm({ width: 20, height: 10 });
573+
let result = term.render([
574+
open("root", {
575+
layout: { width: grow(), height: grow(), direction: "ttb" },
576+
}),
577+
open("box", {
578+
layout: {
579+
width: fixed(14),
580+
direction: "ttb",
581+
padding: { top: 2, bottom: 2 },
582+
},
583+
border: { color: WHITE, top: 1, bottom: 1 },
584+
}),
585+
text("CONTENT"),
586+
close(),
587+
close(),
588+
]);
589+
590+
// effective_top = 2+1 = 3, effective_bottom = 2+1 = 3
591+
// height = 3 + 1 text + 3 = 7
592+
expect(result.info.get("box")?.bounds.height).toBe(7);
593+
});
594+
595+
it("nested two-tone bevel lays out without manual padding compensation", async () => {
596+
let term = await createTerm({ width: 20, height: 10 });
597+
let result = term.render([
598+
open("root", {
599+
layout: { width: grow(), height: grow(), direction: "ttb" },
600+
}),
601+
open("outer", {
602+
layout: { width: fixed(16), direction: "ttb" },
603+
border: { color: WHITE, top: 1, left: 1 },
604+
}),
605+
open("inner", {
606+
layout: { width: grow(), direction: "ttb" },
607+
border: { color: WHITE, bottom: 1, right: 1 },
608+
}),
609+
text("NESTED"),
610+
close(),
611+
close(),
612+
close(),
613+
]);
614+
615+
// inner: effective_bottom=1, effective_right=1 → height = 0 + text(1) + 1 = 2
616+
// outer: effective_top=1, effective_left=1 → height = 1 + inner(2) + 0 = 3
617+
expect(result.info.get("outer")?.bounds.height).toBe(3);
618+
expect(decode(result.output)).toContain("NESTED");
619+
});
620+
621+
it("visual: full border renders border glyphs around content", async () => {
622+
let term = await createTerm({ width: 20, height: 10 });
623+
let out = trim(
624+
print(
625+
decode(
626+
term.render([
627+
open("root", {
628+
layout: { width: grow(), height: grow(), direction: "ttb" },
629+
}),
630+
open("box", {
631+
layout: { width: fixed(14), direction: "ttb" },
632+
border: { color: WHITE, top: 1, right: 1, bottom: 1, left: 1 },
633+
}),
634+
text("CASE A"),
635+
close(),
636+
close(),
637+
]).output,
638+
),
639+
20,
640+
10,
641+
),
642+
);
643+
644+
let lines = out.split("\n");
645+
expect(lines[0]).toBe("┌────────────┐");
646+
expect(lines[1]).toBe("│CASE A │");
647+
expect(lines[2]).toBe("└────────────┘");
648+
});
649+
});

test/clip.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ describe("clip", () => {
9191
width: fixed(10),
9292
height: fixed(3),
9393
direction: "ttb",
94-
padding: pad,
9594
},
9695
border,
9796
}),
@@ -150,7 +149,6 @@ describe("clip", () => {
150149
width: fixed(10),
151150
height: fixed(3),
152151
direction: "ttb",
153-
padding: pad,
154152
},
155153
border,
156154
}),

test/term.test.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ describe("term", () => {
9393
│ │
9494
│ │
9595
│ │
96-
│ padded │
9796
│ │
97+
│ padded │
9898
│ │
9999
│ │
100100
╰──────────────────────────────────────╯`.trim());
@@ -110,7 +110,6 @@ describe("term", () => {
110110
width: grow(),
111111
height: grow(),
112112
direction: "ttb",
113-
padding: { left: 1, top: 1 },
114113
},
115114
border: {
116115
color: rgba(255, 255, 255),
@@ -213,7 +212,6 @@ describe("term", () => {
213212
width: fixed(12),
214213
height: fixed(5),
215214
direction: "ttb",
216-
padding: { left: 1, top: 1 },
217215
},
218216
border: {
219217
color: rgba(255, 255, 255),
@@ -298,7 +296,6 @@ describe("term", () => {
298296
width: grow(),
299297
height: grow(),
300298
direction: "ttb",
301-
padding: { left: 1, top: 1 },
302299
},
303300
border: {
304301
color: rgba(255, 255, 255),
@@ -599,7 +596,6 @@ hi
599596
width: grow(),
600597
height: grow(),
601598
direction: "ttb",
602-
padding: { left: 1, top: 1 },
603599
},
604600
border: {
605601
color: rgba(255, 255, 255),

0 commit comments

Comments
 (0)