fix(measurement-tracking): restore tracked state on undo after Delete all (#5994)

* fix: always show delete confirmation when measurements exist
This commit is contained in:
Ghadeer Albattarni 2026-05-12 13:22:25 -04:00 committed by GitHub
parent 8cd8ccc163
commit 397aa4d0e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 238 additions and 30 deletions

View File

@ -1,4 +1,4 @@
import { eventTarget, Types } from '@cornerstonejs/core'; import { eventTarget, Types, utilities as csUtils } from '@cornerstonejs/core';
import { Enums, annotation, cancelActiveManipulations } from '@cornerstonejs/tools'; import { Enums, annotation, cancelActiveManipulations } from '@cornerstonejs/tools';
import { DicomMetadataStore } from '@ohif/core'; import { DicomMetadataStore } from '@ohif/core';
@ -17,6 +17,8 @@ const { CORNERSTONE_3D_TOOLS_SOURCE_NAME, CORNERSTONE_3D_TOOLS_SOURCE_VERSION }
const { removeAnnotation } = annotation.state; const { removeAnnotation } = annotation.state;
const csToolsEvents = Enums.Events; const csToolsEvents = Enums.Events;
const { DefaultHistoryMemo } = csUtils.HistoryMemo;
const initMeasurementService = ( const initMeasurementService = (
measurementService, measurementService,
displaySetService, displaySetService,
@ -354,7 +356,7 @@ const connectMeasurementServiceToTools = ({
const { MEASUREMENT_REMOVED, MEASUREMENTS_CLEARED, MEASUREMENT_UPDATED, RAW_MEASUREMENT_ADDED } = const { MEASUREMENT_REMOVED, MEASUREMENTS_CLEARED, MEASUREMENT_UPDATED, RAW_MEASUREMENT_ADDED } =
measurementService.EVENTS; measurementService.EVENTS;
measurementService.subscribe(MEASUREMENTS_CLEARED, ({ measurements }) => { measurementService.subscribe(MEASUREMENTS_CLEARED, ({ measurements, trackingContext }) => {
if (!Object.keys(measurements).length) { if (!Object.keys(measurements).length) {
return; return;
} }
@ -373,6 +375,29 @@ const connectMeasurementServiceToTools = ({
options: { deleting: true }, options: { deleting: true },
}); });
} }
// If tracking context was provided, push a memo that keeps XState in sync with
// Cornerstone's annotation history across unlimited undo/redo cycles:
// undo → re-populate trackedStudy/trackedSeries so the panel reflects the
// restored annotations.
// redo → wipe trackedStudy/trackedSeries because the annotations have been
// re-deleted by their own Cornerstone memos; no measurements are
// deleted here (they are already gone), so CLEAR_TRACKING_CONTEXT is
// used instead of UNTRACK_ALL to avoid a double-delete.
if (trackingContext) {
DefaultHistoryMemo.push({
id: csUtils.uuidv4(),
operationType: 'trackingState',
restoreMemo(undo?: boolean) {
if (undo === true) {
commandsManager.run('restoreTrackedSeries', trackingContext);
} else if (undo === false) {
commandsManager.run('clearTrackedSeries');
}
},
});
}
commandsManager.run('endRecordingForAnnotationGroup'); commandsManager.run('endRecordingForAnnotationGroup');
// trigger a render // trigger a render

View File

@ -101,6 +101,7 @@ function ViewerHeader({ appConfig }: withAppTypes<{ appConfig: AppTypes.Config }
<Button <Button
variant="ghost" variant="ghost"
className="hover:bg-muted" className="hover:bg-muted"
data-cy="undo-btn"
onClick={() => { onClick={() => {
commandsManager.run('undo'); commandsManager.run('undo');
}} }}
@ -110,6 +111,7 @@ function ViewerHeader({ appConfig }: withAppTypes<{ appConfig: AppTypes.Config }
<Button <Button
variant="ghost" variant="ghost"
className="hover:bg-muted" className="hover:bg-muted"
data-cy="redo-btn"
onClick={() => { onClick={() => {
commandsManager.run('redo'); commandsManager.run('redo');
}} }}

View File

@ -159,7 +159,13 @@ function TrackedMeasurementsContextProvider(
} }
}, },
clearAllMeasurements: (ctx, evt) => { clearAllMeasurements: (ctx, evt) => {
measurementService.clearMeasurements(); const trackingContext = evt.trackedStudy
? { StudyInstanceUID: evt.trackedStudy, SeriesInstanceUIDs: [...(evt.trackedSeries || [])] }
: undefined;
measurementService.clearMeasurements(
undefined,
trackingContext ? { trackingContext } : undefined
);
measurementService.setIsMeasurementDeletedIndividually(false); measurementService.setIsMeasurementDeletedIndividually(false);
}, },
clearDisplaySetHydratedState: (ctx, evt) => { clearDisplaySetHydratedState: (ctx, evt) => {
@ -250,7 +256,7 @@ function TrackedMeasurementsContextProvider(
simplifiedAndLoadSR: (ctx, evt, condMeta) => { simplifiedAndLoadSR: (ctx, evt, condMeta) => {
return ( return (
appConfig?.measurementTrackingMode === measurementTrackingMode.SIMPLIFIED && appConfig?.measurementTrackingMode === measurementTrackingMode.SIMPLIFIED &&
evt.data.isBackupSave === false evt.data?.isBackupSave === false
); );
}, },
hasDirtyAndSimplified: (ctx, evt, condMeta) => { hasDirtyAndSimplified: (ctx, evt, condMeta) => {
@ -369,11 +375,22 @@ function TrackedMeasurementsContextProvider(
]); ]);
useEffect(() => { useEffect(() => {
// The command needs to be bound to the context's sendTrackedMeasurementsEvent // These commands are bound to the context's sendTrackedMeasurementsEvent, so they have
// so the command has to be registered in a React component. // to be registered from inside this (React) component. Re-registration on each
// render is safe — registerCommand overwrites by key.
commandsManager.registerCommand('DEFAULT', 'loadTrackedSRMeasurements', { commandsManager.registerCommand('DEFAULT', 'loadTrackedSRMeasurements', {
commandFn: props => sendTrackedMeasurementsEvent('HYDRATE_SR', props), commandFn: props => sendTrackedMeasurementsEvent('HYDRATE_SR', props),
}); });
commandsManager.registerCommand('DEFAULT', 'restoreTrackedSeries', {
commandFn: ({ StudyInstanceUID, SeriesInstanceUIDs }) =>
sendTrackedMeasurementsEvent('SET_TRACKED_SERIES', {
StudyInstanceUID,
SeriesInstanceUIDs,
}),
});
commandsManager.registerCommand('DEFAULT', 'clearTrackedSeries', {
commandFn: () => sendTrackedMeasurementsEvent('CLEAR_TRACKING_CONTEXT'),
});
}, [commandsManager, sendTrackedMeasurementsEvent]); }, [commandsManager, sendTrackedMeasurementsEvent]);
return ( return (

View File

@ -43,6 +43,9 @@ const machineConfiguration = {
idle: { idle: {
entry: 'clearContext', entry: 'clearContext',
on: { on: {
// No-op: idle entry already cleared context. Redo can still send
// CLEAR_TRACKING_CONTEXT (tracking memo); required when strict mode is on.
CLEAR_TRACKING_CONTEXT: {},
TRACK_SERIES: [ TRACK_SERIES: [
{ {
target: 'promptLabelAnnotation', target: 'promptLabelAnnotation',
@ -137,6 +140,14 @@ const machineConfiguration = {
], ],
}, },
], ],
// Redo of measurement clear: annotations are already deleted via Cornerstone memo;
// this only clears XState tracking context. UNTRACK_ALL would clear measurements again.
CLEAR_TRACKING_CONTEXT: [
{
target: 'tracking',
actions: ['clearContext', 'setIsDirtyToClean', 'clearDisplaySetHydratedState'],
},
],
SET_TRACKED_SERIES: [ SET_TRACKED_SERIES: [
{ {
target: 'tracking', target: 'tracking',

View File

@ -13,8 +13,7 @@ import {
import { useTrackedMeasurements } from '../getContextModule'; import { useTrackedMeasurements } from '../getContextModule';
import { UntrackSeriesModal } from './PanelStudyBrowserTracking/untrackSeriesModal'; import { UntrackSeriesModal } from './PanelStudyBrowserTracking/untrackSeriesModal';
const { filterMeasurementsBySeriesUID, filterAny } = const { filterMeasurementsBySeriesUID, filterAny } = utils.MeasurementFilters;
utils.MeasurementFilters;
function PanelMeasurementTableTracking(props) { function PanelMeasurementTableTracking(props) {
const [viewportGrid] = useViewportGrid(); const [viewportGrid] = useViewportGrid();
@ -23,28 +22,26 @@ function PanelMeasurementTableTracking(props) {
const [trackedMeasurements, sendTrackedMeasurementsEvent] = useTrackedMeasurements(); const [trackedMeasurements, sendTrackedMeasurementsEvent] = useTrackedMeasurements();
const { trackedStudy, trackedSeries } = trackedMeasurements.context; const { trackedStudy, trackedSeries } = trackedMeasurements.context;
const measurementFilter = trackedStudy const measurementFilter = trackedStudy ? filterMeasurementsBySeriesUID(trackedSeries) : filterAny;
? filterMeasurementsBySeriesUID(trackedSeries)
: filterAny;
const onUntrackConfirm = () => { const onUntrackConfirm = () => {
sendTrackedMeasurementsEvent('UNTRACK_ALL', {}); sendTrackedMeasurementsEvent('UNTRACK_ALL', { trackedStudy, trackedSeries });
}; };
const onDelete = () => { const onDelete = () => {
const hasDirtyMeasurements = measurementService const hasMeasurements = measurementService.getMeasurements().length > 0;
.getMeasurements() if (hasMeasurements) {
.some(measurement => measurement.isDirty); uiModalService.show({
hasDirtyMeasurements title: 'Untrack Study',
? uiModalService.show({ content: UntrackSeriesModal,
title: 'Untrack Study', contentProps: {
content: UntrackSeriesModal, onConfirm: onUntrackConfirm,
contentProps: { message: 'Are you sure you want to untrack study and delete all measurements?',
onConfirm: onUntrackConfirm, },
message: 'Are you sure you want to untrack study and delete all measurements?', });
}, } else {
}) onUntrackConfirm();
: onUntrackConfirm(); }
}; };
const EmptyComponent = () => ( const EmptyComponent = () => (

View File

@ -499,7 +499,7 @@ class MeasurementService extends PubSubService {
mapping => mapping.annotationType === annotationType mapping => mapping.annotationType === annotationType
); );
if (!sourceMapping) { if (!sourceMapping) {
if (!sourceMissing.has(source.uid) ) { if (!sourceMissing.has(source.uid)) {
console.log('No source mapping', source.uid, annotationType, source); console.log('No source mapping', source.uid, annotationType, source);
sourceMissing.add(source.uid); sourceMissing.add(source.uid);
} }
@ -716,12 +716,12 @@ class MeasurementService extends PubSubService {
* That allows, for example, clearing all of a single studies measurements * That allows, for example, clearing all of a single studies measurements
* without needing to clear other measurements. * without needing to clear other measurements.
*/ */
public clearMeasurements(filter?: MeasurementFilter) { public clearMeasurements(filter?: MeasurementFilter, metadata?: Record<string, unknown>) {
// Make a copy of the measurements // Make a copy of the measurements
const toClear = this.getMeasurements(filter); const toClear = this.getMeasurements(filter);
const measurements = [...toClear]; const measurements = [...toClear];
toClear.forEach(measurement => this.measurements.delete(measurement.uid)); toClear.forEach(measurement => this.measurements.delete(measurement.uid));
this._broadcastEvent(this.EVENTS.MEASUREMENTS_CLEARED, { measurements }); this._broadcastEvent(this.EVENTS.MEASUREMENTS_CLEARED, { measurements, ...metadata });
} }
/** /**

View File

@ -1,4 +1,12 @@
import { addLengthMeasurement, expect, scrollVolumeViewport, test, visitStudy } from './utils'; import {
addLengthMeasurement,
addOHIFConfiguration,
expect,
scrollVolumeViewport,
test,
visitStudy,
waitForViewportsRendered,
} from './utils';
test.beforeEach(async ({ page }) => { test.beforeEach(async ({ page }) => {
// Using same one as JumpToMeasurementMPR.spec.ts // Using same one as JumpToMeasurementMPR.spec.ts
@ -213,3 +221,132 @@ test('checks if measurement item can be deleted through the context menu on the
await expect(activeViewport.nthAnnotation(0).locator).toBeHidden(); await expect(activeViewport.nthAnnotation(0).locator).toBeHidden();
expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(0); expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(0);
}); });
test('checks that undo after delete-all restores measurements as tracked', async ({
page,
DOMOverlayPageObject,
rightPanelPageObject,
viewportPageObject,
mainToolbarPageObject,
}) => {
await addLengthMeasurement(page, {
firstClick: [450, 180],
secondClick: [550, 180],
});
await expect(DOMOverlayPageObject.viewport.measurementTracking.locator).toBeVisible();
await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
await addLengthMeasurement(page, {
firstClick: [450, 260],
secondClick: [550, 260],
});
await rightPanelPageObject.measurementsPanel.select();
expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(2);
await rightPanelPageObject.measurementsPanel.panel.deleteAll();
await expect(DOMOverlayPageObject.dialog.title).toHaveText('Untrack Study');
await DOMOverlayPageObject.dialog.confirmation.confirm.click();
await expect(DOMOverlayPageObject.dialog.title).toBeHidden();
expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(0);
await mainToolbarPageObject.undo.click();
await waitForViewportsRendered(page);
await expect(rightPanelPageObject.measurementsPanel.panel.rows).toHaveCount(2);
const activeViewport = await viewportPageObject.active;
const firstMeasurementLine = activeViewport.svg('line').first();
await expect(firstMeasurementLine).not.toHaveAttribute('stroke-dasharray');
const secondMeasurementLine = activeViewport.svg('line').nth(2);
await expect(secondMeasurementLine).not.toHaveAttribute('stroke-dasharray');
});
test('checks that delete-all prompt reappears after undo', async ({
page,
DOMOverlayPageObject,
rightPanelPageObject,
mainToolbarPageObject,
}) => {
await addLengthMeasurement(page, {
firstClick: [450, 180],
secondClick: [550, 180],
});
await expect(DOMOverlayPageObject.viewport.measurementTracking.locator).toBeVisible();
await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
await rightPanelPageObject.measurementsPanel.select();
await rightPanelPageObject.measurementsPanel.panel.deleteAll();
await expect(DOMOverlayPageObject.dialog.title).toHaveText('Untrack Study');
await DOMOverlayPageObject.dialog.confirmation.confirm.click();
await expect(DOMOverlayPageObject.dialog.title).toBeHidden();
await mainToolbarPageObject.undo.click();
await expect(rightPanelPageObject.measurementsPanel.panel.rows).toHaveCount(1);
await rightPanelPageObject.measurementsPanel.panel.deleteAll();
await expect(DOMOverlayPageObject.dialog.title).toHaveText('Untrack Study');
await DOMOverlayPageObject.dialog.confirmation.confirm.click();
await expect(DOMOverlayPageObject.dialog.title).toBeHidden();
await expect(rightPanelPageObject.measurementsPanel.panel.rows).toHaveCount(0);
});
test.describe('simplified tracking mode', () => {
test.beforeEach(async ({ page }) => {
await addOHIFConfiguration(page, {
measurementTrackingMode: 'simplified',
});
const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5';
await visitStudy(page, studyInstanceUID, 'viewer', 2000);
});
test('checks that undo after delete-all restores measurements as tracked (simplified mode)', async ({
page,
DOMOverlayPageObject,
rightPanelPageObject,
viewportPageObject,
mainToolbarPageObject,
}) => {
await addLengthMeasurement(page, {
firstClick: [450, 180],
secondClick: [550, 180],
});
await addLengthMeasurement(page, {
firstClick: [450, 260],
secondClick: [550, 260],
});
await rightPanelPageObject.measurementsPanel.select();
expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(2);
await rightPanelPageObject.measurementsPanel.panel.deleteAll();
await expect(DOMOverlayPageObject.dialog.title).toHaveText('Untrack Study');
await DOMOverlayPageObject.dialog.confirmation.confirm.click();
await expect(DOMOverlayPageObject.dialog.title).toBeHidden();
expect(await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount()).toBe(0);
await mainToolbarPageObject.undo.click();
await waitForViewportsRendered(page);
await expect(rightPanelPageObject.measurementsPanel.panel.rows).toHaveCount(2);
const activeViewport = await viewportPageObject.active;
const firstMeasurementLine = activeViewport.svg('line').first();
await expect(firstMeasurementLine).not.toHaveAttribute('stroke-dasharray');
const secondMeasurementLine = activeViewport.svg('line').nth(2);
await expect(secondMeasurementLine).not.toHaveAttribute('stroke-dasharray');
});
});

View File

@ -338,4 +338,22 @@ export class MainToolbarPageObject {
}, },
}; };
} }
get undo() {
const button = this.page.getByTestId('undo-btn');
return {
button,
async click() {
await button.click();
},
};
}
get redo() {
const button = this.page.getByTestId('redo-btn');
return {
button,
async click() {
await button.click();
},
};
}
} }

View File

@ -125,6 +125,7 @@ export class RightPanelPageObject {
getMeasurementCount: async () => { getMeasurementCount: async () => {
return await page.getByTestId('data-row').count(); return await page.getByTestId('data-row').count();
}, },
rows: page.getByTestId('data-row'),
locator: page.getByTestId('trackedMeasurements-panel').last(), locator: page.getByTestId('trackedMeasurements-panel').last(),
nthMeasurement(index: number) { nthMeasurement(index: number) {
return getMeasurementByIdx(index); return getMeasurementByIdx(index);

View File

@ -10,7 +10,7 @@ import { DataOverlayPageObject } from './DataOverlayPageObject';
import { DOMOverlayPageObject } from './DOMOverlayPageObject'; import { DOMOverlayPageObject } from './DOMOverlayPageObject';
import { MagnifyGlassPageObject } from './MagnifyGlassPageObject'; import { MagnifyGlassPageObject } from './MagnifyGlassPageObject';
type SvgInnerElement = 'circle' | 'path' | 'd'; type SvgInnerElement = 'circle' | 'path' | 'd' | 'line';
type NormalizedDragParams = { type NormalizedDragParams = {
start: { x: number; y: number }; start: { x: number; y: number };