From c2a7ba523c1819054da091f73c25258eecf78150 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 29 Oct 2026 14:53:57 -0700 Subject: [PATCH] fix(library): open the archived shelf's delete review on Cancel (#492) Its only action is a permanent delete, so focus starts on Cancel (openReviewDialog's focusCancel); the spec also pins that the copy says the delete cannot be undone and removes the prompt's saved pairs. --- TESTING.md | 3 ++- tests/browser/workspace-library-delete.spec.mjs | 4 ++++ web/js/workspace/library-delete.mjs | 3 ++- web/js/workspace/workspace-dialogs.mjs | 6 ++++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/TESTING.md b/TESTING.md index b6a15c1d..4bf996c9 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2819,7 +2819,8 @@ is #482's Delete permanently grid ("Saved pairs removed" and "Negatives kept" on pair); an archived prompt used as another prompt's negative cannot be deleted and **stays archived** ("Stays archived: R prompts used as another prompt's negative"); one restored since the shelf was read is "No longer archived (kept)" and never written; a review with nothing to delete is close-only ("None of the selected prompts can be deleted."). The -primary is "Delete N prompts". The same preview, safety backup, review digests, `records` hold and epoch fence apply. +primary is "Delete N prompts"; the review opens with focus on Cancel (it is only a permanent delete; `openReviewDialog`'s +`focusCancel`). The same preview, safety backup, review digests, `records` hold and epoch fence apply. The routes take an optional `shelf` (`"active"`, the default, or `"archived"`; anything else 400). Under it an archived prompt is classified by its pairs and a prompt without an archive row is `not_archived`; the held write classifies the diff --git a/tests/browser/workspace-library-delete.spec.mjs b/tests/browser/workspace-library-delete.spec.mjs index 8bd4267d..36ca84a4 100644 --- a/tests/browser/workspace-library-delete.spec.mjs +++ b/tests/browser/workspace-library-delete.spec.mjs @@ -193,6 +193,10 @@ test('Delete… on the archived shelf deletes an archived prompt and its saved p await openReview(page); await expect(review(page)).toHaveAccessibleName('Delete archived prompts'); await expect(choice(page)).toHaveCount(0); + // A permanent delete only: the review opens on Cancel, and its copy says it cannot be undone and takes the pairs. + await expect(review(page).getByRole('button',{name:'Cancel',exact:true})).toBeFocused(); + await expect(review(page).locator('p.small').first()).toHaveText(/^Deleting a prompt in a saved pair also removes its saved pairs/); + await expect(review(page).locator('p.small').nth(1)).toContainText('the only way back is to restore the safety backup taken first'); await expect(row(page,'Delete permanently')).toHaveText('1 prompt in a saved pair'); await expect(row(page,'Saved pairs removed')).toHaveText(/^1 saved pair/); await expect(row(page,'Backup')).toHaveText('Create and verify before deleting'); diff --git a/web/js/workspace/library-delete.mjs b/web/js/workspace/library-delete.mjs index 086d9d65..b658152d 100644 --- a/web/js/workspace/library-delete.mjs +++ b/web/js/workspace/library-delete.mjs @@ -240,7 +240,8 @@ export function openDeleteReview({ scope, trigger, review, titleOf, onStart, onD const choice = hasPairedChoice(review) ? pairedChoice(value => update(value)) : null; const dialog = openReviewDialog({ scope, trigger, title, grid, options: choice ? [choice, ...sentences] : sentences, cancelLabel: 'Cancel', primary: { label: primaryLabel(current), danger: true }, - onStart: () => onStart(current), onDismiss }); + // #492: the archived shelf's review is only a permanent delete, so it opens on Cancel. + focusCancel: onArchivedShelf(current), onStart: () => onStart(current), onDismiss }); function update(value) { current = withChoice(review, value); dialog.setRows(reviewGrid(current, titleOf)); diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index 25c9f68b..c26d82a0 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -477,7 +477,7 @@ function requireGrid(grid) { * an option, and `{setTitle(term, text)}` for a value's hover title. */ export function openReviewDialog({ scope, title, heading = null, grid, options = [], sentence = null, backup = null, - cancelLabel, primary, onStart, trigger = null, onDismiss = null }) { + cancelLabel, primary, onStart, trigger = null, onDismiss = null, focusCancel = false }) { requireScope(scope); requireCallbacks({ onStart, ...(onDismiss ? { onDismiss } : {}) }); for (const [name, text] of Object.entries({ title, cancelLabel, label: primary?.label })) { @@ -519,7 +519,9 @@ export function openReviewDialog({ scope, title, heading = null, grid, options = actions.append(cancel, start); nodes.push(actions); // Cancel, ×, Escape, a replacing dialog or the page leaving: everything but the primary is a dismissal. - show(title, nodes, { scope, trigger, onClose: onDismiss ? () => { if (!started) onDismiss(); } : null, review: true }); + // `focusCancel` (#492): a review whose primary cannot be undone opens on Cancel instead of ×. + show(title, nodes, { scope, trigger, onClose: onDismiss ? () => { if (!started) onDismiss(); } : null, review: true, + ...(focusCancel ? { focusTarget: cancel } : {}) }); return { setValue(term, text) { const dd = values.get(term);