From 464325418b1a360aa4a2ed6eafb09e627b9a8fee Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 11 Sep 2026 12:09:58 -0400 Subject: [PATCH 1/2] fix(tools): let widgets handle a press before the select tool picks The select tool's press handler runs ahead of the annotation widgets and resolves its own pick at the press position. That pick clears the widget manager's standing selections before the widgets handle the same press, and the ruler reads those selections to tell a handle from its line, so pressing a hovered ruler or rectangle handle with any non-placing tool threw. The throw also skipped the interactor's button bookkeeping, so every pointer move with the button held re-fired the press. Yield a microtask before picking so the widgets see the standing pick they hovered with. The handler stays registered ahead of the widgets so a widget consuming the press cannot skip selection, and the pick still resolves at the press position. --- src/components/tools/SelectTool.vue | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/tools/SelectTool.vue b/src/components/tools/SelectTool.vue index 8f9ebd4ef..af6ad6c6f 100644 --- a/src/components/tools/SelectTool.vue +++ b/src/components/tools/SelectTool.vue @@ -40,10 +40,13 @@ onVTKEvent( } const withModifiers = !!(event.shiftKey || event.controlKey); + const { x, y } = event.position; + // Picking clears the widget manager's standing selections, which the + // widgets read while handling this same press, so let them run first. + await Promise.resolve(); // Pick where the button went down. The widget manager's standing pick is // whatever its last tracked mouse move resolved, which can be a different // position or, mid capture, nothing at all. - const { x, y } = event.position; const selectedData = await view.widgetManager.getSelectedDataForXY(x, y); if ('widget' in selectedData) { const widget = @@ -71,7 +74,7 @@ onVTKEvent( } }, { - // capture all events by calling handler before widgets + // Registered ahead of the widgets so one consuming the press cannot skip it priority: WIDGET_PRIORITY + 1, } ); From 43e4b7d5950511031ee7b6c1fa1e918e577680d1 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 11 Sep 2026 12:10:00 -0400 Subject: [PATCH 2/2] test(tools): press annotations with the select tool Drag a ruler and a rectangle handle, click a ruler's line and drag a polygon's edge with the select tool. The drags must move the handle with no application error, and the line and edge presses must select the annotation without moving a handle. --- .../specs/select-tool-annotation-press.e2e.ts | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 tests/specs/select-tool-annotation-press.e2e.ts diff --git a/tests/specs/select-tool-annotation-press.e2e.ts b/tests/specs/select-tool-annotation-press.e2e.ts new file mode 100644 index 000000000..feb926b9e --- /dev/null +++ b/tests/specs/select-tool-annotation-press.e2e.ts @@ -0,0 +1,210 @@ +import { type ChainablePromiseElement } from 'webdriverio'; +import AppPage from '../pageobjects/volview.page'; +import { clickAt, setupTest, waitForCircleCount } from './annotationTestUtils'; + +// One input source held across action chains, so the press lands exactly where +// the hover left the pointer. Releasing actions would reset it to the origin. +const HOVERING_MOUSE = 'hovering-mouse'; +const hoveringMouse = () => browser.action('pointer', { id: HOVERING_MOUSE }); + +const INSTANT = 0; +const NUDGE_PX = 2; +const DRAG_MS = 200; +const DRAG_DY = 40; +const TOLERANCE_PX = 3; + +// vtk.js reports the first move after an idle period as StartMouseMove, which +// the widget manager does not pick on, so land on (x, y) with a second move. +const nudgeTo = (x: number, y: number) => + hoveringMouse() + .move({ duration: INSTANT, x: x + NUDGE_PX, y: y + NUDGE_PX }) + .move({ duration: INSTANT, x, y }) + .perform(true); + +const pressAtPointer = () => hoveringMouse().down().up().perform(true); + +const dragFromPointer = (x: number, y: number) => + hoveringMouse().down().move({ duration: DRAG_MS, x, y }).up().perform(true); + +// The widget manager sets the hover cursor once its pick lands on an annotation +// and restores the default one once it lands on nothing. +const waitForViewCursor = ( + axialView: ChainablePromiseElement, + cursor: string, + timeoutMsg: string +) => + browser.waitUntil( + async () => { + const current = await axialView.$('div.view').getCSSProperty('cursor'); + return current.value === cursor; + }, + { timeout: 5000, timeoutMsg } + ); + +const getHandleCenters = async (axialView: ChainablePromiseElement) => { + const centers: Array<{ x: number; y: number }> = []; + for (const circle of await axialView.$$('svg circle')) { + centers.push({ + x: Number(await circle.getAttribute('cx')), + y: Number(await circle.getAttribute('cy')), + }); + } + return centers; +}; + +// BoundingRectangle.vue draws this around the selected annotation +const waitForSelection = ( + axialView: ChainablePromiseElement, + timeoutMsg: string +) => + browser.waitUntil( + async () => { + const rects = await axialView.$$('svg rect[stroke="lightgray"]'); + return (await rects.length) === 1; + }, + { timeout: 5000, timeoutMsg } + ); + +const placeAnnotation = async ( + icon: string, + points: Array<[number, number]> +) => { + await AppPage.selectTool(icon); + for (const [x, y] of points) { + await clickAt(x, y); + } +}; + +// Confirms the press landed on the annotation, then hovers empty space so the +// DOM reflects everything the press did before the handles are compared. +const settleAfterPress = async ( + axialView: ChainablePromiseElement, + awayX: number, + awayY: number +) => { + await waitForSelection(axialView, 'Pressing the annotation should select it'); + await nudgeTo(awayX, awayY); + await waitForViewCursor( + axialView, + 'default', + 'Leaving the annotation should restore the default cursor' + ); +}; + +const expectHandlesUnmoved = ( + before: Array<{ x: number; y: number }>, + after: Array<{ x: number; y: number }> +) => { + expect(after.length).toBe(before.length); + before.forEach((handle, index) => { + expect(Math.abs(after[index].x - handle.x)).toBeLessThanOrEqual( + TOLERANCE_PX + ); + expect(Math.abs(after[index].y - handle.y)).toBeLessThanOrEqual( + TOLERANCE_PX + ); + }); +}; + +const TOOL_CASES = [ + { tool: 'ruler', icon: 'mdi-ruler' }, + { tool: 'rectangle', icon: 'mdi-vector-square' }, +]; + +// The select tool resolves its own pick on press. The annotation widgets read +// the widget manager's standing pick while handling that same press. +describe('Pressing an annotation with the select tool', () => { + TOOL_CASES.forEach(({ tool, icon }) => { + it(`drags the ${tool} handle without an application error`, async () => { + const { axialView, centerX, centerY } = await setupTest(); + const handleX = Math.round(centerX - 60); + const handleY = Math.round(centerY - 60); + + await placeAnnotation(icon, [ + [handleX, handleY], + [centerX + 60, centerY + 60], + ]); + await waitForCircleCount(axialView, 2, `${tool} should have two handles`); + const [before] = await getHandleCenters(axialView); + + await AppPage.selectTool('mdi-cursor-default'); + await nudgeTo(handleX, handleY); + await waitForViewCursor( + axialView, + 'pointer', + 'Hovering the handle should pick it' + ); + await dragFromPointer(handleX, handleY + DRAG_DY); + + await browser.waitUntil( + async () => { + const [after] = await getHandleCenters(axialView); + return Math.abs(after.y - before.y - DRAG_DY) <= TOLERANCE_PX; + }, + { + timeout: 5000, + timeoutMsg: `The ${tool} handle should follow the drag`, + } + ); + const [after] = await getHandleCenters(axialView); + expect(Math.abs(after.x - before.x)).toBeLessThanOrEqual(TOLERANCE_PX); + expect(await AppPage.getNotificationsCount()).toBe(0); + }); + }); + + it('keeps the ruler handles in place when its line is clicked', async () => { + const { axialView, centerX, centerY } = await setupTest(); + const midX = Math.round(centerX); + const midY = Math.round(centerY); + + await placeAnnotation('mdi-ruler', [ + [midX - 60, midY - 60], + [midX + 60, midY + 60], + ]); + await waitForCircleCount(axialView, 2, 'Ruler should have two handles'); + const before = await getHandleCenters(axialView); + + await AppPage.selectTool('mdi-cursor-default'); + await nudgeTo(midX, midY); + await waitForViewCursor( + axialView, + 'pointer', + 'Hovering the line should pick the ruler' + ); + await pressAtPointer(); + await settleAfterPress(axialView, midX + 140, midY - 140); + + expectHandlesUnmoved(before, await getHandleCenters(axialView)); + expect(await AppPage.getNotificationsCount()).toBe(0); + }); + + // The polygon moves its active handle on mouse move, so a drag is what + // would expose an unintended grab of the edge. + it('keeps the polygon handles in place when its edge is dragged', async () => { + const { axialView, centerX, centerY } = await setupTest(); + const midX = Math.round(centerX); + const topY = Math.round(centerY - 80); + + await placeAnnotation('mdi-pentagon-outline', [ + [midX - 80, topY], + [midX + 80, topY], + [midX, centerY + 80], + [midX - 80, topY], // close + ]); + await waitForCircleCount(axialView, 3, 'Polygon should have three handles'); + const before = await getHandleCenters(axialView); + + await AppPage.selectTool('mdi-cursor-default'); + await nudgeTo(midX, topY); + await waitForViewCursor( + axialView, + 'pointer', + 'Hovering the edge should pick the polygon' + ); + await dragFromPointer(midX, topY + DRAG_DY); + await settleAfterPress(axialView, midX + 140, topY - 60); + + expectHandlesUnmoved(before, await getHandleCenters(axialView)); + expect(await AppPage.getNotificationsCount()).toBe(0); + }); +});