From fe0835eea3a59c30881ecfa4e3645006e340e227 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Tue, 20 Oct 2026 02:27:52 -0700 Subject: [PATCH] fix(images): concept Load more focus, empty state and review rulings Review of #249 C2: - Load more and Retry focus the first appended image, as the concept does (app.js loadMoreImages). Focus moves without scrolling so the visible cards keep their place (bounded-DOM anchor), and only while focus is still on the pressed control: a viewer or a field the user moved to keeps it. When a last page adds nothing and the button hides, focus goes to the last image, never to the page; a failed page focuses Retry. - The concept's empty state beside the grid: .empty "No matching images. Try another search or category." - D17: the caption date shows only under Newest first (", YYYY" outside the current year) and is hidden under Oldest first and the name and rating sorts. - en-US number grouping in the embedded count too. - Refine "Folder" defaults to the concept's "All output folders". It is not narrowed by Source: the concept has no Source field, and the list holds every source's folders, so any pair can be chosen in one dialog. - The shared toast moves to toast.mjs; library-editor re-exports it. - The D32 overlay (extreme zoom) wraps the layout segmented control and the continuation row instead of pushing them out of the 390px column. - The legacy gallery checks moved to Overview's "Open existing image gallery" card (D29). Mapping additions: grid with no results -> .empty concept copy beside the grid; Load more press -> focus the first new image (concept). --- tests/browser/workspace-images.spec.mjs | 57 ++++++++++++++++++++++++- tests/browser/workspace-visual.spec.mjs | 9 +++- tests/browser/workspace.spec.mjs | 7 +++ tests/comfyui/workspace.spec.mjs | 7 ++- tests/js/workspace-images-state.test.js | 6 ++- web/js/workspace/images-state.mjs | 7 ++- web/js/workspace/images.mjs | 47 ++++++++++++++++---- web/js/workspace/library-editor.mjs | 13 +----- web/js/workspace/toast.mjs | 12 ++++++ web/js/workspace/workspace.css | 4 +- 10 files changed, 139 insertions(+), 30 deletions(-) create mode 100644 web/js/workspace/toast.mjs diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 5ce9547..430aaf7 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -286,11 +286,14 @@ test('continuous loading appends without replacing cards or losing selected deta await expect(cards(page).first()).toHaveAttribute('data-preserved', 'yes'); await expect(cards(page).first()).toHaveAttribute('data-image-key', selected); await expect(details(page)).toBeVisible(); - await expect(more).toBeFocused(); + // The concept (app.js loadMoreImages) focuses the first appended image after a Load more button press. + await expect(page.locator('article[aria-posinset="41"] .image-open')).toBeFocused(); + await more.focus(); await page.keyboard.press('Enter'); await expect(cards(page)).toHaveCount(102); expect(new Set(await cards(page).evaluateAll(elements => elements.map(element => element.dataset.imageKey))).size).toBe(102); await expect(more).toBeHidden(); + await expect(page.locator('article[aria-posinset="81"] .image-open')).toBeFocused(); await expect(page.getByRole('button', { name: /next page|previous page/i })).toHaveCount(0); }); @@ -686,6 +689,8 @@ test('folder discovery offers ancestors that filter nested images', async ({ pag await expect(cards(page)).toHaveCount(3); await page.getByRole('button', { name: 'Filters', exact: true }).click(); await expect(refineDialog(page).getByLabel('Folder', { exact: true }).locator('option[value="project"]')).toHaveCount(1); + // The concept's Folder default. + await expect(refineDialog(page).getByLabel('Folder', { exact: true }).locator('option').first()).toHaveText('All output folders'); await page.keyboard.press('Escape'); await refine(page, { Folder: 'project' }); await expect(cards(page)).toHaveCount(1); @@ -930,3 +935,53 @@ test('Load more toast reports the new total', async ({ page, request }) => { await expect(cards(page)).toHaveCount(0); await expect(page.locator('.infinite-continuation p.small')).toHaveText('No matching images · 0 images'); }); + +test('Load more focus never falls to the page when the last page hides the button', async ({ page, request }) => { + await page.addInitScript(() => { window.IntersectionObserver = undefined; }); + await expect(await request.post('/__test__/reset', { data: { extra_images: 100 } })).toBeOK(); + // The last page repeats already listed files: nothing new to focus, and Load more disappears. + await page.route(url => imagePages(url) && url.searchParams.has('cursor'), async route => { + const response = await route.fetch(); + const json = await response.json(); + await route.fulfill({ response, json: { ...json, images: [], has_more: false, next_cursor: null } }); + }); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(40); + const more = page.getByRole('button', { name: 'Load more images', exact: true }); + await more.focus(); + await page.keyboard.press('Enter'); + await expect(more).toBeHidden(); + await expect(page.locator('article[aria-posinset="40"] .image-open')).toBeFocused(); + expect(await page.evaluate(() => document.activeElement !== document.body)).toBe(true); +}); + +test('a search with no match shows the concept empty state', async ({ page }) => { + await page.goto(workspace); + await expect(cards(page)).toHaveCount(2); + const empty = page.locator('.gallery-body > .empty'); + await expect(empty).toBeHidden(); + await search(page).fill('no-such-image'); + await expect(cards(page)).toHaveCount(0); + await expect(empty).toBeVisible(); + await expect(empty).toHaveText('No matching images. Try another search or category.'); + await search(page).fill(''); + await expect(cards(page)).toHaveCount(2); + await expect(empty).toBeHidden(); +}); + +test('a Load more that finishes after focus moved on leaves focus where the user put it', async ({ page, request }) => { + await page.addInitScript(() => { window.IntersectionObserver = undefined; }); + await expect(await request.post('/__test__/reset', { data: { extra_images: 100 } })).toBeOK(); + let release, arrived; + const held = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { arrived = resolve; }); + await page.route(url => imagePages(url) && url.searchParams.has('cursor'), async route => { arrived(); await held; await route.continue(); }, { times: 1 }); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(40); + await page.getByRole('button', { name: 'Load more images', exact: true }).click(); + await started; + await search(page).focus(); + release(); + await expect(cards(page)).toHaveCount(80); + await expect(search(page)).toBeFocused(); +}); diff --git a/tests/browser/workspace-visual.spec.mjs b/tests/browser/workspace-visual.spec.mjs index ddfe62a..b5943f7 100644 --- a/tests/browser/workspace-visual.spec.mjs +++ b/tests/browser/workspace-visual.spec.mjs @@ -491,7 +491,11 @@ test('supplemental wrapped real footer reservation survives append without movin }); try { const more = page.getByRole('button', { name: 'Load more images', exact: true }); - await more.click(); await expect.poll(() => pending).toBe(true); + // At 4x zoom the concept continuation (copy + Load more) fills the viewport, so keep the last cards in view and + // press Load more from the keyboard without scrolling to it. + await cards.last().scrollIntoViewIfNeeded(); + await more.evaluate(node => node.focus({ preventScroll: true })); await page.keyboard.press('Enter'); + await expect.poll(() => pending).toBe(true); const anchor = await cards.evaluateAll(nodes => { const top = visualViewport?.offsetTop || 0, bottom = top + (visualViewport?.height || innerHeight); const node = nodes.find(node => { const box = node.getBoundingClientRect(); return box.bottom > top && box.top < bottom; }); @@ -502,7 +506,8 @@ test('supplemental wrapped real footer reservation survives append without movin await expect(cards).toHaveCount(80); await expect.poll(async () => Math.abs(await page.locator(`article[data-image-key="${anchor.key}"]`).evaluate(node => node.getBoundingClientRect().top - (visualViewport?.offsetTop || 0)) - anchor.top)).toBeLessThanOrEqual(1); await expect(page.locator(`article[data-image-key="${selectedKey}"]`)).toHaveClass(/selected/); - await expect(more).toBeFocused(); + // The concept focuses the first appended image after a Load more button press. + await expect(page.locator('article[aria-posinset="41"] .image-open')).toBeFocused(); await expect.poll(() => containedImageContent(page)).toEqual([]); } finally { release(); } }); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index 49f251f..761c393 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -81,6 +81,13 @@ test('all shell destinations deep-link, reload and expose working existing appli await page.reload(); await expect(main.getByRole('heading', { level: 1, name: label, exact: true })).toBeVisible(); } + // D29: the legacy image gallery is reached from Overview's tool card, not from Images. + await page.goto(`${workspace}#/overview`); + const gallery = page.locator('#workspace-main').getByRole('link', { name: 'Open existing image gallery', exact: true }); + await expect(gallery).toHaveAttribute('href', '/prompt_manager/gallery'); + await expect(await request.get('/prompt_manager/gallery')).toBeOK(); + await gallery.click(); + await expect(page).toHaveURL(/\/prompt_manager\/gallery$/); }); test('keyboard navigation, browser history and unknown routes remain usable', async ({ page }) => { diff --git a/tests/comfyui/workspace.spec.mjs b/tests/comfyui/workspace.spec.mjs index 56656ad..6bfa81c 100644 --- a/tests/comfyui/workspace.spec.mjs +++ b/tests/comfyui/workspace.spec.mjs @@ -36,8 +36,13 @@ test('workspace serves real library data beside either ComfyUI renderer', async } else { await expect(workspace.getByText('No matching images · 0 images', { exact: true })).toBeVisible(); } - // D29: Images no longer links the legacy gallery; Overview's tool cards do. + // D29: Images no longer links the legacy gallery; Overview's "Open existing image gallery" card does. await expect(workspace.locator('#workspace-main a[href="/prompt_manager/gallery"]')).toHaveCount(0); + await workspace.evaluate(() => { location.hash = '/overview'; }); + const gallery = workspace.locator('#workspace-main').getByRole('link', { name: 'Open existing image gallery', exact: true }); + await expect(gallery).toHaveAttribute('href', '/prompt_manager/gallery'); + expect((await workspace.request.get('/prompt_manager/gallery')).ok()).toBe(true); + await workspace.evaluate(() => { location.hash = '/images'; }); await workspace.reload(); await expect(workspace.locator('#workspace-main h1')).toHaveText('Images'); await expect(workspace.locator('body')).toHaveAttribute('data-ca-theme', 'light'); diff --git a/tests/js/workspace-images-state.test.js b/tests/js/workspace-images-state.test.js index 8071811..31a94af 100644 --- a/tests/js/workspace-images-state.test.js +++ b/tests/js/workspace-images-state.test.js @@ -223,12 +223,14 @@ test('the count line reads N of M images and K positive prompts only when known' assert.equal(countLine({loaded:1200,total:3400,positiveCount:0}),'1,200 of 3,400 images · 0 positive prompts'); }); -test('the caption date names the newest result day under a date sort only', async () => { +test('the caption date names the newest result day under Newest first only', async () => { const {captionDate}=await source; const now=new Date(2026,9,17,15,0); const at=(...parts)=>({modified_time:new Date(...parts).getTime()/1000}); assert.equal(captionDate([at(2026,9,17,9),at(2026,9,10)],'newest',now),'Today · October 17'); - assert.equal(captionDate([at(2026,9,10),at(2026,9,16,23)],'oldest',now),'Yesterday · October 16'); + assert.equal(captionDate([at(2026,9,16,23),at(2026,9,10)],'newest',now),'Yesterday · October 16'); + // D17 ruling: only under Newest first; Oldest first hides the date too. + assert.equal(captionDate([at(2026,9,10),at(2026,9,16,23)],'oldest',now),''); assert.equal(captionDate([at(2026,9,13)],'newest',now),'Tuesday · October 13'); assert.equal(captionDate([at(2025,2,3)],'newest',now),'Monday · March 3, 2025'); for(const sort of ['name','name-desc','rating']) assert.equal(captionDate([at(2026,9,17)],sort,now),''); diff --git a/web/js/workspace/images-state.mjs b/web/js/workspace/images-state.mjs index 2489800..fa928e1 100644 --- a/web/js/workspace/images-state.mjs +++ b/web/js/workspace/images-state.mjs @@ -7,7 +7,6 @@ export function imageKey(image) { /** The workspace listing's sorts: the concept's Recently added, Name A–Z and Highest rated, plus Oldest and Z–A (D14). */ export const IMAGE_SORTS = Object.freeze(['newest', 'oldest', 'name', 'name-desc', 'rating']); -const DATE_SORTS = new Set(['newest', 'oldest']); const isCount = value => Number.isSafeInteger(value) && value >= 0; @@ -168,11 +167,11 @@ const DAY_MS = 86400000; const startOfDay = date => new Date(date.getFullYear(), date.getMonth(), date.getDate()).getTime(); /** - * D17: the caption's right-hand date, " · " of the newest loaded result under a - * date sort, and '' under name or rating sorts. A day outside the current year adds the year so it stays unambiguous. + * D17: the caption's right-hand date, " · " of the newest result under Newest first, + * with ", YYYY" outside the current year; '' under Oldest first and the name or rating sorts. */ export function captionDate(items, sort, now = new Date()) { - if (!DATE_SORTS.has(sort)) return ''; + if (sort !== 'newest') return ''; const newest = items.reduce((latest, item) => Number.isFinite(item.modified_time) ? Math.max(latest, item.modified_time) : latest, -Infinity); if (!Number.isFinite(newest)) return ''; const date = new Date(newest * 1000); diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index cbab9c8..70da5f8 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -11,7 +11,7 @@ import { noun } from './plural.mjs'; import { pageHeading } from './heading.mjs'; import { mountToolbar } from './toolbar.mjs'; import { openImportDialog, openRefineDialog } from './workspace-dialogs.mjs'; -import { showWorkspaceToast } from './library-editor.mjs'; +import { showWorkspaceToast } from './toast.mjs'; function el(tag, text, className) { const element = document.createElement(tag); @@ -152,7 +152,14 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage const more = createButton('Load more images'); const retry = createButton('Retry'); continuation.append(message, more, retry); - collection.append(caption, grid, continuation); + // The concept's empty collection (app.js updateImages), beside the grid because the grid only holds positioned cards. + const empty = el('div', 'No matching images. Try another search or category.', 'empty'); + empty.hidden = true; + collection.append(caption, grid); + if (!embedded) collection.append(empty); + collection.append(continuation); + // After a Load more or Retry press, the index of the first image that batch appends (concept: focus it). + let focusAfterLoad = null; const details = el('section', undefined, 'image-details panel'); details.setAttribute('aria-label', 'Image details'); details.append(el('h2', 'Image details'), el('p', 'Select an image to inspect it.')); @@ -272,7 +279,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage card.setAttribute('aria-posinset', String(index + 1)); card.setAttribute('aria-setsize', String(state.total)); Object.assign(card.style, { left: `${position.x}px`, top: `${position.y}px`, width: `${position.width}px`, height: `${position.height}px` }); } - function focusImage(index, preview = false) { + function focusImage(index, preview = false, {scroll = true} = {}) { if (index < 0 || index >= state.items.length) return; const image = state.items[index], key = imageKey(image); let card = rendered.get(key); @@ -281,7 +288,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage grid.insertBefore(card, [...grid.children].find(child => Number(child.dataset.index) > index) || null); } card.querySelector(preview ? '.image-card-footer button' : '.image-open').focus({ preventScroll: true }); - card.scrollIntoView({ block: 'nearest' }); + if (scroll) card.scrollIntoView({ block: 'nearest' }); cancelAnimationFrame(frame); frame = 0; renderVisible(); } @@ -357,7 +364,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage for (let index = state?.items.length || 0; index < next.items.length; index++) itemIndexes.set(imageKey(next.items[index]), index); } state = next; - count.textContent = embedded ? `${next.loaded.toLocaleString()} loaded · ${next.total.toLocaleString()} ${noun(next.total, 'linked image')}` : countLine(next); + count.textContent = embedded ? `${next.loaded.toLocaleString('en-US')} loaded · ${next.total.toLocaleString('en-US')} ${noun(next.total, 'linked image')}` : countLine(next); if (!embedded) { captionDay.textContent = captionDate(next.items, filters.sort); captionDay.hidden = !captionDay.textContent; @@ -373,6 +380,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage retry.hidden = !next.error; more.hidden = !next.hasMore || !!next.error; more.setAttribute('aria-disabled', String(next.loading || recovering)); + if (!embedded) empty.hidden = recovering || next.loading || !!next.error || next.hasMore || next.loaded > 0; + if (focusAfterLoad !== null && !next.loading) settleLoadFocus(next); if (!selection && next.items.length) inspect(next.items[0]); scheduleLoad(); } @@ -381,8 +390,29 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage stream.dispose(); for (const video of body.querySelectorAll('video')) releaseVideo(video); }); - scope.listen(more, 'click', () => { if (!recovering) stream.loadMore(); }); - scope.listen(retry, 'click', () => { if (!recovering) stream.loadMore(); }); + function pressLoad() { + if (recovering || state?.loading) return; + focusAfterLoad = state?.loaded ?? 0; + stream.loadMore(); + } + /** The concept focuses the first appended image; focus never stays on a control that just hid. */ + function settleLoadFocus(next) { + const first = focusAfterLoad; + focusAfterLoad = null; + const active = document.activeElement; + // Only while focus is still on the pressed control: a viewer or another control the user moved to keeps it. + const stranded = !active || active === document.body || active === more || active === retry; + if (!stranded) return; + if (next.error) { retry.focus(); return; } + // Without scrolling: the visible cards keep their place (bounded-DOM anchor); the next arrow key reveals it. + if (next.loaded > first) focusImage(first, false, {scroll:false}); + else if (more.hidden) { + if (next.loaded) focusImage(next.loaded - 1, false, {scroll:false}); + else focusFallback.focus({preventScroll:true}); + } + } + scope.listen(more, 'click', pressLoad); + scope.listen(retry, 'click', pressLoad); function maybeLoad() { if (!observer || recovering || scope.signal.aborted || !state?.hasMore || state.loading || state.error || main.querySelector('.image-preview[open]')) return; const bounds = continuation.getBoundingClientRect(); @@ -409,6 +439,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage details.replaceChildren(el('h2', 'Image details'), el('p', 'Select an image to inspect it.')); } selection = null; + focusAfterLoad = null; for (const video of details.querySelectorAll('video')) releaseVideo(video); updateScope(); stream.reset(filters, {load:!recovering}); @@ -418,7 +449,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 folders'], ...folders.map(path => [path, path])]}, + {key:'folder', options:[['', 'All output folders'], ...folders.map(path => [path, path])]}, {key:'media', options:MEDIA_OPTIONS}, {key:'tags'}, ], values:{source:filters.root, folder:filters.subfolder, media:filters.media, tags:filters.tags.join(', ')}, diff --git a/web/js/workspace/library-editor.mjs b/web/js/workspace/library-editor.mjs index 67ec411..4eb238b 100644 --- a/web/js/workspace/library-editor.mjs +++ b/web/js/workspace/library-editor.mjs @@ -19,21 +19,12 @@ const NO_PAIR = 'Archive needs a selected saved negative variant.'; const ALREADY_ARCHIVED = 'This negative variant is already archived.'; const REVIEW = 'Loaded the latest saved values. Review your entries, then save.'; const REVIEW_ARCHIVE = 'Loaded the latest saved values. Review them, then archive.'; -const TOAST_MS = 6500; const REFRESH = ['Reload Library', 'refresh']; // The concept has no discard confirmation (its Cancel and × simply close), so the state says it inline: // every exit (Reload Library, Cancel, × or Escape) drops the draft. const DISCARDS = 'Leaving this dialog discards your entries.'; -let toastTimer; - -/** Show one outcome in the shared workspace toast (role="status"); it clears itself like the concept's. */ -export function showWorkspaceToast(message) { - const toast = document.getElementById('workspace-toast'); - if (!toast) return; - toast.textContent = message; - clearTimeout(toastTimer); - toastTimer = setTimeout(() => {toast.textContent = '';}, TOAST_MS); -} +// The toast lives in the shell module; re-exported for the pages that already import it from here. +export { showWorkspaceToast } from './toast.mjs'; function el(tag, text, className) { const node = document.createElement(tag); diff --git a/web/js/workspace/toast.mjs b/web/js/workspace/toast.mjs new file mode 100644 index 0000000..f0cf510 --- /dev/null +++ b/web/js/workspace/toast.mjs @@ -0,0 +1,12 @@ +/** The shell's shared toast (`#workspace-toast`, role="status"), used by every workspace page. */ +const TOAST_MS = 6500; +let toastTimer; + +/** Show one outcome in the shared workspace toast; it clears itself like the concept's. */ +export function showWorkspaceToast(message) { + const toast = document.getElementById('workspace-toast'); + if (!toast) return; + toast.textContent = message; + clearTimeout(toastTimer); + toastTimer = setTimeout(() => {toast.textContent = '';}, TOAST_MS); +} diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index fb6e94c..b2196cc 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -59,9 +59,11 @@ main:focus-visible { outline: 2px solid var(--ca-focus); outline-offset: 4px; } .infinite-continuation button[aria-disabled="true"] { opacity: .55; cursor: wait; } /* #249 D14: the scope chip names real sources and folders, which can be far longer than "All output folders". */ .filter-row span.chip { max-width: 100%; min-width: 0; overflow-wrap: anywhere; } -/* #249 D31: at extreme zoom the 390px shell is narrower than the concept's three layout buttons; they wrap instead of +/* #249 D32: at extreme zoom the 390px shell is narrower than the concept's three layout buttons; they wrap instead of scrolling the page sideways (they never wrap while they fit). */ .segmented { flex-wrap: wrap; max-width: 100%; } +/* #249 D32: likewise the continuation's text and Load more wrap instead of pushing the button out of the column. */ +.infinite-continuation { flex-wrap: wrap; } /* Interim page guards: concept element rules (`.ca-tools dialog`, `pre`) now reach production dialogs and prompt text that keep their own design until their page PR rebuilds them in concept markup. */