From a5b21024accf4996e01e08bf43b5ba00d34a9c74 Mon Sep 17 00:00:00 2001 From: Remex_9 Date: Mon, 2 Nov 2026 14:57:28 -0600 Subject: [PATCH 1/5] Select scene objects with one selection model (#440) - Replace the separate image and annotation selection fields with one `state.selection` ({ kind, index } or null) behind select/deselect helpers. - Clicking an object selects it; clicking it again keeps it selected (the second click is reserved for reaching overlapping objects in #441). Clicking a resize or crop handle without dragging no longer deselects. - Escape or a click on empty canvas clears the selection for every object type. - Tab / Shift+Tab on the canvas step through drawn objects from the top of the stack down; past either end the selection clears and focus leaves. - Each selection is announced with its type, label text, and stack position. Part of #439. Co-Authored-By: Claude Opus 5.5 --- templates/js/scenes.js | 222 ++++++++++++------ templates/tests/sceneAnnotationEditor.test.js | 118 ++++++++++ templates/tests/scenes.test.js | 7 +- 3 files changed, 269 insertions(+), 78 deletions(-) diff --git a/templates/js/scenes.js b/templates/js/scenes.js index 8ab3544..6b76b48 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -191,25 +191,43 @@ const state = { openRequest: 0, fitToWidth: true, busy: false, - selectedImageIndex: null, + // Issue #440: the one selected object, `{ kind: "image" | "annotation", + // index }` (its index in `active.images` or `active.annotations`), or null. + selection: null, cropMode: false, - // Issue #426: the selected annotation's index in `active.annotations`, the - // annotation tool in use ("text" | "line" | "arrow" | "polygon" | - // "pointer"), and the shape being drawn with it. - selectedAnnotationIndex: null, + // Issue #426: the annotation tool in use ("text" | "line" | "arrow" | + // "polygon" | "pointer"), and the shape being drawn with it. tool: null, draft: null, boneOptions: null, importRequest: 0, }; -function deselectImage() { - state.selectedImageIndex = null; - state.cropMode = false; +/** The selected image's index, or null when no image is selected. */ +function selectedImageIndex() { + return state.selection && state.selection.kind === "image" ? state.selection.index : null; +} + +/** The selected annotation's index, or null when no annotation is selected. */ +function selectedAnnotationIndex() { + return state.selection && state.selection.kind === "annotation" ? state.selection.index : null; +} + +/** + * Selects one object, replacing any other selection. Crop mode only lasts + * while the image it was started on stays selected. + * @param {"image"|"annotation"} kind + * @param {number} index + */ +function select(kind, index) { + const isSame = state.selection && state.selection.kind === kind && state.selection.index === index; + if (!isSame) state.cropMode = false; + state.selection = { kind, index }; } -function deselectAnnotation() { - state.selectedAnnotationIndex = null; +function deselect() { + state.selection = null; + state.cropMode = false; } const clone = (value) => JSON.parse(JSON.stringify(value)); @@ -229,8 +247,7 @@ function setActiveScene(scene) { state.conflictUpdatedAt = null; state.missing = false; state.saveAlert = null; - deselectImage(); - deselectAnnotation(); + deselect(); endLineDraw(); state.tool = null; state.draft = null; @@ -497,7 +514,7 @@ function renderLibrary() { */ function updateImageToolbar() { const scene = state.active; - const index = state.selectedImageIndex; + const index = selectedImageIndex(); const hasSelection = Boolean(scene) && index !== null && index >= 0 && index < scene.images.length; dom.imageToolbar.hidden = !hasSelection; if (!hasSelection) return; @@ -522,9 +539,9 @@ function renderCanvas() { if (isEmpty) return; const { svg, unsupported, unlinked, width, height } = renderScene(scene, { - selectedIndex: state.selectedImageIndex, + selectedIndex: selectedImageIndex(), cropMode: state.cropMode, - selectedAnnotationIndex: state.selectedAnnotationIndex, + selectedAnnotationIndex: selectedAnnotationIndex(), draft: state.draft, }); svg.setAttribute("aria-label", `Scene canvas for ${scene.name}`); @@ -983,11 +1000,11 @@ async function handleDuplicate() { /** Removes the selected image from the working copy (Issue #418). */ function handleImageRemove() { - if (!state.active || state.selectedImageIndex === null) return; - const index = state.selectedImageIndex; + if (!state.active || selectedImageIndex() === null) return; + const index = selectedImageIndex(); if (!state.active.images[index]) return; state.active.images.splice(index, 1); - deselectImage(); + deselect(); markChanged(); renderWorkspace(); } @@ -999,8 +1016,8 @@ function handleImageRemove() { * @param {object} fields - The new values to set, e.g. `{ rotation: 90 }`. */ function applySelectedImageUpdate(fields) { - if (!state.active || state.selectedImageIndex === null) return; - const image = state.active.images[state.selectedImageIndex]; + if (!state.active || selectedImageIndex() === null) return; + const image = state.active.images[selectedImageIndex()]; if (!image) return; Object.assign(image, fields); markChanged(); @@ -1013,8 +1030,8 @@ function applySelectedImageUpdate(fields) { * @param {number} deltaDegrees */ function rotateSelectedImage(deltaDegrees) { - if (!state.active || state.selectedImageIndex === null) return; - const image = state.active.images[state.selectedImageIndex]; + if (!state.active || selectedImageIndex() === null) return; + const image = state.active.images[selectedImageIndex()]; if (!image) return; const current = typeof image.rotation === "number" ? image.rotation : 0; const next = ((current + deltaDegrees) % 360 + 360) % 360; @@ -1026,8 +1043,8 @@ function rotateSelectedImage(deltaDegrees) { * @param {"horizontal" | "vertical"} axis */ function flipSelectedImage(axis) { - if (!state.active || state.selectedImageIndex === null) return; - const image = state.active.images[state.selectedImageIndex]; + if (!state.active || selectedImageIndex() === null) return; + const image = state.active.images[selectedImageIndex()]; if (!image) return; const field = axis === "horizontal" ? "flipX" : "flipY"; applySelectedImageUpdate({ [field]: !image[field] }); @@ -1041,13 +1058,13 @@ function flipSelectedImage(axis) { * @param {1 | -1} direction */ function moveSelectedImage(direction) { - if (!state.active || state.selectedImageIndex === null) return; + if (!state.active || selectedImageIndex() === null) return; const images = state.active.images; - const index = state.selectedImageIndex; + const index = selectedImageIndex(); const targetIndex = index + direction; if (targetIndex < 0 || targetIndex >= images.length) return; [images[index], images[targetIndex]] = [images[targetIndex], images[index]]; - state.selectedImageIndex = targetIndex; + state.selection.index = targetIndex; markChanged(); renderWorkspace(); } @@ -1109,7 +1126,7 @@ function unrotateVector(dx, dy, degrees) { */ function startImageDrag(event, svg, corner) { const scene = state.active; - const index = state.selectedImageIndex; + const index = selectedImageIndex(); const image = scene.images[index]; const scale = getSceneScale(svg); const startClientX = event.clientX; @@ -1216,7 +1233,7 @@ function computeCropRect(corner, start, localDelta, imageWidth, imageHeight) { */ function startCropDrag(event, svg, corner) { const scene = state.active; - const index = state.selectedImageIndex; + const index = selectedImageIndex(); const image = scene.images[index]; const scale = getSceneScale(svg); const startClientX = event.clientX; @@ -1252,7 +1269,7 @@ function startCropDrag(event, svg, corner) { /** Enters crop-editing mode for the selected image (Issue #417). */ function enterCropMode() { - if (!state.active || state.selectedImageIndex === null) return; + if (!state.active || selectedImageIndex() === null) return; state.cropMode = true; renderCanvas(); } @@ -1266,8 +1283,8 @@ function exitCropMode() { /** Clears the selected image's crop back to fully visible. */ function resetSelectedImageCrop() { - if (!state.active || state.selectedImageIndex === null) return; - const image = state.active.images[state.selectedImageIndex]; + if (!state.active || selectedImageIndex() === null) return; + const image = state.active.images[selectedImageIndex()]; if (!image) return; applySelectedImageUpdate({ cropX: 0, cropY: 0, cropWidth: image.width, cropHeight: image.height }); } @@ -1303,14 +1320,86 @@ const round2 = (value) => Math.round(value * 100) / 100; function selectedAnnotation() { const scene = state.active; - const index = state.selectedAnnotationIndex; + const index = selectedAnnotationIndex(); if (!scene || index === null || index < 0 || index >= scene.annotations.length) return null; return scene.annotations[index]; } -function selectAnnotation(index) { - deselectImage(); - state.selectedAnnotationIndex = index; +// Issue #440: selecting objects with the mouse and keyboard. A click selects +// the object under it (clicking it again keeps it selected); Escape or a +// click on empty canvas clears the selection; Tab and Shift+Tab step through +// the drawn objects from the top of the stack down. +const MAX_ANNOUNCED_TEXT = 40; + +/** + * Every drawn object as { kind, index }, topmost first: annotations are + * drawn over images, and each later object over the ones before it. + * Objects the canvas can't draw aren't included. + */ +function selectableObjects() { + const svg = dom.canvas.querySelector("svg"); + if (!svg) return []; + const annotationIndexes = new Set( + [...svg.querySelectorAll("[data-scene-annotation-index]")].map((node) => Number(node.dataset.sceneAnnotationIndex)) + ); + const imageIndexes = [...svg.querySelectorAll("[data-scene-image-index]")].map((node) => Number(node.dataset.sceneImageIndex)); + return [ + ...[...annotationIndexes].reverse().map((index) => ({ kind: "annotation", index })), + ...imageIndexes.reverse().map((index) => ({ kind: "image", index })), + ]; +} + +function announceSelection() { + const { kind, index } = state.selection; + let name = "image"; + if (kind === "annotation") { + const annotation = selectedAnnotation(); + name = describeAnnotation(annotation); + if (annotation.type === "text") { + const text = annotation.text.replace(/\s+/g, " ").trim(); + name += ` "${text.length > MAX_ANNOUNCED_TEXT ? `${text.slice(0, MAX_ANNOUNCED_TEXT)}…` : text}"`; + } + } + const objects = selectableObjects(); + const position = objects.findIndex((object) => object.kind === kind && object.index === index); + announce(position === -1 ? `Selected ${name}.` : `Selected ${name}, ${position + 1} of ${plural(objects.length, "object")}.`); +} + +/** Selects an object the user picked, and says which one it is. */ +function selectObject(kind, index) { + select(kind, index); + renderCanvas(); + announceSelection(); +} + +/** Clears the selection the user dismissed. `quiet` skips the announcement. */ +function clearSelection({ quiet = false } = {}) { + if (!state.selection) return; + deselect(); + renderCanvas(); + if (!quiet) announce("Nothing selected."); +} + +/** + * Tab and Shift+Tab on the canvas select the next or previous object. Past + * either end the selection is cleared and focus leaves the canvas as usual. + * @returns {boolean} Whether the key was handled. + */ +function handleSelectionTab(event) { + if (event.key !== "Tab" || event.altKey || event.ctrlKey || event.metaKey) return false; + if (!state.active || state.tool || state.cropMode) return false; + const objects = selectableObjects(); + const current = state.selection + ? objects.findIndex((object) => object.kind === state.selection.kind && object.index === state.selection.index) + : -1; + if (current === -1 && event.shiftKey) return false; + const next = event.shiftKey ? current - 1 : current + 1; + if (next < 0 || next >= objects.length) { + clearSelection({ quiet: true }); + return false; + } + selectObject(objects[next].kind, objects[next].index); + return true; } /** Where a mouse event is in scene coordinates. */ @@ -1376,10 +1465,10 @@ function updateAnnotationControls() { } const last = state.active.annotations.length - 1; - dom.annotationBack.disabled = state.selectedAnnotationIndex === 0; - dom.annotationBackward.disabled = state.selectedAnnotationIndex === 0; - dom.annotationForward.disabled = state.selectedAnnotationIndex === last; - dom.annotationFront.disabled = state.selectedAnnotationIndex === last; + dom.annotationBack.disabled = selectedAnnotationIndex() === 0; + dom.annotationBackward.disabled = selectedAnnotationIndex() === 0; + dom.annotationForward.disabled = selectedAnnotationIndex() === last; + dom.annotationFront.disabled = selectedAnnotationIndex() === last; } /** @@ -1395,10 +1484,7 @@ function setTool(tool) { endLineDraw(); state.tool = next; state.draft = null; - if (next && next !== "pointer") { - deselectImage(); - deselectAnnotation(); - } + if (next && next !== "pointer") deselect(); renderCanvas(); if (next) { announce(TOOL_HINTS[next]); @@ -1420,7 +1506,7 @@ function addAnnotation(annotation) { state.active.annotations.push(annotation); state.tool = null; state.draft = null; - selectAnnotation(state.active.annotations.length - 1); + select("annotation", state.active.annotations.length - 1); markChanged(); renderWorkspace(); } @@ -1755,14 +1841,14 @@ function handleAnnotationOpacityInput() { */ function moveSelectedAnnotation(direction) { const annotations = state.active ? state.active.annotations : null; - const index = state.selectedAnnotationIndex; + const index = selectedAnnotationIndex(); if (!annotations || !selectedAnnotation()) return; const last = annotations.length - 1; const target = direction === "front" ? last : direction === "back" ? 0 : index + direction; if (target < 0 || target > last || target === index) return; const [annotation] = annotations.splice(index, 1); annotations.splice(target, 0, annotation); - state.selectedAnnotationIndex = target; + state.selection.index = target; markChanged(); renderWorkspace(); const position = target === last ? "in front of all other annotations" @@ -1778,7 +1864,7 @@ function deleteSelectedAnnotation() { const scene = state.active; const removed = new Set([annotation, ...pointerLinesOf(annotation, scene).map(({ line }) => line)]); scene.annotations = scene.annotations.filter((other) => !removed.has(other)); - deselectAnnotation(); + deselect(); markChanged(); renderWorkspace(); const kind = describeAnnotation(annotation); @@ -1818,8 +1904,7 @@ function handleAnnotationKey(event) { } if (state.tool || !selectedAnnotation()) return false; if (event.key === "Escape") { - deselectAnnotation(); - renderCanvas(); + clearSelection(); return true; } if (event.key === "Delete" || event.key === "Backspace") { @@ -2293,25 +2378,17 @@ export function initializeSceneEditor(root = document) { handleToolClick(event); return; } - if (event.target.closest("[data-annotation-handle]")) return; + // A click on a handle belongs to the selected object, even without a drag. + if (event.target.closest("[data-annotation-handle], [data-corner]")) return; const annotationTarget = event.target.closest("[data-scene-annotation-index]"); if (annotationTarget) { if (state.cropMode) return; - const annotationIndex = Number(annotationTarget.dataset.sceneAnnotationIndex); - if (state.selectedAnnotationIndex === annotationIndex) deselectAnnotation(); - else selectAnnotation(annotationIndex); - renderCanvas(); + selectObject("annotation", Number(annotationTarget.dataset.sceneAnnotationIndex)); return; } - deselectAnnotation(); - const target = event.target.closest("[data-scene-image-index]"); - const index = target ? Number(target.dataset.sceneImageIndex) : null; - if (state.selectedImageIndex === index) { - deselectImage(); - } else { - state.selectedImageIndex = index; - } - renderCanvas(); + const imageTarget = event.target.closest("[data-scene-image-index]"); + if (imageTarget) selectObject("image", Number(imageTarget.dataset.sceneImageIndex)); + else clearSelection(); }); dom.canvas.addEventListener("dblclick", (event) => { if (state.tool === "polygon") { @@ -2324,20 +2401,17 @@ export function initializeSceneEditor(root = document) { const index = Number(target.dataset.sceneAnnotationIndex); const annotation = state.active && state.active.annotations[index]; if (!annotation || annotation.type !== "text") return; - selectAnnotation(index); + select("annotation", index); renderCanvas(); dom.annotationText.focus(); dom.annotationText.select(); }); dom.canvas.addEventListener("keydown", (event) => { - if (handleAnnotationKey(event)) { + if (handleAnnotationKey(event) || handleSelectionTab(event)) { event.preventDefault(); return; } - if (event.key === "Escape" && state.selectedImageIndex !== null) { - deselectImage(); - renderCanvas(); - } + if (event.key === "Escape" && state.selection) clearSelection(); }); dom.canvas.addEventListener("mousedown", (event) => { // A drag's own click may never arrive (its node was redrawn away), so @@ -2361,14 +2435,14 @@ export function initializeSceneEditor(root = document) { event.preventDefault(); startAnnotationDrag(event, annotationSvg, annotationHandle.dataset.annotationHandle, Number(annotationHandle.dataset.pointIndex)); - } else if (annotationNode && Number(annotationNode.dataset.sceneAnnotationIndex) === state.selectedAnnotationIndex) { + } else if (annotationNode && Number(annotationNode.dataset.sceneAnnotationIndex) === selectedAnnotationIndex()) { event.preventDefault(); startAnnotationDrag(event, annotationSvg, "move"); } return; } - if (state.selectedImageIndex === null) return; - const image = state.active.images[state.selectedImageIndex]; + if (selectedImageIndex() === null) return; + const image = state.active.images[selectedImageIndex()]; if (!image) return; const handle = event.target.closest("[data-corner]"); @@ -2384,7 +2458,7 @@ export function initializeSceneEditor(root = document) { const imageNode = event.target.closest("[data-scene-image-index]"); const isSelectedImageNode = - imageNode && Number(imageNode.dataset.sceneImageIndex) === state.selectedImageIndex; + imageNode && Number(imageNode.dataset.sceneImageIndex) === selectedImageIndex(); if (!handle && !isSelectedImageNode) return; event.preventDefault(); diff --git a/templates/tests/sceneAnnotationEditor.test.js b/templates/tests/sceneAnnotationEditor.test.js index a7817f3..7cbf133 100644 --- a/templates/tests/sceneAnnotationEditor.test.js +++ b/templates/tests/sceneAnnotationEditor.test.js @@ -724,3 +724,121 @@ describe("Annotation selection", () => { expect($("scene-canvas-empty").hidden).toBe(false); }); }); + +describe("Selecting objects (#440)", () => { + // One of each object type. Annotations are drawn over images, and later + // ones over earlier ones, so from the top down: highlight, arrow, line, + // label, image. + const content = { + images: [{ id: "img", src: "/images/a.png", x: 300, y: 300, width: 100, height: 100 }], + annotations: [ + { id: "label", type: "text", text: "Ilium", x: 0, y: 0 }, + { id: "line", type: "line", x1: 10, y1: 400, x2: 200, y2: 400 }, + { id: "arrow", type: "arrow", x1: 10, y1: 450, x2: 200, y2: 450 }, + { id: "poly", type: "polygon", points: [[500, 100], [600, 100], [550, 200]] }, + ], + }; + const imageNode = () => document.querySelector("[data-scene-image-index=\"0\"]"); + const status = () => $("scene-editor-status").textContent; + const outlines = () => document.querySelectorAll(".scene-selection-outline"); + /** Presses a key on the canvas; returns whether the editor handled it (prevented the default). */ + function press(name, options = {}) { + return !$("scene-canvas").dispatchEvent(new KeyboardEvent("keydown", { key: name, bubbles: true, cancelable: true, ...options })); + } + + it("selects each object type with a visible outline, and a second click keeps it selected", async () => { + await openBlankScene(content); + + for (const index of [0, 1, 2, 3]) { + selectAnnotation(index); + expect($("scene-annotation-toolbar").hidden).toBe(false); + expect($("scene-image-toolbar").hidden).toBe(true); + expect(outlines()).toHaveLength(1); + selectAnnotation(index); + expect($("scene-annotation-toolbar").hidden).toBe(false); + } + clickAt(imageNode(), [350, 350]); + expect($("scene-image-toolbar").hidden).toBe(false); + expect($("scene-annotation-toolbar").hidden).toBe(true); + expect(outlines()).toHaveLength(1); + clickAt(imageNode(), [350, 350]); + expect($("scene-image-toolbar").hidden).toBe(false); + }); + + it("says which object was selected and where it is in the stack", async () => { + await openBlankScene(content); + + selectAnnotation(0); + await waitFor(() => expect(status()).toBe("Selected label \"Ilium\", 4 of 5 objects.")); + selectAnnotation(3); + await waitFor(() => expect(status()).toBe("Selected highlight, 1 of 5 objects.")); + clickAt(imageNode(), [350, 350]); + await waitFor(() => expect(status()).toBe("Selected image, 5 of 5 objects.")); + }); + + it("clears the selection with Escape or a click on empty canvas", async () => { + await openBlankScene(content); + + clickAt(imageNode(), [350, 350]); + expect(press("Escape")).toBe(false); + expect($("scene-image-toolbar").hidden).toBe(true); + expect(outlines()).toHaveLength(0); + await waitFor(() => expect(status()).toBe("Nothing selected.")); + + selectAnnotation(1); + clickAt(canvasSvg(), [900, 550]); + expect($("scene-annotation-toolbar").hidden).toBe(true); + expect(outlines()).toHaveLength(0); + }); + + it("keeps an image selected when one of its corner handles is clicked without dragging", async () => { + await openBlankScene(content); + clickAt(imageNode(), [350, 350]); + + clickAt(document.querySelector("[data-corner=\"se\"]"), [400, 400]); + + expect($("scene-image-toolbar").hidden).toBe(false); + expect(scenesModule.getActiveScene().images[0]).toMatchObject({ x: 300, y: 300, width: 100, height: 100 }); + expect(scenesModule.hasUnsavedChanges()).toBe(false); + }); + + it("steps through the objects from the top with Tab and back with Shift+Tab, then lets focus leave", async () => { + await openBlankScene(content); + const selected = () => { + if (!$("scene-image-toolbar").hidden) return "image"; + if (!$("scene-annotation-toolbar").hidden) return "annotation"; + return null; + }; + const order = []; + for (let step = 0; step < 5; step += 1) { + expect(press("Tab")).toBe(true); + await waitFor(() => expect(status()).toMatch(new RegExp(`, ${step + 1} of 5 objects\\.$`))); + order.push(status()); + } + expect(order.map((message) => message.replace(/, \d of 5 objects\.$/, ""))).toEqual([ + "Selected highlight", + "Selected arrow", + "Selected line", + "Selected label \"Ilium\"", + "Selected image", + ]); + expect(selected()).toBe("image"); + + expect(press("Tab", { shiftKey: true })).toBe(true); + await waitFor(() => expect(status()).toBe("Selected label \"Ilium\", 4 of 5 objects.")); + + // Past the last object, Tab clears the selection and isn't held by the canvas. + press("Tab"); + expect(press("Tab")).toBe(false); + expect(selected()).toBeNull(); + // With nothing selected, Shift+Tab leaves the canvas too. + expect(press("Tab", { shiftKey: true })).toBe(false); + }); + + it("leaves Tab alone while a drawing tool is in use", async () => { + await openBlankScene(content); + click("scene-tool-line"); + expect(press("Tab")).toBe(false); + expect(outlines()).toHaveLength(0); + }); +}); diff --git a/templates/tests/scenes.test.js b/templates/tests/scenes.test.js index 764f207..7633387 100644 --- a/templates/tests/scenes.test.js +++ b/templates/tests/scenes.test.js @@ -489,7 +489,7 @@ describe("Scene editor: importing images - Issue 411", () => { expect($("scene-image-import-error").textContent).toMatch(/notes\.pdf/); }); - it("selects an image on click and deselects it on a second click", async () => { + it("selects an image on click and keeps it selected on a second click (Issue 440)", async () => { await openNewScene(); setFiles([{ name: "ilium.png", type: "image/png" }]); await waitFor(() => expect(document.querySelectorAll(".scene-image-object")).toHaveLength(1)); @@ -498,7 +498,7 @@ describe("Scene editor: importing images - Issue 411", () => { await waitFor(() => expect(document.querySelectorAll(".scene-selection-outline")).toHaveLength(1)); document.querySelector(".scene-image-object").dispatchEvent(new MouseEvent("click", { bubbles: true })); - expect(document.querySelectorAll(".scene-selection-outline")).toHaveLength(0); + expect(document.querySelectorAll(".scene-selection-outline")).toHaveLength(1); }); it("clears the selection when clicking empty canvas background", async () => { @@ -651,7 +651,7 @@ describe("Scene editor: remove and reorder images", () => { selectImage(0); expect($("scene-image-toolbar").hidden).toBe(false); - selectImage(0); + $("scene-canvas").dispatchEvent(new MouseEvent("click", { bubbles: true })); expect($("scene-image-toolbar").hidden).toBe(true); }); @@ -662,7 +662,6 @@ describe("Scene editor: remove and reorder images", () => { expect($("scene-image-backward").disabled).toBe(true); expect($("scene-image-forward").disabled).toBe(false); - selectImage(0); // deselect selectImage(1); expect($("scene-image-forward").disabled).toBe(true); expect($("scene-image-backward").disabled).toBe(false); From c3ff89cf7282295c315f5b10739e59005ed43c49 Mon Sep 17 00:00:00 2001 From: Remex_9 Date: Mon, 2 Nov 2026 15:05:33 -0600 Subject: [PATCH 2/5] Reach overlapping scene objects by clicking again (#441) - Add objectsAt(scene, point) to sceneCanvas.js: every drawn object under a scene point, topmost first, matching what renderScene draws (rotated and flipped crop areas, line hit width, polygon fill, linked annotations, and colored regions clipped to their image's crop). - Clicking again in the same spot selects the next object down the stack, wrapping back to the top; Alt+click steps down from anywhere on the stack. A click in a new spot selects the topmost object there. - While objects overlap, a hint under the canvas (also announced) says how many there are and how to reach the next one; it clears on any edit. - Selecting or clearing keeps the canvas in place on screen, so the toolbar that appears above it no longer moves the scene out from under the pointer. - A double-click only opens a label's text when the clicks left that label selected, so quick clicks keep stepping down the stack. Part of #439. Co-Authored-By: Claude Opus 5.5 --- templates/boneset.html | 1 + templates/js/sceneCanvas.js | 105 ++++++++++++++ templates/js/scenes.js | 132 ++++++++++++++---- templates/style.css | 6 + templates/tests/sceneAnnotationEditor.test.js | 118 +++++++++++++++- templates/tests/sceneCanvas.test.js | 89 +++++++++++- 6 files changed, 421 insertions(+), 30 deletions(-) diff --git a/templates/boneset.html b/templates/boneset.html index 40a117d..d274caa 100644 --- a/templates/boneset.html +++ b/templates/boneset.html @@ -321,6 +321,7 @@

+ diff --git a/templates/js/sceneCanvas.js b/templates/js/sceneCanvas.js index 0cecd8a..2ec2cea 100644 --- a/templates/js/sceneCanvas.js +++ b/templates/js/sceneCanvas.js @@ -555,6 +555,111 @@ function renderAnnotation(annotation, defs) { } } +// Issue #441: finding every object under a point, so overlapping objects can +// be reached and not only the one drawn on top. This repeats the browser's +// hit testing in scene geometry, matching what renderScene draws and what +// its hit areas make clickable. + +/** Whether a point is inside a polygon (even-odd rule). */ +function pointInPolygon([px, py], points) { + let inside = false; + for (let i = 0, j = points.length - 1; i < points.length; j = i, i += 1) { + const [xi, yi] = points[i]; + const [xj, yj] = points[j]; + if ((yi > py) !== (yj > py) && px < ((xj - xi) * (py - yi)) / (yj - yi) + xi) inside = !inside; + } + return inside; +} + +function distanceToSegment([px, py], [ax, ay], [bx, by]) { + const dx = bx - ax; + const dy = by - ay; + const lengthSquared = dx * dx + dy * dy; + const t = lengthSquared ? Math.min(1, Math.max(0, ((px - ax) * dx + (py - ay) * dy) / lengthSquared)) : 0; + return Math.hypot(px - (ax + t * dx), py - (ay + t * dy)); +} + +/** Whether a point is within `tolerance` of a polyline (or a closed polygon's outline). */ +function isNearPolyline(point, points, tolerance, closed = false) { + const segments = closed ? points.length : points.length - 1; + for (let i = 0; i < segments; i += 1) { + if (distanceToSegment(point, points[i], points[(i + 1) % points.length]) <= tolerance) return true; + } + return false; +} + +/** The visible (cropped) area of an image as drawn - flipped and rotated - or null if it isn't drawn. */ +function imageHitArea(image) { + if (!image || typeof image.src !== "string" || !SAFE_IMAGE_SRC.test(image.src)) return null; + if (!isValidBox(image)) return null; + const crop = cropRectFor(image); + const corners = [ + [image.x + crop.x, image.y + crop.y], + [image.x + crop.x + crop.width, image.y + crop.y], + [image.x + crop.x + crop.width, image.y + crop.y + crop.height], + [image.x + crop.x, image.y + crop.y + crop.height], + ]; + return corners.map(imageTransform(image, image).toScreen); +} + +/** Whether a drawn annotation (displayed geometry) covers a point. */ +function annotationHit(annotation, point) { + switch (annotation.type) { + case "text": { + if (typeof annotation.text !== "string" || !isNumber(annotation.x) || !isNumber(annotation.y)) return false; + const box = textBox(annotation); + const [x, y] = point; + return x >= box.x && x <= box.x + box.width && y >= box.y && y <= box.y + box.height; + } + case "line": + case "arrow": { + const points = linePoints(annotation); + const width = isNumber(annotation.strokeWidth) ? annotation.strokeWidth : 2; + return Boolean(points) && isNearPolyline(point, points, Math.max(LINE_HIT_WIDTH, width + 8) / 2); + } + case "polygon": { + const points = toPoints(annotation.points); + if (!points || points.length < 3) return false; + const width = isNumber(annotation.strokeWidth) ? annotation.strokeWidth : 1.5; + return pointInPolygon(point, points) || isNearPolyline(point, points, width / 2, true); + } + default: + return false; + } +} + +/** + * Every object drawn at a scene point, topmost first, as { kind, index }: + * annotations (each later one before the ones under it), then images. Objects + * the canvas can't draw are left out, and a colored region clipped to its + * image's crop only counts inside the visible crop area. + * @param {{ images: object[], annotations: object[] }} scene + * @param {[number, number]} point - Scene pixels. + * @returns {{ kind: "image" | "annotation", index: number }[]} + */ +export function objectsAt(scene, point) { + const hits = []; + const images = scene.images || []; + const annotations = scene.annotations || []; + for (let index = annotations.length - 1; index >= 0; index -= 1) { + const annotation = annotations[index]; + if (!annotation) continue; + const transform = linkedTransform(annotation, scene); + if (!annotationHit(transform ? mapLinkedAnnotation(annotation, transform) : annotation, point)) continue; + if (transform && clipsToCrop(annotation)) { + const image = images.find((candidate) => candidate && candidate.id === annotation.sourceImageId); + const visible = visibleCropPolygon(image, transform); + if (visible && !pointInPolygon(point, visible)) continue; + } + hits.push({ kind: "annotation", index }); + } + for (let index = images.length - 1; index >= 0; index -= 1) { + const area = imageHitArea(images[index]); + if (area && pointInPolygon(point, area)) hits.push({ kind: "image", index }); + } + return hits; +} + /** * Renders a scene's images and annotations into an SVG element. Optionally draws a * non-interactive selection outline around one image, identified by its index in diff --git a/templates/js/scenes.js b/templates/js/scenes.js index 6b76b48..7c5b3af 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -1,4 +1,4 @@ -import { renderScene, displayedAnnotation, storeAnnotationGeometry, textBox } from "./sceneCanvas.js"; +import { renderScene, displayedAnnotation, objectsAt, storeAnnotationGeometry, textBox } from "./sceneCanvas.js"; import { DEFAULT_HIGHLIGHT_COLOR, DEFAULT_HIGHLIGHT_OPACITY, @@ -194,6 +194,9 @@ const state = { // Issue #440: the one selected object, `{ kind: "image" | "annotation", // index }` (its index in `active.images` or `active.annotations`), or null. selection: null, + // Issue #441: how many objects were under the click that made the + // selection, while that's more than one (for the "click again" hint). + overlapCount: 0, cropMode: false, // Issue #426: the annotation tool in use ("text" | "line" | "arrow" | // "polygon" | "pointer"), and the shape being drawn with it. @@ -223,11 +226,18 @@ function select(kind, index) { const isSame = state.selection && state.selection.kind === kind && state.selection.index === index; if (!isSame) state.cropMode = false; state.selection = { kind, index }; + state.overlapCount = 0; } function deselect() { state.selection = null; state.cropMode = false; + state.overlapCount = 0; +} + +/** Whether `object` ({ kind, index }) is the selected object. */ +function isSelected(object) { + return Boolean(state.selection) && object.kind === state.selection.kind && object.index === state.selection.index; } const clone = (value) => JSON.parse(JSON.stringify(value)); @@ -394,6 +404,8 @@ function markChanged() { if (!state.active) return; const wasSaved = !hasUnsavedChanges(); state.revision += 1; + // The overlap hint described the click before this edit. + state.overlapCount = 0; if (wasSaved) announce("Unsaved changes."); updateControls(); } @@ -536,6 +548,9 @@ function renderCanvas() { dom.canvas.hidden = isEmpty; dom.fitToggle.hidden = isEmpty; dom.canvasNote.hidden = true; + const showOverlapHint = Boolean(state.selection) && state.overlapCount > 1; + dom.selectionHint.hidden = !showOverlapHint; + dom.selectionHint.textContent = showOverlapHint ? overlapHint() : ""; if (isEmpty) return; const { svg, unsupported, unlinked, width, height } = renderScene(scene, { @@ -1326,10 +1341,17 @@ function selectedAnnotation() { } // Issue #440: selecting objects with the mouse and keyboard. A click selects -// the object under it (clicking it again keeps it selected); Escape or a -// click on empty canvas clears the selection; Tab and Shift+Tab step through -// the drawn objects from the top of the stack down. +// the object under it; Escape or a click on empty canvas clears the +// selection; Tab and Shift+Tab step through the drawn objects from the top of +// the stack down. +// Issue #441: where objects overlap, clicking again in the same spot (or +// Alt+clicking anywhere on the stack) selects the next object down, wrapping +// back to the top, so an image under a highlight can still be reached. const MAX_ANNOUNCED_TEXT = 40; +// How far apart, in screen pixels, two clicks can be and still count as the same spot. +const SAME_SPOT_DISTANCE = 4; +// Where the last selecting click was, in client coordinates, or null. +let lastSelectClick = null; /** * Every drawn object as { kind, index }, topmost first: annotations are @@ -1362,21 +1384,89 @@ function announceSelection() { } const objects = selectableObjects(); const position = objects.findIndex((object) => object.kind === kind && object.index === index); - announce(position === -1 ? `Selected ${name}.` : `Selected ${name}, ${position + 1} of ${plural(objects.length, "object")}.`); + const message = position === -1 ? `Selected ${name}.` : `Selected ${name}, ${position + 1} of ${plural(objects.length, "object")}.`; + announce(state.overlapCount > 1 ? `${message} ${overlapHint()}` : message); +} + +function overlapHint() { + return `${state.overlapCount} objects overlap here. Click again in the same spot, or Alt+click, to select the next one down.`; } -/** Selects an object the user picked, and says which one it is. */ -function selectObject(kind, index) { +/** + * Selects an object the user picked, and says which one it is. + * @param {"image"|"annotation"} kind + * @param {number} index + * @param {{ overlapCount?: number }} [options] - how many objects were under the click. + */ +function selectObject(kind, index, { overlapCount = 0 } = {}) { select(kind, index); - renderCanvas(); + state.overlapCount = overlapCount; + keepCanvasInPlace(renderCanvas); announceSelection(); } +/** + * Redraws while keeping the canvas where it is on screen. Selecting shows the + * image or annotation toolbar above the canvas (and clearing hides it), which + * would otherwise push the scene out from under the pointer, so the next + * click "in the same spot" would land on a different part of it. + */ +function keepCanvasInPlace(redraw) { + const before = dom.canvas.getBoundingClientRect().top; + redraw(); + const shift = dom.canvas.getBoundingClientRect().top - before; + // Instant, not the page's smooth scrolling, so a quick next click finds the canvas already back. + if (shift) window.scrollBy({ top: shift, behavior: "instant" }); +} + +/** The object a canvas node belongs to, as { kind, index }, or null. */ +function objectOfNode(node) { + const annotationNode = node.closest("[data-scene-annotation-index]"); + if (annotationNode) return { kind: "annotation", index: Number(annotationNode.dataset.sceneAnnotationIndex) }; + const imageNode = node.closest("[data-scene-image-index]"); + return imageNode ? { kind: "image", index: Number(imageNode.dataset.sceneImageIndex) } : null; +} + +/** + * Every object under a mouse event, topmost first. The one the browser + * reports as clicked always comes first, so the result agrees with what the + * user sees on top even where hit areas and geometry differ slightly. + */ +function objectsUnderEvent(event) { + const svg = dom.canvas.querySelector("svg"); + const clicked = objectOfNode(event.target); + const stack = svg && svg.contains(event.target) ? objectsAt(state.active, clientToScene(svg, event)) : []; + if (!clicked) return stack; + return [clicked, ...stack.filter((object) => object.kind !== clicked.kind || object.index !== clicked.index)]; +} + +/** A click on the canvas with no tool in use: selects, steps down a stack of objects, or clears. */ +function handleSelectClick(event) { + const sameSpot = Boolean(lastSelectClick) && + Math.hypot(event.clientX - lastSelectClick.x, event.clientY - lastSelectClick.y) <= SAME_SPOT_DISTANCE; + lastSelectClick = { x: event.clientX, y: event.clientY }; + let stack = objectsUnderEvent(event); + if (state.cropMode) { + // While cropping, annotations can't be selected; a click on one does nothing. + if (stack.length > 0 && stack[0].kind === "annotation") return; + stack = stack.filter((object) => object.kind === "image"); + } + if (stack.length === 0) { + clearSelection(); + return; + } + const current = stack.findIndex(isSelected); + let target = stack[0]; + if (current !== -1 && (sameSpot || event.altKey)) target = stack[(current + 1) % stack.length]; + else if (event.altKey && stack.length > 1) target = stack[1]; + selectObject(target.kind, target.index, { overlapCount: stack.length }); +} + /** Clears the selection the user dismissed. `quiet` skips the announcement. */ function clearSelection({ quiet = false } = {}) { if (!state.selection) return; deselect(); - renderCanvas(); + keepCanvasInPlace(renderCanvas); if (!quiet) announce("Nothing selected."); } @@ -1389,9 +1479,7 @@ function handleSelectionTab(event) { if (event.key !== "Tab" || event.altKey || event.ctrlKey || event.metaKey) return false; if (!state.active || state.tool || state.cropMode) return false; const objects = selectableObjects(); - const current = state.selection - ? objects.findIndex((object) => object.kind === state.selection.kind && object.index === state.selection.index) - : -1; + const current = objects.findIndex(isSelected); if (current === -1 && event.shiftKey) return false; const next = event.shiftKey ? current - 1 : current + 1; if (next < 0 || next >= objects.length) { @@ -2272,6 +2360,7 @@ export function initializeSceneEditor(root = document) { canvas: $("scene-canvas"), canvasEmpty: $("scene-canvas-empty"), canvasNote: $("scene-canvas-note"), + selectionHint: $("scene-selection-hint"), importButton: $("scene-import-image"), importButtonEmpty: $("scene-canvas-empty-import"), importInput: $("scene-import-input"), @@ -2380,15 +2469,7 @@ export function initializeSceneEditor(root = document) { } // A click on a handle belongs to the selected object, even without a drag. if (event.target.closest("[data-annotation-handle], [data-corner]")) return; - const annotationTarget = event.target.closest("[data-scene-annotation-index]"); - if (annotationTarget) { - if (state.cropMode) return; - selectObject("annotation", Number(annotationTarget.dataset.sceneAnnotationIndex)); - return; - } - const imageTarget = event.target.closest("[data-scene-image-index]"); - if (imageTarget) selectObject("image", Number(imageTarget.dataset.sceneImageIndex)); - else clearSelection(); + handleSelectClick(event); }); dom.canvas.addEventListener("dblclick", (event) => { if (state.tool === "polygon") { @@ -2396,13 +2477,10 @@ export function initializeSceneEditor(root = document) { return; } if (state.tool) return; - const target = event.target.closest("[data-scene-annotation-index]"); - if (!target) return; - const index = Number(target.dataset.sceneAnnotationIndex); - const annotation = state.active && state.active.annotations[index]; + // The two clicks have already picked the object (stepping down a + // stack of overlapping ones, #441), so only a selected label opens. + const annotation = selectedAnnotation(); if (!annotation || annotation.type !== "text") return; - select("annotation", index); - renderCanvas(); dom.annotationText.focus(); dom.annotationText.select(); }); diff --git a/templates/style.css b/templates/style.css index b10000f..8bdab45 100644 --- a/templates/style.css +++ b/templates/style.css @@ -2983,6 +2983,12 @@ button:disabled { color: #92400e; } +.scene-selection-hint { + margin: var(--space-2) 0 0; + font-size: 0.9rem; + color: var(--text-muted); +} + .scene-image-object { cursor: pointer; } diff --git a/templates/tests/sceneAnnotationEditor.test.js b/templates/tests/sceneAnnotationEditor.test.js index 7cbf133..8d9bcf1 100644 --- a/templates/tests/sceneAnnotationEditor.test.js +++ b/templates/tests/sceneAnnotationEditor.test.js @@ -91,8 +91,18 @@ function drag(target, from, to) { function key(name, options = {}) { $("scene-canvas").dispatchEvent(new KeyboardEvent("keydown", { key: name, bubbles: true, ...options })); } +/** A point on a drawn annotation, where a user would click it. */ +function pointOn(node) { + if (node.tagName === "text") return [Number(node.getAttribute("x")) + 1, Number(node.getAttribute("y")) + 1]; + const points = (node.getAttribute("points") || "").trim().split(/\s+/).map((pair) => pair.split(",").map(Number)); + if (node.tagName === "polygon") { + return [0, 1].map((axis) => points.reduce((sum, point) => sum + point[axis], 0) / points.length); + } + return [(points[0][0] + points[1][0]) / 2, (points[0][1] + points[1][1]) / 2]; +} function selectAnnotation(index) { - clickAt(annotationNode(index), [0, 0]); + const node = annotationNode(index); + clickAt(node, pointOn(node)); } function typeText(text) { $("scene-annotation-text").focus(); @@ -233,7 +243,10 @@ describe("Text labels (#427-#430)", () => { it("opens a label's text for editing on double-click", async () => { await openBlankScene({ annotations: [{ id: "a", type: "text", text: "Ilium", x: 10, y: 10 }] }); - mouse("dblclick", annotationNode(0), [0, 0]); + // A browser sends both clicks before the dblclick. + clickAt(annotationNode(0), [12, 12]); + clickAt(annotationNode(0), [12, 12]); + mouse("dblclick", annotationNode(0), [12, 12]); expect(document.activeElement).toBe($("scene-annotation-text")); expect($("scene-annotation-toolbar").hidden).toBe(false); }); @@ -842,3 +855,104 @@ describe("Selecting objects (#440)", () => { expect(outlines()).toHaveLength(0); }); }); + +describe("Selecting overlapping objects (#441)", () => { + // A label on a highlight on an image: at (320, 315) all three overlap; at + // (390, 390) the highlight and the image; at (285, 285) only the highlight. + const content = { + images: [{ id: "img", src: "/images/a.png", x: 300, y: 300, width: 100, height: 100 }], + annotations: [ + { id: "poly", type: "polygon", points: [[280, 280], [420, 280], [420, 420], [280, 420]] }, + { id: "label", type: "text", text: "Ilium", x: 310, y: 310 }, + ], + }; + const status = () => $("scene-editor-status").textContent; + const hint = () => $("scene-selection-hint"); + const selected = () => { + if (!$("scene-image-toolbar").hidden) return "image"; + if ($("scene-annotation-toolbar").hidden) return null; + return $("scene-annotation-label").textContent.includes("label") ? "label" : "highlight"; + }; + function altClickAt(target, [x, y]) { + target.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, clientX: x, clientY: y, altKey: true })); + document.dispatchEvent(new MouseEvent("mouseup", { clientX: x, clientY: y, altKey: true })); + target.dispatchEvent(new MouseEvent("click", { bubbles: true, clientX: x, clientY: y, altKey: true })); + } + + it("selects the topmost object, then each one below on clicks in the same spot, wrapping to the top", async () => { + await openBlankScene(content); + const order = []; + for (let i = 0; i < 4; i += 1) { + // The browser reports the topmost node as clicked every time. + clickAt(annotationNode(1), [320, 315]); + order.push(selected()); + } + expect(order).toEqual(["label", "highlight", "image", "label"]); + }); + + it("shows and announces a hint while objects overlap, and hides it for a single object", async () => { + await openBlankScene(content); + + clickAt(annotationNode(1), [320, 315]); + expect(hint().hidden).toBe(false); + expect(hint().textContent).toBe( + "3 objects overlap here. Click again in the same spot, or Alt+click, to select the next one down." + ); + await waitFor(() => expect(status()).toBe( + "Selected label \"Ilium\", 1 of 3 objects. 3 objects overlap here. Click again in the same spot, or Alt+click, to select the next one down." + )); + + clickAt(annotationNode(0), [285, 285]); + expect(selected()).toBe("highlight"); + expect(hint().hidden).toBe(true); + + clickAt(annotationNode(1), [320, 315]); + key("Escape"); + expect(hint().hidden).toBe(true); + }); + + it("selects the topmost object again when the click is in a different spot", async () => { + await openBlankScene(content); + clickAt(annotationNode(0), [390, 390]); + clickAt(annotationNode(0), [390, 390]); + expect(selected()).toBe("image"); + + clickAt(annotationNode(0), [395, 380]); + expect(selected()).toBe("highlight"); + }); + + it("steps down the stack with Alt+click from anywhere on it", async () => { + await openBlankScene(content); + + altClickAt(annotationNode(0), [390, 390]); + expect(selected()).toBe("image"); + // The image is under the label too, so the next Alt+click wraps to the top there. + altClickAt(annotationNode(1), [320, 315]); + expect(selected()).toBe("label"); + altClickAt(annotationNode(1), [322, 316]); + expect(selected()).toBe("highlight"); + }); + + it("keeps stepping down on quick clicks instead of a double-click jumping back to the label", async () => { + await openBlankScene(content); + clickAt(annotationNode(1), [320, 315]); + clickAt(annotationNode(1), [320, 315]); + mouse("dblclick", annotationNode(1), [320, 315]); + expect(selected()).toBe("highlight"); + expect(document.activeElement).not.toBe($("scene-annotation-text")); + + clickAt(annotationNode(1), [320, 315]); + expect(selected()).toBe("image"); + }); + + it("clears the hint once the selected object is edited", async () => { + await openBlankScene(content); + clickAt(annotationNode(1), [320, 315]); + expect(hint().hidden).toBe(false); + + key("ArrowRight"); + + expect(hint().hidden).toBe(true); + expect(selected()).toBe("label"); + }); +}); diff --git a/templates/tests/sceneCanvas.test.js b/templates/tests/sceneCanvas.test.js index 537c093..8ad6196 100644 --- a/templates/tests/sceneCanvas.test.js +++ b/templates/tests/sceneCanvas.test.js @@ -1,4 +1,4 @@ -import { renderScene } from "../js/sceneCanvas.js"; +import { objectsAt, renderScene } from "../js/sceneCanvas.js"; function viewBox(svg) { const [x, y, width, height] = svg.getAttribute("viewBox").split(" ").map(Number); @@ -322,3 +322,90 @@ describe("renderScene linked annotations", () => { expect(annotations).toEqual(before); }); }); + +describe("objectsAt (#441)", () => { + const at = (scene, point) => objectsAt({ images: [], annotations: [], ...scene }, point); + const label = (overrides = {}) => ({ type: "text", text: "Ilium", x: 0, y: 0, fontSize: 20, ...overrides }); + + it("lists every object under a point, topmost first: later annotations, then later images", () => { + const scene = { + images: [image(), image({ x: 50 })], + annotations: [ + label({ x: 60, y: 10 }), + { type: "polygon", points: [[40, 0], [160, 0], [160, 90], [40, 90]] }, + ], + }; + expect(at(scene, [70, 20])).toEqual([ + { kind: "annotation", index: 1 }, + { kind: "annotation", index: 0 }, + { kind: "image", index: 1 }, + { kind: "image", index: 0 }, + ]); + expect(at(scene, [10, 20])).toEqual([{ kind: "image", index: 0 }]); + expect(at(scene, [900, 500])).toEqual([]); + }); + + it("tests a rotated image against its rotated outline", () => { + // 200x100 at the origin turned 90° about (100, 50) covers x 50-150, y -50-150. + const scene = { images: [image({ rotation: 90 })] }; + expect(at(scene, [100, 140])).toEqual([{ kind: "image", index: 0 }]); + expect(at(scene, [10, 50])).toEqual([]); + }); + + it("only counts the visible area of a cropped image, as it is flipped", () => { + const cropped = image({ cropX: 0, cropY: 0, cropWidth: 50, cropHeight: 100 }); + expect(at({ images: [cropped] }, [25, 50])).toEqual([{ kind: "image", index: 0 }]); + expect(at({ images: [cropped] }, [150, 50])).toEqual([]); + // Flipped horizontally, the visible strip is drawn at the right-hand end. + const flipped = { ...cropped, flipX: true }; + expect(at({ images: [flipped] }, [175, 50])).toEqual([{ kind: "image", index: 0 }]); + expect(at({ images: [flipped] }, [25, 50])).toEqual([]); + }); + + it("hits a line within its click tolerance and a highlight on its fill", () => { + const scene = { + annotations: [ + { type: "line", x1: 0, y1: 300, x2: 200, y2: 300 }, + { type: "polygon", points: [[400, 400], [500, 400], [450, 500]] }, + ], + }; + expect(at(scene, [100, 305])).toEqual([{ kind: "annotation", index: 0 }]); + expect(at(scene, [100, 320])).toEqual([]); + expect(at(scene, [450, 450])).toEqual([{ kind: "annotation", index: 1 }]); + expect(at(scene, [400, 480])).toEqual([]); + }); + + it("leaves out objects the canvas can't draw", () => { + const scene = { + images: [image({ src: "javascript:alert(1)" })], + annotations: [{ type: "circle", x: 0, y: 0, r: 50 }, { type: "polygon", points: [[0, 0], [100, 100]] }], + }; + expect(at(scene, [10, 10])).toEqual([]); + }); + + it("finds a linked annotation where it is drawn, following its moved image", () => { + const frame = { x: 0, y: 0, width: 200, height: 100 }; + const scene = { + images: [image({ id: "img", x: 300 })], + annotations: [label({ sourceImageId: "img", sourceImageFrame: frame })], + }; + expect(at(scene, [305, 5])).toEqual([{ kind: "annotation", index: 0 }, { kind: "image", index: 0 }]); + expect(at(scene, [5, 5])).toEqual([]); + }); + + it("ignores a colored region outside its image's visible crop", () => { + const frame = { x: 0, y: 0, width: 200, height: 100 }; + const scene = { + images: [image({ id: "img", cropX: 0, cropY: 0, cropWidth: 100, cropHeight: 100 })], + annotations: [{ + type: "polygon", + points: [[0, 0], [200, 0], [200, 100], [0, 100]], + source: { type: "legacy-colored-region" }, + sourceImageId: "img", + sourceImageFrame: frame, + }], + }; + expect(at(scene, [50, 50])).toEqual([{ kind: "annotation", index: 0 }, { kind: "image", index: 0 }]); + expect(at(scene, [150, 50])).toEqual([]); + }); +}); From e38ef3391e63cfe49734af2f4f7ce6665131454f Mon Sep 17 00:00:00 2001 From: Remex_9 Date: Mon, 2 Nov 2026 15:07:34 -0600 Subject: [PATCH 3/5] Move and resize selected objects safely (#442) - A press anywhere on the selected object drags it, even where another object is drawn over it (an image reached under a highlight with #441's click-again); a press on any unselected object never moves anything. - The selected image now has the same keyboard controls as annotations: arrow keys move it by 1 scene pixel (10 with Shift), and Delete or Backspace removes it (announced). Both are off while a crop is edited. - Tests cover a crowded scene: dragging, nudging, and resizing a covered image, then saving and reopening; drags on unselected objects moving nothing; a selected highlight moving under the label drawn on it; and deleting images from the keyboard. Part of #439. Co-Authored-By: Claude Opus 5.5 --- templates/js/scenes.js | 38 +++++-- templates/tests/sceneAnnotationEditor.test.js | 99 +++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/templates/js/scenes.js b/templates/js/scenes.js index 7c5b3af..be299eb 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -1022,6 +1022,7 @@ function handleImageRemove() { deselect(); markChanged(); renderWorkspace(); + announce("Image removed. Save to keep the change."); } /** @@ -1065,6 +1066,29 @@ function flipSelectedImage(axis) { applySelectedImageUpdate({ [field]: !image[field] }); } +/** + * Issue #442: keyboard use of the selected image on the canvas, matching + * annotations - the arrow keys move it (1 scene pixel, or 10 with Shift) + * and Delete or Backspace removes it. Not while its crop is being edited. + * @returns {boolean} Whether the key was handled. + */ +function handleImageKey(event) { + if (state.tool || state.cropMode || event.altKey || event.ctrlKey || event.metaKey) return false; + const image = state.active && selectedImageIndex() !== null ? state.active.images[selectedImageIndex()] : null; + if (!image) return false; + if (event.key === "Delete" || event.key === "Backspace") { + handleImageRemove(); + dom.canvas.focus(); + return true; + } + const step = event.shiftKey ? NUDGE_STEP_LARGE : NUDGE_STEP; + const deltas = { ArrowLeft: [-step, 0], ArrowRight: [step, 0], ArrowUp: [0, -step], ArrowDown: [0, step] }; + if (!deltas[event.key]) return false; + const [dx, dy] = deltas[event.key]; + applySelectedImageUpdate({ x: image.x + dx, y: image.y + dy }); + return true; +} + /** * Swaps the selected image with its neighbor toward the front (direction=1) * or back (direction=-1) of the paint order (Issue #419). No-op at either end @@ -2485,7 +2509,7 @@ export function initializeSceneEditor(root = document) { dom.annotationText.select(); }); dom.canvas.addEventListener("keydown", (event) => { - if (handleAnnotationKey(event) || handleSelectionTab(event)) { + if (handleAnnotationKey(event) || handleImageKey(event) || handleSelectionTab(event)) { event.preventDefault(); return; } @@ -2504,16 +2528,19 @@ export function initializeSceneEditor(root = document) { return; } if (state.tool) return; + // Issue #442: a press anywhere on the selected object drags it, even + // where another object is drawn over it (it was reached with #441's + // click-again), and a press on anything else never moves anything. + const isOnSelected = () => objectsUnderEvent(event).some(isSelected); if (selectedAnnotation()) { const annotationSvg = dom.canvas.querySelector("svg"); const annotationHandle = event.target.closest("[data-annotation-handle]"); - const annotationNode = event.target.closest("[data-scene-annotation-index]"); if (!annotationSvg) return; if (annotationHandle) { event.preventDefault(); startAnnotationDrag(event, annotationSvg, annotationHandle.dataset.annotationHandle, Number(annotationHandle.dataset.pointIndex)); - } else if (annotationNode && Number(annotationNode.dataset.sceneAnnotationIndex) === selectedAnnotationIndex()) { + } else if (isOnSelected()) { event.preventDefault(); startAnnotationDrag(event, annotationSvg, "move"); } @@ -2534,10 +2561,7 @@ export function initializeSceneEditor(root = document) { return; } - const imageNode = event.target.closest("[data-scene-image-index]"); - const isSelectedImageNode = - imageNode && Number(imageNode.dataset.sceneImageIndex) === selectedImageIndex(); - if (!handle && !isSelectedImageNode) return; + if (!handle && !isOnSelected()) return; event.preventDefault(); startImageDrag(event, svg, handle ? handle.dataset.corner : null); diff --git a/templates/tests/sceneAnnotationEditor.test.js b/templates/tests/sceneAnnotationEditor.test.js index 8d9bcf1..c96deb1 100644 --- a/templates/tests/sceneAnnotationEditor.test.js +++ b/templates/tests/sceneAnnotationEditor.test.js @@ -956,3 +956,102 @@ describe("Selecting overlapping objects (#441)", () => { expect(selected()).toBe("label"); }); }); + +describe("Moving and resizing selected objects (#442)", () => { + // A crowded scene: image "a" under a highlight and a label, image "b" to + // its right, and a line below. + const content = () => ({ + images: [ + { id: "a", src: "/images/a.png", x: 300, y: 300, width: 100, height: 100 }, + { id: "b", src: "/images/b.png", x: 600, y: 300, width: 100, height: 100 }, + ], + annotations: [ + { id: "poly", type: "polygon", points: [[280, 280], [420, 280], [420, 420], [280, 420]] }, + { id: "label", type: "text", text: "Ilium", x: 310, y: 310 }, + { id: "line", type: "line", x1: 10, y1: 500, x2: 200, y2: 500 }, + ], + }); + const images = () => scenesModule.getActiveScene().images; + const storedImages = () => backend.scenes.get("scene-1").images; + const imageNode = (index) => document.querySelector(`[data-scene-image-index="${index}"]`); + const status = () => $("scene-editor-status").textContent; + /** Selects image "a" the way a user reaches it under the highlight: clicking twice in one spot. */ + function selectCoveredImage() { + clickAt(annotationNode(0), [390, 390]); + clickAt(annotationNode(0), [390, 390]); + expect($("scene-image-toolbar").hidden).toBe(false); + } + + it("moves, nudges, and resizes an image covered by a highlight, and keeps it after reopening", async () => { + await openBlankScene(content()); + selectCoveredImage(); + + // The press lands on the highlight, but the selected image is under it, so the image moves. + drag(annotationNode(0), [390, 390], [440, 400]); + expect(images()[0]).toMatchObject({ x: 350, y: 310 }); + expect($("scene-image-toolbar").hidden).toBe(false); + + key("ArrowRight"); + key("ArrowDown", { shiftKey: true }); + expect(images()[0]).toMatchObject({ x: 351, y: 320 }); + + // The se handle is at (451, 420); dragging it 50 right scales 100 -> 150 about the center. + drag(document.querySelector("[data-corner=\"se\"]"), [451, 420], [501, 420]); + expect(images()[0]).toMatchObject({ x: 326, y: 295, width: 150, height: 150 }); + + await saveAndReopen(); + expect(storedImages()[0]).toMatchObject({ x: 326, y: 295, width: 150, height: 150 }); + expect(storedImages()[1]).toEqual(content().images[1]); + expect(stored()).toEqual(content().annotations); + }); + + it("never moves an object that isn't selected", async () => { + await openBlankScene(content()); + selectCoveredImage(); + + // Pressing and dragging on the line or on image "b" moves nothing. + drag(annotationNode(2), [100, 500], [150, 550]); + drag(imageNode(1), [650, 350], [700, 400]); + + expect(images()).toEqual(content().images); + expect(annotations()).toEqual(content().annotations); + expect(scenesModule.hasUnsavedChanges()).toBe(false); + }); + + it("moves a selected highlight when the press lands on the label drawn over it", async () => { + await openBlankScene(content()); + clickAt(annotationNode(0), [390, 390]); + + drag(annotationNode(1), [320, 315], [330, 325]); + + expect(annotations()[0].points).toEqual([[290, 290], [430, 290], [430, 430], [290, 430]]); + expect(annotations()[1]).toEqual(content().annotations[1]); + expect(images()).toEqual(content().images); + }); + + it("removes the selected image with Delete or Backspace", async () => { + await openBlankScene(content()); + selectCoveredImage(); + + key("Delete"); + expect(images().map((image) => image.id)).toEqual(["b"]); + expect($("scene-image-toolbar").hidden).toBe(true); + await waitFor(() => expect(status()).toBe("Image removed. Save to keep the change.")); + + clickAt(imageNode(0), [650, 350]); + key("Backspace"); + expect(images()).toEqual([]); + expect(annotations()).toEqual(content().annotations); + }); + + it("leaves the arrow keys and Delete alone while an image's crop is being edited", async () => { + await openBlankScene(content()); + clickAt(imageNode(1), [650, 350]); + click("scene-image-crop-start"); + + key("ArrowLeft"); + key("Delete"); + + expect(images()).toEqual(content().images); + }); +}); From 197f6e83f0f9adce6d88704b06962f62d706b42d Mon Sep 17 00:00:00 2001 From: Remex_9 Date: Thu, 5 Nov 2026 11:11:01 -0600 Subject: [PATCH 4/5] Undo and redo scene edits (#443) - Every recorded change to the working copy (images, annotations, and the import log) becomes an undo step, up to 100. Image src strings, which can be megabytes of base64, are shared between steps rather than copied. - A run of small edits of one kind on one object - typing a label, picking a color, dragging the opacity slider, nudging with the arrow keys - undoes as one step; selecting another object or saving starts a new step. - Revisions now only count up, so undoing or redoing back to the saved version shows "Saved" again, and a new edit after an undo never looks saved. - Undo and Redo buttons next to Save, plus Ctrl/Cmd+Z and Ctrl/Cmd+Shift+Z or Ctrl+Y; text fields keep their own undo. Each step is announced. - The selected object stays selected if it still exists. History is cleared when another scene is opened or a version is reloaded, so undo never reaches into another scene. Part of #439. Co-Authored-By: Claude Opus 5.5 --- templates/boneset.html | 2 + templates/js/scenes.js | 181 ++++++++++++++++-- templates/tests/sceneAnnotationEditor.test.js | 160 ++++++++++++++++ 3 files changed, 328 insertions(+), 15 deletions(-) diff --git a/templates/boneset.html b/templates/boneset.html index 487589a..7ebbe0b 100644 --- a/templates/boneset.html +++ b/templates/boneset.html @@ -250,6 +250,8 @@

