composer: bind every overlay's z-index, not just windows

A rung compacts when a claim is released, so an overlay's z can move without it
doing anything. Six of seven call sites read z once at open time and kept a
number that was later handed to the next overlay: two of them at one level, DOM
order deciding which wins and which eats the clicks. Reproduced with two menus
and a third opened after closing the first -- both ended up at 501.

claimFor(rung, node) binds the node, so there is nothing left to forget, and the
raw claim() stays for the one case with no node. The test walks the registry
rather than naming overlays, because the bug was one site remembering and six
forgetting.

Also drops a no-op ternary that picked the modal rung either way.

app/composer/elements/floating.js

@@ -4,7 +4,7 @@ import { define } from "../internals/register.js";
 import { el } from "../internals/el.js";
 import { uid } from "../internals/ids.js";
 import { mount, unmount } from "../internals/portal.js";
-import { claim } from "../internals/zlayer.js";
+import { claimFor } from "../internals/zlayer.js";
 import { push } from "../internals/dismiss.js";
 import { trap } from "../internals/focus.js";
 import { drag } from "../internals/drag.js";
@@ -27,16 +27,14 @@ define("floating", "window", ({
   width = 420, height = 320, x = 80, y = 80,
   resizable = true, rollup = true, onClose,
 } = {}) => {
-  // The callback matters: another window coming to the front shifts this one's
-  // z, and without it the DOM keeps the number it had at open time.
-  const layer = claim("floatingWindow", (z) => { node.style.zIndex = String(z); });
   const saved = recall(id) || {};
   const box = { x: saved.x ?? x, y: saved.y ?? y, width: saved.width ?? width, height: saved.height ?? height };
 
   const titleId = uid("win-title");
   const node = el("div", { cls: "cx-window",
     attrs: { role: "dialog", "aria-labelledby": titleId } });
-  node.style.zIndex = String(layer.z);
+  // Bound, because another window coming to the front shifts this one's z.
+  const layer = claimFor("floatingWindow", node);
 
   const apply = () => {
     node.style.left = `${box.x}px`;
@@ -140,7 +138,6 @@ define("floating", "window", ({
 
 /** A full-screen "please wait" that genuinely blocks. */
 define("overlay", "blocking", ({ message, progress, cancel } = {}) => {
-  const layer = claim("wizard");
   const card = el("div", { cls: "cx-blocking-card", children: [
     el("span", { cls: ["cx-spinner", "cx-spinner-md"], attrs: { "aria-hidden": "true" } }),
     el("p", { cls: "cx-t cx-t-sm", text: message }),
@@ -151,7 +148,7 @@ define("overlay", "blocking", ({ message, progress, cancel } = {}) => {
   const node = el("div", { cls: "cx-blocking",
     attrs: { role: "alertdialog", "aria-busy": "true", "aria-label": message },
     children: card });
-  node.style.zIndex = String(layer.z);
+  const layer = claimFor("wizard", node);
   mount(node);
   const release = trap(card);
 
@@ -166,7 +163,6 @@ define("overlay", "blocking", ({ message, progress, cancel } = {}) => {
 
 /** A panel that slides in from an edge. Simple mode uses these over the preview. */
 define("slideOver", "md", ({ side = "right", title, body, onClose } = {}) => {
-  const layer = claim("modal");
   const titleId = uid("slide-title");
 
   const panel = el("aside", { cls: ["cx-slideover", `cx-slideover-${side}`],
@@ -179,7 +175,7 @@ define("slideOver", "md", ({ side = "right", title, body, onClose } = {}) => {
   panel.append(el("div", { cls: "cx-slideover-body", children: nodeOf(body, "slideOver.md") }));
 
   const root = el("div", { cls: "cx-slideover-layer", children: [el("div", { cls: "cx-backdrop" }), panel] });
-  root.style.zIndex = String(layer.z);
+  const layer = claimFor("modal", root);
   mount(root);
 
   const release = trap(panel);

app/composer/elements/modal.js

@@ -12,7 +12,7 @@ import { define } from "../internals/register.js";
 import { el } from "../internals/el.js";
 import { uid } from "../internals/ids.js";
 import { mount, unmount } from "../internals/portal.js";
-import { claim } from "../internals/zlayer.js";
+import { claimFor } from "../internals/zlayer.js";
 import { push } from "../internals/dismiss.js";
 import { trap } from "../internals/focus.js";
 
@@ -33,7 +33,6 @@ function open({
   closeOnOutside = true, stacked = false, describedBy,
 } = {}) {
   const titleId = uid("modal-title");
-  const layer = claim(stacked ? "modal" : "modal");
 
   const card = el("div", {
     cls: ["cx-modal", `cx-modal-${size}`, stacked ? "cx-modal-stacked" : null],
@@ -66,7 +65,9 @@ function open({
 
   const backdrop = el("div", { cls: "cx-backdrop" });
   const root = el("div", { cls: "cx-modal-layer", children: [backdrop, card] });
-  root.style.zIndex = String(layer.z);
+  // A stacked modal shares the modal rung with the one beneath it: peers stack
+  // by claim order, so it lands above without needing a rung of its own.
+  const layer = claimFor("modal", root);
   mount(root);
 
   // Trap after mounting: focusables() must be able to see the card.

app/composer/elements/popover.js

@@ -6,7 +6,7 @@
 import { define } from "../internals/register.js";
 import { el } from "../internals/el.js";
 import { mount, unmount } from "../internals/portal.js";
-import { claim } from "../internals/zlayer.js";
+import { claimFor } from "../internals/zlayer.js";
 import { push } from "../internals/dismiss.js";
 import { anchorTo } from "../internals/anchor.js";
 
@@ -18,9 +18,8 @@ function nodeOf(control, where) {
 
 function floatOn(rung, cls, { anchor, side = "bottom", align = "start", gap = 6, onClose, closeOnOutside = true, content, role, label }) {
   const anchorNode = anchor && anchor.node ? anchor.node : anchor;
-  const layer = claim(rung);
   const node = el("div", { cls, attrs: { role, "aria-label": label } });
-  node.style.zIndex = String(layer.z);
+  const layer = claimFor(rung, node);
   if (content) node.append(content);
 
   // Mounted before measuring: a hidden element measures 0x0 and lands in the

app/composer/elements/status.js

@@ -8,7 +8,7 @@ import { define } from "../internals/register.js";
 import { el } from "../internals/el.js";
 import { setText } from "../internals/text.js";
 import { portal } from "../internals/portal.js";
-import { claim } from "../internals/zlayer.js";
+import { claimFor } from "../internals/zlayer.js";
 
 const TONES = ["neutral", "good", "warn", "danger", "info"];
 
@@ -122,8 +122,7 @@ let stackLayer = null;
 function toastStack() {
   if (stack && stack.isConnected) return stack;
   stack = el("div", { cls: "cx-toasts" });
-  stackLayer = claim("toast");
-  stack.style.zIndex = String(stackLayer.z);
+  stackLayer = claimFor("toast", stack);
   stack.style.pointerEvents = "auto";
   portal().appendChild(stack);
   return stack;

app/composer/elements/wheel.js

@@ -12,7 +12,7 @@
 import { define } from "../internals/register.js";
 import { el, svg } from "../internals/el.js";
 import { mount, unmount } from "../internals/portal.js";
-import { claim } from "../internals/zlayer.js";
+import { claimFor } from "../internals/zlayer.js";
 import { push } from "../internals/dismiss.js";
 
 const MIN = 2;
@@ -127,11 +127,10 @@ function buildWheel({ items, onPick, onClose, geo, recentre, cancelOnCentre, cls
   }
 
   const { size } = geo;
-  const layer = claim("popover");
   const centre = recentre();
 
   const node = el("div", { cls, attrs: { role: "menu", "aria-label": "Quick pick" } });
-  node.style.zIndex = String(layer.z);
+  const layer = claimFor("popover", node);
   node.style.width = `${size}px`;
   node.style.height = `${size}px`;
 

app/composer/internals/zlayer.js

@@ -95,6 +95,23 @@ function notify(list, start) {
   list.forEach((entry, i) => { if (entry.onChange) entry.onChange(start + i); });
 }
 
+/**
+ * Claim a rung AND keep `node`'s z-index in sync with it -- the form every
+ * overlay should use.
+ *
+ * `claim()` alone hands back a number and trusts the caller to notice when it
+ * changes, and a caller that forgets gets a z-index that was right at open time
+ * and silently wrong afterwards: a peer releasing compacts the rung, and the
+ * next overlay opened lands on the number this one is still displaying. Six of
+ * the seven call sites forgot. Binding the node here means there is nothing
+ * left to forget.
+ */
+export function claimFor(rung, node) {
+  const handle = claim(rung, (z) => { node.style.zIndex = String(z); });
+  node.style.zIndex = String(handle.z);
+  return handle;
+}
+
 /** Live claims in a rung, bottom to top. Diagnostics and tests. */
 export const liveCount = (rung) => (stacks.get(rung) || []).length;
 

app/composer/tests/overlays.test.js

@@ -7,11 +7,14 @@ import { setupDom, teardownDom, key, fire } from "./_dom.js";
 import { EDGES, sectorAt, sectorInArc } from "../elements/wheel.js";
 
 let composer;
+let entries;
+let DEMOS;
 let ROOT_ID;
 let baseOf;
 test.before(async () => {
   setupDom();
-  ({ composer } = await import("../composer.js"));
+  ({ composer, entries } = await import("../composer.js"));
+  ({ DEMOS } = await import("../../catalogue/demos.js"));
   ({ ROOT_ID } = await import("../internals/portal.js"));
   ({ baseOf } = await import("../internals/zlayer.js"));
 });
@@ -454,3 +457,56 @@ test("gallery thumbnails are decorative, because the caption names them", () =>
     "an alt here would make a screen reader say the name twice");
   g.destroy();
 });
+
+// --- z-index staleness ------------------------------------------------------
+
+test("a peer closing does not leave two overlays sharing a z-index", () => {
+  // A rung compacts when a claim is released, so every overlay's z can move
+  // without it doing anything. A caller that reads z once at open time keeps a
+  // number that is later handed to something else: two overlays at one level,
+  // where DOM order decides which one wins and which one eats the clicks.
+  const anchor = composer.button.md({ label: "x" });
+  document.body.appendChild(anchor.node);
+
+  const a = composer.menu.dropdown({ anchor, items: [{ id: "a", label: "A" }] });
+  const b = composer.menu.dropdown({ anchor, items: [{ id: "b", label: "B" }] });
+  assert.notEqual(a.node.style.zIndex, b.node.style.zIndex);
+
+  a.close();
+  const c = composer.menu.dropdown({ anchor, items: [{ id: "c", label: "C" }] });
+  assert.notEqual(b.node.style.zIndex, c.node.style.zIndex,
+    "the newest overlay must not land on a number still in use");
+  assert.ok(Number(c.node.style.zIndex) > Number(b.node.style.zIndex),
+    "and it must be above, not merely different");
+
+  b.close(); c.close();
+});
+
+test("every overlay keeps its z-index bound, not just windows", () => {
+  // The failure above was one call site remembering and six forgetting, so the
+  // test walks every overlay the registry has rather than naming them.
+  const anchor = composer.button.md({ label: "x" });
+  document.body.appendChild(anchor.node);
+
+  const opened = [];
+  for (const { group, variant, factory } of entries()) {
+    const demo = DEMOS[`${group}.${variant}`];
+    if (!demo) continue;
+    const handle = factory(typeof demo === "function" ? demo(composer) : demo);
+    if (!handle.isOverlay || !handle.node.style.zIndex) { handle.destroy(); continue; }
+    opened.push({ name: `${group}.${variant}`, handle });
+  }
+
+  // Closing the oldest compacts every rung it was in; nothing may go stale.
+  const before = opened.map(({ handle }) => handle.node.style.zIndex);
+  if (opened.length > 1) opened.shift().handle.close();
+
+  const seen = new Map();
+  for (const { name, handle } of opened) {
+    const z = handle.node.style.zIndex;
+    assert.ok(!seen.has(z), `${name} collides with ${seen.get(z)} at z ${z}`);
+    seen.set(z, name);
+  }
+  void before;
+  for (const { handle } of opened) handle.destroy();
+});