fix(hotkeyBindings): consolidate Escape hotkey behavior for contour drawing tools (#6104)

* fix(hotkeyBindings): consolidate Escape hotkey behavior for contour drawing tools

* PR feedback.

* PR feedback.
This commit is contained in:
Joe Boccanfuso 2026-06-26 14:20:22 +02:00 committed by GitHub
parent b7c8b865dd
commit 661ecb5b2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 442 additions and 12 deletions

View File

@ -11,6 +11,7 @@ import {
annotation,
Types as ToolTypes,
SplineContourSegmentationTool,
cancelActiveManipulations,
} from '@cornerstonejs/tools';
import {
SegmentInfo,
@ -306,6 +307,20 @@ function commandsModule({
commandsManager.run('setDisplaySetsForViewports', { viewportsToUpdate: updatedViewports });
},
/**
* Cancels any in-progress annotation manipulation (e.g. drawing a Spline,
* Livewire or PlanarFreehand contour) on the active viewport. Reached on
* Escape via the `cancelActiveOperation` command. `cancelActiveManipulations`
* invokes the `cancel` method of each active/passive tool that has an
* in-progress annotation, so it is a no-op when nothing is being drawn.
*/
cancelMeasurement: () => {
const element = _getActiveViewportEnabledElement()?.viewport?.element;
if (element) {
cancelActiveManipulations(element);
}
},
hydrateSecondaryDisplaySet: async ({ displaySet, viewportId }) => {
if (!displaySet) {
return;
@ -2028,6 +2043,24 @@ function commandsModule({
rejectPreview: () => {
actions._handlePreviewAction('reject');
},
/**
* Generic Escape handler. A single Escape press should discard whatever the
* user has in progress, but that can be one of two unrelated things: a
* provisional segmentation preview, or an annotation being drawn. Rather
* than bind both `rejectPreview` and `cancelMeasurement` to `esc` (Mousetrap
* keeps only one handler per key, so the second silently shadows the first),
* this command orchestrates both single-purpose commands. Each is a no-op
* when its state is not active, so running both is safe and order-independent.
*/
cancelActiveOperation: () => {
try {
actions.rejectPreview();
} catch (error) {
console.debug('Error rejecting active preview', error);
} finally {
actions.cancelMeasurement();
}
},
clearMarkersForMarkerLabelmap: () => {
const { viewport } = _getActiveViewportEnabledElement();
const toolGroup = cornerstoneTools.ToolGroupManager.getToolGroupForViewport(viewport.id);
@ -2550,6 +2583,9 @@ function commandsModule({
removeMeasurement: {
commandFn: actions.removeMeasurement,
},
cancelMeasurement: {
commandFn: actions.cancelMeasurement,
},
toggleLockMeasurement: {
commandFn: actions.toggleLockMeasurement,
},
@ -2798,6 +2834,7 @@ function commandsModule({
toggleSegmentSelect: actions.toggleSegmentSelect,
acceptPreview: actions.acceptPreview,
rejectPreview: actions.rejectPreview,
cancelActiveOperation: actions.cancelActiveOperation,
toggleUseCenterSegmentIndex: actions.toggleUseCenterSegmentIndex,
toggleLabelmapAssist: actions.toggleLabelmapAssist,
interpolateScrollForMarkerLabelmap: actions.interpolateScrollForMarkerLabelmap,

View File

@ -531,10 +531,8 @@ const connectMeasurementServiceToTools = ({
}
// 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);
}
commandsManager.run('cancelMeasurement');
const removedAnnotation = annotation.state.getAnnotation(removedMeasurementId);
removeAnnotation(removedMeasurementId);
// Ensure `removedAnnotation` is available before triggering the memo,

View File

@ -134,8 +134,8 @@ const bindings = [
isEditable: true,
},
{
commandName: 'cancelMeasurement',
label: 'Cancel Measurement',
commandName: 'cancelActiveOperation',
label: 'Cancel',
keys: ['esc'],
},
{
@ -172,11 +172,6 @@ const bindings = [
label: 'Accept Preview',
keys: ['enter'],
},
{
commandName: 'rejectPreview',
label: 'Reject Preview',
keys: ['esc'],
},
{
commandName: 'undo',
label: 'Undo',

View File

@ -0,0 +1,89 @@
---
sidebar_position: 7
sidebar_label: Escape / cancelActiveOperation hotkey
title: Escape hotkey consolidation
---
# Escape hotkey single `cancelActiveOperation` command
The default `Escape` hotkey behavior has been consolidated. Previously the
`esc` key was bound to **two** separate commands in the default hotkey
bindings — `cancelMeasurement` and `rejectPreview`. Because the underlying
hotkey library (Mousetrap) keeps only **one** handler per key, the
later-registered binding (`rejectPreview`) silently shadowed the other, so
`cancelMeasurement` never ran and an in-progress Spline/Livewire/PlanarFreehand
contour (or any annotation being drawn) could not be cancelled with `Escape`.
`Escape` is now bound to a single generic command, **`cancelActiveOperation`**,
which orchestrates the two single-purpose commands. Each is a no-op when its
state is not active, so one `Escape` press consistently:
- rejects a provisional segmentation **preview** (via `rejectPreview`), and
- cancels an **in-progress annotation** being drawn (via `cancelMeasurement`).
## Change
| 3.12 (old) | 3.13 (new) |
|----------------------------------------------|-----------------------------------------|
| `esc``cancelMeasurement` **and** `esc``rejectPreview` (the latter shadowed the former) | `esc``cancelActiveOperation` |
| `enter``acceptPreview` | `enter``acceptPreview` (unchanged) |
New command (cornerstone extension, `CORNERSTONE` context):
```ts
// extensions/cornerstone/src/commandsModule.ts
cancelActiveOperation: () => {
actions.rejectPreview(); // discard a provisional segmentation preview
actions.cancelMeasurement(); // cancel an in-progress annotation draw
},
```
Both `rejectPreview` and `cancelMeasurement` remain available as standalone
commands; only their default `esc` hotkey bindings changed.
## Migration
Most applications need no changes — the default behavior simply works as
expected now.
**If you provide a custom `ohif.hotkeyBindings` customization** and want the
same consolidated behavior, bind `esc` to the new command and remove the old
duplicate `esc` entries:
**Before (3.12):**
```ts
'ohif.hotkeyBindings': [
// ...
{ commandName: 'cancelMeasurement', label: 'Cancel Measurement', keys: ['esc'] },
{ commandName: 'acceptPreview', label: 'Accept Preview', keys: ['enter'] },
{ commandName: 'rejectPreview', label: 'Reject Preview', keys: ['esc'] },
];
```
**After (3.13):**
```ts
'ohif.hotkeyBindings': [
// ...
{ commandName: 'cancelActiveOperation', label: 'Cancel', keys: ['esc'] },
{ commandName: 'acceptPreview', label: 'Accept Preview', keys: ['enter'] },
];
```
**If you ran the `cancelMeasurement` or `rejectPreview` command directly** (for
example from a toolbar button or another command), no change is required — both
commands still exist with the same behavior.
> **User key remaps:** the per-user hotkey overrides stored in `localStorage`
> (`user-preferred-keys`) are keyed by command hash. A user who had personally
> remapped the old `cancelMeasurement` or `rejectPreview` key will fall back to
> the new `esc` default for `cancelActiveOperation`, since those old command
> hashes no longer exist in the defaults.
## Reason
Binding two separate commands to the same key is inherently fragile — only the
last registered binding survives. Routing `Escape` through a single
orchestrating command removes the shadowing bug, keeps each underlying command
single-purpose, and makes the "discard whatever is in progress" intent explicit.

View File

@ -217,7 +217,10 @@ const renderSelectSetting = option => {
key={option.id}
>
<div className="w-1/3 text-[13px]">{renderLabelWithTooltip(option.name, option.tooltip)}</div>
<div className="w-2/3">
<div
className="w-2/3"
data-cy={option.id}
>
<Select
onValueChange={value => option.onChange?.(value)}
value={option.value}
@ -230,6 +233,7 @@ const renderSelectSetting = option => {
<SelectItem
key={value.id}
value={value.id}
data-cy={value.id}
>
{value.label}
</SelectItem>

View File

@ -0,0 +1,47 @@
import { expect, test, visitStudy } from './utils';
import { press } from './utils/keyboardUtils';
test.beforeEach(async ({ page }) => {
const studyInstanceUID = '1.3.12.2.1107.5.2.32.35162.30000015050317233592200000046';
const mode = 'segmentation';
await visitStudy(page, studyInstanceUID, mode, 2000);
});
test('should cancel an in-progress B-Spline contour segmentation via Escape', async ({
page,
rightPanelPageObject,
viewportPageObject,
}) => {
const contourPanel = rightPanelPageObject.contourSegmentationPanel;
// Create a Contour-type segmentation so the contour drawing tools become enabled.
// Creating it adds a default "Segment 1"; wait for that row before drawing.
await contourPanel.addSegmentation();
await expect(contourPanel.panel.rows).toHaveCount(1);
// Activate the Spline Contour tool, then switch to the B-Spline variant.
await contourPanel.tools.splineContour.click();
await contourPanel.tools.splineContour.selectType('bSpline');
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress B-Spline contour
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw again to verify the contour tool is still interactive after cancellation
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
await expect(activeViewport.svg('circle')).toHaveCount(3);
});

View File

@ -0,0 +1,47 @@
import { expect, test, visitStudy } from './utils';
import { press } from './utils/keyboardUtils';
test.beforeEach(async ({ page }) => {
const studyInstanceUID = '1.3.12.2.1107.5.2.32.35162.30000015050317233592200000046';
const mode = 'segmentation';
await visitStudy(page, studyInstanceUID, mode, 2000);
});
test('should cancel an in-progress Catmull-Rom spline contour segmentation via Escape', async ({
page,
rightPanelPageObject,
viewportPageObject,
}) => {
const contourPanel = rightPanelPageObject.contourSegmentationPanel;
// Create a Contour-type segmentation so the contour drawing tools become enabled.
// Creating it adds a default "Segment 1"; wait for that row before drawing.
await contourPanel.addSegmentation();
await expect(contourPanel.panel.rows).toHaveCount(1);
// Activate the Spline Contour tool, then arm the Catmull-Rom spline variant.
await contourPanel.tools.splineContour.click();
await contourPanel.tools.splineContour.selectType('catmullRom');
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress Catmull-Rom spline contour
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw again to verify the contour tool is still interactive after cancellation
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
await expect(activeViewport.svg('circle')).toHaveCount(3);
});

View File

@ -0,0 +1,47 @@
import { expect, test, visitStudy } from './utils';
import { press } from './utils/keyboardUtils';
test.beforeEach(async ({ page }) => {
const studyInstanceUID = '1.3.12.2.1107.5.2.32.35162.30000015050317233592200000046';
const mode = 'segmentation';
await visitStudy(page, studyInstanceUID, mode, 2000);
});
test('should cancel an in-progress Linear spline contour segmentation via Escape', async ({
page,
rightPanelPageObject,
viewportPageObject,
}) => {
const contourPanel = rightPanelPageObject.contourSegmentationPanel;
// Create a Contour-type segmentation so the contour drawing tools become enabled.
// Creating it adds a default "Segment 1"; wait for that row before drawing.
await contourPanel.addSegmentation();
await expect(contourPanel.panel.rows).toHaveCount(1);
// Activate the Spline Contour tool, then switch to the Linear spline variant.
await contourPanel.tools.splineContour.click();
await contourPanel.tools.splineContour.selectType('linear');
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress Linear spline contour
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw again to verify the contour tool is still interactive after cancellation
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
await expect(activeViewport.svg('circle')).toHaveCount(3);
});

View File

@ -84,6 +84,40 @@ test('should restore viewport interactivity after deleting an in-progress Livewi
await expect(activeViewport.nthAnnotation(0).locator).toBeVisible();
});
test('should cancel an in-progress Livewire annotation via Escape', async ({
page,
DOMOverlayPageObject,
mainToolbarPageObject,
viewportPageObject,
}) => {
await mainToolbarPageObject.measurementTools.livewireContour.click();
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress Livewire annotation
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw and complete a new Livewire annotation to verify interactivity is restored
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
{ x: 581, y: 287 },
{ x: 482, y: 333 },
{ x: 383, y: 301 },
]);
await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
await expect(activeViewport.nthAnnotation(0).locator).toBeVisible();
});
test('should restore viewport interactivity after deleting an in-progress Livewire annotation via Backspace', async ({
page,
DOMOverlayPageObject,

View File

@ -0,0 +1,46 @@
import { expect, test, visitStudy } from './utils';
import { press } from './utils/keyboardUtils';
test.beforeEach(async ({ page }) => {
const studyInstanceUID = '1.3.12.2.1107.5.2.32.35162.30000015050317233592200000046';
const mode = 'segmentation';
await visitStudy(page, studyInstanceUID, mode, 2000);
});
test('should cancel an in-progress Livewire contour segmentation via Escape', async ({
page,
rightPanelPageObject,
viewportPageObject,
}) => {
const contourPanel = rightPanelPageObject.contourSegmentationPanel;
// Create a Contour-type segmentation so the contour drawing tools become enabled.
// Creating it adds a default "Segment 1"; wait for that row before drawing.
await contourPanel.addSegmentation();
await expect(contourPanel.panel.rows).toHaveCount(1);
// Activate the Livewire Contour tool (a plain click arms it; no variants).
await contourPanel.tools.livewireContour.click();
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress Livewire contour
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw again to verify the contour tool is still interactive after cancellation
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
await expect(activeViewport.svg('circle')).toHaveCount(3);
});

View File

@ -84,6 +84,41 @@ test('should restore viewport interactivity after deleting an in-progress Spline
await expect(activeViewport.nthAnnotation(0).locator).toBeVisible();
});
test('should cancel an in-progress Spline annotation via Escape', async ({
page,
DOMOverlayPageObject,
mainToolbarPageObject,
viewportPageObject,
}) => {
await mainToolbarPageObject.measurementTools.splineROI.click();
const activeViewport = await viewportPageObject.active;
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
]);
// Ensure the three points clicked above are rendered in the DOM before pressing Escape
await expect(activeViewport.svg('circle')).toHaveCount(3);
await press({ page, key: 'Escape' });
// Pressing Escape should cancel the in-progress Spline annotation
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
// Draw and complete a new Spline annotation to verify interactivity is restored
await activeViewport.clickAt([
{ x: 380, y: 299 },
{ x: 420, y: 236 },
{ x: 523, y: 232 },
{ x: 581, y: 287 },
{ x: 482, y: 333 },
{ x: 383, y: 301 },
]);
await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
await expect(activeViewport.nthAnnotation(0).locator).toBeVisible();
});
test('should restore viewport interactivity after deleting an in-progress Spline annotation via Backspace', async ({
page,
DOMOverlayPageObject,

View File

@ -344,6 +344,57 @@ export class RightPanelPageObject {
select: async () => {
await menuButton.click();
},
// Switches to the contour tab and creates a new Contour-type segmentation,
// which enables the contour drawing tools (Spline / Livewire / Freehand).
addSegmentation: async () => {
await menuButton.click();
await addSegmentationButton.click();
},
tools: {
get splineContour() {
const button = page.getByTestId('SplineContourSegmentationTool');
// Maps a friendly spline name to the underlying cornerstone tool name,
// which is also the data-cy of its option in the Spline Type dropdown.
const splineTypeToolNames = {
catmullRom: 'CatmullRomSplineROI',
linear: 'LinearSplineROI',
bSpline: 'BSplineROI',
} as const;
return {
button,
// Activates the Spline Contour tool, arming the spline variant currently
// selected in the Spline Type dropdown. This defaults to Catmull-Rom until
// selectType is used to switch it.
click: async () => {
await button.click();
},
// Opens the Spline Type dropdown (rendered once the tool is active) and
// switches to the requested spline variant.
selectType: async (type: keyof typeof splineTypeToolNames) => {
await page.getByTestId('splineTypeSelect').getByRole('combobox').click();
await page.getByTestId(splineTypeToolNames[type]).click();
},
};
},
get livewireContour() {
const button = page.getByTestId('LivewireContourSegmentationTool');
return {
button,
click: async () => {
await button.click();
},
};
},
get freehandContour() {
const button = page.getByTestId('PlanarFreehandContourSegmentationTool');
return {
button,
click: async () => {
await button.click();
},
};
},
},
};
}
get labelMapSegmentationPanel() {