+ + diff --git a/templates/js/scenes.js b/templates/js/scenes.js index a859791..7390012 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -224,6 +224,12 @@ const state = { // "polygon" | "pointer"), and the shape being drawn with it. tool: null, draft: null, + // Issue #443: undo and redo for the open scene. `present` is a copy of + // the working copy's content as of the last recorded change; `past` and + // `future` hold the copies before and after it. `lastKey` names the kind + // of the last change, so a run of small edits of one kind (typing a + // label, nudging with the arrow keys) is undone as one step. + history: { past: [], present: null, future: [], lastKey: null }, boneOptions: null, importRequest: 0, }; @@ -246,7 +252,11 @@ function selectedAnnotationIndex() { */ function select(kind, index) { const isSame = state.selection && state.selection.kind === kind && state.selection.index === index; - if (!isSame) state.cropMode = false; + if (!isSame) { + state.cropMode = false; + // Edits of another object are never grouped with these. + state.history.lastKey = null; + } state.selection = { kind, index }; state.overlapCount = 0; } @@ -255,6 +265,7 @@ function deselect() { state.selection = null; state.cropMode = false; state.overlapCount = 0; + state.history.lastKey = null; } /** Whether `object` ({ kind, index }) is the selected object. */ @@ -274,6 +285,8 @@ function setActiveScene(scene) { state.saved = scene ? clone(scene) : null; state.revision = 0; state.savedRevision = 0; + // Undo never reaches into another scene, or a version that was reloaded. + state.history = { past: [], present: scene ? contentSnapshot(scene) : null, future: [], lastKey: null }; state.activeToken += 1; state.saveFailed = false; state.conflictUpdatedAt = null; @@ -411,6 +424,8 @@ function updateControls() { dom.duplicateButton.disabled = locked || !state.active; updatePublishControls(locked); dom.saveButton.disabled = locked || !hasUnsavedChanges(); + dom.undoButton.disabled = !canUndo(); + dom.redoButton.disabled = !canRedo(); if (dom.importToggle) { dom.importToggle.disabled = state.busy || !state.active; dom.importSubmit.disabled = state.busy || !state.boneOptions; @@ -451,17 +466,145 @@ function updatePublishControls(locked) { } -/** Records a change to the working copy (Issue #451). Callers re-render. */ -function markChanged() { +const MAX_UNDO_STEPS = 100; +// Revisions only ever count up, even after an undo, so a revision number +// always names one version of the content: undoing back to the saved version +// shows it as saved again, and a new edit after an undo never looks saved. +let lastRevision = 0; + +/** A copy of an image for the undo history, sharing its `src` (possibly megabytes of base64, never changed in place). */ +function copyImage(image) { + const { src, ...rest } = image; + const copy = clone(rest); + if (src !== undefined) copy.src = src; + return copy; +} + +/** A copy of the content a save stores, with its revision, for the undo history (Issue #443). */ +function contentSnapshot(scene) { + const snapshot = { + images: (scene.images || []).map(copyImage), + annotations: clone(scene.annotations || []), + revision: state.revision, + }; + if (Array.isArray(scene.importLog)) snapshot.importLog = clone(scene.importLog); + return snapshot; +} + +/** + * Records a change to the working copy (Issue #451), as one undo step + * (Issue #443). Callers re-render. + * @param {{ group?: string }} [options] - group names a kind of small, + * repeated edit; consecutive changes with the same group undo as one step. + */ +function markChanged({ group = null } = {}) { if (!state.active) return; const wasSaved = !hasUnsavedChanges(); - state.revision += 1; + lastRevision += 1; + state.revision = lastRevision; + const history = state.history; + const continuesGroup = group !== null && group === history.lastKey; + if (!continuesGroup && history.present) { + history.past.push(history.present); + if (history.past.length > MAX_UNDO_STEPS) history.past.shift(); + } + history.present = contentSnapshot(state.active); + history.future = []; + history.lastKey = group; // The overlap hint described the click before this edit. state.overlapCount = 0; if (wasSaved) announce("Unsaved changes."); updateControls(); } +function canUndo() { + return Boolean(state.active) && !state.busy && !activeDrag && state.history.past.length > 0; +} + +function canRedo() { + return Boolean(state.active) && !state.busy && !activeDrag && state.history.future.length > 0; +} + +/** Issue #443: puts the working copy back the way it was before the last recorded change. */ +function undo() { + if (!canUndo()) return; + const history = state.history; + history.future.push(history.present); + history.present = history.past.pop(); + showHistoryStep("Undone"); +} + +/** Issue #443: reapplies the last undone change. */ +function redo() { + if (!canRedo()) return; + const history = state.history; + history.past.push(history.present); + history.present = history.future.pop(); + showHistoryStep("Redone"); +} + +/** + * Makes the working copy match `history.present`. A drawing tool in use is + * put away first. The selected object stays selected if it still exists. + * @param {string} verb - "Undone" or "Redone", for the announcement. + */ +function showHistoryStep(verb) { + const scene = state.active; + const snapshot = state.history.present; + const selected = state.selection + ? (state.selection.kind === "image" ? scene.images : scene.annotations)[state.selection.index] + : null; + const selectedKind = state.selection && state.selection.kind; + endLineDraw(); + state.tool = null; + state.draft = null; + + scene.images = snapshot.images.map(copyImage); + scene.annotations = clone(snapshot.annotations); + if (snapshot.importLog) scene.importLog = clone(snapshot.importLog); + else delete scene.importLog; + state.revision = snapshot.revision; + + const objects = selectedKind === "image" ? scene.images : scene.annotations; + const index = selected && selected.id !== undefined ? objects.findIndex((object) => object && object.id === selected.id) : -1; + if (index === -1) deselect(); + else { + state.selection = { kind: selectedKind, index }; + state.cropMode = false; + state.overlapCount = 0; + } + state.history.lastKey = null; + updateControls(); + renderWorkspace(); + const remaining = verb === "Undone" ? state.history.past.length : state.history.future.length; + announce(`${verb}. ${remaining === 0 + ? `Nothing more to ${verb === "Undone" ? "undo" : "redo"}.` + : `${plural(remaining, "more change")} can be ${verb === "Undone" ? "undone" : "redone"}.`}`); +} + +/** Whether a key press belongs to a text field, which has its own undo. */ +function isTextEntry(target) { + if (!target || typeof target.closest !== "function") return false; + if (target.closest("textarea, [contenteditable=\"true\"]")) return true; + const input = target.closest("input"); + return Boolean(input) && ["text", "search", "number", "email", "url", "tel", "password"].includes(input.type); +} + +/** Ctrl/Cmd+Z undoes; Ctrl/Cmd+Shift+Z or Ctrl+Y redoes, while the editor is open. */ +function handleUndoKey(event) { + // isConnected: an editor whose page was replaced (as tests do) stays out of the way. + if (!dom || !dom.editorView.isConnected || dom.editorView.hidden || !state.active) return; + if (!(event.ctrlKey || event.metaKey) || event.altKey) return; + const key = event.key.toLowerCase(); + const isUndo = key === "z" && !event.shiftKey; + const isRedo = (key === "z" && event.shiftKey) || (key === "y" && !event.shiftKey); + if ((!isUndo && !isRedo) || isTextEntry(event.target)) return; + if (isUndo ? !canUndo() : !canRedo()) return; + event.preventDefault(); + if (isUndo) undo(); + else redo(); +} + /** * Applies a change to the open scene's working copy and marks it unsaved, for * editing tools outside this module (e.g. future annotation tools). @@ -1023,6 +1166,7 @@ async function handleSave({ overwrite = false } = {}) { if (token === state.activeToken) { state.saved = clone(updated); state.savedRevision = sentRevision; + state.history.lastKey = null; state.conflictUpdatedAt = null; scene.name = updated.name; scene.updatedAt = updated.updatedAt; @@ -1110,6 +1254,7 @@ async function saveAsNewScene() { else delete scene.publishedAt; state.saved = clone(created); state.savedRevision = revision; + state.history.lastKey = null; state.missing = false; state.saveFailed = false; renderWorkspace(); @@ -1184,12 +1329,12 @@ function handleImageRemove() { * toolbar buttons; a drag applies its own changes live frame-by-frame. * @param {object} fields - The new values to set, e.g. `{ rotation: 90 }`. */ -function applySelectedImageUpdate(fields) { +function applySelectedImageUpdate(fields, { group = null } = {}) { if (!state.active || selectedImageIndex() === null) return; const image = state.active.images[selectedImageIndex()]; if (!image) return; Object.assign(image, fields); - markChanged(); + markChanged({ group }); renderWorkspace(); } @@ -1238,7 +1383,7 @@ function handleImageKey(event) { const deltas = { ArrowLeft: [-step, 0], ArrowRight: [step, 0], ArrowUp: [0, -step], ArrowDown: [0, step] }; if (!deltas[event.key]) return false; const [dx, dy] = deltas[event.key]; - applySelectedImageUpdate({ x: image.x + dx, y: image.y + dy }); + applySelectedImageUpdate({ x: image.x + dx, y: image.y + dy }, { group: "nudge" }); return true; } @@ -2054,23 +2199,24 @@ function nudgeSelectedAnnotation(dx, dy) { const pointers = pointerLinesOf(annotation, scene); storeAnnotationGeometry(annotation, translateAnnotation(start, dx, dy), scene); movePointerLines(pointers, startBox, dx, dy, scene); - markChanged(); + markChanged({ group: "nudge" }); renderWorkspace(); } /** * Applies a toolbar change to the selected annotation. * @param {(annotation: object, scene: object) => void} edit - * @param {{ reanchor?: boolean }} [options] - reanchor puts a label's - * pointer lines back on its edge after its size or text changed. + * @param {{ reanchor?: boolean, group?: string }} [options] - reanchor puts a + * label's pointer lines back on its edge after its size or text changed; + * group is passed to markChanged, so a run of these edits undoes as one. */ -function editSelectedAnnotation(edit, { reanchor = false } = {}) { +function editSelectedAnnotation(edit, { reanchor = false, group = null } = {}) { const annotation = selectedAnnotation(); if (!annotation) return; const scene = state.active; edit(annotation, scene); if (reanchor) reanchorPointerLines(annotation, scene); - markChanged(); + markChanged({ group }); renderWorkspace(); } @@ -2087,7 +2233,7 @@ function handleAnnotationTextInput() { dom.annotationText.removeAttribute("aria-invalid"); editSelectedAnnotation((annotation) => { annotation.text = text; - }, { reanchor: true }); + }, { reanchor: true, group: "text" }); } /** Leaving the text box empty keeps the label's last text. */ @@ -2131,7 +2277,7 @@ function handleAnnotationColorInput() { } else { annotation.stroke = color; } - }); + }, { group: "color" }); } /** @@ -2144,7 +2290,7 @@ function handleAnnotationOpacityInput() { editSelectedAnnotation((annotation) => { annotation.fillOpacity = clampHighlightOpacity(opacity); delete annotation.opacity; - }); + }, { group: "opacity" }); } /** @@ -2574,6 +2720,8 @@ export function initializeSceneEditor(root = document) { sceneSources: $("scene-sources"), saveStatus: $("scene-save-status"), saveButton: $("scene-save"), + undoButton: $("scene-undo"), + redoButton: $("scene-redo"), duplicateButton: $("scene-duplicate"), renameButton: $("scene-rename"), deleteButton: $("scene-delete"), @@ -2672,6 +2820,9 @@ export function initializeSceneEditor(root = document) { }); dom.deleteButton.addEventListener("click", handleDelete); if (dom.saveButton) dom.saveButton.addEventListener("click", () => handleSave()); + dom.undoButton.addEventListener("click", undo); + dom.redoButton.addEventListener("click", redo); + document.addEventListener("keydown", handleUndoKey); if (dom.duplicateButton) dom.duplicateButton.addEventListener("click", handleDuplicate); if (dom.publishButton) dom.publishButton.addEventListener("click", handlePublishToggle); dom.fitToggle.addEventListener("click", () => setFitToWidth(!state.fitToWidth)); diff --git a/templates/tests/sceneAnnotationEditor.test.js b/templates/tests/sceneAnnotationEditor.test.js index 6cab063..d2cd162 100644 --- a/templates/tests/sceneAnnotationEditor.test.js +++ b/templates/tests/sceneAnnotationEditor.test.js @@ -1280,3 +1280,163 @@ describe("Mouse editing with pointer events (#446)", () => { expect(document.querySelector(".scene-draft")).toBeNull(); }); }); + +describe("Undo and redo (#443)", () => { + const content = () => ({ + images: [{ id: "img", src: "/images/a.png", x: 300, y: 300, width: 100, height: 100 }], + annotations: [ + { id: "label", type: "text", text: "Ilium", x: 20, y: 20 }, + { id: "poly", type: "polygon", points: [[600, 100], [700, 100], [650, 200]] }, + ], + }); + const clone = (value) => JSON.parse(JSON.stringify(value)); + const images = () => scenesModule.getActiveScene().images; + const imageNode = () => document.querySelector("[data-scene-image-index=\"0\"]"); + const saveStatus = () => $("scene-save-status").textContent; + const status = () => $("scene-editor-status").textContent; + function shortcut(name, options = {}, target = $("scene-canvas")) { + const event = new KeyboardEvent("keydown", { key: name, bubbles: true, cancelable: true, ...options }); + target.dispatchEvent(event); + return event.defaultPrevented; + } + + it("undoes and redoes a sequence of image and annotation edits, one step at a time", async () => { + await openBlankScene(content()); + expect($("scene-undo").disabled).toBe(true); + expect($("scene-redo").disabled).toBe(true); + + clickAt(imageNode(), [350, 350]); + drag(imageNode(), [350, 350], [400, 380]); + selectAnnotation(1); + click("scene-annotation-delete"); + await addLabel([100, 400], "Pubis"); + const edited = clone(scenesModule.getActiveScene()); + expect(edited.annotations.map((a) => a.text || a.id)).toEqual(["Ilium", "Pubis"]); + + // Typing the new label's text, then adding it, are two steps. + click("scene-undo"); + expect(annotations().map((a) => a.text || a.id)).toEqual(["Ilium", "New label"]); + click("scene-undo"); + expect(annotations().map((a) => a.text || a.id)).toEqual(["Ilium"]); + click("scene-undo"); + expect(annotations()).toEqual(content().annotations); + expect(images()[0]).toMatchObject({ x: 350, y: 330 }); + click("scene-undo"); + expect(images()).toEqual(content().images); + expect($("scene-undo").disabled).toBe(true); + expect(saveStatus()).toBe("Saved"); + + for (let step = 0; step < 4; step += 1) click("scene-redo"); + expect(clone(scenesModule.getActiveScene()).images).toEqual(edited.images); + expect(clone(scenesModule.getActiveScene()).annotations).toEqual(edited.annotations); + expect($("scene-redo").disabled).toBe(true); + await waitFor(() => expect(status()).toBe("Redone. Nothing more to redo.")); + }); + + it("undoes a run of typing or arrow-key nudges as one step, but not edits of another object", async () => { + await openBlankScene(content()); + selectAnnotation(0); + typeText("Il"); + typeText("Ili"); + typeText("Ilio"); + key("ArrowRight"); + key("ArrowRight"); + key("ArrowDown"); + clickAt(imageNode(), [350, 350]); + key("ArrowLeft"); + key("ArrowLeft"); + + click("scene-undo"); + expect(images()[0].x).toBe(300); + expect(annotations()[0]).toMatchObject({ text: "Ilio", x: 22, y: 21 }); + click("scene-undo"); + expect(annotations()[0]).toMatchObject({ text: "Ilio", x: 20, y: 20 }); + click("scene-undo"); + expect(annotations()[0]).toEqual(content().annotations[0]); + expect($("scene-undo").disabled).toBe(true); + }); + + it("shows the scene as saved again when undo or redo returns to the saved version", async () => { + await openBlankScene(content()); + selectAnnotation(0); + key("ArrowRight", { shiftKey: true }); + await saveAndWait(); + + click("scene-undo"); + expect(saveStatus()).toBe("Unsaved changes"); + expect(scenesModule.hasUnsavedChanges()).toBe(true); + click("scene-redo"); + expect(saveStatus()).toBe("Saved"); + expect(scenesModule.hasUnsavedChanges()).toBe(false); + + // A new edit after an undo replaces what could be redone, and is never mistaken for the saved version. + click("scene-undo"); + key("ArrowDown"); + expect($("scene-redo").disabled).toBe(true); + expect(scenesModule.hasUnsavedChanges()).toBe(true); + }); + + it("keeps the selected object selected when it still exists, and saves the undone state", async () => { + await openBlankScene(content()); + selectAnnotation(0); + key("ArrowRight"); + click("scene-annotation-delete"); + expect($("scene-annotation-toolbar").hidden).toBe(true); + + click("scene-undo"); + expect(annotations()).toHaveLength(2); + expect($("scene-annotation-toolbar").hidden).toBe(true); + + selectAnnotation(0); + click("scene-undo"); + expect(annotations()[0]).toEqual(content().annotations[0]); + expect($("scene-annotation-toolbar").hidden).toBe(false); + expect($("scene-annotation-text").value).toBe("Ilium"); + + click("scene-redo"); + await saveAndReopen(); + expect(stored()[0]).toMatchObject({ x: 21, y: 20 }); + // A reopened scene starts with nothing to undo. + expect($("scene-undo").disabled).toBe(true); + }); + + it("undoes with Ctrl/Cmd+Z and redoes with Ctrl/Cmd+Shift+Z or Ctrl+Y, leaving text fields their own undo", async () => { + await openBlankScene(content()); + clickAt(imageNode(), [350, 350]); + key("ArrowRight", { shiftKey: true }); + key("ArrowDown", { shiftKey: true }); + clickAt(canvasSvg(), [900, 550]); + selectAnnotation(1); + click("scene-annotation-delete"); + + expect(shortcut("z", { ctrlKey: true })).toBe(true); + expect(annotations()).toHaveLength(2); + expect(shortcut("z", { metaKey: true })).toBe(true); + expect(images()[0]).toMatchObject({ x: 300, y: 300 }); + expect(shortcut("Z", { metaKey: true, shiftKey: true })).toBe(true); + expect(images()[0]).toMatchObject({ x: 310, y: 310 }); + expect(shortcut("y", { ctrlKey: true }, document.body)).toBe(true); + expect(annotations()).toHaveLength(1); + + selectAnnotation(0); + typeText("Ischium"); + expect(shortcut("z", { ctrlKey: true }, $("scene-annotation-text"))).toBe(false); + expect(annotations()[0].text).toBe("Ischium"); + }); + + it("never undoes into another scene", async () => { + backend.seed({ id: "scene-2", name: "Another", ...content() }); + await openBlankScene(content()); + selectAnnotation(0); + key("ArrowRight"); + + jest.spyOn(window, "confirm").mockReturnValue(true); + [...document.querySelectorAll(".scene-list-item")].find((b) => b.textContent.includes("Another")).click(); + await waitFor(() => expect($("scene-title").textContent).toBe("Another")); + + expect($("scene-undo").disabled).toBe(true); + expect(shortcut("z", { ctrlKey: true })).toBe(false); + expect(annotations()).toEqual(content().annotations); + expect(scenesModule.hasUnsavedChanges()).toBe(false); + }); +}); From 77e6a0275997b97638d4921256f746bcb04133ee Mon Sep 17 00:00:00 2001 From: Remex_9 Date: Thu, 5 Nov 2026 11:12:23 -0600 Subject: [PATCH 5/5] Warn before leaving the Scene Editor with unsaved changes (#444) - The Back button, pressing Scene Editor again, and a scene's "view in the bone viewer" links now ask before leaving when the open scene has unsaved changes. Choosing to stay keeps the editor open (announced); choosing to leave keeps the working copy, which is still there on coming back. - With everything saved, the editor is left without asking, as before. - Closing or reloading the page, opening another scene, New, Delete, and Duplicate already warned (#507); this closes the remaining gap. Part of #439. Co-Authored-By: Claude Opus 5.5 --- templates/js/scenes.js | 28 ++++++++- templates/tests/sceneAnnotationEditor.test.js | 57 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/templates/js/scenes.js b/templates/js/scenes.js index 7390012..9bfd97f 100644 --- a/templates/js/scenes.js +++ b/templates/js/scenes.js @@ -828,7 +828,7 @@ function renderSceneSources(scene) { } function showInViewer(boneId) { - leaveEditor(); + if (!requestLeaveEditor()) return; document.dispatchEvent(new CustomEvent(SHOW_IN_VIEWER_EVENT, { detail: { boneId } })); } @@ -2683,6 +2683,28 @@ function enterEditor() { loadLibrary(); } +/** + * Issue #444: leaves the editor, asking first when the open scene has unsaved + * changes. Leaving doesn't discard them - the working copy stays open and is + * still here on coming back - but nothing outside the editor shows them, and + * closing or reloading the page would lose them. + * @returns {boolean} Whether the editor was left. + */ +function requestLeaveEditor() { + if (hasUnsavedChanges()) { + const leave = window.confirm( + `"${state.active.name}" has unsaved changes. Leave the Scene Editor without saving? ` + + "They stay here until you close or reload the page, but the bone viewer only shows saved scenes." + ); + if (!leave) { + announce("Still in the Scene Editor. Your changes aren't saved yet."); + return false; + } + } + leaveEditor(); + return true; +} + function leaveEditor() { dom.editorView.hidden = true; dom.viewer.hidden = false; @@ -2793,9 +2815,9 @@ export function initializeSceneEditor(root = document) { dom.enterButton.addEventListener("click", () => { if (dom.editorView.hidden) enterEditor(); - else leaveEditor(); + else requestLeaveEditor(); }); - dom.backButton.addEventListener("click", leaveEditor); + dom.backButton.addEventListener("click", requestLeaveEditor); const editorView = dom.editorView; document.addEventListener(OPEN_SCENE_EVENT, (event) => { // Skip if this page was re-rendered and this editor is no longer on it. diff --git a/templates/tests/sceneAnnotationEditor.test.js b/templates/tests/sceneAnnotationEditor.test.js index d2cd162..782546b 100644 --- a/templates/tests/sceneAnnotationEditor.test.js +++ b/templates/tests/sceneAnnotationEditor.test.js @@ -1440,3 +1440,60 @@ describe("Undo and redo (#443)", () => { expect(scenesModule.hasUnsavedChanges()).toBe(false); }); }); + +describe("Warning about unsaved changes when leaving the editor (#444)", () => { + const content = { + images: [{ id: "img", src: "/images/a.png", x: 300, y: 300, width: 100, height: 100, source: { type: "legacy-image", boneId: "ilium" } }], + annotations: [{ id: "label", type: "text", text: "Ilium", x: 20, y: 20 }], + }; + const inEditor = () => !$("scene-editor-view").hidden; + async function openEditedScene() { + await openBlankScene(content); + selectAnnotation(0); + key("ArrowRight"); + expect(scenesModule.hasUnsavedChanges()).toBe(true); + } + + it("lets the user stay in the editor from the Back button, the Scene Editor button, or a viewer link", async () => { + await openEditedScene(); + const confirm = jest.spyOn(window, "confirm").mockReturnValue(false); + const shownInViewer = jest.fn(); + document.addEventListener("bonebox:show-in-viewer", shownInViewer); + + click("scene-editor-back"); + click("text-button-SceneEditor"); + document.querySelector(".scene-source-link").click(); + + expect(confirm).toHaveBeenCalledTimes(3); + expect(confirm).toHaveBeenLastCalledWith(expect.stringContaining("\"Plate\" has unsaved changes")); + expect(inEditor()).toBe(true); + expect(shownInViewer).not.toHaveBeenCalled(); + expect(annotations()[0].x).toBe(21); + await waitFor(() => expect($("scene-editor-status").textContent).toBe("Still in the Scene Editor. Your changes aren't saved yet.")); + document.removeEventListener("bonebox:show-in-viewer", shownInViewer); + }); + + it("leaves when the user chooses to, keeping the unsaved changes for when they come back", async () => { + await openEditedScene(); + jest.spyOn(window, "confirm").mockReturnValue(true); + + click("scene-editor-back"); + expect(inEditor()).toBe(false); + + click("text-button-SceneEditor"); + expect(inEditor()).toBe(true); + expect(annotations()[0].x).toBe(21); + expect(scenesModule.hasUnsavedChanges()).toBe(true); + }); + + it("leaves without asking when everything is saved", async () => { + await openEditedScene(); + await saveAndWait(); + const confirm = jest.spyOn(window, "confirm"); + + click("scene-editor-back"); + + expect(inEditor()).toBe(false); + expect(confirm).not.toHaveBeenCalled(); + }); +});