From 7c362a53ae41e653e0501981086cd5e0e8f5b2c8 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 20 Oct 2026 09:51:40 -0700 Subject: [PATCH] fix(images): Refine's Folder list follows the chosen Source The workspace subfolder discovery returned only the union of every source's folders, so "Source: temp" could be paired with a folder that exists only under output and silently match nothing. In workspace mode each root now lists its own folders, and "Refine this view" offers only the chosen Source's folders: changing Source keeps a folder it also has and otherwise returns Folder to "All output folders", as the old field row did when it reloaded a Source's folders. Failing before: the API test found no per-root folders (KeyError), the dialog unit test and the browser test still offered output-only folders under the input source. --- py/api/images.py | 67 ++++++++++++++----------- tests/browser/workspace-images.spec.mjs | 27 ++++++++++ tests/js/workspace-toolbar.test.js | 36 +++++++++++++ tests/test_api_workspace_images.py | 27 ++++++++++ web/js/workspace/images.mjs | 17 +++++-- web/js/workspace/workspace-dialogs.mjs | 35 ++++++++++--- 6 files changed, 170 insertions(+), 39 deletions(-) diff --git a/py/api/images.py b/py/api/images.py index e46e49f..75a51d1 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -1422,45 +1422,56 @@ class ImageRoutesMixin: {"success": False, "error": "Failed to rescan the gallery"}, status=500 ) + async def _root_subfolders(self, output_path, include_ancestors): + """The distinct folders holding listed files under one output root.""" + subfolders = set() + for f, _mtime in await self._get_gallery_files(output_path): + rel_dir = str(f.relative_to(output_path).parent) + if rel_dir and rel_dir != ".": + subfolders.add(rel_dir) + if include_ancestors: + ancestors = set() + for folder in subfolders: + parts = folder.replace("\\", "/").split("/") + for i in range(1, len(parts)): + ancestors.add("/".join(parts[:i])) + subfolders.update(ancestors) + return subfolders + async def get_gallery_subfolders(self, request): - """Get distinct subfolders from gallery output directories.""" + """Get distinct subfolders from gallery output directories. + + In workspace mode every root also lists its own folders, so the + Refine dialog can offer only the chosen Source's folders. + """ try: output_dirs = list(self._get_all_output_dirs()) workspace = request.query.get("mode") == "workspace" - roots = None + include_ancestors = ( + request.query.get("include_ancestors", "").lower() == "true" + ) + selected = None if workspace: try: selected = _workspace_root(request.query, output_dirs) except ValueError as exc: return bad_request(str(exc)) - roots = [ - {"index": index, "label": _public_path(root)} - for index, root in enumerate(output_dirs) - ] - if selected is not None: - output_dirs = [output_dirs[selected]] - subfolders = set() - for output_path in output_dirs: - files = await self._get_gallery_files(output_path) - for f, _mtime in files: - rel_dir = str(f.relative_to(output_path).parent) - if rel_dir and rel_dir != ".": - subfolders.add(rel_dir) - - include_ancestors = ( - request.query.get("include_ancestors", "").lower() == "true" - ) - if include_ancestors: - ancestors = set() - for folder in subfolders: - parts = folder.replace("\\", "/").split("/") - for i in range(1, len(parts)): - ancestors.add("/".join(parts[:i])) - subfolders.update(ancestors) - + walked = [ + await self._root_subfolders(output_path, include_ancestors) + for output_path in output_dirs + ] + chosen = walked if selected is None else [walked[selected]] + subfolders = set().union(*chosen) payload = {"success": True, "subfolders": sorted(subfolders)} if workspace: - payload["roots"] = roots + payload["roots"] = [ + { + "index": index, + "label": _public_path(root), + "subfolders": sorted(walked[index]), + } + for index, root in enumerate(output_dirs) + ] return web.json_response(payload) except Exception as e: self.logger.error(f"Subfolders error: {e}") diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index e9ebf2b..f449c9d 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1458,3 +1458,30 @@ test('a star press leaves the continuation status, count and caption date untouc await expect(toast(page)).toHaveText('Added to favorites.'); expect(await page.evaluate(() => window.__statusMutations)).toEqual([]); }); + +// #249 final review m1: as in the old field row, a Source offers only its own folders. +test('Refine offers only the chosen source folders', async ({ page, request }) => { + await expect(await request.post('/__test__/add-nested-image')).toBeOK(); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(3); + await page.getByRole('button', { name: 'Filters', exact: true }).click(); + const source = refineDialog(page).getByLabel('Source', { exact: true }); + const folder = refineDialog(page).getByLabel('Folder', { exact: true }); + const values = () => folder.locator('option').evaluateAll(options => options.map(option => option.value)); + expect(await values()).toEqual(['', 'project', 'project/day', 'shared']); + await folder.selectOption('project'); + await source.selectOption('1'); + expect(await values()).toEqual(['', 'shared']); + await expect(folder).toHaveValue(''); + await source.selectOption('0'); + expect(await values()).toEqual(['', 'project', 'project/day', 'shared']); + await folder.selectOption('shared'); + await source.selectOption('1'); + await expect(folder).toHaveValue('shared'); + await refineDialog(page).getByRole('button', { name: 'Apply filters', exact: true }).click(); + await expect(cards(page)).toHaveCount(1); + await expect(cards(page)).toHaveAttribute('data-image-key', '1:shared/same.png'); + // Reopened on that Source, the dialog again lists only its folders. + await page.getByRole('button', { name: 'Filters', exact: true }).click(); + expect(await values()).toEqual(['', 'shared']); +}); diff --git a/tests/js/workspace-toolbar.test.js b/tests/js/workspace-toolbar.test.js index 3e602fb..7085dda 100644 --- a/tests/js/workspace-toolbar.test.js +++ b/tests/js/workspace-toolbar.test.js @@ -331,6 +331,42 @@ test('refine dialog round-trips values and clear resets them', async () => { } }); +test('the Folder list follows the chosen Source', async () => { + const { openRefineDialog, closeWorkspaceDialog } = await dialogsModule; + const dom = fakeDOM(); + const scope = (await coreModule).createScope(); + try { + const bySource = { all: ['portraits', 'temp-only'], 0: ['portraits'], 1: ['temp-only'] }; + const optionsFor = source => [['', 'All output folders'], ...(bySource[source] ?? []).map(path => [path, path])]; + const fields = [REFINE_FIELDS[0], { key: 'folder', options: optionsFor('all'), optionsFor }, REFINE_FIELDS[2]]; + const applied = []; + openRefineDialog({ scope, fields, values: { source: '1', folder: 'temp-only', media: 'all' }, + onApply: value => applied.push(value), onClear() {} }); + const [source, folder] = byId(dom, 'modal-body').children.filter(child => child.className === 'field') + .map(field => field.children[1]); + // Opened on a chosen Source: only its folders. + assert.deepEqual(optionPairs(folder), [['', 'All output folders'], ['temp-only', 'temp-only']]); + assert.equal(folder.value, 'temp-only'); + // Another Source drops a folder it does not have. + source.value = '0'; + source.dispatchEvent(new Event('change')); + assert.deepEqual(optionPairs(folder), [['', 'All output folders'], ['portraits', 'portraits']]); + assert.equal(folder.value, ''); + folder.value = 'portraits'; + // A folder the next Source also has stays chosen. + source.value = 'all'; + source.dispatchEvent(new Event('change')); + assert.deepEqual(optionPairs(folder).map(([value]) => value), ['', 'portraits', 'temp-only']); + assert.equal(folder.value, 'portraits'); + button(byId(dom, 'modal-body'), 'Apply filters').click(); + assert.deepEqual(applied, [{ source: 'all', folder: 'portraits', media: 'all' }]); + } finally { + closeWorkspaceDialog(); + scope.dispose(); + dom.restore(); + } +}); + test('a running rescan marks Rescan busy and ignores a second press', async () => { const { openImportDialog, closeWorkspaceDialog } = await dialogsModule; const dom = fakeDOM(); diff --git a/tests/test_api_workspace_images.py b/tests/test_api_workspace_images.py index 131588e..d879af2 100644 --- a/tests/test_api_workspace_images.py +++ b/tests/test_api_workspace_images.py @@ -247,6 +247,33 @@ class TestWorkspaceImages(ImageAPITestCase): ) self.assertEqual(response.status, 400) + async def test_workspace_roots_list_their_own_folders(self): + # "Refine this view" offers only the chosen Source's folders (#249 m1). + second = self.comfy_dir / "second" + second.mkdir() + self.media("first/deep/a.png") + self.media("nested/b.png", root=second) + self.api._get_all_output_dirs = lambda: [self.output_dir, second] + deep = os.path.join("first", "deep") + response = await self.client.get( + "/prompt_manager/gallery/subfolders?mode=workspace&include_ancestors=true" + ) + folders = await response.json() + self.assertEqual(folders["subfolders"], sorted(["first", deep, "nested"])) + self.assertEqual( + [(r["index"], r["subfolders"]) for r in folders["roots"]], + [(0, sorted(["first", deep])), (1, ["nested"])], + ) + # A chosen root narrows the union; every root still lists its own. + response = await self.client.get( + "/prompt_manager/gallery/subfolders?mode=workspace&root=1" + ) + folders = await response.json() + self.assertEqual(folders["subfolders"], ["nested"]) + self.assertEqual( + [r["subfolders"] for r in folders["roots"]], [[deep], ["nested"]] + ) + async def test_workspace_dimensions_are_header_only_and_video_is_unknown(self): self.media("a.png") self.media("b.mp4") diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index c5ab1c9..97952c9 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -95,7 +95,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage let metrics = {...INITIAL_METRICS}; let layout = visit?.layout || 'grid', selection = visit?.selection || null, frame = 0, loadFrame = 0, restoreFrame = 0, observer; let state, geometry = { positions: [], columns: 1, height: 0, maxHeight: 0 }, galleryWidth = 0, optionsRequest, categoryRequest, recovering = false, rescanning = false; - // Discovered sources and folders for "Refine this view" and the scope chip. + // Discovered sources (each with its own folders) and every folder, for "Refine this view" and the scope chip. let roots = [], folders = []; const rendered = new Map(); const itemIndexes = new Map(); @@ -554,7 +554,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage if (recovering) return; openRefineDialog({scope, trigger, fields:[ {key:'source', options:[['all', 'All sources'], ...roots.map(root => [String(root.index), root.label])]}, - {key:'folder', options:[['', 'All output folders'], ...folders.map(path => [path, path])]}, + {key:'folder', options:folderOptions('all'), optionsFor:folderOptions}, {key:'media', options:MEDIA_OPTIONS}, {key:'tags'}, ], values:{source:filters.root, folder:filters.subfolder, media:filters.media, tags:filters.tags.join(', ')}, @@ -564,6 +564,11 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage applyRefinement(cleared); }}); } + /** The chosen Source's folders, as the old field row reloaded them; every folder under All sources. */ + function folderOptions(source) { + const root = source === 'all' ? null : roots.find(item => String(item.index) === source); + return [['', 'All output folders'], ...(root?.subfolders ?? folders).map(path => [path, path])]; + } function applyRefinement({source, folder, media, tags}) { if (recovering) return; Object.assign(filters, {root:source, subfolder:folder, media, @@ -617,13 +622,15 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage optionsRequest?.abort(); const request = new AbortController(); optionsRequest = request; try { - // Every source's folders (with ancestors), so any Source and Folder pair can be chosen in one dialog. + // Every source's folders (with ancestors), listed per source too, so the dialog can follow its Source. const params = new URLSearchParams({mode:'workspace', include_ancestors:'true'}); const data = await api.get(`/prompt_manager/gallery/subfolders?${params}`, {signal:request.signal}); if (scope.signal.aborted || request !== optionsRequest) return false; if (!Array.isArray(data?.roots) || !Array.isArray(data.subfolders)) throw new TypeError('Invalid image sources'); - roots = data.roots.filter(root => Number.isSafeInteger(root?.index) && typeof root.label === 'string'); - folders = data.subfolders.filter(path => typeof path === 'string' && path); + const paths = list => Array.isArray(list) ? list.filter(path => typeof path === 'string' && path) : null; + roots = data.roots.filter(root => Number.isSafeInteger(root?.index) && typeof root.label === 'string') + .map(root => ({index:root.index, label:root.label, subfolders:paths(root.subfolders)})); + folders = paths(data.subfolders); updateScope(); optionsStatus.hidden = true; return true; diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index b5295ca..c238678 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -113,24 +113,35 @@ export function closeWorkspaceDialog() { shell = null; } -function selectControl(key, options) { +function fillSelect(select, key, options) { if (!Array.isArray(options) || options.length === 0 || options.some(pair => !Array.isArray(pair) || typeof pair[0] !== 'string' || typeof pair[1] !== 'string')) { throw new TypeError(`The ${key} filter needs [value, label] options`); } - const select = el('select'); select.replaceChildren(...options.map(([value, label]) => { const node = el('option', label); node.value = value; return node; })); +} + +function selectControl(key, options) { + const select = el('select'); + fillSelect(select, key, options); return select; } +/** Show `value` when the select offers it, else its first (cleared) option. */ +function choose(select, value) { + const known = [...select.options].some(option => option.value === value); + select.value = known ? value : select.options[0].value; +} + /** * "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 + * Tags; Folder may add `optionsFor(source)`, the options of one Source, so its list follows the chosen Source and a + * folder that Source lacks returns to the first option. `values` holds the current value per key. Apply reports the edited values; Clear resets every field and * reports the cleared values plus `category: ''`, so the page clears its toolbar category too. Both close first. */ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, trigger = null }) { @@ -149,16 +160,28 @@ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, return [{ key, control: input, cleared: '' }]; } const select = selectControl(key, item.options); - const known = item.options.some(([value]) => value === values[key]); - select.value = known ? values[key] : item.options[0][0]; - return [{ key, control: select, cleared: item.options[0][0] }]; + choose(select, values[key]); + return [{ key, control: select, cleared: item.options[0][0], optionsFor: item.optionsFor }]; }); + const source = controls.find(item => item.key === 'source'); + const folder = controls.find(item => item.key === 'folder' && typeof item.optionsFor === 'function'); + const followSource = () => { + const chosen = folder.control.value; + fillSelect(folder.control, 'folder', folder.optionsFor(source.control.value)); + choose(folder.control, chosen); + }; + if (source && folder) { + followSource(); + source.control.addEventListener('change', followSource); + } const read = () => Object.fromEntries(controls.map(({ key, control }) => [key, key === 'tags' ? control.value.trim() : control.value])); const clear = createButton('Clear filters'); const apply = createButton('Apply filters', 'ca-primary'); clear.addEventListener('click', () => { + // The cleared Source's folders first, so the cleared Folder value is one of them. + if (source && folder) { source.control.value = source.cleared; followSource(); } for (const { control, cleared } of controls) control.value = cleared; const reset = read(); finish();