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

fix(workspace): bind shared dialogs to the page and its route

Browse Source 
- Drop the invented Refine note; every field is real (D14).
- Ignore a late close event once the shell was reopened or replaced, so
  an action that opens the next dialog keeps it open.
- Dialog openers require the page scope; disposing it closes and removes
  the shell unless a newer page's dialog is open. Every route change
  closes the shared dialog before the next page mounts.
- Clear filters also reports category '' so the page clears the toolbar
  category, as the concept does.
- The toolbar requires its scope and falls back to the first sort for an
  unknown value; dialog callbacks are validated up front.

Refs #249
This commit is contained in:
Vito Sansevero
2026-10-20 00:29:25 -07:00
parent bb2b7067a4
commit 996f3ce54d
4 changed files with 183 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/js/workspace-toolbar.test.js
+126 -23
View File
@@ -57,7 +57,8 @@ function fakeDOM() {
close() {
if (!this.open) return;
this.open = false;
this.dispatchEvent(new Event('close'));
// Browsers queue the close event as a later task.
setTimeout(() => this.dispatchEvent(new Event('close')), 0);
}
click() { this.dispatchEvent(new Event('click', { cancelable: true })); }
}
@@ -85,6 +86,8 @@ const byId = (dom, id) => dom.all(node => node.id === id)[0];
const tags = node => node.children.map(child => child.tagName.toLowerCase());
const optionPairs = select => select.options.map(option => [option.value, option.textContent]);
const modal = dom => byId(dom, 'modal');
const tick = () => new Promise(resolve => setTimeout(resolve, 5));
const coreModule = import('../../web/js/shared/toolkit/core.js');
const button = (root, text) => {
const found = [];
const walk = node => {
@@ -160,6 +163,14 @@ test('toolbar emits concept markup and order', async () => {
toolbar.setValue({ q: 'moon', category: 'Archive', sort: 'newest' });
assert.deepEqual([input.value, category.value, sort.value], ['moon', 'Archive', 'newest']);
assert.deepEqual(toolbar.value(), { q: 'moon', category: 'Archive', sort: 'newest' });
// A selection that is no longer listed drops to All categories.
toolbar.setCategories(['Portraits']);
assert.equal(category.value, '');
// An unknown sort falls back to the first option instead of an empty select.
toolbar.setValue({ sort: 'bogus' });
assert.equal(sort.value, 'newest');
assert.throws(() => mountToolbar(container, { categories: [], sorts: [['newest', 'Recently added']],
onChange() {}, onFilters() {} }), TypeError);
} finally {
scope.dispose();
dom.restore();
@@ -208,6 +219,9 @@ test('sort lists concept options first', async () => {
// A page without a concept option keeps the ones it has, concept first.
assert.deepEqual(orderSorts([['oldest', 'Oldest first'], ['newest', 'Recently added']]),
[['newest', 'Recently added'], ['oldest', 'Oldest first']]);
// Matching is by the concept's exact labels (en dash): a variant label is an appended option.
assert.deepEqual(orderSorts([['name', 'Name A-Z'], ['newest', 'Recently added']]),
[['newest', 'Recently added'], ['name', 'Name A-Z']]);
assert.throws(() => orderSorts([]), TypeError);
assert.throws(() => orderSorts([['newest']]), TypeError);
@@ -237,13 +251,14 @@ const REFINE_FIELDS = [
test('refine dialog round-trips values and clear resets them', async () => {
const { openRefineDialog, closeWorkspaceDialog } = await dialogsModule;
const dom = fakeDOM();
const scope = (await coreModule).createScope();
try {
const trigger = new dom.Element('button');
dom.body.append(trigger);
const applied = [];
const cleared = [];
const values = { source: '1', folder: 'portraits', media: 'video', tags: 'cinematic' };
openRefineDialog({ fields: REFINE_FIELDS, values, trigger,
openRefineDialog({ scope, fields: REFINE_FIELDS, values, trigger,
onApply: value => applied.push(value), onClear: value => cleared.push(value) });
const dialog = modal(dom);
assert.equal(dialog.open, true);
@@ -257,10 +272,10 @@ test('refine dialog round-trips values and clear resets them', async () => {
assert.deepEqual(optionPairs(source), REFINE_FIELDS[0].options);
assert.deepEqual([source.value, folder.value, media.value, tagsInput.value], ['1', 'portraits', 'video', 'cinematic']);
assert.equal(tagsInput.placeholder, 'e.g. cinematic');
assert.deepEqual(tags(body), ['div', 'div', 'div', 'div', 'p', 'div']);
assert.equal(body.children[4].className, 'small');
assert.ok(body.children[4].textContent.length > 0);
const actions = body.children[5];
// D14: no mock-status note, because every field is a real filter.
assert.deepEqual(tags(body), ['div', 'div', 'div', 'div', 'div']);
assert.equal(body.children.some(child => child.tagName === 'P'), false);
const actions = body.children[4];
assert.equal(actions.className, 'dialog-actions');
assert.deepEqual(actions.children.map(child => [child.textContent, child.className]),
[['Clear filters', ''], ['Apply filters', 'ca-primary']]);
@@ -275,29 +290,31 @@ test('refine dialog round-trips values and clear resets them', async () => {
assert.equal(trigger.isConnected && document.activeElement, trigger);
// Reopening with the applied values shows them again (a round trip).
openRefineDialog({ fields: REFINE_FIELDS, values: applied[0], trigger, onApply: value => applied.push(value), onClear() {} });
openRefineDialog({ scope, fields: REFINE_FIELDS, values: applied[0], trigger, onApply: value => applied.push(value), onClear() {} });
button(byId(dom, 'modal-body'), 'Apply filters').click();
assert.deepEqual(applied[1], applied[0]);
// Clear resets every field to its first option (Tags to empty) and reports that.
openRefineDialog({ fields: REFINE_FIELDS, values: applied[0], trigger, onApply: value => applied.push(value),
openRefineDialog({ scope, fields: REFINE_FIELDS, values: applied[0], trigger, onApply: value => applied.push(value),
onClear: value => cleared.push(value) });
const reopened = byId(dom, 'modal-body');
button(reopened, 'Clear filters').click();
assert.deepEqual(cleared, [{ source: 'all', folder: '', media: 'all', tags: '' }]);
// The concept's Clear also clears the toolbar category; the page applies `category`.
assert.deepEqual(cleared, [{ source: 'all', folder: '', media: 'all', tags: '', category: '' }]);
assert.equal(applied.length, 2);
assert.equal(modal(dom).open, false);
// A page offers only its own fields; the modal is the one shared shell.
openRefineDialog({ fields: [{ key: 'tags' }], values: {}, onApply: value => applied.push(value), onClear() {} });
openRefineDialog({ scope, fields: [{ key: 'tags' }], values: {}, onApply: value => applied.push(value), onClear() {} });
const only = byId(dom, 'modal-body').children.filter(child => child.className === 'field');
assert.deepEqual(only.map(field => field.children[0].textContent), ['Tags']);
button(byId(dom, 'modal-body'), 'Apply filters').click();
assert.deepEqual(applied[2], { tags: '' });
assert.equal(dom.all(node => node.id === 'modal').length, 1);
assert.throws(() => openRefineDialog({ fields: [{ key: 'evidence' }], values: {}, onApply() {}, onClear() {} }), TypeError);
assert.throws(() => openRefineDialog({ scope, fields: [{ key: 'evidence' }], values: {}, onApply() {}, onClear() {} }), TypeError);
} finally {
closeWorkspaceDialog();
scope.dispose();
dom.restore();
}
});
@@ -305,9 +322,10 @@ test('refine dialog round-trips values and clear resets them', async () => {
test('import dialog lists only real operations', async () => {
const { openImportDialog, closeWorkspaceDialog } = await dialogsModule;
const dom = fakeDOM();
const scope = (await coreModule).createScope();
try {
const calls = [];
openImportDialog({ onInspect: () => calls.push('inspect'), onRescan: () => calls.push('rescan') });
openImportDialog({ scope, onInspect: () => calls.push('inspect'), onRescan: () => calls.push('rescan') });
assert.equal(byId(dom, 'modal-title').textContent, 'Import & inspect');
const body = byId(dom, 'modal-body');
assert.deepEqual(tags(body), ['p', 'div']);
@@ -331,12 +349,13 @@ test('import dialog lists only real operations', async () => {
button(body, 'Rescan').click();
assert.deepEqual(calls, ['rescan']);
assert.equal(modal(dom).open, false);
openImportDialog({ onInspect: () => calls.push('inspect'), onRescan: () => calls.push('rescan') });
openImportDialog({ scope, onInspect: () => calls.push('inspect'), onRescan: () => calls.push('rescan') });
button(byId(dom, 'modal-body'), 'Open inspector').click();
assert.deepEqual(calls, ['rescan', 'inspect']);
assert.equal(modal(dom).open, false);
} finally {
closeWorkspaceDialog();
scope.dispose();
dom.restore();
}
});
@@ -344,9 +363,10 @@ test('import dialog lists only real operations', async () => {
test('copy fallback selects the text', async () => {
const { openCopyFallback, closeWorkspaceDialog } = await dialogsModule;
const dom = fakeDOM();
const scope = (await coreModule).createScope();
try {
const text = '<b>a castle</b>, "moonlight" & fog';
openCopyFallback(text);
openCopyFallback(text, { scope });
assert.equal(byId(dom, 'modal-title').textContent, 'Copy prompt');
const body = byId(dom, 'modal-body');
assert.deepEqual(tags(body), ['div', 'p']);
@@ -363,9 +383,10 @@ test('copy fallback selects the text', async () => {
assert.equal(document.activeElement, area);
assert.equal(note.className, 'small');
assert.equal(note.textContent, 'Automatic clipboard access requires a secure browser context.');
assert.throws(() => openCopyFallback(42), TypeError);
assert.throws(() => openCopyFallback(42, { scope }), TypeError);
} finally {
closeWorkspaceDialog();
scope.dispose();
dom.restore();
}
});
@@ -373,11 +394,12 @@ test('copy fallback selects the text', async () => {
test('dialogs restore focus to the trigger on Escape', async () => {
const { openRefineDialog, openImportDialog, openCopyFallback, closeWorkspaceDialog } = await dialogsModule;
const dom = fakeDOM();
const scope = (await coreModule).createScope();
try {
const opens = [
trigger => openRefineDialog({ fields: [{ key: 'tags' }], values: {}, trigger, onApply() {}, onClear() {} }),
trigger => openImportDialog({ trigger, onInspect() {}, onRescan() {} }),
trigger => openCopyFallback('text', { trigger }),
trigger => openRefineDialog({ scope, fields: [{ key: 'tags' }], values: {}, trigger, onApply() {}, onClear() {} }),
trigger => openImportDialog({ scope, trigger, onInspect() {}, onRescan() {} }),
trigger => openCopyFallback('text', { trigger, scope }),
];
for (const open of opens) {
const trigger = new dom.Element('button');
@@ -404,11 +426,11 @@ test('dialogs restore focus to the trigger on Escape', async () => {
// × closes and restores focus too; a removed trigger leaves focus alone.
const trigger = new dom.Element('button');
dom.body.append(trigger);
openImportDialog({ trigger, onInspect() {}, onRescan() {} });
openImportDialog({ scope, trigger, onInspect() {}, onRescan() {} });
byId(dom, 'modal-close').click();
assert.equal(modal(dom).open, false);
assert.equal(document.activeElement, trigger);
openImportDialog({ trigger, onInspect() {}, onRescan() {} });
openImportDialog({ scope, trigger, onInspect() {}, onRescan() {} });
trigger.remove();
const close = byId(dom, 'modal-close');
close.click();
@@ -417,16 +439,97 @@ test('dialogs restore focus to the trigger on Escape', async () => {
// Opening over an open dialog swaps its content and keeps the first trigger; dispose removes the shell.
const first = new dom.Element('button');
dom.body.append(first);
openImportDialog({ trigger: first, onInspect() {}, onRescan() {} });
openCopyFallback('swap');
openImportDialog({ scope, trigger: first, onInspect() {}, onRescan() {} });
openCopyFallback('swap', { scope });
assert.equal(byId(dom, 'modal-title').textContent, 'Copy prompt');
modal(dom).dispatchEvent(new Event('cancel', { cancelable: true }));
assert.equal(document.activeElement, first);
openImportDialog({ trigger: first, onInspect() {}, onRescan() {} });
openImportDialog({ scope, trigger: first, onInspect() {}, onRescan() {} });
closeWorkspaceDialog();
assert.equal(modal(dom), undefined);
} finally {
closeWorkspaceDialog();
scope.dispose();
dom.restore();
}
});
test('an action may open the next dialog without the late close event closing it', async () => {
const { openImportDialog, openCopyFallback, closeWorkspaceDialog } = await dialogsModule;
const dom = fakeDOM();
const scope = (await coreModule).createScope();
try {
const first = new dom.Element('button');
const second = new dom.Element('button');
dom.body.append(first, second);
openImportDialog({ scope, trigger: first, onInspect() {},
onRescan: () => openCopyFallback('next', { scope, trigger: second }) });
button(byId(dom, 'modal-body'), 'Rescan').click();
await tick();
assert.equal(modal(dom).open, true);
assert.equal(byId(dom, 'modal-title').textContent, 'Copy prompt');
assert.equal(document.activeElement, byId(dom, 'copy-text'));
// Its own Escape still restores focus to its own trigger.
modal(dom).dispatchEvent(new Event('cancel', { cancelable: true }));
await tick();
assert.equal(document.activeElement, second);
// Replacing the shell ignores the old shell's late close event too.
openImportDialog({ scope, trigger: first, onInspect() {}, onRescan() {} });
closeWorkspaceDialog();
openImportDialog({ scope, trigger: second, onInspect() {}, onRescan() {} });
await tick();
assert.equal(modal(dom).open, true);
assert.equal(byId(dom, 'modal-title').textContent, 'Import & inspect');
} finally {
closeWorkspaceDialog();
scope.dispose();
dom.restore();
}
});
test('dialogs need the page scope and close with it', async () => {
const { openRefineDialog, openImportDialog, openCopyFallback, closeWorkspaceDialog } = await dialogsModule;
const { createScope } = await coreModule;
const dom = fakeDOM();
try {
assert.throws(() => openImportDialog({ onInspect() {}, onRescan() {} }), /page scope/);
assert.throws(() => openRefineDialog({ fields: [], onApply() {}, onClear() {} }), /page scope/);
assert.throws(() => openCopyFallback('text'), /page scope/);
const scope = createScope();
assert.throws(() => openImportDialog({ scope, onInspect() {} }), TypeError);
assert.throws(() => openRefineDialog({ scope, fields: [], onApply() {} }), TypeError);
assert.equal(modal(dom), undefined);
// Disposing the page scope closes and removes the shell, however often the page opened it.
const trigger = new dom.Element('button');
dom.body.append(trigger);
openImportDialog({ scope, trigger, onInspect() {}, onRescan() {} });
openCopyFallback('again', { scope });
scope.dispose();
assert.equal(modal(dom), undefined);
assert.equal(document.activeElement, trigger);
// An old page's scope never closes the next page's dialog.
const older = createScope();
const newer = createScope();
openImportDialog({ scope: older, onInspect() {}, onRescan() {} });
openCopyFallback('next page', { scope: newer });
older.dispose();
assert.equal(modal(dom).open, true);
newer.dispose();
assert.equal(modal(dom), undefined);
} finally {
closeWorkspaceDialog();
dom.restore();
}
});
test('every route change closes the shared workspace dialog', async () => {
const { readFile } = require('node:fs/promises');
const path = require('node:path');
const source = await readFile(path.join(__dirname, '../../web/js/workspace/index.mjs'), 'utf8');
assert.match(source, /import \{[^}]*\bcloseWorkspaceDialog\b[^}]*\} from '\.\/workspace-dialogs\.mjs';/);
const render = source.slice(source.indexOf('function render(route) {'));
const body = render.slice(0, render.indexOf('viewScope = createScope();'));
assert.match(body, /closeWorkspaceDialog\(\);/, 'render closes the dialog before the next page mounts');
});
web/js/workspace/index.mjs
+3
View File
@@ -9,6 +9,7 @@ import { mountVocabulary } from './vocabulary.mjs';
import { createWorkspaceIcon } from './icons.mjs';
import { bindCanvasLink, canvasHref } from './topbar.mjs';
import { renderHeading } from './heading.mjs';
import { closeWorkspaceDialog } from './workspace-dialogs.mjs';
const { createApiClient, resolveUrl } = window.ApiClient;
@@ -85,6 +86,8 @@ function mountWorkspace() {
scope.cleanup(() => viewScope?.dispose());
function render(route) {
// A shared page dialog never stays open over the next route.
closeWorkspaceDialog();
viewScope?.dispose();
images = null;
viewScope = createScope();
web/js/workspace/toolbar.mjs
+6 -2
View File
@@ -10,7 +10,8 @@ import { createWorkspaceIcon } from './icons.mjs';
/** The search waits this long after the last keystroke before it reports a change. */
export const SEARCH_DELAY = 250;
// The concept's sort labels, in the concept's order.
// The concept's sort labels, in the concept's order. Matching is by exact label (note the en dash in "Name A–Z"); any
// other label is one of the page's appended options.
const CONCEPT_SORT_LABELS = Object.freeze(['Recently added', 'Name A–Z', 'Highest rated']);
const isText = value => typeof value === 'string';
@@ -51,6 +52,9 @@ function categoryOptions(categories) {
* The toolbar leaves `container` when `scope` is disposed. Returns `{element, value, setValue, setCategories}`.
*/
export function mountToolbar(container, { scope, categories = [], sorts, value = {}, onChange, onFilters }) {
if (typeof scope?.listen !== 'function' || typeof scope.debounce !== 'function' || typeof scope.cleanup !== 'function') {
throw new TypeError('The toolbar needs the page scope');
}
if (typeof onChange !== 'function' || typeof onFilters !== 'function') {
throw new TypeError('The toolbar needs onChange and onFilters');
}
@@ -84,7 +88,7 @@ export function mountToolbar(container, { scope, categories = [], sorts, value =
function setValue(next) {
if (isText(next?.q)) search.value = next.q;
if (isText(next?.category)) category.value = next.category;
if (isText(next?.sort)) sort.value = next.sort;
if (isText(next?.sort)) sort.value = sort.options.some(node => node.value === next.sort) ? next.sort : sort.options[0].value;
}
function setCategories(next) {
const selected = category.value;
web/js/workspace/workspace-dialogs.mjs
+48 -16
View File
@@ -4,17 +4,19 @@
* swaps its content, as the concept's `dialog()` does. Closing, by ×, Escape or an action, returns focus to the
* control that opened it.
*
* - "Refine this view" (a1348–1364): Source / Folder / Media / Tags, each optional per page; "Apply filters" because
* every field is a real filter, and no "Prompt evidence" field because evidence is only known per file (D14).
* - "Refine this view" (a1348–1364): Source / Folder / Media / Tags, each optional per page; "Apply filters" and no
* mock-status note because every field is a real filter, and no "Prompt evidence" field because evidence is only
* known per file (D14). Clear also clears the toolbar category, as the concept's does; the page applies it.
* - "Import & inspect" (a1483–1491): only the real operations, Inspect a local image and Scan an output folder →
* Rescan; no "Import portable recipes" (D15).
* - "Copy prompt" (a1223–1241): the select-to-copy fallback when the clipboard is unavailable (D22).
*
* Every opener takes the page `scope`: disposing it closes and removes the shell, so a dialog never outlives its page.
*/
import { createButton } from '../shared/toolkit/core.js';
const FIELD_LABELS = Object.freeze({ source: 'Source', folder: 'Folder', media: 'Media', tags: 'Tags' });
const FIELD_ORDER = Object.keys(FIELD_LABELS);
const REFINE_NOTE = 'These filters apply to this view and combine with the search and category above.';
let shell = null;
let session = null;
@@ -54,8 +56,11 @@ function createShell() {
event.preventDefault();
finish();
});
// A browser may still close a modal on its own; that is a dismissal too.
dialog.addEventListener('close', () => finish());
// A browser may still close a modal on its own; that is a dismissal too. `close` arrives as a later task, so it
// is ignored once this shell was reopened or replaced.
dialog.addEventListener('close', () => {
if (shell?.dialog === dialog && !dialog.open) finish();
});
return { dialog, title, body, close };
}
@@ -68,10 +73,33 @@ function finish() {
if (trigger?.isConnected) trigger.focus();
}
function show(heading, nodes, trigger, focusTarget) {
const boundScopes = new WeakSet();
function requireScope(scope) {
if (typeof scope?.cleanup !== 'function') throw new TypeError('Workspace dialogs need the page scope');
}
function requireCallbacks(callbacks) {
for (const [name, callback] of Object.entries(callbacks)) {
if (typeof callback !== 'function') throw new TypeError(`The dialog needs ${name}`);
}
}
/** Disposing a page scope closes the shell unless a newer page's dialog is open in it. */
function bindScope(scope) {
if (boundScopes.has(scope)) return;
boundScopes.add(scope);
scope.cleanup(() => {
boundScopes.delete(scope);
if (!session || session.scope === scope) closeWorkspaceDialog();
});
}
function show(heading, nodes, { scope, trigger, focusTarget }) {
shell ??= createShell();
if (!shell.dialog.isConnected) document.body.append(shell.dialog);
session = { trigger: trigger ?? session?.trigger ?? null };
bindScope(scope);
session = { scope, trigger: trigger ?? session?.trigger ?? null };
shell.title.textContent = heading;
shell.body.replaceChildren(...nodes);
if (!shell.dialog.open) shell.dialog.showModal();
@@ -103,9 +131,11 @@ function selectControl(key, options) {
* "Refine this view". `fields` lists the page's filters in any subset of Source, Folder, Media and Tags, as
* `{key, options}` with `[value, label]` options for the selects (the first is the cleared value) and no options for
* Tags. `values` holds the current value per key. Apply reports the edited values; Clear resets every field and
* reports the cleared values. Both close the dialog first.
* reports the cleared values plus `category: ''`, so the page clears its toolbar category too. Both close first.
*/
export function openRefineDialog({ fields, values = {}, onApply, onClear, trigger = null, note = REFINE_NOTE }) {
export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, trigger = null }) {
requireScope(scope);
requireCallbacks({ onApply, onClear });
if (!Array.isArray(fields) || fields.some(item => !FIELD_ORDER.includes(item?.key))) {
throw new TypeError('Refine fields are source, folder, media and tags');
}
@@ -132,7 +162,7 @@ export function openRefineDialog({ fields, values = {}, onApply, onClear, trigge
for (const { control, cleared } of controls) control.value = cleared;
const reset = read();
finish();
onClear(reset);
onClear({ ...reset, category: '' });
});
apply.addEventListener('click', () => {
const applied = read();
@@ -143,9 +173,8 @@ export function openRefineDialog({ fields, values = {}, onApply, onClear, trigge
actions.append(clear, apply);
show('Refine this view', [
...controls.map(({ key, control }) => field(FIELD_LABELS[key], control, `filter-${key}`)),
el('p', note, 'small'),
actions,
], trigger);
], { scope, trigger });
}
function operation(title, description, label, onAction) {
@@ -164,7 +193,9 @@ function operation(title, description, label, onAction) {
}
/** "Import & inspect" with the two real operations (D15). */
export function openImportDialog({ onInspect, onRescan, trigger = null }) {
export function openImportDialog({ scope, onInspect, onRescan, trigger = null }) {
requireScope(scope);
requireCallbacks({ onInspect, onRescan });
const list = el('div', undefined, 'operation-list');
list.append(
operation('Inspect a local image', 'Read embedded metadata before deciding to add it.', 'Open inspector', onInspect),
@@ -173,11 +204,12 @@ export function openImportDialog({ onInspect, onRescan, trigger = null }) {
show('Import & inspect', [
el('p', 'Image inspection and output scanning have different effects. Choose the workflow you want.', 'small'),
list,
], trigger);
], { scope, trigger });
}
/** "Copy prompt": the text in a readonly, pre-selected textarea for a manual copy. */
export function openCopyFallback(text, { trigger = null } = {}) {
export function openCopyFallback(text, { scope, trigger = null } = {}) {
requireScope(scope);
if (typeof text !== 'string') throw new TypeError('Copy text must be a string');
const area = el('textarea');
area.readOnly = true;
@@ -185,6 +217,6 @@ export function openCopyFallback(text, { trigger = null } = {}) {
show('Copy prompt', [
field('Select and copy this text', area, 'copy-text'),
el('p', 'Automatic clipboard access requires a secure browser context.', 'small'),
], trigger, area);
], { scope, trigger, focusTarget: area });
area.select();
}