Skip to content

Commit c16a32d

Browse files
committed
fix(segmentation): remove deletion success notifications
1 parent 299a4b2 commit c16a32d

7 files changed

Lines changed: 42 additions & 81 deletions

File tree

‎src/segmentation/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ image; the mask's bounded labelmap is allocated only when needed.
1212
appearance. Stores holding masks or annotation tools declare their segment
1313
references on the registry, which hands them the removal. Removing the last
1414
image deletes every segment no config entry holds.
15-
- `deleteSegment.ts` deletes a segment with its masks and shapes and reports
16-
what went with it.
15+
- `deleteSegment.ts` guards locked segments before deleting their masks and
16+
shapes.
1717
- `store.ts` owns mask identity, attachment, lookup, and lifecycle. Import and
1818
restore use its operations to create records and attach prepared storage.
1919
- `masks/` contains allocation, growth, overlap operations, and voxel access.

‎src/segmentation/__tests__/deleteSegment.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { beforeEach, describe, expect, it } from 'vitest';
22
import { createPinia, setActivePinia } from 'pinia';
33

44
import { createSegmentRegistry } from '@/src/segmentation/segmentRegistry';
5-
import { deleteSegmentAndReport } from '@/src/segmentation/deleteSegment';
5+
import { deleteUnlockedSegment } from '@/src/segmentation/deleteSegment';
66

