From 99e392436e26cacc2ed10f5ac5c5f607d91d04d6 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 30 Oct 2026 13:41:48 -0700 Subject: [PATCH] fix(images): at one column Info goes to the panel and loading waits for the next move (#531) Opening the viewer on Info blocked the inspector and zoom contracts, and popped up on a zoomed desktop. --- TESTING.md | 5 ++-- tests/browser/images-concept.spec.mjs | 2 -- tests/browser/workspace-images.spec.mjs | 10 ------- .../browser/workspace-sticky-chrome.spec.mjs | 26 ++++++++++++------- web/js/workspace/images.mjs | 16 ++++++++++-- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/TESTING.md b/TESTING.md index 70d020f1..aadb95a6 100644 --- a/TESTING.md +++ b/TESTING.md @@ -5631,9 +5631,8 @@ Beyond the concept, from the maintainer's test of #528; the concept is unchanged - **D187:** Back to top: one shell chip past 200px of scroll, above the toast's spot; it rises only for a toast taller than two lines. It never animates, because the pages' height corrections would cancel a smooth scroll, and focus moves to `#workspace-main`. D42's band grows to 140px. - **D188:** the Needs review panel is sticky like the Images panel, with its own scroll on a window under 720px tall, and static at 700px and below. -- **D189:** where a panel is one column (static): Info on a review card brings the panel into view; Info on an image opens - the viewer's details, because the Images panel follows the whole list, past infinite scroll. The removed-images view keeps - today's panel, because the viewer can write there. +- **D189:** where a panel is one column (static): Info brings the panel into view. On + Images the panel follows the whole list, past infinite scroll, so loading waits for the reader's next move (wheel, touch or key) rather than pushing the panel away. ## Workspace data fields for the concept pages (#249) diff --git a/tests/browser/images-concept.spec.mjs b/tests/browser/images-concept.spec.mjs index 9d23784d..55866eab 100644 --- a/tests/browser/images-concept.spec.mjs +++ b/tests/browser/images-concept.spec.mjs @@ -318,8 +318,6 @@ async function behaviorsOf(page,scope) { await settle(page); const focusAfterViewer=await activeOf(page); await page.locator(CARDS).nth(2).locator('[aria-label^="Inspect"]').click(); - // D189: at one column Info also opens the viewer's details, over the panel it updates. - if(page.viewportSize().width<=700){await page.keyboard.press('Escape');await expect(page.locator('#viewer-dialog')).toBeHidden();} await expect.poll(()=>selectedOf(page)).toEqual([2]); const selected=await selectedOf(page); const layouts=await layoutsOf(page,scope); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 332fed8c..b6cb2630 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -70,7 +70,6 @@ test('native Images keeps colliding filenames distinct and inspects the selected await page.setViewportSize({width:390,height:844}); for (const [index,source] of [[1,'input'],[0,'output']]) { await cards(page).nth(index).getByRole('button',{name:'Inspect same.png',exact:true}).click(); - await closeInfoViewer(page); // D189 await expectPanelSource(page, source); await details(page).scrollIntoViewIfNeeded(); await capture(`after-images-source-collision-${source}-390`); @@ -78,13 +77,6 @@ test('native Images keeps colliding filenames distinct and inspects the selected expect((await (await request.get('/prompt_manager/stats')).json()).stats).toEqual(snapshot.stats); }); -/** D189: at one column Info also opens the viewer's details; the panel is updated behind it. */ -async function closeInfoViewer(page) { - await expect(page.locator('#viewer-dialog')).toBeVisible(); - await page.keyboard.press('Escape'); - await expect(page.locator('#viewer-dialog')).toBeHidden(); -} - test('workspace defaults to the native Images collection without a hash route', async ({ page }) => { await page.goto('/prompt_manager/workspace'); await expect(page.locator('#workspace-main h1')).toHaveText('Images'); @@ -677,7 +669,6 @@ test('mobile Images keeps filters, layouts and selected details within the viewp for (const layout of ['List', 'Masonry', 'Grid']) { await page.getByRole('button', { name: layout, exact: true }).click(); await cards(page).last().getByRole('button', { name: 'Inspect same.png', exact: true }).click(); - await closeInfoViewer(page); // D189 await expectPanelSource(page, 'input'); expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true); } @@ -2269,7 +2260,6 @@ test('at 390px in light theme the Set category dialog and the category chip fit const card = cardOf(page, '0:bulk/0001.png'); // The chip wraps inside the side panel; its origin never splits. await card.getByRole('button', { name: 'Inspect 0001.png', exact: true }).click(); - await closeInfoViewer(page); // D189 const chip = details(page).locator('span.chip', { has: page.locator('span.chip-source') }); await expect(chip).toHaveText(`Image category: ${LONG} ยท this image`); const fit = await chip.evaluate(node => { diff --git a/tests/browser/workspace-sticky-chrome.spec.mjs b/tests/browser/workspace-sticky-chrome.spec.mjs index 12fd3a10..2fb311d7 100644 --- a/tests/browser/workspace-sticky-chrome.spec.mjs +++ b/tests/browser/workspace-sticky-chrome.spec.mjs @@ -217,21 +217,27 @@ test('at 390px Info on a lower review card brings the panel into view', async ({ await expect(reviewPanel(page)).toBeInViewport(); }); -// D189: at one column the Images panel follows the whole list, so scrolling to it would run into infinite scroll. -test('at 390px Info on an image opens the viewer with its details; loading the page does not', async ({ page, request }) => { +// D189: at one column the Images panel follows the whole list: Info goes there, and a batch doesn't push it away. +test('at 390px Info on an image brings its panel into view and loading waits for the next move', async ({ page, request }) => { await page.setViewportSize(PHONE); - await openImages(page, request, 20); - await expect(page.locator('#viewer-dialog')).toBeHidden(); + await openImages(page, request); + expect(await page.evaluate(() => scrollY)).toBe(0); + await expect(page.locator('#image-inspector')).not.toBeInViewport(); + let reads = 0; + page.on('request', item => { if (new URL(item.url()).pathname.endsWith('/images/recent')) reads++; }); const card = imageCards(page).nth(3); await card.scrollIntoViewIfNeeded(); - const scrolled = await page.evaluate(() => scrollY); await card.getByRole('button', { name: /^Inspect / }).click(); - await expect(page.locator('#viewer-dialog')).toBeVisible(); - await expect(page.locator('#viewer-details')).toBeVisible(); - await page.keyboard.press('Escape'); + await expect(page.locator('#image-inspector')).toBeInViewport(); await expect(page.locator('#viewer-dialog')).toBeHidden(); - await expect(card.getByRole('button', { name: /^Inspect / })).toBeFocused(); - expect(await page.evaluate(() => scrollY)).toBe(scrolled); + const held = (await box(page.locator('#image-inspector'))).top; + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)))); + expect((await box(page.locator('#image-inspector'))).top).toBe(held); + expect(reads).toBe(0); + // The next move releases it: loading resumes. + const next = page.waitForRequest(item => new URL(item.url()).pathname.includes('/images')); + await page.mouse.wheel(0, 40); + await next; }); // Review's review-state load more is a button, so Library's anchor stays honest through a compensated bar. diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index 37c8e6b6..1527d623 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -302,6 +302,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage 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; + let panelHold = false; // The concept's side panel (app.js a426, D20); the Library-embedded gallery has its own page around it. const details = el('aside', undefined, 'detail-panel'); if (!embedded) details.id = 'image-inspector'; @@ -407,8 +408,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage else if (action && !openCardTagging({api, scope, image, action, embedded, onSourceChanged:recovery.recover, onRefresh:refreshInspectionCollection})) { inspect(image); - // D189: at one column the panel follows the whole list, past infinite scroll, so Info opens the viewer's details. - if (getComputedStyle(details).position === 'static') previewImage(image); + bringPanelIntoView(); } }); @@ -644,7 +644,19 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage } scope.listen(more, 'click', pressLoad); scope.listen(retry, 'click', () => (state?.error?.code === 'cursor-stale' ? refreshInspectionCollection() : pressLoad())); + /** + * D189: at one column the panel follows the whole list, so Info goes to it. That passes the continuation, so loading + * waits for the reader's next move: a batch appended now would push the panel away. + */ + function bringPanelIntoView() { + if (getComputedStyle(details).position !== 'static') return; + panelHold = true; + details.scrollIntoView({ block: 'start' }); + } + const releasePanel = () => { if (panelHold) { panelHold = false; scheduleLoad(); } }; + for (const name of ['wheel', 'touchmove', 'keydown']) scope.listen(window, name, releasePanel, { passive: true }); function maybeLoad() { + if (panelHold) return; if (!observer || recovering || scope.signal.aborted || !state?.hasMore || state.loading || state.error || main.querySelector('#viewer-dialog[open]')) return; const bounds = continuation.getBoundingClientRect(); if (bounds.top <= window.innerHeight + 250 && bounds.bottom >= 0) stream.loadMore();