From b6167966a0899f865c4e4aecf1bfbe7dfe6c0b47 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 4 Dec 2025 14:24:14 -0500 Subject: [PATCH 1/4] fix(tools): prevent fill interference when placing annotations - Suppress hover/context menu for existing annotations when actively placing a new polygon or rectangle - Add cross-tool awareness between polygon and rectangle tools - Check getActiveWidget() in widget behaviors to defer to focused widget - Ensure grabFocus properly sets hasFocus to maintain activeState --- src/components/tools/polygon/PolygonTool.vue | 46 ++++++++++++++++++- .../tools/rectangle/RectangleTool.vue | 31 ++++++++++++- src/vtk/PolygonWidget/behavior.ts | 18 +++++++- src/vtk/RulerWidget/behavior.ts | 17 +++++++ 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/components/tools/polygon/PolygonTool.vue b/src/components/tools/polygon/PolygonTool.vue index e639ff3c4..1fe0d91ca 100644 --- a/src/components/tools/polygon/PolygonTool.vue +++ b/src/components/tools/polygon/PolygonTool.vue @@ -92,6 +92,7 @@ import { Tools } from '@/src/store/tools/types'; import { getLPSAxisFromDir } from '@/src/utils/lps'; import { LPSAxisDir } from '@/src/types/lps'; import { usePolygonStore } from '@/src/store/tools/polygons'; +import { useRectangleStore } from '@/src/store/tools/rectangles'; import { useContextMenu, useCurrentTools, @@ -235,7 +236,31 @@ export default defineComponent({ // --- // - const { contextMenu, openContextMenu } = useContextMenu(); + const { contextMenu, openContextMenu: baseOpenContextMenu } = + useContextMenu(); + + // Check if any rectangle is actively being placed + const rectangleStore = useRectangleStore(); + const isAnyRectanglePlacing = () => { + return rectangleStore.tools.some( + (tool) => tool.placing && tool.firstPoint && tool.secondPoint + ); + }; + + // Suppress context menu for non-placing polygons when actively placing + // or when a rectangle is actively being placed + const openContextMenu = (id: ToolID, event: any) => { + if (isAnyRectanglePlacing()) { + return; + } + if (placingTool.id.value && id !== placingTool.id.value) { + const placingToolData = activeToolStore.toolByID[placingTool.id.value]; + if (placingToolData?.points?.length > 0) { + return; + } + } + baseOpenContextMenu(id, event); + }; const currentTools = useCurrentTools( activeToolStore, @@ -247,7 +272,24 @@ export default defineComponent({ }) ); - const { onHover, overlayInfo } = useHover(currentTools, slice); + const { onHover: baseOnHover, overlayInfo } = useHover(currentTools, slice); + + // Suppress hover for non-placing polygons when actively placing (has points) + // or when a rectangle is actively being placed + const onHover = (id: ToolID, event: any) => { + if (isAnyRectanglePlacing()) { + baseOnHover(id, { ...event, hovering: false }); + return; + } + if (placingTool.id.value && id !== placingTool.id.value) { + const placingToolData = activeToolStore.toolByID[placingTool.id.value]; + if (placingToolData?.points?.length > 0) { + baseOnHover(id, { ...event, hovering: false }); + return; + } + } + baseOnHover(id, event); + }; const mergePossible = computed( () => activeToolStore.mergeableTools.length >= 1 diff --git a/src/components/tools/rectangle/RectangleTool.vue b/src/components/tools/rectangle/RectangleTool.vue index 4b994fd4e..e0b5464c8 100644 --- a/src/components/tools/rectangle/RectangleTool.vue +++ b/src/components/tools/rectangle/RectangleTool.vue @@ -28,6 +28,7 @@ import { Tools } from '@/src/store/tools/types'; import { getLPSAxisFromDir } from '@/src/utils/lps'; import { LPSAxisDir } from '@/src/types/lps'; import { useRectangleStore } from '@/src/store/tools/rectangles'; +import { usePolygonStore } from '@/src/store/tools/polygons'; import { useCurrentTools, useContextMenu, @@ -39,6 +40,7 @@ import AnnotationInfo from '@/src/components/tools/AnnotationInfo.vue'; import { useFrameOfReference } from '@/src/composables/useFrameOfReference'; import { Maybe } from '@/src/types'; import { useSliceInfo } from '@/src/composables/useSliceInfo'; +import { ToolID } from '@/src/types/annotation-tool'; import { watchImmediate } from '@vueuse/core'; import RectangleWidget2D from './RectangleWidget2D.vue'; @@ -121,7 +123,8 @@ export default defineComponent({ // --- // - const { contextMenu, openContextMenu } = useContextMenu(); + const { contextMenu, openContextMenu: baseOpenContextMenu } = + useContextMenu(); const currentTools = useCurrentTools( activeToolStore, @@ -133,7 +136,31 @@ export default defineComponent({ }) ); - const { onHover, overlayInfo } = useHover(currentTools, slice); + const { onHover: baseOnHover, overlayInfo } = useHover(currentTools, slice); + + // Check if any polygon is actively being placed (has points) + const polygonStore = usePolygonStore(); + const isAnyPolygonPlacing = () => { + return polygonStore.tools.some( + (tool) => tool.placing && tool.points.length > 0 + ); + }; + + // Suppress hover/context menu when a polygon is actively being placed + const onHover = (id: ToolID, event: any) => { + if (isAnyPolygonPlacing()) { + baseOnHover(id, { ...event, hovering: false }); + return; + } + baseOnHover(id, event); + }; + + const openContextMenu = (id: ToolID, event: any) => { + if (isAnyPolygonPlacing()) { + return; + } + baseOpenContextMenu(id, event); + }; return { tools: currentTools, diff --git a/src/vtk/PolygonWidget/behavior.ts b/src/vtk/PolygonWidget/behavior.ts index bd11d34ef..bb94adf4f 100644 --- a/src/vtk/PolygonWidget/behavior.ts +++ b/src/vtk/PolygonWidget/behavior.ts @@ -192,8 +192,6 @@ export default function widgetBehavior(publicAPI: any, model: any) { if (model.widgetState.getPlacing() && manipulator) { // Dropping first point? if (model.widgetState.getHandles().length === 0) { - // update variables used by updateActiveStateHandle - model.activeState = model.widgetState.getMoveHandle(); model._widgetManager.grabFocus(publicAPI); } updateActiveStateHandle(event); @@ -252,6 +250,16 @@ export default function widgetBehavior(publicAPI: any, model: any) { model._widgetManager.disablePicking(); } + // Don't emit hover events if another widget has focus (e.g., is placing) + const activeWidget = model._widgetManager.getActiveWidget(); + if (activeWidget && activeWidget !== publicAPI) { + publicAPI.invokeHoverEvent({ + ...event, + hovering: false, + }); + return macro.VOID; + } + publicAPI.invokeHoverEvent({ ...event, hovering: !!model.activeState || checkOverFill(), @@ -424,6 +432,12 @@ export default function widgetBehavior(publicAPI: any, model: any) { return macro.EVENT_ABORT; } + // If another widget has focus (e.g., is placing), don't show context menu + const activeWidget = model._widgetManager.getActiveWidget(); + if (activeWidget && activeWidget !== publicAPI) { + return macro.VOID; + } + const eventWithWidgetAction = { ...eventData, widgetActions: makeWidgetActions(eventData), diff --git a/src/vtk/RulerWidget/behavior.ts b/src/vtk/RulerWidget/behavior.ts index b3b03cb9e..725384073 100644 --- a/src/vtk/RulerWidget/behavior.ts +++ b/src/vtk/RulerWidget/behavior.ts @@ -184,6 +184,16 @@ export default function widgetBehavior(publicAPI: any, model: any) { return macro.EVENT_ABORT; } + // Don't emit hover events if another widget has focus (e.g., is placing) + const activeWidget = model._widgetManager.getActiveWidget(); + if (activeWidget && activeWidget !== publicAPI) { + publicAPI.invokeHoverEvent({ + ...eventData, + hovering: false, + }); + return macro.VOID; + } + publicAPI.invokeHoverEvent({ ...eventData, hovering: !!model.activeState || checkOverFill(), @@ -220,6 +230,13 @@ export default function widgetBehavior(publicAPI: any, model: any) { ) { return macro.VOID; } + + // If another widget has focus (e.g., is placing), don't show context menu + const activeWidget = model._widgetManager.getActiveWidget(); + if (activeWidget && activeWidget !== publicAPI) { + return macro.VOID; + } + publicAPI.invokeRightClickEvent(eventData); return macro.EVENT_ABORT; }; From 9b598f133a4be2db38a2dbf2bfaf759d241daca5 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 5 Dec 2025 09:06:44 -0500 Subject: [PATCH 2/4] fix(polygon): handle events correctly when placing polygon inside another When placing a new polygon inside an existing one: - Right-click to remove points now works correctly - Old polygon handles no longer show on hover - Tooltip doesn't appear for containing polygon after placing finishes Key changes: - Check getPlacing() before activeState in handleRightButtonPress - Check if any other widget has focus before handling events - Properly release focus when polygon finishes placing - Hide handles on finished polygons when another is placing with points --- .../tools/polygon/PolygonWidget2D.vue | 11 +- src/vtk/PolygonWidget/behavior.ts | 34 +- tests/specs/polygon-nested-interaction.e2e.ts | 330 ++++++++++++++++++ 3 files changed, 360 insertions(+), 15 deletions(-) create mode 100644 tests/specs/polygon-nested-interaction.e2e.ts diff --git a/src/components/tools/polygon/PolygonWidget2D.vue b/src/components/tools/polygon/PolygonWidget2D.vue index 6c151f6ec..da252fc3c 100644 --- a/src/components/tools/polygon/PolygonWidget2D.vue +++ b/src/components/tools/polygon/PolygonWidget2D.vue @@ -101,8 +101,17 @@ export default defineComponent({ onVTKEvent(widget, 'onDraggingEvent', (eventData: any) => { dragging.value = eventData.dragging; }); + const anotherToolPlacing = computed(() => + toolStore.tools.some( + (t) => t.placing && t.id !== toolId.value && t.points.length > 0 + ) + ); const showHandles = computed(() => { - return lastHoverEventData.value?.hovering && !dragging.value; + return ( + lastHoverEventData.value?.hovering && + !dragging.value && + !anotherToolPlacing.value + ); }); watchEffect(() => { if (!lastHoverEventData.value) return; diff --git a/src/vtk/PolygonWidget/behavior.ts b/src/vtk/PolygonWidget/behavior.ts index bb94adf4f..039b3febb 100644 --- a/src/vtk/PolygonWidget/behavior.ts +++ b/src/vtk/PolygonWidget/behavior.ts @@ -246,13 +246,12 @@ export default function widgetBehavior(publicAPI: any, model: any) { // So we can rely on getSelections() to be up to date now overUnselectedHandle = false; - if (model.hasFocus) { - model._widgetManager.disablePicking(); - } - // Don't emit hover events if another widget has focus (e.g., is placing) - const activeWidget = model._widgetManager.getActiveWidget(); - if (activeWidget && activeWidget !== publicAPI) { + const widgets = model._widgetManager.getWidgets(); + const anotherWidgetHasFocus = widgets.some( + (w: any) => w !== publicAPI && w.hasFocus() + ); + if (anotherWidgetHasFocus) { publicAPI.invokeHoverEvent({ ...event, hovering: false, @@ -344,7 +343,6 @@ export default function widgetBehavior(publicAPI: any, model: any) { (model.hasFocus && !model.activeState) || (model.activeState && !model.activeState.getActive()) ) { - // update if mouse hovered over handle/activeState for next onDown model._widgetManager.enablePicking(); model._interactor.render(); } @@ -423,18 +421,22 @@ export default function widgetBehavior(publicAPI: any, model: any) { }; publicAPI.handleRightButtonPress = (eventData: any) => { - if (!model.activeState) { - return macro.VOID; - } - + // When placing, handle right-click regardless of what widget manager picked if (model.widgetState.getPlacing()) { removeLastHandle(); return macro.EVENT_ABORT; } + if (!model.activeState) { + return macro.VOID; + } + // If another widget has focus (e.g., is placing), don't show context menu - const activeWidget = model._widgetManager.getActiveWidget(); - if (activeWidget && activeWidget !== publicAPI) { + const widgets = model._widgetManager.getWidgets(); + const anotherWidgetHasFocus = widgets.some( + (w: any) => w !== publicAPI && w.hasFocus() + ); + if (anotherWidgetHasFocus) { return macro.VOID; } @@ -465,7 +467,8 @@ export default function widgetBehavior(publicAPI: any, model: any) { // Called after we are finished/placed. publicAPI.loseFocus = () => { - if (model.hasFocus) { + const hadFocus = model.hasFocus; + if (hadFocus) { model._interactor.cancelAnimation(publicAPI); publicAPI.invokeEndInteractionEvent(); } @@ -475,6 +478,9 @@ export default function widgetBehavior(publicAPI: any, model: any) { model.widgetState.getMoveHandle().setOrigin(null); model.activeState = null; model.hasFocus = false; + if (hadFocus) { + model._widgetManager.releaseFocus(); + } model._widgetManager.enablePicking(); }; diff --git a/tests/specs/polygon-nested-interaction.e2e.ts b/tests/specs/polygon-nested-interaction.e2e.ts new file mode 100644 index 000000000..2c9c4635c --- /dev/null +++ b/tests/specs/polygon-nested-interaction.e2e.ts @@ -0,0 +1,330 @@ +import AppPage from '../pageobjects/volview.page'; +import { MINIMAL_DICOM } from './configTestUtils'; + +describe('Polygon tool nested interaction', () => { + it('should not show first polygon handles when hovering over it while placing second polygon', async () => { + await AppPage.open( + `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` + ); + await AppPage.waitForViews(); + + const views2D = await AppPage.getViews2D(); + const axialView = views2D[0]; + const canvas = await axialView.$('canvas'); + const location = await canvas.getLocation(); + const size = await canvas.getSize(); + + const centerX = location.x + size.width / 2; + const centerY = location.y + size.height / 2; + + const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); + await polygonButton.click(); + + // Create first polygon (outer, larger) + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 80), y: Math.round(centerY + 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY + 80) }) + .down() + .up() + .perform(); + + // Close first polygon by clicking first point + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + // Verify first polygon has 4 handles + const firstPolygonHandles = await axialView.$$('svg circle'); + expect(await firstPolygonHandles.length).toBe(4); + + // Start second polygon inside first - place first point + await browser + .action('pointer') + .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 40) }) + .down() + .up() + .perform(); + + // Place second point of new polygon + await browser + .action('pointer') + .move({ x: Math.round(centerX + 40), y: Math.round(centerY - 40) }) + .down() + .up() + .perform(); + + // Now hover over the FIRST polygon's corner (top-left) to trigger hover handles + // Bug: This should NOT show handles on first polygon while placing second + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) + .perform(); + + // Wait a moment for hover state to update + await browser.waitUntil( + async () => { + const allCircles = await axialView.$$('svg circle'); + // Count visible circles - should only be from placing polygon (2 points) + // not from first polygon's hover handles + let visibleCount = 0; + for (const circle of allCircles) { + const visibility = await circle.getAttribute('visibility'); + if (visibility !== 'hidden') { + visibleCount++; + } + } + // Should be 2 (placing polygon points) + 4 (first polygon, but hidden) + // If bug exists: will be 6+ (first polygon handles become visible on hover) + return visibleCount <= 2; + }, + { + timeout: 3000, + timeoutMsg: + 'First polygon handles should NOT be visible when hovering while placing second polygon', + } + ); + }); + + it('should allow right-click to remove points on third polygon inside existing one', async () => { + await AppPage.open( + `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` + ); + await AppPage.waitForViews(); + + const views2D = await AppPage.getViews2D(); + const axialView = views2D[0]; + const canvas = await axialView.$('canvas'); + const location = await canvas.getLocation(); + const size = await canvas.getSize(); + + const centerX = location.x + size.width / 2; + const centerY = location.y + size.height / 2; + + const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); + await polygonButton.click(); + + // Create first (outer) polygon + await browser + .action('pointer') + .move({ x: Math.round(centerX - 100), y: Math.round(centerY - 100) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX + 100), y: Math.round(centerY - 100) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX + 100), y: Math.round(centerY + 100) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX - 100), y: Math.round(centerY + 100) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX - 100), y: Math.round(centerY - 100) }) + .down() + .up() + .perform(); + + // Create second polygon inside first, finish it + await browser + .action('pointer') + .move({ x: Math.round(centerX - 60), y: Math.round(centerY - 60) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX - 20), y: Math.round(centerY - 60) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 20) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX - 60), y: Math.round(centerY - 60) }) + .down() + .up() + .perform(); + + // Start third polygon inside first, place 2 points + await browser + .action('pointer') + .move({ x: Math.round(centerX + 20), y: Math.round(centerY + 20) }) + .down() + .up() + .perform(); + await browser + .action('pointer') + .move({ x: Math.round(centerX + 60), y: Math.round(centerY + 20) }) + .down() + .up() + .perform(); + + const circlesBeforeRightClick = await axialView.$$('svg circle'); + const countBefore = await circlesBeforeRightClick.length; + + // Right-click to remove second point of third polygon + await browser + .action('pointer') + .move({ x: Math.round(centerX + 40), y: Math.round(centerY + 40) }) + .down({ button: 2 }) + .up({ button: 2 }) + .perform(); + + await browser.waitUntil( + async () => { + const circlesAfterRightClick = await axialView.$$('svg circle'); + const countAfter = await circlesAfterRightClick.length; + return countAfter === countBefore - 1; + }, + { + timeout: 5000, + timeoutMsg: + 'Right-click should remove one point from the third polygon', + } + ); + }); + + it('should allow right-click to remove points while placing new polygon inside existing one', async () => { + await AppPage.open( + `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` + ); + await AppPage.waitForViews(); + + const views2D = await AppPage.getViews2D(); + const axialView = views2D[0]; + const canvas = await axialView.$('canvas'); + const location = await canvas.getLocation(); + const size = await canvas.getSize(); + + const centerX = location.x + size.width / 2; + const centerY = location.y + size.height / 2; + + const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); + await polygonButton.click(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 80), y: Math.round(centerY + 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY + 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 40) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 40), y: Math.round(centerY - 40) }) + .down() + .up() + .perform(); + + await browser + .action('pointer') + .move({ x: Math.round(centerX + 40), y: Math.round(centerY + 40) }) + .down() + .up() + .perform(); + + // Count total circles (handles may not be visible when placing) + const circlesBeforeRightClick = await axialView.$$('svg circle'); + const countBefore = await circlesBeforeRightClick.length; + + // Right-click inside the first polygon to test that placing polygon handles it + await browser + .action('pointer') + .move({ + x: Math.round(centerX), + y: Math.round(centerY), + }) + .down({ button: 2 }) + .up({ button: 2 }) + .perform(); + + await browser.waitUntil( + async () => { + const circlesAfterRightClick = await axialView.$$('svg circle'); + const countAfter = await circlesAfterRightClick.length; + // Should remove exactly one point + return countAfter === countBefore - 1; + }, + { + timeout: 5000, + timeoutMsg: + 'Right-click should remove exactly one point from the placing polygon', + } + ); + }); +}); From b6026d54aaf7a82fe009378f7f57129798d2b990 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 5 Dec 2025 09:42:13 -0500 Subject: [PATCH 3/4] fix: polygon nested interaction issues - Prevent hover handles on existing polygons while placing new one - Allow right-click to remove points when placing inside other annotations - Clear stale activeState on all widgets when placing polygon loses focus - Add hasFocus check to RulerWidget (used by RectangleWidget) --- src/components/tools/AnnotationInfo.vue | 4 +- src/components/tools/polygon/PolygonTool.vue | 41 +- src/vtk/PolygonWidget/behavior.ts | 24 +- src/vtk/RulerWidget/behavior.ts | 13 +- tests/specs/polygon-nested-interaction.e2e.ts | 494 +++++++----------- 5 files changed, 224 insertions(+), 352 deletions(-) diff --git a/src/components/tools/AnnotationInfo.vue b/src/components/tools/AnnotationInfo.vue index 1b0f26ceb..38e90b594 100644 --- a/src/components/tools/AnnotationInfo.vue +++ b/src/components/tools/AnnotationInfo.vue @@ -4,9 +4,8 @@ import { useElementSize } from '@vueuse/core'; import { AnnotationToolStore } from '@/src/store/tools/useAnnotationTool'; import { OverlayInfo } from '@/src/composables/annotationTool'; -// These seem to work ¯\_(ツ)_/¯ const TOOLTIP_PADDING_X = 30; -const TOOLTIP_PADDING_Y = 10; +const TOOLTIP_PADDING_Y = 16; const props = defineProps<{ info: OverlayInfo; @@ -78,6 +77,7 @@ const offset = computed(() => { background: rgba(255, 255, 255, 0.9) !important; padding-left: 0; padding-right: 0; + pointer-events: none; } .tooltip-text { diff --git a/src/components/tools/polygon/PolygonTool.vue b/src/components/tools/polygon/PolygonTool.vue index 1fe0d91ca..14cbc7f20 100644 --- a/src/components/tools/polygon/PolygonTool.vue +++ b/src/components/tools/polygon/PolygonTool.vue @@ -239,55 +239,36 @@ export default defineComponent({ const { contextMenu, openContextMenu: baseOpenContextMenu } = useContextMenu(); - // Check if any rectangle is actively being placed const rectangleStore = useRectangleStore(); - const isAnyRectanglePlacing = () => { - return rectangleStore.tools.some( + const shouldSuppressInteraction = (id: ToolID) => { + const rectanglePlacing = rectangleStore.tools.some( (tool) => tool.placing && tool.firstPoint && tool.secondPoint ); - }; - - // Suppress context menu for non-placing polygons when actively placing - // or when a rectangle is actively being placed - const openContextMenu = (id: ToolID, event: any) => { - if (isAnyRectanglePlacing()) { - return; - } + if (rectanglePlacing) return true; if (placingTool.id.value && id !== placingTool.id.value) { const placingToolData = activeToolStore.toolByID[placingTool.id.value]; - if (placingToolData?.points?.length > 0) { - return; - } + if (placingToolData?.points?.length > 0) return true; } - baseOpenContextMenu(id, event); + return false; + }; + + const openContextMenu = (id: ToolID, event: any) => { + if (!shouldSuppressInteraction(id)) baseOpenContextMenu(id, event); }; const currentTools = useCurrentTools( activeToolStore, viewAxis, - // only show this view's placing tool - computed(() => { - if (placingTool.id.value) return [placingTool.id.value]; - return []; - }) + computed(() => (placingTool.id.value ? [placingTool.id.value] : [])) ); const { onHover: baseOnHover, overlayInfo } = useHover(currentTools, slice); - // Suppress hover for non-placing polygons when actively placing (has points) - // or when a rectangle is actively being placed const onHover = (id: ToolID, event: any) => { - if (isAnyRectanglePlacing()) { + if (shouldSuppressInteraction(id)) { baseOnHover(id, { ...event, hovering: false }); return; } - if (placingTool.id.value && id !== placingTool.id.value) { - const placingToolData = activeToolStore.toolByID[placingTool.id.value]; - if (placingToolData?.points?.length > 0) { - baseOnHover(id, { ...event, hovering: false }); - return; - } - } baseOnHover(id, event); }; diff --git a/src/vtk/PolygonWidget/behavior.ts b/src/vtk/PolygonWidget/behavior.ts index 039b3febb..6fb2d7db9 100644 --- a/src/vtk/PolygonWidget/behavior.ts +++ b/src/vtk/PolygonWidget/behavior.ts @@ -23,6 +23,11 @@ const DOUBLE_CLICK_SLIP_DISTANCE_MAX_SQUARED = export default function widgetBehavior(publicAPI: any, model: any) { model.classHierarchy.push('vtkPolygonWidgetBehavior'); + const anotherWidgetHasFocus = () => + model._widgetManager + .getWidgets() + .some((w: any) => w !== publicAPI && w.hasFocus()); + const setDragging = (isDragging: boolean) => { model._dragging = isDragging; publicAPI.invokeDraggingEvent({ @@ -246,12 +251,7 @@ export default function widgetBehavior(publicAPI: any, model: any) { // So we can rely on getSelections() to be up to date now overUnselectedHandle = false; - // Don't emit hover events if another widget has focus (e.g., is placing) - const widgets = model._widgetManager.getWidgets(); - const anotherWidgetHasFocus = widgets.some( - (w: any) => w !== publicAPI && w.hasFocus() - ); - if (anotherWidgetHasFocus) { + if (anotherWidgetHasFocus()) { publicAPI.invokeHoverEvent({ ...event, hovering: false, @@ -431,12 +431,7 @@ export default function widgetBehavior(publicAPI: any, model: any) { return macro.VOID; } - // If another widget has focus (e.g., is placing), don't show context menu - const widgets = model._widgetManager.getWidgets(); - const anotherWidgetHasFocus = widgets.some( - (w: any) => w !== publicAPI && w.hasFocus() - ); - if (anotherWidgetHasFocus) { + if (anotherWidgetHasFocus()) { return macro.VOID; } @@ -480,6 +475,11 @@ export default function widgetBehavior(publicAPI: any, model: any) { model.hasFocus = false; if (hadFocus) { model._widgetManager.releaseFocus(); + // Deactivate all widgets so stale activeStates don't persist + // (user may right-click again without moving mouse) + model._widgetManager + .getWidgets() + .forEach((w: any) => w.deactivateAllHandles()); } model._widgetManager.enablePicking(); }; diff --git a/src/vtk/RulerWidget/behavior.ts b/src/vtk/RulerWidget/behavior.ts index 725384073..0676c6c9a 100644 --- a/src/vtk/RulerWidget/behavior.ts +++ b/src/vtk/RulerWidget/behavior.ts @@ -16,6 +16,11 @@ export function shouldIgnoreEvent(e: any) { export default function widgetBehavior(publicAPI: any, model: any) { model.classHierarchy.push('vtkRulerWidgetProp'); + const anotherWidgetHasFocus = () => + model._widgetManager + .getWidgets() + .some((w: any) => w !== publicAPI && w.hasFocus()); + model.interactionState = InteractionState.Select; let draggingState: any = null; @@ -184,9 +189,7 @@ export default function widgetBehavior(publicAPI: any, model: any) { return macro.EVENT_ABORT; } - // Don't emit hover events if another widget has focus (e.g., is placing) - const activeWidget = model._widgetManager.getActiveWidget(); - if (activeWidget && activeWidget !== publicAPI) { + if (anotherWidgetHasFocus()) { publicAPI.invokeHoverEvent({ ...eventData, hovering: false, @@ -231,9 +234,7 @@ export default function widgetBehavior(publicAPI: any, model: any) { return macro.VOID; } - // If another widget has focus (e.g., is placing), don't show context menu - const activeWidget = model._widgetManager.getActiveWidget(); - if (activeWidget && activeWidget !== publicAPI) { + if (anotherWidgetHasFocus()) { return macro.VOID; } diff --git a/tests/specs/polygon-nested-interaction.e2e.ts b/tests/specs/polygon-nested-interaction.e2e.ts index 2c9c4635c..0e7fb9706 100644 --- a/tests/specs/polygon-nested-interaction.e2e.ts +++ b/tests/specs/polygon-nested-interaction.e2e.ts @@ -1,106 +1,149 @@ +import { type ChainablePromiseElement } from 'webdriverio'; import AppPage from '../pageobjects/volview.page'; import { MINIMAL_DICOM } from './configTestUtils'; +// Low-level mouse helpers +const clickAt = async (x: number, y: number) => { + await browser + .action('pointer') + .move({ x: Math.round(x), y: Math.round(y) }) + .down() + .up() + .perform(); +}; + +const rightClickAt = async (x: number, y: number) => { + await browser + .action('pointer') + .move({ x: Math.round(x), y: Math.round(y) }) + .down({ button: 2 }) + .up({ button: 2 }) + .perform(); +}; + +const moveTo = async (x: number, y: number) => { + await browser + .action('pointer') + .move({ x: Math.round(x), y: Math.round(y) }) + .perform(); +}; + +// Test setup +const setupTest = async () => { + await AppPage.open(`?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}`); + await AppPage.waitForViews(); + + const views2D = await AppPage.getViews2D(); + const axialView = views2D[0]; + const canvas = await axialView.$('canvas'); + const location = await canvas.getLocation(); + const size = await canvas.getSize(); + + return { + axialView, + centerX: location.x + size.width / 2, + centerY: location.y + size.height / 2, + }; +}; + +// Tool selection +const selectPolygonTool = async () => { + const btn = await $('button span i[class~=mdi-pentagon-outline]'); + await btn.click(); +}; + +const selectRectangleTool = async () => { + const btn = await $('button span i[class~=mdi-vector-square]'); + await btn.click(); +}; + +// High-level shape creation +const createSquarePolygon = async ( + centerX: number, + centerY: number, + halfSize: number +) => { + const s = halfSize; + await clickAt(centerX - s, centerY - s); + await clickAt(centerX + s, centerY - s); + await clickAt(centerX + s, centerY + s); + await clickAt(centerX - s, centerY + s); + await clickAt(centerX - s, centerY - s); // close +}; + +const createTrianglePolygon = async ( + centerX: number, + centerY: number, + offsets: [number, number][] +) => { + for (const [dx, dy] of offsets) { + await clickAt(centerX + dx, centerY + dy); + } + await clickAt(centerX + offsets[0][0], centerY + offsets[0][1]); // close +}; + +const startPolygonPoints = async ( + centerX: number, + centerY: number, + offsets: [number, number][] +) => { + for (const [dx, dy] of offsets) { + await clickAt(centerX + dx, centerY + dy); + } +}; + +const createRectangle = async ( + centerX: number, + centerY: number, + halfSize: number +) => { + await clickAt(centerX - halfSize, centerY - halfSize); + await clickAt(centerX + halfSize, centerY + halfSize); +}; + +// Assertions +const getCircleCount = async (axialView: ChainablePromiseElement) => { + const circles = await axialView.$$('svg circle'); + return circles.length; +}; + +const waitForPointRemoved = async ( + axialView: ChainablePromiseElement, + countBefore: number, + msg: string +) => { + await browser.waitUntil( + async () => (await getCircleCount(axialView)) === countBefore - 1, + { timeout: 5000, timeoutMsg: msg } + ); +}; + +const getVisibleCircleCount = async (axialView: ChainablePromiseElement) => { + const circles = await axialView.$$('svg circle'); + let count = 0; + for (const circle of circles) { + if ((await circle.getAttribute('visibility')) !== 'hidden') count++; + } + return count; +}; + describe('Polygon tool nested interaction', () => { it('should not show first polygon handles when hovering over it while placing second polygon', async () => { - await AppPage.open( - `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` - ); - await AppPage.waitForViews(); - - const views2D = await AppPage.getViews2D(); - const axialView = views2D[0]; - const canvas = await axialView.$('canvas'); - const location = await canvas.getLocation(); - const size = await canvas.getSize(); - - const centerX = location.x + size.width / 2; - const centerY = location.y + size.height / 2; - - const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); - await polygonButton.click(); - - // Create first polygon (outer, larger) - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 80), y: Math.round(centerY + 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY + 80) }) - .down() - .up() - .perform(); - - // Close first polygon by clicking first point - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - // Verify first polygon has 4 handles - const firstPolygonHandles = await axialView.$$('svg circle'); - expect(await firstPolygonHandles.length).toBe(4); + const { axialView, centerX, centerY } = await setupTest(); + await selectPolygonTool(); - // Start second polygon inside first - place first point - await browser - .action('pointer') - .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 40) }) - .down() - .up() - .perform(); + await createSquarePolygon(centerX, centerY, 80); + expect(await getCircleCount(axialView)).toBe(4); - // Place second point of new polygon - await browser - .action('pointer') - .move({ x: Math.round(centerX + 40), y: Math.round(centerY - 40) }) - .down() - .up() - .perform(); + await startPolygonPoints(centerX, centerY, [ + [-40, -40], + [40, -40], + ]); - // Now hover over the FIRST polygon's corner (top-left) to trigger hover handles - // Bug: This should NOT show handles on first polygon while placing second - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) - .perform(); + await moveTo(centerX - 80, centerY - 80); - // Wait a moment for hover state to update await browser.waitUntil( - async () => { - const allCircles = await axialView.$$('svg circle'); - // Count visible circles - should only be from placing polygon (2 points) - // not from first polygon's hover handles - let visibleCount = 0; - for (const circle of allCircles) { - const visibility = await circle.getAttribute('visibility'); - if (visibility !== 'hidden') { - visibleCount++; - } - } - // Should be 2 (placing polygon points) + 4 (first polygon, but hidden) - // If bug exists: will be 6+ (first polygon handles become visible on hover) - return visibleCount <= 2; - }, + async () => (await getVisibleCircleCount(axialView)) <= 2, { timeout: 3000, timeoutMsg: @@ -110,221 +153,68 @@ describe('Polygon tool nested interaction', () => { }); it('should allow right-click to remove points on third polygon inside existing one', async () => { - await AppPage.open( - `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` - ); - await AppPage.waitForViews(); - - const views2D = await AppPage.getViews2D(); - const axialView = views2D[0]; - const canvas = await axialView.$('canvas'); - const location = await canvas.getLocation(); - const size = await canvas.getSize(); - - const centerX = location.x + size.width / 2; - const centerY = location.y + size.height / 2; - - const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); - await polygonButton.click(); - - // Create first (outer) polygon - await browser - .action('pointer') - .move({ x: Math.round(centerX - 100), y: Math.round(centerY - 100) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX + 100), y: Math.round(centerY - 100) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX + 100), y: Math.round(centerY + 100) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX - 100), y: Math.round(centerY + 100) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX - 100), y: Math.round(centerY - 100) }) - .down() - .up() - .perform(); - - // Create second polygon inside first, finish it - await browser - .action('pointer') - .move({ x: Math.round(centerX - 60), y: Math.round(centerY - 60) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX - 20), y: Math.round(centerY - 60) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 20) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX - 60), y: Math.round(centerY - 60) }) - .down() - .up() - .perform(); - - // Start third polygon inside first, place 2 points - await browser - .action('pointer') - .move({ x: Math.round(centerX + 20), y: Math.round(centerY + 20) }) - .down() - .up() - .perform(); - await browser - .action('pointer') - .move({ x: Math.round(centerX + 60), y: Math.round(centerY + 20) }) - .down() - .up() - .perform(); - - const circlesBeforeRightClick = await axialView.$$('svg circle'); - const countBefore = await circlesBeforeRightClick.length; - - // Right-click to remove second point of third polygon - await browser - .action('pointer') - .move({ x: Math.round(centerX + 40), y: Math.round(centerY + 40) }) - .down({ button: 2 }) - .up({ button: 2 }) - .perform(); - - await browser.waitUntil( - async () => { - const circlesAfterRightClick = await axialView.$$('svg circle'); - const countAfter = await circlesAfterRightClick.length; - return countAfter === countBefore - 1; - }, - { - timeout: 5000, - timeoutMsg: - 'Right-click should remove one point from the third polygon', - } + const { axialView, centerX, centerY } = await setupTest(); + await selectPolygonTool(); + + await createSquarePolygon(centerX, centerY, 100); + await createTrianglePolygon(centerX, centerY, [ + [-60, -60], + [-20, -60], + [-40, -20], + ]); + await startPolygonPoints(centerX, centerY, [ + [20, 20], + [60, 20], + ]); + + const countBefore = await getCircleCount(axialView); + await rightClickAt(centerX + 40, centerY + 40); + await waitForPointRemoved( + axialView, + countBefore, + 'Right-click should remove one point from the third polygon' ); }); it('should allow right-click to remove points while placing new polygon inside existing one', async () => { - await AppPage.open( - `?urls=${MINIMAL_DICOM.url}&names=${MINIMAL_DICOM.name}` + const { axialView, centerX, centerY } = await setupTest(); + await selectPolygonTool(); + + await createSquarePolygon(centerX, centerY, 80); + await startPolygonPoints(centerX, centerY, [ + [-40, -40], + [40, -40], + [40, 40], + ]); + + const countBefore = await getCircleCount(axialView); + await rightClickAt(centerX, centerY); + await waitForPointRemoved( + axialView, + countBefore, + 'Right-click should remove exactly one point from the placing polygon' ); - await AppPage.waitForViews(); - - const views2D = await AppPage.getViews2D(); - const axialView = views2D[0]; - const canvas = await axialView.$('canvas'); - const location = await canvas.getLocation(); - const size = await canvas.getSize(); - - const centerX = location.x + size.width / 2; - const centerY = location.y + size.height / 2; - - const polygonButton = await $('button span i[class~=mdi-pentagon-outline]'); - await polygonButton.click(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 80), y: Math.round(centerY + 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY + 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX - 80), y: Math.round(centerY - 80) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX - 40), y: Math.round(centerY - 40) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 40), y: Math.round(centerY - 40) }) - .down() - .up() - .perform(); - - await browser - .action('pointer') - .move({ x: Math.round(centerX + 40), y: Math.round(centerY + 40) }) - .down() - .up() - .perform(); - - // Count total circles (handles may not be visible when placing) - const circlesBeforeRightClick = await axialView.$$('svg circle'); - const countBefore = await circlesBeforeRightClick.length; - - // Right-click inside the first polygon to test that placing polygon handles it - await browser - .action('pointer') - .move({ - x: Math.round(centerX), - y: Math.round(centerY), - }) - .down({ button: 2 }) - .up({ button: 2 }) - .perform(); + }); - await browser.waitUntil( - async () => { - const circlesAfterRightClick = await axialView.$$('svg circle'); - const countAfter = await circlesAfterRightClick.length; - // Should remove exactly one point - return countAfter === countBefore - 1; - }, - { - timeout: 5000, - timeoutMsg: - 'Right-click should remove exactly one point from the placing polygon', - } + it('should allow right-click to remove polygon points when placing inside a rectangle', async () => { + const { axialView, centerX, centerY } = await setupTest(); + + await selectRectangleTool(); + await createRectangle(centerX, centerY, 80); + + await selectPolygonTool(); + await startPolygonPoints(centerX, centerY, [ + [-40, -40], + [40, -40], + [40, 40], + ]); + + const countBefore = await getCircleCount(axialView); + await rightClickAt(centerX, centerY); + await waitForPointRemoved( + axialView, + countBefore, + 'Right-click should remove one point from the polygon when placing inside a rectangle' ); }); }); From 8fd332c3aa8c30c047de529606abee366d328caf Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 5 Dec 2025 10:28:35 -0500 Subject: [PATCH 4/4] fix(e2e): fix flaky remote-manifest test and unskip working test --- tests/specs/remote-manifest.e2e.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/specs/remote-manifest.e2e.ts b/tests/specs/remote-manifest.e2e.ts index 6920afcdf..64b509c82 100644 --- a/tests/specs/remote-manifest.e2e.ts +++ b/tests/specs/remote-manifest.e2e.ts @@ -9,12 +9,14 @@ describe('VolView loading of remoteManifest.json', () => { }; const fileName = 'remoteFilesBadUrl.json'; await writeManifestToFile(manifest, fileName); - await openVolViewPage(fileName); + + const urlParams = `?urls=[tmp/${fileName}]`; + await volViewPage.open(urlParams); await volViewPage.waitForNotification(); }); - it.skip('should load relative URI with no name property', async () => { + it('should load relative URI with no name property', async () => { await downloadFile(MINIMAL_DICOM.url, MINIMAL_DICOM.name); const manifest = {