7-
describe('deleteSegmentAndReport', () => {
7+
describe('deleteUnlockedSegment', () => {
88
beforeEach(() => {
99
setActivePinia(createPinia());
1010
});
@@ -13,7 +13,7 @@ describe('deleteSegmentAndReport', () => {
1313
const registry = createSegmentRegistry();
1414
const id = registry.addSegment({ name: 'Tumor', locked: true });
1515

16-
deleteSegmentAndReport(registry, id);
16+
deleteUnlockedSegment(registry, id);
1717

1818
expect(registry.getSegment(id)).toBeDefined();
1919
});
@@ -22,7 +22,7 @@ describe('deleteSegmentAndReport', () => {
2222
const registry = createSegmentRegistry();
2323
const id = registry.addSegment({ name: 'Tumor' });
2424

25-
deleteSegmentAndReport(registry, id);
25+
deleteUnlockedSegment(registry, id);
2626

2727
expect(registry.getSegment(id)).toBeUndefined();
2828
});

‎src/segmentation/components/SegmentList.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import SaveSegmentationDialog from '@/src/segmentation/components/SaveSegmentati
1010
import SegmentEditor from '@/src/segmentation/components/SegmentEditor.vue';
1111
import SegmentListActions from '@/src/segmentation/components/SegmentListActions.vue';
1212
import { useCurrentImage } from '@/src/composables/useCurrentImage';
13-
import { deleteSegmentAndReport } from '@/src/segmentation/deleteSegment';
13+
import { deleteUnlockedSegment } from '@/src/segmentation/deleteSegment';
1414
import { useSegmentEditing } from '@/src/segmentation/composables/useSegmentEditing';
1515
import { pulseSegmentMask } from '@/src/segmentation/rendering/revealPulse';
1616
import {
@@ -408,7 +408,7 @@ const {
408408
@reveal="revealSlice(item)"
409409
@edit="editing.startEditing(item.id)"
410410
@toggle-visible="toggleVisible(item.id)"
411-
@delete="deleteSegmentAndReport(registry, item.id)"
411+
@delete="deleteUnlockedSegment(registry, item.id)"
412412
/>
413413
</template>
414414
</editable-item-list>

‎src/segmentation/components/__tests__/SegmentList.spec.ts‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,7 +1086,7 @@ describe('locked segment editor routes', () => {
10861086
);
10871087
});
10881088

1089-
describe('deleting a segment says what went with it', () => {
1089+
describe('deleting a segment without a success notification', () => {
10901090
beforeEach(async () => {
10911091
setActivePinia(createPinia());
10921092
await seatSpecImage('img-1');
@@ -1115,31 +1115,40 @@ describe('deleting a segment says what went with it', () => {
11151115
await nextTick();
11161116
};
11171117

1118-
it('counts the masks, the images they were on, and the annotations', async () => {
1118+
const expectDeleted = async (wrapper: VueWrapper) => {
1119+
for (const imageId of ['img-1', 'img-2']) {
1120+
await showImage(imageId);
1121+
expect(rowIds(wrapper)).toEqual([]);
1122+
expect(
1123+
wrapper
1124+
.get('[data-testid="save-segments-button"]')
1125+
.attributes('disabled')
1126+
).toBeDefined();
1127+
}
1128+
expect(messageTitles()).toEqual([]);
1129+
};
1130+
1131+
it('removes a segment and its painted masks across images', async () => {
11191132
const segmentId = spreadSegment(['img-1', 'img-2']);
11201133
const wrapper = mountList();
11211134
await nextTick();
11221135

11231136
await deleteRow(wrapper, segmentId);
11241137

1125-
expect(messageTitles()).toEqual([
1126-
'Deleted 2 masks on 2 images and 2 annotations',
1127-
]);
1138+
await expectDeleted(wrapper);
11281139
});
11291140

1130-
it('says one of each in the singular', async () => {
1141+
it('removes a segment with content only on another image', async () => {
11311142
const segmentId = spreadSegment(['img-2']);
11321143
const wrapper = mountList();
11331144
await nextTick();
11341145

11351146
await deleteRow(wrapper, segmentId);
11361147

1137-
expect(messageTitles()).toEqual([
1138-
'Deleted 1 mask on 1 image and 1 annotation',
1139-
]);
1148+
await expectDeleted(wrapper);
11401149
});
11411150

1142-
it('names only what the segment had', async () => {
1151+
it('deletes painted and annotation-only segments', async () => {
11431152
const painted = makeMask('img-1', 'Painted');
11441153
seedVoxel(painted.maskId, [1, 1, 0]);
11451154
const shaped = segments().addSegment({ name: 'Shaped' });
@@ -1153,18 +1162,16 @@ describe('deleting a segment says what went with it', () => {
11531162
await nextTick();
11541163

11551164
await deleteRow(wrapper, painted.segmentId);
1165+
expect(rowIds(wrapper)).toEqual([shaped]);
11561166
await deleteRow(wrapper, shaped);
11571167

1158-
expect(messageTitles()).toEqual([
1159-
'Deleted 1 mask on 1 image',
1160-
'Deleted 1 annotation',
1161-
]);
1168+
await expectDeleted(wrapper);
11621169
});
11631170

11641171
// A record is minted the moment a segment is resolved as an edit target, so
11651172
// an image can hold one for a segment that was never painted there. Deleting
11661173
// drops the record, but there was nothing on that image to lose.
1167-
it('counts no mask on an image the segment was only resolved on', async () => {
1174+
it('deletes empty mask records with their segment', async () => {
11681175
const recorded = makeMask('img-1', 'Resolved');
11691176
const allocated = maskOn('img-2', recorded.segmentId);
11701177
store().maskVoxels(allocated.id).materialize();
@@ -1173,8 +1180,7 @@ describe('deleting a segment says what went with it', () => {
11731180

11741181
await deleteRow(wrapper, recorded.segmentId);
11751182

1176-
expect(rowIds(wrapper)).toEqual([]);
1177-
expect(messageTitles()).toEqual([]);
1183+
await expectDeleted(wrapper);
11781184
});
11791185

11801186
it('stays quiet when the segment held nothing', async () => {
@@ -1184,11 +1190,10 @@ describe('deleting a segment says what went with it', () => {
11841190

11851191
await deleteRow(wrapper, empty);
11861192

1187-
expect(rowIds(wrapper)).toEqual([]);
1188-
expect(messageTitles()).toEqual([]);
1193+
await expectDeleted(wrapper);
11891194
});
11901195

1191-
it('reports the same cascade when the editor deletes', async () => {
1196+
it('removes the segment and its painted masks when the editor deletes', async () => {
11921197
const segmentId = spreadSegment(['img-1', 'img-2']);
11931198
const wrapper = mountList();
11941199
await nextTick();
@@ -1199,10 +1204,7 @@ describe('deleting a segment says what went with it', () => {
11991204
editor(wrapper).vm.$emit('delete');
12001205
await nextTick();
12011206

1202-
expect(rowIds(wrapper)).toEqual([]);
1203-
expect(messageTitles()).toEqual([
1204-
'Deleted 2 masks on 2 images and 2 annotations',
1205-
]);
1207+
await expectDeleted(wrapper);
12061208
});
12071209
});
12081210

‎src/segmentation/composables/useSegmentEditing.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { SegmentRegistry } from '@/src/segmentation/segmentRegistry';
44
import type { Maybe } from '@/src/types';
55
import { cssColorToRGBA } from '@/src/segmentation/color';
66
import { cleanUndefined } from '@/src/utils';
7-
import { deleteSegmentAndReport } from '@/src/segmentation/deleteSegment';
7+
import { deleteUnlockedSegment } from '@/src/segmentation/deleteSegment';
88

99
/**
1010
* The segment edit dialog: one editor, one set of fields, one place that
@@ -86,7 +86,7 @@ export function useSegmentEditing(registry: SegmentRegistry) {
8686
// Deleting a segment takes its masks on every image and its shapes with it.
8787
function deleteEditingSegment() {
8888
const id = editingSegmentId.value;
89-
if (id) deleteSegmentAndReport(registry, id);
89+
if (id) deleteUnlockedSegment(registry, id);
9090
stopEditing(false);
9191
}
9292

‎src/segmentation/deleteSegment.ts‎

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,6 @@
1-
import { maskHasContent } from '@/src/segmentation/model';
21
import type { SegmentRegistry } from '@/src/segmentation/segmentRegistry';
3-
import { useSegmentationStore } from '@/src/segmentation/store';
4-
import { useMessageStore } from '@/src/store/messages';
5-
import { AnnotationToolStoreMap } from '@/src/store/tools';
6-
import { plural } from '@/src/utils';
72

8-
/**
9-
* What deleting a segment is about to take with it, counted before the cascade
10-
* runs: its mask on every image, and every finished annotation naming it. A
11-
* tool still being placed is not counted, because the cascade leaves it alone,
12-
* and neither is a mask record with nothing in it: the cascade drops the
13-
* record, but the user never put anything on that image to lose.
14-
*/
15-
function countCascade(segmentId: string) {
16-
// A segment has at most one mask per image, so this counts both.
17-
const images = useSegmentationStore()
18-
.masksOfSegment(segmentId)
19-
.filter(maskHasContent).length;
20-
const annotations = Object.values(AnnotationToolStoreMap).reduce(
21-
(total, useStore) =>
22-
total +
23-
useStore().finishedTools.filter((tool) => tool.segmentId === segmentId)
24-
.length,
25-
0
26-
);
27-
return { images, annotations };
28-
}
29-
30-
/**
31-
* Deletes a segment and says what went with it. The cascade reaches masks on
32-
* images this one is not viewing and annotations on other slices and axes, so
33-
* its scope is invisible from here and there is no undo: the same reason
34-
* `removeSelectedTools` reports its count. No dialog asks first, which is what
35-
* the rest of the app does.
36-
*/
37-
export function deleteSegmentAndReport(
3+
export function deleteUnlockedSegment(
384
registry: SegmentRegistry,
395
segmentId: string
406
) {
@@ -43,14 +9,5 @@ export function deleteSegmentAndReport(
439
registry.appearanceOf(segmentId).locked
4410
)
4511
return;
46-
const { images, annotations } = countCascade(segmentId);
4712
registry.deleteSegment(segmentId);
48-
49-
const removed = [
50-
images > 0 &&
51-
`${images} ${plural(images, 'mask')} on ${images} ${plural(images, 'image')}`,
52-
annotations > 0 && `${annotations} ${plural(annotations, 'annotation')}`,
53-
].filter((part): part is string => !!part);
54-
if (removed.length > 0)
55-
useMessageStore().addInfo(`Deleted ${removed.join(' and ')}`);
5613
}

‎tests/specs/annotations-sidebar.e2e.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ const DRAWING_TOOLS = [
5151
];
5252

5353
describe('Annotations sidebar', () => {
54-
it('deletes a segment with its painted mask and measurement and reports what was removed', async () => {
54+
it('deletes a segment with its painted mask and measurement without a notification', async () => {
5555
const { axialView } = await placeRectangle();
5656
await volViewPage.activatePaint();
5757
await volViewPage.paintStrokeOnView(axialView);
@@ -67,9 +67,11 @@ describe('Annotations sidebar', () => {
6767
await expect(axialView.$('svg rect')).not.toExist();
6868
await expect(volViewPage.saveSegmentsButtons[0]).toBeDisabled();
6969
await volViewPage.notifications.click();
70-
await expect($('.message-center .header > span')).toHaveText(
71-
'Deleted 1 mask on 1 image and 1 annotation'
70+
await expect($('.message-center')).toBeDisplayed();
71+
const titles = await $$('.message-center .header > span').map((title) =>
72+
title.getText()
7273
);
74+
expect(titles.some((title) => title.startsWith('Deleted '))).toBe(false);
7375
});
7476

7577
it('keeps the Segments list in place and selected across tool switches', async () => {

0 commit comments

Comments
 (0)