From 000109d8e34ce95432f0909e9dfe20795214f5e4 Mon Sep 17 00:00:00 2001 From: Clayton Date: Thu, 25 Jun 2026 02:35:05 -0500 Subject: [PATCH] Stop a locked canvas layer from being reordered or displaced [#1248] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The summary copy promises "Lock brand elements so they cannot move by accident," but the Up/Down buttons only disabled at the top/bottom of the stack — a locked layer could still be moved directly, and an unlocked neighbour's Up/Down could displace it by swapping positions, since a single step only ever swaps with the adjacent layer. Extract canMoveLayer()/reorderLayers() as pure, exported functions: a move is allowed only when neither the layer being moved nor its target neighbour is locked. Both the button disabled-state and the actual move() now go through the same check, so a locked layer can never move and can never be displaced. --- prototype/canvas-layer-controls.html | 44 ++++++++++++++++++++----- prototype/canvas-layer-controls.test.js | 42 ++++++++++++++++++++++- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/prototype/canvas-layer-controls.html b/prototype/canvas-layer-controls.html index e34e5443..0632c139 100644 --- a/prototype/canvas-layer-controls.html +++ b/prototype/canvas-layer-controls.html @@ -353,6 +353,29 @@

Adapts when reused

return ""; } + // Locking a layer's stated purpose is "so it cannot move by accident" (see the summary + // note above the list). That promise covers two paths: the locked layer's own up/down + // controls, and an unlocked neighbour's move that would swap positions with it. A single + // up/down step only ever swaps with the adjacent layer, so checking that one neighbour is + // enough to decide whether a move is allowed. + function canMoveLayer(list, index, delta) { + const target = index + delta; + if (target < 0 || target >= list.length) { return false; } + const layer = list[index]; + const neighbor = list[target]; + if (!layer || !neighbor) { return false; } + return !layer.locked && !neighbor.locked; + } + + function reorderLayers(list, index, delta) { + if (!canMoveLayer(list, index, delta)) { return list; } + const target = index + delta; + const copy = list.slice(); + const item = copy.splice(index, 1)[0]; + copy.splice(target, 0, item); + return copy; + } + function renderLayer(layer, index) { const type = LAYER_TYPES[layer.type]; @@ -368,10 +391,10 @@

Adapts when reused

const main = el("div", { class: "layer-main" }, [nameRow, meta]); const up = el("button", { type: "button", class: "tiny secondary", "aria-label": "Move " + type.label + " up", text: "▲" }, []); - if (index === 0) { up.disabled = true; } + if (!canMoveLayer(layers, index, -1)) { up.disabled = true; } up.addEventListener("click", function () { move(index, -1); }); const down = el("button", { type: "button", class: "tiny secondary", "aria-label": "Move " + type.label + " down", text: "▼" }, []); - if (index === layers.length - 1) { down.disabled = true; } + if (!canMoveLayer(layers, index, 1)) { down.disabled = true; } down.addEventListener("click", function () { move(index, 1); }); const vis = el("button", { type: "button", class: "tiny secondary", text: layer.visible ? "Hide" : "Show" }, []); @@ -403,17 +426,20 @@

Adapts when reused

} function move(index, delta) { - const target = index + delta; - if (target < 0 || target >= layers.length) { return; } - const copy = layers.slice(); - const item = copy.splice(index, 1)[0]; - copy.splice(target, 0, item); - layers = copy; + const next = reorderLayers(layers, index, delta); + if (next === layers) { return; } + layers = next; render(); } if (typeof module !== "undefined" && module.exports) { - module.exports = { evaluate: evaluate, LAYER_TYPES: LAYER_TYPES, layoutSignature: layoutSignature }; + module.exports = { + evaluate: evaluate, + LAYER_TYPES: LAYER_TYPES, + layoutSignature: layoutSignature, + canMoveLayer: canMoveLayer, + reorderLayers: reorderLayers, + }; } render(); diff --git a/prototype/canvas-layer-controls.test.js b/prototype/canvas-layer-controls.test.js index 6b69e3e5..d0d373ce 100644 --- a/prototype/canvas-layer-controls.test.js +++ b/prototype/canvas-layer-controls.test.js @@ -122,4 +122,44 @@ if (typeof M.layoutSignature === "function") { ); } -console.log("canvas layer controls: stack guardrails (cover, lock, hidden speaker) verified"); +// Locking a layer's stated purpose is "so it cannot move by accident" (see the lede / +// summary-note copy). That must hold both when the locked layer's own controls are used and +// when an unlocked neighbour's move would swap positions with it. +assert.ok(typeof M.canMoveLayer === "function", "prototype exports canMoveLayer()"); +assert.ok(typeof M.reorderLayers === "function", "prototype exports reorderLayers()"); + +const lockedStack = [ + { id: "captions", type: "captions", visible: true, locked: false }, + { id: "brand", type: "brand", visible: true, locked: false }, + { id: "background", type: "background", visible: true, locked: true }, +]; + +assert.strictEqual(M.canMoveLayer(lockedStack, 2, -1), false, "the locked layer itself cannot move up"); +assert.strictEqual(M.canMoveLayer(lockedStack, 1, 1), false, "an unlocked neighbour cannot displace the locked layer by moving down"); +assert.strictEqual(M.canMoveLayer(lockedStack, 0, 1), true, "two unlocked layers may still swap with each other"); + +assert.strictEqual( + M.reorderLayers(lockedStack, 2, -1), + lockedStack, + "moving a locked layer is refused and returns the same list reference", +); +assert.strictEqual( + M.reorderLayers(lockedStack, 1, 1), + lockedStack, + "moving an unlocked layer into a locked neighbour is refused", +); + +const reordered = M.reorderLayers(lockedStack, 0, 1); +assert.notStrictEqual(reordered, lockedStack, "an allowed reorder returns a new list"); +assert.deepStrictEqual( + reordered.map((layer) => layer.id), + ["brand", "captions", "background"], + "an allowed reorder swaps the requested neighbour", +); +assert.deepStrictEqual( + lockedStack.map((layer) => layer.id), + ["captions", "brand", "background"], + "reorderLayers does not mutate its input list", +); + +console.log("canvas layer controls: stack guardrails (cover, lock, hidden speaker, locked reorder) verified");