nodes/ComfyAssets-ComfyUI_PromptManager
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(images): the delete review starts nothing without a chosen mode, and dims its primary until then (#480)

Browse Source 
An aria-disabled primary ignores every activation, a chosen one starts once,
and the apply helper refuses a missing mode. Tests cover a forced click, a
double click, Enter and Space on the primary, Enter in the dialog and a
submit event, and the server refusing an apply without an exact mode.
This commit is contained in:
Vito Sansevero
2026-10-29 12:09:29 -07:00
parent 392e4bc7a8
commit f8cbeb0482
5 changed files with 94 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/browser/workspace-images-delete.spec.mjs
+47
View File
@@ -122,3 +122,50 @@ test('Delete… in the viewer closes it after the delete', async ({ page }) => {
await expect(viewer).toBeHidden();
await expect(page.locator(CARDS)).toHaveCount(before - 1);
});
// The review never starts a delete without an explicit choice (live test 2026-10-29): every activation of the
// aria-disabled primary is ignored, the dialog has no form to submit, and a chosen primary starts once.
test('without a chosen mode nothing can start a delete', async ({ page }) => {
const before = await openImages(page);
await check(page, 0);
const applies = [];
page.on('request', request => { if (request.url().includes('/maintenance/apply')) applies.push(request.postDataJSON()); });
await bar(page).getByRole('button', { name: 'Delete…', exact: true }).click();
const dialog = review(page);
await expect(dialog).toBeVisible();
const primary = dialog.locator('.dialog-actions button.danger');
await expect(primary).toHaveAttribute('aria-disabled', 'true');
await expect(primary).toHaveText('Choose what happens to the files');
// Visibly dimmed while no mode is chosen.
await expect(primary).toHaveCSS('opacity', '0.55');
await expect(primary).toHaveCSS('cursor', 'not-allowed');
await primary.click({ force: true });
await primary.dblclick({ force: true });
await primary.focus();
await page.keyboard.press('Enter');
await page.keyboard.press('Space');
await dialog.locator('#modal-title').click();
await page.keyboard.press('Enter');
expect(await dialog.locator('form').count()).toBe(0);
await page.evaluate(() => document.querySelector('dialog#modal[open]')?.dispatchEvent(new Event('submit', { bubbles: true })));
await expect(dialog).toBeVisible();
expect(applies).toEqual([]);
for (const radio of await dialog.getByRole('radio').all()) await expect(radio).not.toBeChecked();
await dialog.getByRole('button', { name: 'Cancel', exact: true }).click();
await expect(page.locator(CARDS)).toHaveCount(before);
expect(applies).toEqual([]);
});
test('a chosen mode starts exactly one delete, with that mode, however often it is pressed', async ({ page }) => {
const before = await openImages(page);
await check(page, 0);
const applies = [];
page.on('request', request => { if (request.url().includes('/maintenance/apply')) applies.push(request.postDataJSON()); });
const { primary } = await deleteChecked(page, 'library');
await primary.dblclick();
await expect(toast(page)).toContainText('Removed 1 image from the library.');
await expect(page.locator(CARDS)).toHaveCount(before - 1);
expect(applies.length).toBe(1);
expect(applies[0].mode).toBe('library');
expect(Object.keys(applies[0]).sort()).toEqual(['mode', 'token']);
});
tests/js/workspace-images-delete.test.js
+8
View File
@@ -139,3 +139,11 @@ test('apply sends the token and the mode, and toasts that the job started', asyn
store, plan: plan(), mode: 'library', notify: text => toasts.push(text) });
assert.equal(refused, null);
});
test('apply refuses to send a delete without an explicit mode', async () => {
const { applyDelete } = module();
const api = { post: () => assert.fail('no request without a mode') };
for (const mode of [undefined, null, '', 'TRASH', 'delete']) {
await assert.rejects(applyDelete({ api, store: null, plan: plan(), mode, notify: () => {} }), TypeError);
}
});
tests/test_image_delete_routes.py
+30 -2
View File
@@ -106,10 +106,38 @@ class TestReview(ImageDeleteTestCase):
async def test_mode_is_required_and_only_for_this_plan(self):
self.write("a.png", b"aaa")
self.link("a.png")
plan = await self.review(["a.png"])
await self.post(APPLY, {"token": plan["token"]}, 400)
await self.post(APPLY, {"token": plan["token"], "mode": "delete"}, 400)
# No explicit, exact mode: refused, never a default (least of all trash).
for body in (
{"token": plan["token"]},
{"token": plan["token"], "mode": None},
{"token": plan["token"], "mode": ""},
{"token": plan["token"], "mode": "TRASH"},
{"token": plan["token"], "mode": "delete"},
{"token": plan["token"], "mode": ["trash"]},
{"token": plan["token"], "mode": True},
):
with self.subTest(body=body):
await self.post(APPLY, body, 400)
self.assertTrue((self.output_dir / "a.png").exists())
self.assertEqual(self.fake.moved, [])
self.assertEqual(self.count("SELECT COUNT(*) FROM generated_images"), 1)
# The refusals left the token usable: an explicit mode still applies.
body = await self.post(APPLY, {"token": plan["token"], "mode": "library"}, 202)
job = self.api._jobs.get(body["job"]["id"])
await asyncio.wait_for(asyncio.shield(job.task), DISK_BOUND_SECONDS)
self.assertEqual(job.scope["mode"], "library")
self.assertTrue((self.output_dir / "a.png").exists())
self.assertEqual(self.fake.moved, [])
async def test_mode_on_another_plan_is_refused(self):
plan = await self.post(
"/prompt_manager/maintenance/preview",
{"operation": "missing_links", "options": {}},
200,
)
await self.post(APPLY, {"token": plan["token"], "mode": "trash"}, 400)
class TestLibraryMode(ImageDeleteTestCase):
web/js/workspace/images-delete.mjs
+8 -3
View File
@@ -195,12 +195,12 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply})
openCloseOnlyReview({scope, title, grid: deleteRows(plan, 'library'), sentence: reason, trigger});
return null;
}
let mode = null;
let mode = null, started = false;
const list = el('dl', undefined, 'meta-grid');
list.hidden = true;
const notes = el('div', undefined, 'image-delete-notes');
const cancel = createButton('Cancel');
const start = createButton(primaryLabel('library', modeCount(plan, 'library')), 'danger');
const start = createButton('Choose what happens to the files', 'danger image-delete-start');
start.setAttribute('aria-disabled', 'true');
function render() {
list.replaceChildren();
@@ -219,8 +219,12 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply})
}
const group = modeGroup(value => { mode = value; render(); });
cancel.addEventListener('click', () => closeWorkspaceDialog());
// Nothing starts without an explicit mode: an aria-disabled primary ignores every activation (click, Enter, Space),
// and a chosen one starts once however often it is pressed. The dialog has no form, so Enter elsewhere submits nothing.
start.addEventListener('click', () => {
if (!mode || start.getAttribute('aria-disabled') === 'true') return;
if (started || !MODES.some(choice => choice.value === mode) || start.getAttribute('aria-disabled') === 'true') return;
started = true;
start.setAttribute('aria-disabled', 'true');
const chosen = mode;
closeWorkspaceDialog();
onApply(chosen);
@@ -233,6 +237,7 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply})
/** Apply `plan` in `mode`; the started job, or null (a toast said why). */
export async function applyDelete({api, store, plan, mode, notify = showWorkspaceToast}) {
if (!MODES.some(choice => choice.value === mode)) throw new TypeError('A delete needs an explicit mode');
let result;
try {
result = await api.post(APPLY_URL, {token: plan.token, mode});
web/js/workspace/workspace.css
+1
View File
@@ -478,6 +478,7 @@ pre.prompt-text { font: inherit; font-size: 12px; line-height: 1.75; margin-bloc
/* D147 (#430): a state that blocks Save shows its reason on its own row above the sticky actions, with Refresh for a
changed source; the disabled actions look disabled; a refused capture's empty preview says so. */
.tagging-modal .dialog-actions [aria-disabled="true"], .tagging-modal .tag-model-panel [aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D147 */ }
#modal .dialog-actions .image-delete-start[aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* #480 D159: no mode chosen yet */ }
.tagging-modal .tag-action-notice { flex: 1 0 100%; display: flex; flex-wrap: wrap; align-items: center; justify-content: space-between; gap: 8px; margin: 0; color: var(--ca-text); font-size: 12px; /* D147 */ }
.tagging-modal .tag-action-notice > span { flex: 1 1 16em; }
.tag-image-preview.tag-preview-empty { display: grid; place-items: center; padding: 8px; text-align: center; font-size: 12px; color: var(--ca-muted); background: var(--ca-field); box-sizing: border-box; /* D147 */ }