From febc514e42283958b07ae579063a0f4c83205abd Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 20 Oct 2026 09:51:42 -0700 Subject: [PATCH] fix(images): an unchanged Refine apply keeps the loaded page "Apply filters" (and "Clear filters" with nothing set) reset the stream, refetched the first page and dropped the reader's scroll position even when no filter changed. Like the toolbar's equality guard, an unchanged refinement now does nothing. Failing before: the browser test counted one listing fetch after an unchanged Apply; now none, and the loaded cards stay in place. --- tests/browser/workspace-images.spec.mjs | 45 +++++++++++++++++++++++++ web/js/workspace/images.mjs | 13 ++++--- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index f449c9d..82d8e54 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1485,3 +1485,48 @@ test('Refine offers only the chosen source folders', async ({ page, request }) = await page.getByRole('button', { name: 'Filters', exact: true }).click(); expect(await values()).toEqual(['', 'shared']); }); + +/** + * Record every fetch the page's code calls from now on, as `{method, path, search}`. The API client calls fetch in the + * same task as the action that needs it, so after a press the record already holds any request the press started. + */ +async function recordFetches(page) { + await page.evaluate(() => { + window.__fetches = []; + const original = window.fetch; + window.fetch = (resource, init = {}) => { + const url = new URL(String(resource instanceof Request ? resource.url : resource), window.location.href); + window.__fetches.push({ method: (init.method || 'GET').toUpperCase(), path: url.pathname, search: url.search }); + return original.call(window, resource, init); + }; + }); + return () => page.evaluate(() => window.__fetches); +} +/** The listing page reads among them (the sidebar count's `limit=1` read is not one). */ +async function countImageFetches(page) { + const fetches = await recordFetches(page); + return async () => (await fetches()).filter(item => imagePages(new URL(`http://host${item.path}${item.search}`))).length; +} + +// #249 final review m2: like the toolbar, an unchanged refinement reloads nothing, so the loaded cards (and with them +// the reader's place) stay as they were. +test('Apply or Clear without a change keeps the loaded page', async ({ page, request }) => { + await expect(await request.post('/__test__/reset', { data: { extra_images: 80 } })).toBeOK(); + await page.setViewportSize({ width: 1200, height: 700 }); + await page.goto(workspace); + await expect(caption(page)).toContainText(/of 82 images/); + const fetches = await countImageFetches(page); + await cards(page).first().evaluate(card => { card.dataset.kept = 'yes'; }); + const before = await caption(page).textContent(); + for (const action of ['Apply filters', 'Clear filters']) { + await page.getByRole('button', { name: 'Filters', exact: true }).click(); + await refineDialog(page).getByRole('button', { name: action, exact: true }).click(); + await expect(refineDialog(page)).toBeHidden(); + expect(await fetches()).toBe(0); + await expect(cards(page).first()).toHaveAttribute('data-kept', 'yes'); + await expect(caption(page)).toHaveText(before); + } + // A real change still applies. + await refine(page, { Media: 'image' }); + await expect.poll(fetches).toBe(1); +}); diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index 97952c9..2ca3d2d 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -560,7 +560,6 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage ], values:{source:filters.root, folder:filters.subfolder, media:filters.media, tags:filters.tags.join(', ')}, onApply:applyRefinement, onClear:cleared => { toolbar.setValue({category:cleared.category}); - filters.category = cleared.category; applyRefinement(cleared); }}); } @@ -569,10 +568,16 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage 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}) { + /** Apply the dialog's values (Clear adds `category`); unchanged filters keep the loaded page, as the toolbar does. */ + function applyRefinement({source, folder, media, tags, category = filters.category}) { if (recovering) return; - Object.assign(filters, {root:source, subfolder:folder, media, - tags:[...new Set(tags.split(',').map(tag => tag.trim()).filter(Boolean))]}); + const next = {root:source, subfolder:folder, media, category, + tags:[...new Set(tags.split(',').map(tag => tag.trim()).filter(Boolean))]}; + const same = Object.entries(next).every(([key, value]) => key === 'tags' + ? value.length === filters.tags.length && value.every((tag, index) => tag === filters.tags[index]) + : filters[key] === value); + if (same) return; + Object.assign(filters, next); reset(); } function setControlsDisabled(disabled) {