Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b5bdba2f38 | ||
|
|
245647f12b | ||
|
|
3479ae8d39 | ||
|
|
e23e6f9a54 |
@@ -5029,3 +5029,52 @@ Findings offer it for the 300ms tap delay; this app's viewport is
|
||||
the stated benefit is not there to win. What it would change is the
|
||||
gesture stack #63 tuned by measurement on the device (`pan-y` plus a
|
||||
non-passive `preventDefault`), and that is not measurable from here.
|
||||
|
||||
## Art pop-in is measurable in a browser, if you count frames rather than milliseconds (measured 2026-08-24)
|
||||
|
||||
#65 is an Android report ("scrolling through albums, the art pops in")
|
||||
and the desktop harness can measure it, which was not obvious: the
|
||||
first attempt waited 220 ms after each scroll jump and found **zero**
|
||||
blank covers on either build. The metric only discriminates at one and
|
||||
two animation frames after the jump, which is where a pop-in actually
|
||||
lives.
|
||||
|
||||
Protocol, on `make dev-headless SEED=bulk` (4 988 albums), ten
|
||||
2 400px jumps of `.grid-scroll-container`, counting covers whose rect
|
||||
intersects the viewport with `naturalWidth === 0`:
|
||||
|
||||
| build | blank at frame 1 | at frame 2 | at 50 ms |
|
||||
|---|---|---|---|
|
||||
| `main` | 254 / 258 | 214 / 258 | 0 |
|
||||
| `main`, second run | 254 / 258 | 190 / 258 | 0 |
|
||||
| prefetch | 117 / 258 | 77 / 258 | 0 |
|
||||
| prefetch, second run | 118 / 258 | 96 / 258 | 0 |
|
||||
|
||||
Two things this protocol gets wrong if repeated carelessly. **A second
|
||||
run in the same browser session measures the HTTP cache**, not the
|
||||
build — the skill already warns about this for `make perf`, and it
|
||||
applies to any image measurement; every row above is a fresh
|
||||
`playwright-cli close` + `open`. And **the frontend is embedded**, so
|
||||
comparing builds is a `git stash` *and* a rebuild, not a stash.
|
||||
|
||||
**The bulk library's covers are 300x300 and ~3.7 kB**, which is why
|
||||
both builds are clean by 50 ms here and why the phone's number cannot
|
||||
be inferred from this one — same caveat the skill already records
|
||||
about full-size artwork.
|
||||
|
||||
**`rangeChanged` and `visibilityChanged` are different ranges**, and
|
||||
the difference is the whole of this fix's value.
|
||||
`@lit-labs/virtualizer` reports `_first`/`_last` (rendered, including
|
||||
the ~1000px overhang) on the former and `_firstVisible`/`_lastVisible`
|
||||
on the latter. Both grids listen to `visibilityChanged` for scroll
|
||||
persistence, which wants the visible range and is correct; a prefetch
|
||||
window measured from it lands mostly on cards that already exist.
|
||||
Anchored there, the component test could see only one row past the
|
||||
last rendered card.
|
||||
|
||||
**`_overhang` is not configurable.** It is a `protected` field set to
|
||||
1000 in `BaseLayout` and read by every layout; there is no option on
|
||||
`grid()`/`flow()` and no property on the element. The issue's Direction
|
||||
("ask the virtualizer for a larger overscan") is therefore not
|
||||
available without patching a private, which is why the request is
|
||||
issued ahead of the element instead.
|
||||
|
||||
@@ -1832,6 +1832,21 @@ is not it.** A `placeholder` is an accname fallback, so an
|
||||
Explore's search box — the audit's own `a11y.26` — as clean. A sweep
|
||||
for *empty* names cannot see a *weak* one.
|
||||
|
||||
**`title` is the same trap one rung lower, and it defeats the obvious
|
||||
spec as well as the obvious sweep.** `queue-panel`'s Clear queue and
|
||||
Add queue to playlist were named by `title` alone, so
|
||||
`getByRole('button', { name: 'Clear queue' })` matched them **before**
|
||||
the fix as well as after — a `getByRole` assertion, which is what
|
||||
catches every other nameless control in this app, would have been
|
||||
green on the broken build. `title` is the *last* fallback in the
|
||||
accname order, so content put inside the button later silently
|
||||
outranks it, and it is the one name a phone cannot show, having no
|
||||
hover. The property is therefore asserted as *the name is not the
|
||||
tooltip*: `queue-overlay.spec.ts` removes the `title` attributes and
|
||||
asks again, which is 1 and 1 with `aria-label` and was measured at 0
|
||||
and 0 without it. The `title`s stay, because on a desktop they are
|
||||
also the tooltip for an icon-only control and that is a different job.
|
||||
|
||||
**The shell scrolls sideways and not down.** `body` is
|
||||
`overflow-x: auto; overflow-y: hidden`, and both halves are measured.
|
||||
Vertically there is nothing to fix: the middle grid row is `1fr` and
|
||||
@@ -3403,6 +3418,46 @@ rather than searching it — the store replaces that array when its
|
||||
contents change and shares the unchanged members, which is the same
|
||||
signal `track-list`'s memoized caches key on.
|
||||
|
||||
**And the right tier arriving late still reads as no art at all**, so
|
||||
the two grids ask for it before the card exists (#65).
|
||||
`utils/image-prefetch.ts` warms the images a scroll is about to reach,
|
||||
from `cover-grid`'s and `artists-view`'s virtualizers. Measured on the
|
||||
50 000-track bulk seed over ten 2 400px jumps: of 258 covers arriving
|
||||
in view, **254 were still blank one frame later and 214 two frames
|
||||
later**; with the prefetch, 117 and 77. Both builds are clean by 50 ms
|
||||
on a desktop with 3.7 kB fixture covers, which is where the reference
|
||||
device's slower engine and 27 kB covers spend their pop-in.
|
||||
|
||||
Four things about it are load-bearing.
|
||||
|
||||
**The overscan the obvious fix asks for does not exist.**
|
||||
`@lit-labs/virtualizer`'s `_overhang` is a hard-coded 1000px
|
||||
`protected` field on `BaseLayout` with no configuration surface, so
|
||||
raising it means monkey-patching a private. 1000px is about two
|
||||
screens on a 439px viewport, and the *image* cannot be requested until
|
||||
the card it lives in is rendered — which is what this asks for
|
||||
instead.
|
||||
|
||||
**It hangs off `rangeChanged`, not `visibilityChanged`.** Those report
|
||||
different ranges: visibility is what is on screen, and the virtualizer
|
||||
has already rendered that 1000px past it. Anchored to the visible
|
||||
range the window is spent on cards that already exist and have already
|
||||
asked for their own art — measured as the difference between the
|
||||
prefetch reaching one row past the last card and reaching a full
|
||||
window past it.
|
||||
|
||||
**It is not the `LRUMap` path, and saying so is the bound.** That
|
||||
ceiling holds Explore's base64 data URLs in JS; a library cover is a
|
||||
plain URL under `Cache-Control: immutable` (the filenames are content
|
||||
hashes), so what retains the bytes is the browser's own cache. What
|
||||
this module retains is the *set of URLs already asked for*, capped at
|
||||
512 and reported to `window.__yjCacheStats()` — 497 entries and 15 407
|
||||
chars after the run above.
|
||||
|
||||
**The prefetch asks for what the card will draw.** `artists-view`'s
|
||||
tier ladder moved into `artistAvatarURL()` so the two cannot disagree;
|
||||
a second copy would be a warm cache for a tier nothing renders.
|
||||
|
||||
**The same rule, on the selection path, was the worst stall in the
|
||||
app.** Five components turned selected file paths back into tracks with
|
||||
`filePaths.map(fp => tracks.find(…))`, so "Select all → Edit tags" at
|
||||
|
||||
@@ -157,6 +157,62 @@ test.describe('an overlaid queue says it is over the content', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #170 — the other two buttons in that same row.
|
||||
*
|
||||
* Clear queue and Add queue to playlist predate the close button and
|
||||
* were named by a `title` attribute and nothing else. Unlike the
|
||||
* sliders in `control-names.spec.ts`, that is not a *missing* name:
|
||||
* `title` is the last fallback in the accname order, so
|
||||
* `getByRole('button', { name: 'Clear queue' })` matched them before
|
||||
* this fix as well as after it — measured, 1 and 1. A sweep for empty
|
||||
* names cannot see a weak one, which is `a11y.26`'s complaint and the
|
||||
* reason this file could have grown a green test that proved nothing.
|
||||
*
|
||||
* So the name is asserted twice, and the second assertion is the one
|
||||
* that fails on the broken build. Taking the tooltip away and asking
|
||||
* again is the property in words: **the name is not the tooltip**. It
|
||||
* is what makes the button survive content being put inside it later,
|
||||
* and it is the only one of the two a phone has — there is no hover on
|
||||
* the surface #55 turned into a full screen. Measured on `main` before
|
||||
* the fix: 0 and 0.
|
||||
*
|
||||
* Both buttons are disabled here, because the queue starts empty and
|
||||
* naming is not enablement. A disabled button is still in the
|
||||
* accessibility tree, which is exactly where the complaint was.
|
||||
*/
|
||||
test.describe('the queue header says what its actions do', () => {
|
||||
const ACTIONS = ['Clear queue', 'Add queue to playlist'];
|
||||
|
||||
test('names both of the older actions', async ({ app }) => {
|
||||
await openQueue(app);
|
||||
|
||||
for (const name of ACTIONS) {
|
||||
await expect(
|
||||
app.getByRole('button', { name, exact: true }),
|
||||
).toHaveCount(1);
|
||||
}
|
||||
});
|
||||
|
||||
test('and the names do not come from the tooltip', async ({ app }) => {
|
||||
await openQueue(app);
|
||||
|
||||
await app.locator('#queue-panel').evaluate((el) => {
|
||||
for (const button of el.shadowRoot!.querySelectorAll(
|
||||
'.header-action-button',
|
||||
)) {
|
||||
button.removeAttribute('title');
|
||||
}
|
||||
});
|
||||
|
||||
for (const name of ACTIONS) {
|
||||
await expect(
|
||||
app.getByRole('button', { name, exact: true }),
|
||||
).toHaveCount(1);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The inline panel is the mode that already worked, and the one every
|
||||
* other queue spec is written against. It keeps its resize handle and
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import '@lit-labs/virtualizer';
|
||||
import type {
|
||||
LitVirtualizer,
|
||||
RangeChangedEvent,
|
||||
VisibilityChangedEvent,
|
||||
} from '@lit-labs/virtualizer';
|
||||
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
||||
@@ -30,6 +31,7 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||
import { RovingGridController } from '@utils/roving-grid';
|
||||
import { prefetchImageWindow } from '@utils/image-prefetch';
|
||||
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
@@ -582,6 +584,26 @@ export class ArtistsView
|
||||
* Scroll position persistence
|
||||
* ================================================================ */
|
||||
|
||||
/**
|
||||
* Warm the avatars just past the rendered range (#65).
|
||||
*
|
||||
* `rangeChanged` is the rendered range and `visibilityChanged` is
|
||||
* what is on screen; the virtualizer has already drawn about
|
||||
* 1000px past the latter, so that is the wrong anchor to measure a
|
||||
* prefetch window from. It is deliberately outside the
|
||||
* `restoringScroll` guard below: a restored scroll lands in the
|
||||
* middle of the grid, which is exactly when nothing around it is
|
||||
* cached.
|
||||
*/
|
||||
private onRangeChanged = (e: RangeChangedEvent) => {
|
||||
prefetchImageWindow(
|
||||
this.cachedGridEntries,
|
||||
e.first,
|
||||
e.last,
|
||||
(entry) => this.artistAvatarURL(entry.artist),
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Save the first visible item index on scroll.
|
||||
*/
|
||||
@@ -1145,7 +1167,16 @@ export class ArtistsView
|
||||
* Helpers
|
||||
* ================================================================ */
|
||||
|
||||
private renderArtistAvatar(artist: library.Artist) {
|
||||
/**
|
||||
* The image this artist's card will draw, or `''` for the initial
|
||||
* placeholder.
|
||||
*
|
||||
* Split out of `renderArtistAvatar` so the prefetch (#65) asks for
|
||||
* exactly what the card is going to ask for — a second copy of the
|
||||
* tier ladder would be a second thing to keep in step, and warming
|
||||
* the wrong tier is a download that buys nothing.
|
||||
*/
|
||||
private artistAvatarURL(artist: library.Artist): string {
|
||||
const needed = (this.imageSize ?? 176) * window.devicePixelRatio;
|
||||
let imageURL = '';
|
||||
|
||||
@@ -1172,6 +1203,12 @@ export class ArtistsView
|
||||
) ?? '';
|
||||
}
|
||||
|
||||
return imageURL;
|
||||
}
|
||||
|
||||
private renderArtistAvatar(artist: library.Artist) {
|
||||
const imageURL = this.artistAvatarURL(artist);
|
||||
|
||||
if (imageURL) {
|
||||
return html`<img
|
||||
class="avatar-image"
|
||||
@@ -1531,6 +1568,7 @@ export class ArtistsView
|
||||
.keyFunction=${(entry: ArtistEntry) => entry.artist.ID}
|
||||
.layout=${this.gridLayout}
|
||||
@visibilityChanged=${this.onVisibilityChanged}
|
||||
@rangeChanged=${this.onRangeChanged}
|
||||
></lit-virtualizer>
|
||||
</div>
|
||||
${this.renderContextMenu()}
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
import '@lit-labs/virtualizer';
|
||||
import type {
|
||||
LitVirtualizer,
|
||||
RangeChangedEvent,
|
||||
VisibilityChangedEvent,
|
||||
} from '@lit-labs/virtualizer';
|
||||
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
||||
@@ -30,6 +31,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@components/playlist-picker/playlist-picker.js';
|
||||
import { loadTrackDetails } from '@utils/lazy-track-details.js';
|
||||
import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js';
|
||||
import { prefetchImageWindow } from '@utils/image-prefetch.js';
|
||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||
import type { CoverArtUrls } from '@components/track-details/track-details.js';
|
||||
import { AlbumSelectionManager } from './album-selection.js';
|
||||
@@ -910,6 +912,31 @@ export class CoverGrid
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Warm the covers just past the rendered range (#65).
|
||||
*
|
||||
* `rangeChanged` rather than `visibilityChanged`, because the two
|
||||
* report different ranges and only one of them is the right
|
||||
* anchor: visibility is what is on screen, and the virtualizer has
|
||||
* already rendered about 1000px past that. Measured from the
|
||||
* visible range this would spend most of its window on cards that
|
||||
* already exist and have already asked for their own art.
|
||||
*
|
||||
* The entry lists are memoized, so asking for one here costs a
|
||||
* reference compare.
|
||||
*/
|
||||
private onRangeChanged = (e: RangeChangedEvent) => {
|
||||
const entries = this.splitMode
|
||||
? this.getBeforeEntries()
|
||||
: this.buildGridEntries();
|
||||
|
||||
prefetchImageWindow(entries, e.first, e.last, (entry) =>
|
||||
entry.album.CoverArtPath
|
||||
? this.getCoverUrl(entry.album)
|
||||
: '',
|
||||
);
|
||||
};
|
||||
|
||||
/* ====================================================================
|
||||
* Virtualizer items
|
||||
* ==================================================================== */
|
||||
@@ -2003,6 +2030,7 @@ export class CoverGrid
|
||||
@keydown=${this.onGridAlbumKeydown}
|
||||
@contextmenu=${this.onGridAlbumContextMenu}
|
||||
@visibilityChanged=${this.onVisibilityChanged}
|
||||
@rangeChanged=${this.onRangeChanged}
|
||||
></lit-virtualizer>
|
||||
`;
|
||||
}
|
||||
@@ -2037,6 +2065,7 @@ export class CoverGrid
|
||||
@keydown=${this.onGridAlbumKeydown}
|
||||
@contextmenu=${this.onGridAlbumContextMenu}
|
||||
@visibilityChanged=${this.onVisibilityChanged}
|
||||
@rangeChanged=${this.onRangeChanged}
|
||||
></lit-virtualizer>
|
||||
|
||||
<album-dropdown
|
||||
|
||||
@@ -2201,11 +2201,25 @@ export class QueuePanel
|
||||
`
|
||||
: nothing}
|
||||
</div>
|
||||
<!-- **Every action here is named by aria-label**, like
|
||||
the close button #24 added beside them (#170). A
|
||||
title alone *is* a name, which is why a sweep for
|
||||
empty names reports these clean and why an
|
||||
assertion by role and name is green either way --
|
||||
but it is the weakest one: title is the last
|
||||
fallback in the accname order, so any content put
|
||||
inside the button later silently outranks it, and
|
||||
a phone has no hover to show it as a tooltip.
|
||||
|
||||
The titles stay. On a desktop they are the tooltip
|
||||
for an icon-only control, which is a different job
|
||||
from naming it, and aria-label does not do it. -->
|
||||
<div class="header-actions">
|
||||
<button
|
||||
class="header-action-button"
|
||||
@click=${() => void this.handleClearQueue()}
|
||||
?disabled=${tracks.length === 0}
|
||||
aria-label="Clear queue"
|
||||
title="Clear queue"
|
||||
>
|
||||
<wa-icon
|
||||
@@ -2216,6 +2230,7 @@ export class QueuePanel
|
||||
class="header-action-button add-to-playlist-button"
|
||||
@click=${this.handleAddToPlaylist}
|
||||
?disabled=${tracks.length === 0}
|
||||
aria-label="Add queue to playlist"
|
||||
title="Add queue to playlist"
|
||||
>
|
||||
<wa-icon
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
/**
|
||||
* Warm the browser's image cache for the cards a scroll is about to
|
||||
* reach.
|
||||
*
|
||||
* #65: album art pops in while scrolling. The rule this app already
|
||||
* follows is that a row image is `loading="lazy" decoding="async"` and
|
||||
* draws the smallest adequate tier, and both halves are in place —
|
||||
* `cover-grid.getCoverUrl()` and `artists-view`'s avatar both pick
|
||||
* `_sm`/`_md`/`_lg` from the card size and the device pixel ratio. What
|
||||
* is left is *when* the fetch starts: the grids are virtualized, so the
|
||||
* `<img>` does not exist at all until the virtualizer decides to render
|
||||
* its card, and only then can the browser ask for anything.
|
||||
*
|
||||
* The issue's Direction asks for a larger overscan, and that is not
|
||||
* available: `@lit-labs/virtualizer`'s `_overhang` is a hard-coded
|
||||
* 1000px `protected` field on `BaseLayout` with no configuration
|
||||
* surface, so raising it means monkey-patching a private. 1000px is
|
||||
* about two screens on the reference device's 439px viewport, which is
|
||||
* a fraction of a second at speed.
|
||||
*
|
||||
* So the request is issued ahead of the element instead. Cover art and
|
||||
* artist images are plain URLs served by `coverart.Handler` /
|
||||
* `explore`'s image handler under `Cache-Control: public,
|
||||
* max-age=31536000, immutable` — the filenames are content hashes — so
|
||||
* a prefetched image is a cache hit by the time the card is drawn, and
|
||||
* a second pass over the same rows costs nothing at all.
|
||||
*
|
||||
* Three things about it are load-bearing.
|
||||
*
|
||||
* **This is not the `LRUMap` path the issue's Findings warn about.**
|
||||
* That ceiling (`ARTIST_IMAGE_CACHE_LIMIT` and friends) bounds
|
||||
* Explore's base64 data URLs, which are held in JS. A library cover is
|
||||
* a URL, and what retains the bytes is the browser's own HTTP cache,
|
||||
* which evicts on its own terms. What this module retains is the *set
|
||||
* of URLs already asked for*, which is why that set has a cap and
|
||||
* reports itself to `window.__yjCacheStats()` — the measurement the
|
||||
* issue asks for.
|
||||
*
|
||||
* **A window is warmed on both sides of the rendered range.** The
|
||||
* event carries no direction, and scrolling back up needs the same
|
||||
* treatment; the rows behind are already in `requested` from the pass
|
||||
* that rendered them, so the backward half issues nothing in the
|
||||
* common case and is free.
|
||||
*
|
||||
* **An in-flight image is held.** `new Image().src = url` and drop it
|
||||
* is the usual idiom and usually survives, but "usually" is an engine
|
||||
* detail and the engine that matters here is a two-year-old WebView.
|
||||
* The element is kept until it loads or fails, and no longer — nothing
|
||||
* here holds a decoded bitmap on purpose.
|
||||
*/
|
||||
|
||||
import { registerCacheProbe } from './cache-stats.js';
|
||||
import { LRUMap } from './lru-map.js';
|
||||
|
||||
/**
|
||||
* How many entries past each edge of the rendered range to warm.
|
||||
*
|
||||
* Entries rather than pixels, because that is what the event reports
|
||||
* and what the caller has an array of. Twelve rows on the phone's
|
||||
* two-column grid and four on a desktop's six, on top of the
|
||||
* virtualizer's own 1000px — enough to cover a flick, and bounded so a
|
||||
* fast scroll through 5 000 albums cannot ask for 5 000 covers.
|
||||
*/
|
||||
export const PREFETCH_AHEAD = 24;
|
||||
|
||||
/** Ceiling on the record of what has already been asked for. */
|
||||
export const PREFETCH_MEMORY = 512;
|
||||
|
||||
/** URLs already requested; the value is a placeholder, the key is the record. */
|
||||
const requested = new LRUMap<string, true>(PREFETCH_MEMORY);
|
||||
|
||||
/** Images still loading, held so the request cannot be collected. */
|
||||
const inFlight = new Set<HTMLImageElement>();
|
||||
|
||||
registerCacheProbe('imagePrefetch', () => {
|
||||
let chars = 0;
|
||||
|
||||
for (const url of requested.keys()) chars += url.length;
|
||||
|
||||
return { entries: requested.size, chars, limit: PREFETCH_MEMORY };
|
||||
});
|
||||
|
||||
/** Whether this URL has already been asked for. */
|
||||
export function imagePrefetched(url: string): boolean {
|
||||
return requested.has(url);
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask the browser for `url` unless it has already been asked for.
|
||||
* Returns whether a request was issued.
|
||||
*/
|
||||
export function prefetchImage(url: string): boolean {
|
||||
if (!url || requested.has(url)) return false;
|
||||
|
||||
requested.set(url, true);
|
||||
|
||||
const img = new Image();
|
||||
|
||||
inFlight.add(img);
|
||||
|
||||
const done = () => {
|
||||
inFlight.delete(img);
|
||||
};
|
||||
|
||||
img.addEventListener('load', done, { once: true });
|
||||
img.addEventListener('error', done, { once: true });
|
||||
img.decoding = 'async';
|
||||
img.src = url;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Warm the images either side of a virtualizer's rendered range.
|
||||
*
|
||||
* `first`/`last` are the indices the `visibilityChanged` event
|
||||
* reported; `urlOf` returns the image the card at that index will
|
||||
* draw, or `''` where it draws a placeholder. Returns how many
|
||||
* requests were issued, which is what a test can assert on and what
|
||||
* makes "a second run does approximately nothing" checkable.
|
||||
*/
|
||||
export function prefetchImageWindow<T>(
|
||||
items: readonly T[],
|
||||
first: number,
|
||||
last: number,
|
||||
urlOf: (item: T) => string,
|
||||
ahead: number = PREFETCH_AHEAD,
|
||||
): number {
|
||||
if (items.length === 0 || first < 0 || last < first) return 0;
|
||||
|
||||
const from = Math.max(0, first - ahead);
|
||||
const to = Math.min(items.length - 1, last + ahead);
|
||||
let issued = 0;
|
||||
|
||||
// Forward first: it is the direction a scroll is usually going, so
|
||||
// it is the half that has to win the race.
|
||||
for (let i = last + 1; i <= to; i++) {
|
||||
const item = items[i];
|
||||
|
||||
if (item !== undefined && prefetchImage(urlOf(item))) issued++;
|
||||
}
|
||||
|
||||
for (let i = from; i < first; i++) {
|
||||
const item = items[i];
|
||||
|
||||
if (item !== undefined && prefetchImage(urlOf(item))) issued++;
|
||||
}
|
||||
|
||||
return issued;
|
||||
}
|
||||
|
||||
/** Forget what has been asked for. For tests; the app never needs it. */
|
||||
export function resetImagePrefetch(): void {
|
||||
requested.clear();
|
||||
inFlight.clear();
|
||||
}
|
||||
@@ -0,0 +1,190 @@
|
||||
/**
|
||||
* The grids ask for the art below the fold before the card exists
|
||||
* (#65).
|
||||
*
|
||||
* Reported as "scrolling through albums, the art pops in". The cards
|
||||
* already draw the smallest adequate tier and are already
|
||||
* `loading="lazy"`, so what was left is *when*: `<lit-virtualizer>`
|
||||
* renders about 1000px past the viewport and the `<img>` — and
|
||||
* therefore the request — does not exist until it does. On the
|
||||
* reference device that is about two screens.
|
||||
*
|
||||
* These assert the mechanism, since no tier here can photograph a
|
||||
* pop-in: that the rows past the rendered range are requested, that
|
||||
* the request is for the same tier the card will draw, and that the
|
||||
* window has an end — an unbounded prefetch of a 5 000-album library
|
||||
* is the failure this trades against.
|
||||
*
|
||||
* What is *not* asserted here is that a rendered card was never
|
||||
* prefetched. It often was, honestly: the grid lays out more than once
|
||||
* on mount, so a row warmed by the first pass is drawn by the second,
|
||||
* which is the whole point. The rule that a single pass skips its own
|
||||
* rendered range is `image-prefetch.test.ts`'s, where one call can be
|
||||
* looked at on its own.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
import type { LitElement } from 'lit';
|
||||
|
||||
import '@components/cover-grid/cover-grid';
|
||||
import '@components/artists-view/artists-view';
|
||||
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
||||
import { Events } from '../../src/events';
|
||||
import { fixture, shadowAll } from '@test/support/render';
|
||||
import {
|
||||
PREFETCH_AHEAD,
|
||||
imagePrefetched,
|
||||
resetImagePrefetch,
|
||||
} from '@utils/image-prefetch';
|
||||
|
||||
/** Enough albums that the virtualizer's own window is nowhere near the end. */
|
||||
const ALBUMS = Array.from({ length: 400 }, (_, i) => {
|
||||
const n = String(i + 1).padStart(4, '0');
|
||||
|
||||
return {
|
||||
ID: i + 1,
|
||||
Name: `Album ${n}`,
|
||||
ArtistName: 'Aurora Fields',
|
||||
Year: 2020,
|
||||
CoverArtPath: `/covers/${n}.jpg`,
|
||||
CoverArtSmall: `/covers/${n}_sm.jpg`,
|
||||
CoverArtMedium: `/covers/${n}_md.jpg`,
|
||||
CoverArtLarge: `/covers/${n}_lg.jpg`,
|
||||
};
|
||||
});
|
||||
|
||||
const ARTISTS = Array.from({ length: 400 }, (_, i) => {
|
||||
const n = String(i + 1).padStart(4, '0');
|
||||
|
||||
return {
|
||||
ID: i + 1,
|
||||
Name: `Artist ${n}`,
|
||||
AlbumCount: 2,
|
||||
TrackCount: 9,
|
||||
ImageSmall: `/artists/${n}_sm.jpg`,
|
||||
ImageMedium: `/artists/${n}_md.jpg`,
|
||||
ImageLarge: `/artists/${n}_lg.jpg`,
|
||||
};
|
||||
});
|
||||
|
||||
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
|
||||
function sized(el: HTMLElement): void {
|
||||
el.style.display = 'block';
|
||||
el.style.height = '600px';
|
||||
el.style.width = '900px';
|
||||
}
|
||||
|
||||
async function settle(el: LitElement): Promise<void> {
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
await new Promise((r) => setTimeout(r, 200));
|
||||
}
|
||||
|
||||
/** The `src` of every card the grid actually rendered. */
|
||||
function renderedSources(el: LitElement, selector: string): string[] {
|
||||
return shadowAll(el, selector)
|
||||
.map((img) => (img as HTMLImageElement).getAttribute('src') ?? '')
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
/**
|
||||
* The last index the virtualizer has rendered, read off the cards
|
||||
* rather than counted: the rendered range is what the prefetch window
|
||||
* is measured from, and a count assumes it starts at 0 and has no
|
||||
* gaps.
|
||||
*/
|
||||
function lastRenderedIndex(el: LitElement, selector: string): number {
|
||||
const indices = shadowAll(el, selector).map((card) =>
|
||||
Number(card.getAttribute('data-index')),
|
||||
);
|
||||
|
||||
return Math.max(...indices);
|
||||
}
|
||||
|
||||
/**
|
||||
* The tier the cards chose, read off a rendered card rather than
|
||||
* recomputed — the point of the assertion is that the prefetch and the
|
||||
* card agree, so deriving both from the same ladder here would prove
|
||||
* nothing.
|
||||
*/
|
||||
function tierSuffix(src: string): string {
|
||||
const m = /_(sm|md|lg)\.jpg$/.exec(src);
|
||||
|
||||
return m ? `_${m[1]}` : '';
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
resetHarness();
|
||||
resetImagePrefetch();
|
||||
localStorage.clear();
|
||||
stub('library.Library.GetAlbums', ALBUMS);
|
||||
stub('library.Library.GetArtists', ARTISTS);
|
||||
stub('library.Library.GetTracks', []);
|
||||
stub('library.Library.GetGenres', []);
|
||||
emit(Events.LibraryScanComplete);
|
||||
});
|
||||
|
||||
describe('the albums grid warms the covers below the fold', () => {
|
||||
it('asks for the covers past the rendered range, in the tier the card draws', async () => {
|
||||
const el = await fixture<LitElement>('cover-grid');
|
||||
|
||||
sized(el);
|
||||
await settle(el);
|
||||
|
||||
const rendered = renderedSources(el, 'img.cover-image');
|
||||
|
||||
expect(rendered.length).toBeGreaterThan(0);
|
||||
|
||||
const tier = tierSuffix(rendered[0]!);
|
||||
const url = (index: number) =>
|
||||
`/covers/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
|
||||
|
||||
// The grid starts at the top and never scrolls here, so the whole
|
||||
// window lies past the last card drawn.
|
||||
const last = lastRenderedIndex(el, '.album-card');
|
||||
|
||||
expect(imagePrefetched(url(last + 1))).toBe(true);
|
||||
expect(imagePrefetched(url(last + PREFETCH_AHEAD))).toBe(true);
|
||||
});
|
||||
|
||||
it('stops at the end of the window rather than warming the library', async () => {
|
||||
const el = await fixture<LitElement>('cover-grid');
|
||||
|
||||
sized(el);
|
||||
await settle(el);
|
||||
|
||||
const rendered = renderedSources(el, 'img.cover-image');
|
||||
const tier = tierSuffix(rendered[0]!);
|
||||
const url = (index: number) =>
|
||||
`/covers/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
|
||||
|
||||
// Not "exactly `last + PREFETCH_AHEAD`": the grid lays out more
|
||||
// than once on mount and each pass warms a window from wherever
|
||||
// the rendered range was then, so the reachable set is a few
|
||||
// windows wide. The property that matters is that it is a window
|
||||
// at all rather than the library.
|
||||
expect(imagePrefetched(url(399))).toBe(false);
|
||||
expect(window.__yjCacheStats?.()['imagePrefetch']?.entries ?? 0)
|
||||
.toBeLessThan(ALBUMS.length / 2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the artists grid warms its avatars the same way', () => {
|
||||
it('asks for the avatars past the rendered range', async () => {
|
||||
const el = await fixture<LitElement>('artists-view');
|
||||
|
||||
sized(el);
|
||||
await settle(el);
|
||||
|
||||
const rendered = renderedSources(el, 'img.avatar-image');
|
||||
|
||||
expect(rendered.length).toBeGreaterThan(0);
|
||||
|
||||
const tier = tierSuffix(rendered[0]!);
|
||||
const last = lastRenderedIndex(el, '.artist-card');
|
||||
const url = (index: number) =>
|
||||
`/artists/${String(index + 1).padStart(4, '0')}${tier}.jpg`;
|
||||
|
||||
expect(imagePrefetched(url(last + 1))).toBe(true);
|
||||
expect(imagePrefetched(url(399))).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,2 @@
|
||||
<!-- A real, servable image for the prefetch tests: one transparent pixel. -->
|
||||
<svg xmlns="http://www.w3.org/2000/svg" width="1" height="1"></svg>
|
||||
|
After Width: | Height: | Size: 147 B |
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* What the grids ask for ahead of the scroll (#65).
|
||||
*
|
||||
* The virtualizer renders about 1000px past its viewport and nothing
|
||||
* else can be asked for, because the `<img>` does not exist until the
|
||||
* card does — two screens on the reference device, which is a fraction
|
||||
* of a second at speed. `prefetchImageWindow` issues the request
|
||||
* before the element, so the assertions here are about *which* rows
|
||||
* are asked for, that none is asked for twice, and that a request is
|
||||
* really made rather than merely recorded.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
|
||||
import {
|
||||
PREFETCH_MEMORY,
|
||||
imagePrefetched,
|
||||
prefetchImage,
|
||||
prefetchImageWindow,
|
||||
resetImagePrefetch,
|
||||
} from '@utils/image-prefetch';
|
||||
|
||||
/** A hundred cards, each with its own cover URL. */
|
||||
const CARDS = Array.from({ length: 100 }, (_, i) => ({ url: `/covers/${i}_sm.jpg` }));
|
||||
|
||||
const urlOf = (card: { url: string }) => card.url;
|
||||
|
||||
beforeEach(() => {
|
||||
resetImagePrefetch();
|
||||
});
|
||||
|
||||
describe('warming the images a scroll is about to reach', () => {
|
||||
it('asks for the rows just past the rendered range, and no further', () => {
|
||||
const issued = prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
|
||||
|
||||
// Three past each edge: 51-53 and 37-39.
|
||||
expect(issued).toBe(6);
|
||||
expect(imagePrefetched('/covers/51_sm.jpg')).toBe(true);
|
||||
expect(imagePrefetched('/covers/53_sm.jpg')).toBe(true);
|
||||
expect(imagePrefetched('/covers/54_sm.jpg')).toBe(false);
|
||||
expect(imagePrefetched('/covers/39_sm.jpg')).toBe(true);
|
||||
expect(imagePrefetched('/covers/37_sm.jpg')).toBe(true);
|
||||
expect(imagePrefetched('/covers/36_sm.jpg')).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves the rendered rows alone — they have their own <img>', () => {
|
||||
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
|
||||
|
||||
expect(imagePrefetched('/covers/45_sm.jpg')).toBe(false);
|
||||
});
|
||||
|
||||
it('asks for nothing twice, so a scroll back over the same rows is free', () => {
|
||||
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
|
||||
|
||||
expect(prefetchImageWindow(CARDS, 40, 50, urlOf, 3)).toBe(0);
|
||||
});
|
||||
|
||||
it('clamps at both ends of the list', () => {
|
||||
// At the top of a five-item list nothing precedes the range, and
|
||||
// the tail runs out after two.
|
||||
expect(prefetchImageWindow(CARDS.slice(0, 5), 0, 2, urlOf, 10)).toBe(2);
|
||||
});
|
||||
|
||||
it('asks for nothing when the virtualizer reports an empty range', () => {
|
||||
// `visibilityChanged` reports -1/-1 before anything is laid out.
|
||||
expect(prefetchImageWindow(CARDS, -1, -1, urlOf)).toBe(0);
|
||||
});
|
||||
|
||||
it('skips a card that draws a placeholder rather than an image', () => {
|
||||
expect(prefetchImageWindow(CARDS, 40, 50, () => '', 3)).toBe(0);
|
||||
});
|
||||
|
||||
it('really issues the request, rather than only recording it', async () => {
|
||||
// A served file, so the load succeeds and the resource timing entry
|
||||
// is unambiguous; the query string keeps it distinct per run.
|
||||
const url = `/test/support/pixel.svg?prefetch=${Date.now()}`;
|
||||
const href = new URL(url, location.href).href;
|
||||
|
||||
expect(prefetchImage(url)).toBe(true);
|
||||
|
||||
for (let i = 0; i < 100; i++) {
|
||||
if (performance.getEntriesByName(href).length > 0) break;
|
||||
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
}
|
||||
|
||||
expect(performance.getEntriesByName(href)).toHaveLength(1);
|
||||
expect(prefetchImage(url)).toBe(false);
|
||||
expect(performance.getEntriesByName(href)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('reports what it is holding, with its cap, to the cache stats', () => {
|
||||
prefetchImageWindow(CARDS, 40, 50, urlOf, 3);
|
||||
|
||||
const stat = window.__yjCacheStats?.()['imagePrefetch'];
|
||||
|
||||
expect(stat).toBeTruthy();
|
||||
expect(stat!.entries).toBe(6);
|
||||
expect(stat!.limit).toBe(PREFETCH_MEMORY);
|
||||
// It holds URLs, not images — the bytes are the browser's cache.
|
||||
expect(stat!.chars).toBe(6 * '/covers/51_sm.jpg'.length);
|
||||
});
|
||||
|
||||
it('keeps its record bounded, so a 50 000-album scroll cannot grow it', () => {
|
||||
const many = Array.from(
|
||||
{ length: PREFETCH_MEMORY * 2 },
|
||||
(_, i) => ({ url: `/covers/bulk-${i}_sm.jpg` }),
|
||||
);
|
||||
|
||||
prefetchImageWindow(many, 0, 0, urlOf, many.length);
|
||||
|
||||
expect(window.__yjCacheStats?.()['imagePrefetch']?.entries).toBe(PREFETCH_MEMORY);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user