fix: Restore viewport interactivity when deleting in-progress Spline or Livewire measurement#5905
Conversation
… Spline/Livewire measurement
…leting an in-progress Spline or Livewire annotation
✅ Deploy Preview for ohif-dev canceled.
|
tests/Livewire.spec.ts
Outdated
| test('should restore viewport interactivity after deleting an in-progress Livewire annotation via context menu', async ({ | ||
| page, | ||
| DOMOverlayPageObject, | ||
| mainToolbarPageObject, | ||
| viewportPageObject, | ||
| rightPanelPageObject, |
There was a problem hiding this comment.
Unused
rightPanelPageObject parameter
rightPanelPageObject is destructured in the test function signature but is never referenced anywhere inside the test body. This will typically trigger a lint/TypeScript "unused variable" warning. It should be removed from the parameter list to keep the signature consistent with what is actually used (matching the pattern in Spline.spec.ts where this parameter is correctly omitted).
| test('should restore viewport interactivity after deleting an in-progress Livewire annotation via context menu', async ({ | |
| page, | |
| DOMOverlayPageObject, | |
| mainToolbarPageObject, | |
| viewportPageObject, | |
| rightPanelPageObject, | |
| test('should restore viewport interactivity after deleting an in-progress Livewire annotation via context menu', async ({ | |
| page, | |
| DOMOverlayPageObject, | |
| mainToolbarPageObject, | |
| viewportPageObject, | |
| }) => { |
| // Cancel any active tool manipulation (e.g., Spline/Livewire) to avoid leaving the tool | ||
| // in a drawing state after deleting a not completed measurement, which can block viewport interactivity. | ||
| const element = getActiveViewportEnabledElement(viewportGridService)?.viewport?.element; | ||
| if (element) { | ||
| cancelActiveManipulations(element); | ||
| } |
There was a problem hiding this comment.
cancelActiveManipulations called unconditionally on all removals
cancelActiveManipulations is invoked for every MEASUREMENT_REMOVED event regardless of whether the removed annotation was actually in-progress. Consider a scenario where a user is actively drawing a Spline/Livewire annotation while a different (completed) annotation is removed — e.g., via the measurements side-panel, an undo operation, or a programmatic delete from another source. In that case, the call would cancel the user's active drawing unexpectedly.
A safer approach would be to only cancel when the removed annotation is the one currently being drawn. You could check the annotation's isDrawing flag or whether it has no closed contour before calling cancelActiveManipulations:
const removedAnnotation = annotation.state.getAnnotation(removedMeasurementId);
// Only cancel if the annotation was still in-progress (i.e. not yet completed/closed)
if (element && removedAnnotation?.data?.contour?.closed === false) {
cancelActiveManipulations(element);
}
While this edge case may be difficult to hit in practice (since the viewport focus is typically held by the drawing interaction), it is worth being explicit about the intent.
Context
Fixes #5673
This PR fixes loss of viewport interactivity when a user deletes an in-progress Spline or Livewire measurement from the context menu.
When the user starts drawing with Spline or Livewire tool, then attempt to delete the measurement before it's completed (whether through backspace or right click), the viewport becomes non-interactive, the user cannot draw again, switch tools, until they double-click on the image or refresh the viewer.
Root cause
Deleting the measurement only removed the annotation from annotation state. The Spline/Livewire tool was never told to cancel, so it kept its in-progress state (
isDrawing). The tool stayed in “active manipulation” mode and continued to block other interaction with the viewport.Changes & Results
initMeasurementService.tsIn the
MEASUREMENT_REMOVEDsubscription, get the active viewport element (same pattern asMEASUREMENT_UPDATED) and callcancelActiveManipulations(element)beforeremoveAnnotation.Before
After deleting a non completed measurement, the viewport becomes interactive and can't use any tools.
After
The viewport stay active after deleting a measurement
spline-tool-delete-fix.mov
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Greptile Summary
This PR fixes a viewport interactivity bug where deleting an in-progress Spline or Livewire annotation (via the context menu or backspace) left the tool in its
isDrawingstate, blocking all further interaction until the user double-clicked or refreshed. The root cause was thatMEASUREMENT_REMOVEDonly removed the annotation from state without notifying the active tool to cancel its manipulation.Key changes:
initMeasurementService.ts: ImportscancelActiveManipulationsfrom@cornerstonejs/toolsand calls it on the active viewport element at the start of theMEASUREMENT_REMOVEDhandler — following the samegetActiveViewportEnabledElementpattern already used in theMEASUREMENT_UPDATEDhandler. The fix also uses?.viewport?.element(with extra optional chaining onviewport) which is slightly safer than the existing?.viewport.elementusage on line 408.tests/Livewire.spec.ts&tests/Spline.spec.ts: Two new E2E tests each verify that after deleting an in-progress annotation via the context menu, the viewport remains interactive and a new annotation can be successfully drawn and confirmed.Minor concerns:
cancelActiveManipulationsis called for everyMEASUREMENT_REMOVEDevent, not only when the annotation is in-progress. This could interrupt an active drawing session if a separate completed annotation is deleted concurrently (e.g., from the measurements side-panel).Livewire.spec.tstest includes an unusedrightPanelPageObjectparameter that should be removed.Confidence Score: 4/5
cancelActiveManipulationsunconditionally for all removals (not just in-progress ones), and there is an unused parameter in the Livewire test. Neither issue is blocking, but the unconditional cancellation could introduce subtle regressions in edge cases.extensions/cornerstone/src/initMeasurementService.ts— review the unconditionalcancelActiveManipulationscall to ensure it doesn't affect scenarios where a completed annotation is deleted while drawing is active.Important Files Changed
cancelActiveManipulationscall in theMEASUREMENT_REMOVEDhandler to restore viewport interactivity after deleting an in-progress annotation. The fix is correct for the reported bug but is applied unconditionally to all removals, which could inadvertently cancel an active drawing if a different annotation is removed concurrently.rightPanelPageObjectparameter that should be removed.Sequence Diagram
sequenceDiagram participant User participant Viewport participant ContextMenu participant MeasurementService participant initMeasurementService participant CornerstoneTools User->>Viewport: Start drawing (Spline/Livewire) Viewport->>CornerstoneTools: Tool enters isDrawing=true state User->>ContextMenu: Right-click → Delete measurement ContextMenu->>MeasurementService: remove(measurementId) MeasurementService->>initMeasurementService: MEASUREMENT_REMOVED event Note over initMeasurementService: NEW: cancelActiveManipulations(element) initMeasurementService->>CornerstoneTools: cancelActiveManipulations(element) CornerstoneTools->>Viewport: Reset isDrawing=false, restore interactivity initMeasurementService->>CornerstoneTools: removeAnnotation(measurementId) Viewport-->>User: Viewport is interactive againLast reviewed commit: c076066