diff --git a/app/composer/elements/input.js b/app/composer/elements/input.js index f6692cb5..0cef4a0b 100644 --- a/app/composer/elements/input.js +++ b/app/composer/elements/input.js @@ -152,6 +152,8 @@ define("textarea", "md", ({ value = "", rows = 4, autoGrow = false, placeholder, * Items are DATA -- {id, label, hint, icon} -- never a render callback, so a * module cannot smuggle markup in through a "just render this" escape hatch. */ +const FILTER_ICON_TONES = new Set(["accent", "danger", "warn", "good", "neutral"]); + define("filterList", "md", ({ items = [], value, onChange, placeholder = "Search", empty = "Nothing matches" } = {}) => { let selected = value; const search = el("input", { cls: ["cx-input", "cx-input-sm", "cx-search", "cx-focusable"], @@ -168,13 +170,18 @@ define("filterList", "md", ({ items = [], value, onChange, placeholder = "Search list.replaceChildren(); if (!shown.length) { list.append(el("div", { cls: "cx-filter-empty", text: empty })); return; } for (const item of shown) { + // `tone` picks one of a FIXED set of chip colours -- never a raw + // colour from data, which is the rule `el()`'s style refusal exists to + // hold everywhere else too. Enforced, not just claimed: an unknown + // tone throws, the same as button.js's own tone check, rather than + // silently rendering an unstyled chip. + if (item.tone && !FILTER_ICON_TONES.has(item.tone)) { + throw new RangeError(`Unknown filterList icon tone "${item.tone}". Known: ${[...FILTER_ICON_TONES].join(", ")}.`); + } const row = el("button", { cls: ["cx-filter-row", "cx-focusable", item.id === selected ? "cx-on" : null], attrs: { type: "button", role: "option", "aria-selected": String(item.id === selected) }, children: [ - // `tone` picks one of a FIXED set of chip colours -- never a raw - // colour from data, which is the rule `el()`'s style refusal exists - // to hold everywhere else too. item.icon ? el("span", { cls: ["cx-filter-icon", item.tone ? `cx-filter-icon-${item.tone}` : null], text: item.icon, diff --git a/app/composer/tests/elements_behaviour.test.js b/app/composer/tests/elements_behaviour.test.js index 89e1d99e..09b9f259 100644 --- a/app/composer/tests/elements_behaviour.test.js +++ b/app/composer/tests/elements_behaviour.test.js @@ -238,6 +238,12 @@ test("filterList gives an icon a coloured chip only from a fixed tone, never fro assert.ok(icon.classList.contains("cx-filter-icon-good")); }); +test("an unknown filterList icon tone is refused rather than rendered as a blank chip", () => { + assert.throws(() => mount(composer.filterList.md({ + items: [{ id: "a", label: "Node packs", icon: "▣", tone: "sparkly-typo" }], + })), RangeError); +}); + test("filterList says so when nothing matches", () => { const f = mount(composer.filterList.md({ items: [{ id: "a", label: "one" }] })); const search = f.node.querySelector("input"); diff --git a/app/shell/settings_window.js b/app/shell/settings_window.js index 38111519..b8d46628 100644 --- a/app/shell/settings_window.js +++ b/app/shell/settings_window.js @@ -83,6 +83,14 @@ export function createSettingsWindow({ sections = [], gitStatus } = {}) { const body = composer.splitPane.h({ panes: [nav, contentStack], size: 25, min: 15, label: "Settings" }); const modal = composer.modal.generic({ title: "Settings", size: "xl", body, + // Mirrors pipeline_window.js's own standalone modal: a half-finished + // edit (Models and pipeline's draft, most concretely) must not vanish + // because a click landed on the backdrop. This window has no way to + // ask "does the section on screen have unsaved work" per section, so + // it holds the door for all of them, the same conservative call + // pipeline_window.js already made for itself -- the X button and every + // section's own "Close" still work. + closeOnOutside: false, onClose: () => { if (activeCleanup) { activeCleanup.destroy?.(); activeCleanup = null; } if (current === handle) current = null; diff --git a/app/shell/tests/settings_window.test.js b/app/shell/tests/settings_window.test.js index 9383a161..70127849 100644 --- a/app/shell/tests/settings_window.test.js +++ b/app/shell/tests/settings_window.test.js @@ -4,7 +4,7 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { setupDom, teardownDom } from "../../composer/tests/_dom.js"; +import { setupDom, teardownDom, fire } from "../../composer/tests/_dom.js"; let createSettingsWindow; test.before(async () => { @@ -119,6 +119,20 @@ test("a gitStatus rejection is shown as a hint rather than left blank or thrown" w.close(); }); +test("a click outside the window does not close it -- a half-finished edit in any section is not a click away from gone", () => { + // The bug this guards: pipeline_window.js's own standalone modal has + // ALWAYS refused to close on an outside click (a group edit's draft is + // the only copy of it), but Settings' own modal was built without + // carrying that over, so hosting Models and pipeline in here silently + // lost the protection its own file was written to guarantee. + const alpha = fakeSection("a", "Alpha"); + const w = createSettingsWindow({ sections: [alpha] }); + w.open("a"); + fire(document.body, "pointerdown"); + assert.ok(document.querySelector(".cx-modal"), "an outside click closed the window"); + w.close(); +}); + test("closing tears down the active section and a later open starts fresh", () => { const alpha = fakeSection("a", "Alpha"); const w = createSettingsWindow({ sections: [alpha] });