From 34a55c4154e33c33f4a4e6a01d179215c2ab0824 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:33:01 +0300 Subject: [PATCH] composer: a stacked modal dimmed twice, through a viewport-sized shadow Every modal already mounts a .cx-backdrop element over its layer, so the stacked variant painting another one as `0 0 0 100vmax` of shadow spread was redundant, and the area beneath it was darkened twice. It was also the most expensive declaration in the sheet: a spread the size of the viewport diagonal, on an element inside a fixed layer, with an entry animation running. Removed in favour of a bounded shadow, and a test reads the stylesheet for the pattern -- asserting on node.style in jsdom passes against any rule at all, since no CSS is applied there. Co-Authored-By: Claude Opus 5 --- app/composer/elements/overlay.css | 15 +++++++++++- app/composer/tests/overlays.test.js | 38 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/app/composer/elements/overlay.css b/app/composer/elements/overlay.css index 404925d9..7ffcb271 100644 --- a/app/composer/elements/overlay.css +++ b/app/composer/elements/overlay.css @@ -14,7 +14,20 @@ box-shadow: var(--shadow-3); animation: cx-pop var(--dur-2) var(--ease-standard); } -.cx-modal-stacked { box-shadow: var(--shadow-3), 0 0 0 100vmax var(--backdrop); } +/* A stacked modal gets a deeper shadow and nothing else. + * + * It used to paint its own backdrop as `0 0 0 100vmax` of shadow spread, which + * was redundant -- every modal, stacked or not, already mounts a real + * .cx-backdrop element that covers the layer -- so the area under a stacked + * modal was dimmed twice. It was also the most expensive declaration in this + * sheet by a distance: a shadow spread the size of the viewport diagonal, on an + * element inside a fixed layer, with an entry animation running. That is a + * standing invitation to a stale compositor layer, and one showed up as the + * whole app painted twice, side by side, until a window resize forced a + * repaint. */ +.cx-modal-stacked { + box-shadow: 0 20px 48px color-mix(in srgb, var(--shadow-ink) 90%, transparent); +} @keyframes cx-pop { from { opacity: 0; transform: translateY(-6px) scale(.99); } } .cx-modal-head { diff --git a/app/composer/tests/overlays.test.js b/app/composer/tests/overlays.test.js index c6366728..3b723f70 100644 --- a/app/composer/tests/overlays.test.js +++ b/app/composer/tests/overlays.test.js @@ -1,8 +1,14 @@ // Overlay behaviour: stacking, dismissal order, and the promises. +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; + import test from "node:test"; import assert from "node:assert/strict"; +const here = dirname(fileURLToPath(import.meta.url)); + import { setupDom, teardownDom, key, fire } from "./_dom.js"; import { EDGES, sectorAt, sectorInArc } from "../elements/wheel.js"; @@ -566,3 +572,35 @@ test("every overlay keeps its z-index bound, not just windows", () => { second.close(); anchor.destroy(); }); + +test("a stacked modal dims what is under it with a real backdrop, once", () => { + // The dimming used to come from TWO places at once: the layer's own backdrop + // element, and a box-shadow with a 100vmax spread on the card. So the area + // under a stacked modal was darkened twice, and a shadow the size of the + // viewport diagonal -- on an element in a fixed layer, with an entry + // animation running -- is how a stale compositor layer gets made. One showed + // up as the whole app painted twice until a resize forced a repaint. + const under = composer.modal.generic({ title: "Under", body: composer.text.sm({ text: "a" }) }); + const over = composer.modal.stacked({ title: "Over", body: composer.text.sm({ text: "b" }) }); + + const layer = over.node.closest(".cx-modal-layer"); + assert.ok(layer, "a stacked modal has no layer of its own"); + assert.equal(layer.querySelectorAll(".cx-backdrop").length, 1, + "a stacked modal must dim through its own backdrop element, exactly once"); + + over.close(); + under.close(); +}); + +test("no overlay paints a backdrop with a viewport-sized shadow spread", () => { + // Read from the stylesheet, because jsdom applies no CSS: asserting on + // node.style.boxShadow here passes against any rule at all, since the value + // lives in the sheet and the inline style is always empty. The first version + // of this check did exactly that. + const css = readFileSync(join(here, "..", "elements", "overlay.css"), "utf8"); + const offenders = css.split("\n") + .filter((line) => /box-shadow/.test(line) && /vmax|vmin|\b\d{3,}(px|vh|vw)\b/.test(line)); + assert.deepEqual(offenders, [], + "a shadow spread that large is a stale-compositor-layer bug waiting to happen; " + + "dim with a .cx-backdrop element instead"); +});