amadan

rastrillo / fiesta Public

Give the modal room, and make a second sweep change nothing

The modal's preview box held its backdrop and nothing else: 170px
(212px Mobile), because the open-state reading skipped any document
with a backdrop and the reading at rest sees only the page behind the
overlay. The drives now read a modal's panel: what it needs unclipped
(its height, or what anything scrolling in it would need, plus the
overlay's padding) with the box made tall, and, back at the box the
table gives it, whether the panel or anything in it holds more than it
shows. heights.go gains previewReserved, a hand-written floor under the
measured table that no sweep prints, with the modal's 620px from
Rastrillo: a modal wants a window to be modal over. heightOf raises a
key to its reservation; Mobile follows at 775px.

printTable no longer grows a document as tall as its frame (a shell,
the stage) by its slack on every sweep: a key whose every reading was
such a document with its content fitting keeps the reservation the
table gives it, and grows only when a reading finds the content taller
than the box. Mobile entries the table already has are kept unless a
reading needs more, since the 320px drive raises them and prints no
table. A sweep, pasted, then swept again prints the same table.

Red first, by assertion: the enforced wide drive against the 170px
box reported "en/modal.html at 1500px Desktop modal/default: the
modal's panel is clipped in a frame of 170px; it needs 224px to show
whole", and the same at Mobile (303 in 212). Green after.

Idempotence: two full twelve-locale sweeps, the second on the first's
pasted table, print identical tables; the first already equals the
committed table but for the modal's measured row (170 to 250), so the
hand correction the last calibration needed is now the printout's own.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 4c3a6a91539f7ff290ddbac669185be04b632cad parent aba08dd
2 files changed, +152 −31
  • internal/gallery/heights.go +22 −5
  • internal/gallery/heights_browser_test.go +130 −26

CI passed — run details

CI log 
=== gofmt ===
=== no-rastrillo ===
go test -run '^(TestFiestaNeverImportsRastrillo|TestThePublicPackagesLinkNoBrowser)$' -count=1 .
ok  	amadan.net/rastrillo/fiesta	0.112s
=== size ===
go test ./internal/build -run '^TestTheOneLinkIsUnder25KiBGzipped$' -count=1 -v
=== RUN   TestTheOneLinkIsUnder25KiBGzipped
    build_test.go:138: fiesta-plain.css: 152856 bytes, 22303 bytes gzipped (21.8 KiB of the 25.0 KiB limit)
    build_test.go:138: fiesta-signal.css: 152951 bytes, 22414 bytes gzipped (21.9 KiB of the 25.0 KiB limit)
    build_test.go:138: fiesta.css: 155770 bytes, 23018 bytes gzipped (22.5 KiB of the 25.0 KiB limit)
--- PASS: TestTheOneLinkIsUnder25KiBGzipped (0.02s)
PASS
ok  	amadan.net/rastrillo/fiesta/internal/build	0.021s
=== suite ===
--- shard 0 of 15 ---
==> shard 0 of 15: make root
go build ./...
go vet ./...
FIESTA_TEST_REQUIRE_NODE=1 FIESTA_TEST_REQUIRE_GIT=1 go test -count=1 ./...
ok  	amadan.net/rastrillo/fiesta	1.557s
ok  	amadan.net/rastrillo/fiesta/check	6.007s
ok  	amadan.net/rastrillo/fiesta/cmd/fiesta	89.798s
ok  	amadan.net/rastrillo/fiesta/example	0.286s
ok  	amadan.net/rastrillo/fiesta/internal/axe	0.217s
ok  	amadan.net/rastrillo/fiesta/internal/build	1.329s
ok  	amadan.net/rastrillo/fiesta/internal/codeview	0.018s
ok  	amadan.net/rastrillo/fiesta/internal/gallery	85.757s
?   	amadan.net/rastrillo/fiesta/internal/harness	[no test files]
ok  	amadan.net/rastrillo/fiesta/internal/markup	0.017s
ok  	amadan.net/rastrillo/fiesta/internal/nodetest	0.369s
ok  	amadan.net/rastrillo/fiesta/internal/site	3.320s
ok  	amadan.net/rastrillo/fiesta/internal/tokens	0.016s
ok  	amadan.net/rastrillo/fiesta/internal/www	76.921s
--- shard 1 of 15 ---
==> shard 1 of 15: make staticcheck
go run honnef.co/go/tools/cmd/staticcheck@v0.8.2 -tags browser ./...
--- shard 2 of 15 ---
==> shard 2 of 15: make govulncheck
go run golang.org/x/vuln/cmd/govulncheck@v1.8.0 ./...
=== Symbol Results ===

No vulnerabilities found.

Your code is affected by 0 vulnerabilities.
This scan also found 2 vulnerabilities in packages you import and 5
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.
Use '-show verbose' for more details.
--- shard 3 of 15 ---
==> shard 3 of 15: make gitleaks
go run github.com/zricethezav/gitleaks/v8@v8.30.1 git --no-banner --redact --log-opts='--full-history --diff-filter=tuxdb HEAD' .
6:52AM INF Unknown SCM platform. Use --platform to include links in findings. host=amadan.net
6:52AM INF 1 commits scanned.
6:52AM INF scanned ~6493828 bytes (6.49 MB) in 4.12s
6:52AM INF no leaks found
--- shard 4 of 15 ---
==> shard 4 of 15: make browser.internal.harness
./hack/gotest.sh browser 'internal.harness'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
ok  	amadan.net/rastrillo/fiesta/internal/harness	12.989s
--- shard 5 of 15 ---
==> shard 5 of 15: make browser.check~1of3
./hack/gotest.sh browser 'check~1of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 1 of 3 of ./check/ runs 95 of 286 tests
ok  	amadan.net/rastrillo/fiesta/check	227.248s
--- shard 6 of 15 ---
==> shard 6 of 15: make browser.check~2of3
./hack/gotest.sh browser 'check~2of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 2 of 3 of ./check/ runs 95 of 286 tests
ok  	amadan.net/rastrillo/fiesta/check	241.032s
--- shard 7 of 15 ---
==> shard 7 of 15: make browser.check~3of3
./hack/gotest.sh browser 'check~3of3'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 3 of 3 of ./check/ runs 96 of 286 tests
ok  	amadan.net/rastrillo/fiesta/check	245.612s
--- shard 8 of 15 ---
==> shard 8 of 15: make browser.internal.gallery~1of4
./hack/gotest.sh browser 'internal.gallery~1of4'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 1 of 4 of ./internal/gallery/ runs 1 of 145 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	313.044s
--- shard 9 of 15 ---
==> shard 9 of 15: make browser.internal.gallery~2of4
./hack/gotest.sh browser 'internal.gallery~2of4'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 2 of 4 of ./internal/gallery/ runs 47 of 145 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	195.246s
--- shard 10 of 15 ---
==> shard 10 of 15: make browser.internal.gallery~3of4
./hack/gotest.sh browser 'internal.gallery~3of4'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 3 of 4 of ./internal/gallery/ runs 49 of 145 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	215.883s
--- shard 11 of 15 ---
==> shard 11 of 15: make browser.internal.gallery~4of4
./hack/gotest.sh browser 'internal.gallery~4of4'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 4 of 4 of ./internal/gallery/ runs 48 of 145 tests
ok  	amadan.net/rastrillo/fiesta/internal/gallery	204.308s
--- shard 12 of 15 ---
==> shard 12 of 15: make browser.internal.gallery.history
./hack/gotest.sh browser 'internal.gallery.history'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
ok  	amadan.net/rastrillo/fiesta/internal/gallery/history	34.278s
--- shard 13 of 15 ---
==> shard 13 of 15: make browser.internal.www~1of2
./hack/gotest.sh browser 'internal.www~1of2'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 1 of 2 of ./internal/www/ runs 17 of 34 tests
ok  	amadan.net/rastrillo/fiesta/internal/www	130.819s
--- shard 14 of 15 ---
==> shard 14 of 15: make browser.internal.www~2of2
./hack/gotest.sh browser 'internal.www~2of2'
gotest: on DOCS-T1, driving the harness's own choice (/usr/bin/chromium)
gotest: slice 2 of 2 of ./internal/www/ runs 17 of 34 tests
ok  	amadan.net/rastrillo/fiesta/internal/www	118.175s

Intent

full ledger: gallery

diff --git a/internal/gallery/heights.go b/internal/gallery/heights.go
index 9e45460..7b4dbc7 100644
--- a/internal/gallery/heights.go
+++ b/internal/gallery/heights.go
@@ -149,7 +149,7 @@ var previewHeights = map[string]int{
"media-grid/screenshots-a-logo-and-a-banner": 720,
"media-grid/select-mode-one-tile-selected": 400,
"meter/row": 100,
- "modal/default": 170,
+ "modal/default": 250,
"notice/a-saved-message": 120,
"page-header/title-only": 120,
"page-header/title-subhead-and-a-primary-action": 150,
@@ -236,12 +236,29 @@ var previewMobileHeights = map[string]int{
// previewMobileHeights allows, read for that language's pages only.
var previewMobileLocaleHeights = map[string]map[string]int{}
-// heightOf is one key's Desktop height, or the default.
+// previewReserved is the room a box is given beyond what any sweep
+// measures, written by hand with its reason, and never printed by a
+// sweep: a floor under the measured table. The modal's is Rastrillo's
+// 620px: a modal wants a window to be modal over, and its panel is
+// sized from the window (85vh, at most 640px), so a box that only just
+// holds the panel shows a modal in a letterbox rather than one over a
+// page. The drives still read the panel for clipping at whatever the
+// box comes to.
+var previewReserved = map[string]int{
+ "modal/default": 620,
+}
+
+// heightOf is one key's Desktop height: the measured table's, or the
+// default, raised to its reservation where it has one.
func heightOf(key string) int {
- if h, ok := previewHeights[key]; ok {
- return h
+ h, ok := previewHeights[key]
+ if !ok {
+ h = previewHeight
+ }
+ if r := previewReserved[key]; r > h {
+ h = r
}
- return previewHeight
+ return h
}
// previewStyle writes one widget's two virtual heights. Mobile is a
diff --git a/internal/gallery/heights_browser_test.go b/internal/gallery/heights_browser_test.go
index c78c687..7a36f51 100644
--- a/internal/gallery/heights_browser_test.go
+++ b/internal/gallery/heights_browser_test.go
@@ -88,14 +88,18 @@ const measureHeights = `(() => {
try { d = f.contentDocument; } catch (e) {}
const need = d && d.body ? Math.ceil(Math.max(d.body.getBoundingClientRect().height, d.body.scrollHeight)) : -1;
const shell = !!(d && d.querySelector("[rst-shell-sidebar]"));
- out[key] = {Need: need, Box: Math.round(parseFloat(getComputedStyle(f).height)), Shell: shell};
+ const viewport = !!(d && d.querySelector("[rst-shell-sidebar], [rst-shell-topbar], [rst-shell-console], [rst-stage]"));
+ out[key] = {Need: need, Box: Math.round(parseFloat(getComputedStyle(f).height)), Shell: shell, Viewport: viewport};
}
return JSON.stringify(out);
})()`
type heightReading struct {
Need, Box int
- Shell bool
+ // Shell is a frame holding a sidebar shell, whose rail is 100dvh;
+ // Viewport any document as tall as its frame (a shell, the stage),
+ // which reads its own box when its content fits.
+ Shell, Viewport bool
}
// readHeights decodes one measurement, failing on what makes it
@@ -137,10 +141,42 @@ const measureOpen = `(async (prop) => {
const kept = views.map(v => v.style.getPropertyValue(prop));
views.forEach(v => v.style.setProperty(prop, "1600px"));
await settle(window);
+ // What a modal's panel holds: its own height, or, where something in
+ // it scrolls, the height that thing would need unclipped, plus the
+ // overlay's padding round the panel.
+ const panelNeed = (d, w, panel) => {
+ const top = panel.getBoundingClientRect().top;
+ let h = Math.max(panel.getBoundingClientRect().height, panel.scrollHeight);
+ for (const e of panel.querySelectorAll("*")) {
+ if (!/auto|scroll|hidden|clip/.test(w.getComputedStyle(e).overflowY)) continue;
+ const r = e.getBoundingClientRect();
+ h = Math.max(h, r.top - top + e.scrollHeight + (panel.getBoundingClientRect().bottom - r.bottom));
+ }
+ const m = w.getComputedStyle(panel.closest("[rst-modal-overlay]") || d.body);
+ return Math.ceil(h + (parseFloat(m.paddingTop) || 0) + (parseFloat(m.paddingBottom) || 0));
+ };
+ // Whether a modal's panel clips anything at the box it has: the
+ // panel, or anything in it that scrolls, holding more than it shows.
+ const panelClipped = (w, panel) => [panel, ...panel.querySelectorAll("*")].some(e =>
+ (e === panel || /auto|scroll|hidden|clip/.test(w.getComputedStyle(e).overflowY)) && e.scrollHeight > e.clientHeight + 1);
const out = {};
+ const modals = [];
for (const f of document.querySelectorAll(".ds-view__frame")) {
const d = f.contentDocument;
- if (!d || !d.body || d.querySelector("[rst-shell-sidebar], [rst-shell-topbar], [rst-shell-console], [rst-stage], [rst-backdrop]")) continue;
+ if (!d || !d.body) continue;
+ // A modal is laid out on a fixed overlay the size of the frame, so
+ // what it asks of the box is its panel unclipped; the page behind
+ // it is the backdrop's and reads as content at rest.
+ const panel = d.querySelector("[rst-modal-panel]");
+ if (panel) {
+ const w = d.defaultView;
+ await settle(w);
+ const h = Math.ceil(d.body.getBoundingClientRect().height);
+ out[frameKey(f)] = {Need: panelNeed(d, w, panel), What: "modal panel", Closed: h, After: h};
+ modals.push([f, w, panel]);
+ continue;
+ }
+ if (d.querySelector("[rst-shell-sidebar], [rst-shell-topbar], [rst-shell-console], [rst-stage], [rst-backdrop]")) continue;
const w = d.defaultView;
const opens = [];
for (const det of d.querySelectorAll("details[rst-dropdown], details[rst-row-menu]")) {
@@ -197,6 +233,13 @@ const measureOpen = `(async (prop) => {
}
views.forEach((v, i) => kept[i] ? v.style.setProperty(prop, kept[i]) : v.style.removeProperty(prop));
await settle(window);
+ // Back at the boxes the table gives them, the modal panels are read
+ // for clipping: a panel that scrolls in its own box is a modal a
+ // reader sees cut short, whatever the arithmetic above says.
+ for (const [f, w, panel] of modals) {
+ await settle(w);
+ out[frameKey(f)].Clipped = panelClipped(w, panel);
+ }
return JSON.stringify(out);
})`
@@ -204,6 +247,9 @@ type openReading struct {
Need int
What string
Closed, After int
+ // Clipped is set for a modal whose panel, back in the box the
+ // table gives it, holds more than it shows.
+ Clipped bool
}
// readOpen measures what each frame's box must hold with its menus and
@@ -246,10 +292,21 @@ func shortfall(t *testing.T, where, key string, r heightReading, grace int) bool
}
// openShortfall reports one frame's box against what it must hold open:
-// no grace, since a menu cut short is a menu a reader cannot use.
+// no grace, since a menu cut short is a menu a reader cannot use; and a
+// modal whose panel is clipped in its box, read directly.
func openShortfall(t *testing.T, where, key string, open openReading, box int) bool {
t.Helper()
- return shortfall(t, where+" with its "+open.What+" open", key, heightReading{Need: open.Need, Box: box}, 0)
+ short := shortfall(t, where+" with its "+open.What+" open", key, heightReading{Need: open.Need, Box: box}, 0)
+ if open.Clipped {
+ msg := fmt.Sprintf("%s %s: the modal's panel is clipped in a frame of %dpx; it needs %dpx to show whole", where, key, box, open.Need)
+ if heightsEnforced {
+ t.Error(msg)
+ } else {
+ t.Log("not enforced yet: " + msg)
+ }
+ short = true
+ }
+ return short
}
// selfCheck proves the measurement can see a short box: every widget's
@@ -281,8 +338,19 @@ func heightPages(t *testing.T) []string {
// printTable writes the readings out as Go source: one Desktop height
// per key, the tallest over the locales read plus slack, and the
-// Mobile entries where the derived 1.25× does not cover a reading.
-func printTable(t *testing.T, desk, mob map[string]map[string]int) {
+// Mobile entries where the derived 1.25x does not cover a reading.
+//
+// It is written so that a second sweep after pasting the first changes
+// nothing. A document as tall as the frame it is in (a shell, the
+// stage) measures its own box when its content fits, so slack on that
+// reading would grow the box on every sweep; such a key keeps the
+// reservation the table already gives it, unless a reading found its
+// content taller than its box (fit says which keys fitted every time).
+// Mobile entries the table already has are kept unless a reading needs
+// more: the 320px drive raises them and prints no table, so the wide
+// printout carrying them is what keeps a paste from dropping them.
+// previewReserved is never printed: it is a floor, not a measurement.
+func printTable(t *testing.T, desk, mob map[string]map[string]int, deskFit, mobFit map[string]bool) {
t.Helper()
keys := map[string]bool{}
for _, m := range desk {
@@ -298,44 +366,65 @@ func printTable(t *testing.T, desk, mob map[string]map[string]int) {
// Slack, then rounded up to the next ten: twelve languages and three
// type families do not lay out to the same pixel.
up := func(n int) int { return (n + 20 + 9) / 10 * 10 }
+ tallest := func(m map[string]map[string]int, k string) int {
+ n := 0
+ for _, byKey := range m {
+ n = max(n, byKey[k])
+ }
+ return n
+ }
var b strings.Builder
b.WriteString("var previewHeights = map[string]int{\n")
h := map[string]int{}
for _, k := range sorted {
- n := 0
- for _, m := range desk {
- if m[k] > n {
- n = m[k]
- }
+ cur, ok := previewHeights[k]
+ if !ok {
+ cur = previewHeight
+ }
+ h[k] = up(tallest(desk, k))
+ if deskFit[k] {
+ h[k] = cur
}
- h[k] = up(n)
fmt.Fprintf(&b, "\t%q: %d,\n", k, h[k])
}
b.WriteString("}\nvar previewMobileHeights = map[string]int{\n")
mh := map[string]int{}
for _, k := range sorted {
- if n := mob["en"][k]; n > h[k]*5/4 {
- mh[k] = up(n)
- fmt.Fprintf(&b, "\t%q: %d,\n", k, mh[k])
+ m := 0
+ if n := mob["en"][k]; n > h[k]*5/4 && !mobFit[k] {
+ m = up(n)
+ }
+ m = max(m, previewMobileHeights[k])
+ if m > h[k]*5/4 {
+ mh[k] = m
+ fmt.Fprintf(&b, "\t%q: %d,\n", k, m)
}
}
b.WriteString("}\nvar previewMobileLocaleHeights = map[string]map[string]int{\n")
- var locales []string
+ locales := map[string]bool{}
for l := range mob {
if l != "en" {
- locales = append(locales, l)
+ locales[l] = true
}
}
- sort.Strings(locales)
- for _, l := range locales {
+ for l := range previewMobileLocaleHeights {
+ locales[l] = true
+ }
+ var sortedLocales []string
+ for l := range locales {
+ sortedLocales = append(sortedLocales, l)
+ }
+ sort.Strings(sortedLocales)
+ for _, l := range sortedLocales {
var lines []string
for _, k := range sorted {
- base := h[k] * 5 / 4
- if mh[k] > base {
- base = mh[k]
+ base := max(h[k]*5/4, mh[k])
+ n := previewMobileLocaleHeights[l][k]
+ if r := mob[l][k]; r > base && !mobFit[k] {
+ n = max(n, up(r))
}
- if n := mob[l][k]; n > base {
- lines = append(lines, fmt.Sprintf("\t\t%q: %d,", k, up(n)))
+ if n > base {
+ lines = append(lines, fmt.Sprintf("\t\t%q: %d,", k, n))
}
}
if len(lines) > 0 {
@@ -369,6 +458,17 @@ func TestPreviewFrameHeightsFitTheirContent(t *testing.T) {
m[key] = n
}
}
+ // fit says, per key, whether every reading of it was a viewport-sized
+ // document whose content fitted its box: printTable keeps those at
+ // the reservation they have.
+ deskFit, mobFit := map[string]bool{}, map[string]bool{}
+ fits := func(fit map[string]bool, key string, r heightReading) {
+ ok := r.Viewport && r.Need <= r.Box
+ if was, read := fit[key]; read {
+ ok = ok && was
+ }
+ fit[key] = ok
+ }
for _, locale := range wideLocales() {
desk[locale], mob[locale] = map[string]int{}, map[string]int{}
for _, file := range heightPages(t) {
@@ -379,6 +479,7 @@ func TestPreviewFrameHeightsFitTheirContent(t *testing.T) {
for key, r := range desktop {
measured++
keep(desk[locale], key, r.Need)
+ fits(deskFit, key, r)
if shortfall(t, where+" Desktop", key, r, 48) {
short++
}
@@ -386,6 +487,7 @@ func TestPreviewFrameHeightsFitTheirContent(t *testing.T) {
for key, o := range readOpen(t, ctx, where+" Desktop", "--ds-h") {
opened++
keep(desk[locale], key, o.Need)
+ deskFit[key] = false
if openShortfall(t, where+" Desktop", key, o, desktop[key].Box) {
short++
}
@@ -395,12 +497,14 @@ func TestPreviewFrameHeightsFitTheirContent(t *testing.T) {
mobile := readHeights(t, ctx, where+" Mobile")
for key, r := range mobile {
keep(mob[locale], key, r.Need)
+ fits(mobFit, key, r)
if shortfall(t, where+" Mobile", key, r, 48) {
short++
}
}
for key, o := range readOpen(t, ctx, where+" Mobile", "--ds-hm") {
keep(mob[locale], key, o.Need)
+ mobFit[key] = false
if openShortfall(t, where+" Mobile", key, o, mobile[key].Box) {
short++
}
@@ -420,7 +524,7 @@ func TestPreviewFrameHeightsFitTheirContent(t *testing.T) {
}
t.Logf("%d frames measured and %d opened over %d locale(s); %d short (enforced: %v)", measured, opened, len(desk), short, heightsEnforced)
if sweepFull() {
- printTable(t, desk, mob)
+ printTable(t, desk, mob, deskFit, mobFit)
}
}