Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f8c8d374d1 | ||
|
|
a113b7bd62 | ||
|
|
245647f12b | ||
|
|
3479ae8d39 | ||
|
|
e23e6f9a54 | ||
|
|
52d095e3c6 | ||
|
|
939915b1fa |
@@ -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.
|
||||
|
||||
@@ -1113,9 +1113,9 @@ not the fix and cannot be: that function is the `document` listener for
|
||||
`navigate`, so it is an infinite loop.
|
||||
|
||||
**It is a store rather than an event, because a component that mounts
|
||||
after a navigation still has to know.** `bottom-nav`'s "More" drawer
|
||||
after a navigation still has to know.** `bottom-nav`'s "More" sheet
|
||||
creates its `<app-sidebar>` on open, and that copy had heard no
|
||||
`navigate` at all — standing on Albums, the drawer opened highlighting
|
||||
`navigate` at all — standing on Albums, it opened highlighting
|
||||
Home. An event has no answer for a listener that was not there.
|
||||
|
||||
**A detail view is not a view here**, so the destination it was opened
|
||||
@@ -1216,7 +1216,7 @@ and then vanishing.
|
||||
than a general rule about phones.** `PHONE_COLUMN_IDS` is the precedent
|
||||
for "what a phone shows is a different question", and it would apply —
|
||||
except that `bottom-nav`'s "More" opens the *same* `<app-sidebar>`,
|
||||
which filters, so an unfiltered bar would contradict its own drawer one
|
||||
which filters, so an unfiltered bar would contradict its own sheet one
|
||||
tap away. Which four tabs is still plan 016's committed subset; this
|
||||
only removes from it, and "More" is never filtered because it is how
|
||||
everything else stays reachable.
|
||||
@@ -1874,7 +1874,7 @@ listing the destinations again — but rendering it unconditionally put a
|
||||
second copy of every `data-testid="nav-*"` in the DOM, and 30 existing
|
||||
specs failed with "strict mode violation: resolved to 2 elements" on a
|
||||
desktop viewport where the element is not even visible. It renders only
|
||||
while the drawer is open, and `bottom-nav.test.ts` asserts its absence
|
||||
while the sheet is open, and `bottom-nav.test.ts` asserts its absence
|
||||
before that.
|
||||
|
||||
**The tab bar is four destinations and a way to the rest.** Three to
|
||||
@@ -1883,6 +1883,33 @@ is 32px each. Which four is plan 016's committed subset, and everything
|
||||
else — Settings included, because a phone still needs it — is behind
|
||||
"More".
|
||||
|
||||
**And "More" rises from the bottom, on #60's sheet rather than a
|
||||
second one** (#71). It was a `wa-drawer placement="start"`: a 200px
|
||||
column of a 424px screen, opening away from the thumb that asked for
|
||||
it, with the rest of its 400px band empty. It is the *same element*
|
||||
with `placement="bottom"` and `without-header`, which is what keeps
|
||||
the change to where it comes from — `wa-drawer` renders a native
|
||||
`<dialog>` and opens it with `showModal()`, so #60's containment
|
||||
finding carries over with nothing new to prove, and the focus trap,
|
||||
Escape, tap-outside and `wa-after-hide` all come along. Measured at
|
||||
424x439: 424 wide, 373 tall (85vh, so there is an outside to tap),
|
||||
48px rows.
|
||||
|
||||
Three things about it are load-bearing. **The sidebar is mounted
|
||||
rather than re-listed as data**, which the issue offers as the
|
||||
alternative: the shell's own `<app-sidebar>` is `display: none` below
|
||||
600px rather than removed, so a second list drawing `nav-*` handles is
|
||||
the duplication above, and it would be a second place to add the next
|
||||
view to. **There is one scroller, and it is the sheet's body** — the
|
||||
reported "only part of the screen scrolls under my finger" is three
|
||||
nested ones (the dialog, its body, and the sidebar's own
|
||||
`overflow-y: auto` host), so which box a drag moves depends on where
|
||||
the finger landed; `overscroll-behavior: contain` is the other half.
|
||||
And **`expanded` means the host owns the box, not just the labels**:
|
||||
`app-sidebar` writes an *inline* width and caps itself at 400px, which
|
||||
beats any rule the host could write, so the width, the scrolling and
|
||||
the mouse-only resize handle all follow that attribute.
|
||||
|
||||
**There are three supported size bands, and the queue is part of the
|
||||
promise.** Plan 018 (#24) wrote them down: **Phone** below 600 (bottom
|
||||
nav, reflows, fits 320px exactly), **Compact** 600–899 (icon sidebar),
|
||||
@@ -3376,6 +3403,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
|
||||
|
||||
@@ -63,12 +63,15 @@ func Parse(r io.Reader) ([]Chunk, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
data := make([]byte, size)
|
||||
if _, err := io.ReadFull(r, data); err != nil {
|
||||
// Copied rather than allocated up front, as ID3Chunk does: the
|
||||
// size is four bytes off the file, so a truncated one is free to
|
||||
// declare a chunk larger than the whole of itself.
|
||||
var data bytes.Buffer
|
||||
if _, err := io.CopyN(&data, r, int64(size)); err != nil {
|
||||
return nil, fmt.Errorf("read chunk data for %q: %w", id, err)
|
||||
}
|
||||
|
||||
chunks = append(chunks, Chunk{ID: id, Data: data})
|
||||
chunks = append(chunks, Chunk{ID: id, Data: data.Bytes()})
|
||||
|
||||
// Odd-length chunks have a padding byte. Lenient: if the
|
||||
// read fails (e.g. EOF), just break rather than error.
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"encoding/binary"
|
||||
"errors"
|
||||
"runtime"
|
||||
"testing"
|
||||
|
||||
"yellowjacket/backend/riff"
|
||||
@@ -209,3 +210,45 @@ func TestParse_ReadsEveryChunkInOrder(t *testing.T) {
|
||||
t.Errorf("odd chunk data: got %q, want %q", chunks[1].Data, "INFOodd")
|
||||
}
|
||||
}
|
||||
|
||||
// A chunk size is four bytes read off the file, so a truncated or
|
||||
// malformed WAV is free to declare a chunk larger than the whole of
|
||||
// itself. Parse must grow with what arrives rather than with what was
|
||||
// claimed.
|
||||
//
|
||||
// This measures the allocation instead of the error because the error
|
||||
// is the same either way: a build sizing its buffer from the header
|
||||
// reports the truncation correctly, having asked the allocator for a
|
||||
// gigabyte on the way. Deliberately not parallel — TotalAlloc is
|
||||
// process-wide, and a test paused beside another one is measuring it
|
||||
// too.
|
||||
func TestParse_DoesNotAllocateWhatAChunkClaims(t *testing.T) {
|
||||
// Large enough that a header-sized buffer is unmistakable, in a
|
||||
// container of a few dozen bytes.
|
||||
const declared = 1 << 30
|
||||
|
||||
var raw bytes.Buffer
|
||||
|
||||
raw.WriteString("RIFF")
|
||||
_ = binary.Write(&raw, binary.LittleEndian, uint32(declared+12))
|
||||
raw.WriteString("WAVE")
|
||||
raw.WriteString("data")
|
||||
_ = binary.Write(&raw, binary.LittleEndian, uint32(declared))
|
||||
raw.WriteString("and then the file ends")
|
||||
|
||||
var before, after runtime.MemStats
|
||||
|
||||
runtime.GC()
|
||||
runtime.ReadMemStats(&before)
|
||||
|
||||
if _, err := riff.Parse(bytes.NewReader(raw.Bytes())); err == nil {
|
||||
t.Fatal("Parse: got nil error for a chunk larger than the file holding it")
|
||||
}
|
||||
|
||||
runtime.ReadMemStats(&after)
|
||||
|
||||
if grew := after.TotalAlloc - before.TotalAlloc; grew > 1<<20 {
|
||||
t.Errorf("Parse allocated %d bytes reading a %d-byte file whose chunk header claimed %d",
|
||||
grew, raw.Len(), declared)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,6 +98,58 @@ test.describe('the shell on a phone', () => {
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
test('draws "More" as a sheet on the bottom edge (#71)', async ({ app }) => {
|
||||
await app.getByTestId('tab-more').click();
|
||||
await expect(app.getByTestId('nav-drawer').locator('app-sidebar'))
|
||||
.toBeVisible();
|
||||
|
||||
// What the report is about is geometry, and geometry is what no
|
||||
// other assertion here can see: the side drawer was a 200px column
|
||||
// opening away from the thumb that asked for it, with the rest of
|
||||
// its 400px band empty. Measured rather than screenshotted, since
|
||||
// the failure is a number.
|
||||
//
|
||||
// Polled, because a sheet *arrives*: the drawer's show animation
|
||||
// translates it a full height below the fold, so a measurement
|
||||
// taken the moment its content is visible reports a box hanging
|
||||
// 412px off the bottom of the screen. Asking for the settled
|
||||
// number is the assertion; asking once is a race.
|
||||
const measure = () => app.evaluate(() => {
|
||||
const nav = document.querySelector('bottom-nav');
|
||||
const drawer = nav?.shadowRoot?.querySelector('wa-drawer');
|
||||
const dialog = drawer?.shadowRoot?.querySelector('[part~="dialog"]');
|
||||
const sidebar = nav?.shadowRoot?.querySelector('app-sidebar');
|
||||
const row = sidebar?.shadowRoot?.querySelector('li button');
|
||||
const box = dialog?.getBoundingClientRect();
|
||||
|
||||
return {
|
||||
left: Math.round(box?.left ?? -1),
|
||||
right: Math.round(box?.right ?? -1),
|
||||
bottom: Math.round(box?.bottom ?? -1),
|
||||
height: Math.round(box?.height ?? -1),
|
||||
row: Math.round(row?.getBoundingClientRect().height ?? -1),
|
||||
viewport: [window.innerWidth, window.innerHeight],
|
||||
};
|
||||
});
|
||||
|
||||
await expect
|
||||
.poll(async () => (await measure()).bottom)
|
||||
.toBe(PHONE.height);
|
||||
|
||||
const sheet = await measure();
|
||||
|
||||
expect(sheet.left).toBe(0);
|
||||
expect(sheet.right).toBe(sheet.viewport[0]);
|
||||
|
||||
// A surface covering the whole screen is a page, not a sheet --
|
||||
// which is also what leaves an outside to tap on, the only pointer
|
||||
// route out of it (#171 is the same question one surface over).
|
||||
expect(sheet.height).toBeLessThan(sheet.viewport[1]);
|
||||
|
||||
// 48px rows, from #186's touch floor and #60's context sheet.
|
||||
expect(sheet.row).toBeGreaterThanOrEqual(48);
|
||||
});
|
||||
|
||||
for (const vp of [PHONE, SMALL_PHONE]) {
|
||||
test(`does not scroll sideways at ${vp.width}×${vp.height}`, async ({ app }) => {
|
||||
await app.setViewportSize(vp);
|
||||
|
||||
@@ -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()}
|
||||
|
||||
@@ -27,15 +27,43 @@ interface Tab {
|
||||
* three to five items before the targets stop being thumb-sized —
|
||||
* 360 px over eleven sidebar entries is 32 px each — so the four here
|
||||
* are the ones plan 016's subset says a phone is *for*, and "More"
|
||||
* opens the existing `<app-sidebar>` in a drawer. That is deliberately
|
||||
* opens the existing `<app-sidebar>` in a sheet. That is deliberately
|
||||
* a reuse rather than a second nav: two lists of destinations is two
|
||||
* places to add the next view to, and the sidebar already carries the
|
||||
* drag-to-navigate behaviour, the active state and the labels.
|
||||
*
|
||||
* **"More" rises from the bottom, and it is the same sheet a context
|
||||
* menu is** (#71). It was a `wa-drawer` sliding in from the side: a
|
||||
* 200px column of a 424px screen, opening away from the thumb that
|
||||
* asked for it, with three nested scrollers in it — the dialog, its
|
||||
* body, and the sidebar's own `overflow-y: auto` host — which is the
|
||||
* "only part of the screen scrolls under my finger" in the report.
|
||||
*
|
||||
* Three things about the replacement are load-bearing.
|
||||
*
|
||||
* **It is the same element with another `placement`, not a new
|
||||
* surface.** `wa-drawer` renders a native `<dialog>` and opens it with
|
||||
* `showModal()`, which is exactly what `menu-surface`'s sheet relies
|
||||
* on — Chrome 37, the real top layer — so #60's containment finding
|
||||
* carries over with nothing new to prove, and the focus trap, Escape,
|
||||
* tap-outside and `wa-after-hide` all come along unchanged.
|
||||
*
|
||||
* **The body is the only scroller**, with `overscroll-behavior:
|
||||
* contain`, and the sidebar is told to stop being one. Nesting them is
|
||||
* what makes a drag scroll the wrong box.
|
||||
*
|
||||
* **The sidebar is still mounted rather than re-listed as data**,
|
||||
* which the issue offers as an alternative. Its `data-testid` per
|
||||
* destination is the reason: the shell's own sidebar is `display:
|
||||
* none` below 600px rather than removed, so a second list drawing
|
||||
* `nav-*` handles is the duplication this component already renders
|
||||
* conditionally to avoid — and it would be a second place to add the
|
||||
* next view to, with its own copy of #25's visibility filter.
|
||||
*
|
||||
* It emits the same bubbling, composed `navigate` event the sidebar
|
||||
* does, so `index.ts` needs no knowledge of it, and it listens for that
|
||||
* event globally for the same reason the sidebar does: a navigation it
|
||||
* did not send (a card click, a detail view, the drawer) still has to
|
||||
* did not send (a card click, a detail view, the sheet) still has to
|
||||
* move the highlight.
|
||||
*/
|
||||
@customElement('bottom-nav')
|
||||
@@ -115,15 +143,50 @@ export class BottomNav extends LitElement {
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
wa-drawer::part(body) {
|
||||
padding: 0;
|
||||
/* The sheet. --size is the drawer's own API for the axis its
|
||||
placement uses, so auto is what makes it hug its content
|
||||
instead of being a fixed 25rem band; the rest is the shape
|
||||
the menu-surface context sheet already has, so a phone meets
|
||||
one sheet rather than two. 85vh for its reason too: a surface
|
||||
covering the whole screen is a page, not a sheet. */
|
||||
wa-drawer {
|
||||
--size: auto;
|
||||
}
|
||||
|
||||
app-sidebar {
|
||||
/* The sidebar sizes itself inline and collapses to icons
|
||||
below 900px, which is every phone. In the drawer there
|
||||
is room for the labels, so it is told not to. */
|
||||
height: 100%;
|
||||
wa-drawer::part(dialog) {
|
||||
max-height: 85vh;
|
||||
border-radius: 12px 12px 0 0;
|
||||
/* The sidebar paints its own surface, so the sheet takes
|
||||
that colour rather than the menus' elevated one: two
|
||||
greys in one sheet is a seam across the middle of it. */
|
||||
background-color: var(--yj-bg-surface, #212529);
|
||||
/* One scroller, and it is the body below. The dialog's own
|
||||
overflow: auto is what let the sheet scroll as well as
|
||||
its content, and it is also what would square off the
|
||||
corners this rule just rounded. */
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
wa-drawer::part(body) {
|
||||
padding: 0;
|
||||
/* A scroll that reaches the end of this list must not
|
||||
become a scroll of the page underneath it. */
|
||||
overscroll-behavior: contain;
|
||||
/* The sheet sits on the bottom edge, so the last
|
||||
destination would otherwise be under the home indicator
|
||||
on a gesture-navigation phone -- the same allowance the
|
||||
bar itself makes above. */
|
||||
padding-bottom: env(safe-area-inset-bottom, 0);
|
||||
}
|
||||
|
||||
/* A sheet is dragged at with a thumb, so it says where its top
|
||||
edge is. Decorative: the destinations are below it. */
|
||||
.grip {
|
||||
width: 36px;
|
||||
height: 4px;
|
||||
margin: 8px auto 4px;
|
||||
border-radius: 2px;
|
||||
background: var(--yj-text-tertiary, #888);
|
||||
}
|
||||
`];
|
||||
|
||||
@@ -158,7 +221,7 @@ export class BottomNav extends LitElement {
|
||||
private visibilityCtrl = new ViewVisibilityController(this);
|
||||
|
||||
/**
|
||||
* Whether the drawer has been asked for.
|
||||
* Whether the sheet has been asked for.
|
||||
*
|
||||
* The sidebar inside it is rendered only while this is true, and
|
||||
* that is not an optimisation. `app-sidebar` carries a
|
||||
@@ -200,15 +263,17 @@ export class BottomNav extends LitElement {
|
||||
|
||||
override updated() {
|
||||
// Web Awesome renders its heading into its own shadow root and
|
||||
// never points aria-labelledby at it, so the drawer would
|
||||
// never points aria-labelledby at it, so the sheet would
|
||||
// otherwise be announced unnamed -- the same fix, and the same
|
||||
// reason, as every wa-dialog in the app. A drawer's shadow root
|
||||
// has the same shape, so the helper needs no change.
|
||||
// has the same shape, so the helper needs no change; under
|
||||
// `without-header` there is no heading to point at, which is
|
||||
// that helper's documented `aria-label` path.
|
||||
nameDialog(this.drawer);
|
||||
}
|
||||
|
||||
private onGlobalNavigate = () => {
|
||||
// A navigation from inside the drawer is the drawer's job done.
|
||||
// A navigation from inside the sheet is the sheet's job done.
|
||||
// The highlight is not this listener's business any more.
|
||||
this.drawerOpen = false;
|
||||
};
|
||||
@@ -274,12 +339,14 @@ export class BottomNav extends LitElement {
|
||||
</nav>
|
||||
|
||||
<wa-drawer
|
||||
placement="start"
|
||||
placement="bottom"
|
||||
without-header
|
||||
label="All views"
|
||||
data-testid="nav-drawer"
|
||||
?open=${this.drawerOpen}
|
||||
@wa-after-hide=${this.onDrawerHide}
|
||||
>
|
||||
<div class="grip"></div>
|
||||
${this.drawerOpen
|
||||
? html`<app-sidebar expanded></app-sidebar>`
|
||||
: nothing}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -42,6 +42,26 @@ export class AppSidebar extends LitElement {
|
||||
scrollbar-width: thin;
|
||||
}
|
||||
|
||||
/* A host that has made room owns the box, not just the labels
|
||||
(#71). The bottom-nav sheet is the width of the screen and
|
||||
provides the one scroll container it needs; left to itself
|
||||
the sidebar is a 200px column with a second scroller inside
|
||||
it, which is what a nested scroll region feels like under a
|
||||
thumb -- part of the surface moves and part of it does not. */
|
||||
:host([expanded]) {
|
||||
max-width: none;
|
||||
height: auto;
|
||||
overflow: visible;
|
||||
}
|
||||
|
||||
/* And the width is not draggable there. It is a mouse
|
||||
affordance (mousedown, col-resize) sitting on the right edge
|
||||
of a touch surface, where the compatibility mouse events a
|
||||
tap synthesises can start a resize nobody asked for. */
|
||||
:host([expanded]) .resize-handle {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.resize-handle {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
@@ -160,6 +180,25 @@ export class AppSidebar extends LitElement {
|
||||
:host(.collapsed) li button wa-icon {
|
||||
font-size: var(--yj-icon-md);
|
||||
}
|
||||
|
||||
/* Below 600px the only place this renders is the bottom-nav
|
||||
sheet -- the shell's own copy is display: none there -- so
|
||||
the rows are sized for the thumb that opened it: 48px, which
|
||||
is #186's floor and the height every row in #60's context
|
||||
sheet already has. A media query inside a shadow root is
|
||||
answered by the viewport, so the component states this
|
||||
itself rather than the sheet reaching in. */
|
||||
@media (max-width: 599px) {
|
||||
ul {
|
||||
padding: 4px 8px 8px;
|
||||
}
|
||||
|
||||
li button {
|
||||
min-height: 48px;
|
||||
padding: 12px 10px;
|
||||
gap: 14px;
|
||||
}
|
||||
}
|
||||
`];
|
||||
|
||||
/** Delay in ms before a drag-hover triggers navigation. */
|
||||
@@ -188,11 +227,14 @@ export class AppSidebar extends LitElement {
|
||||
|
||||
/**
|
||||
* Keep the labels regardless of the viewport, for a host that has
|
||||
* made room for them -- `bottom-nav`'s drawer, which is the whole
|
||||
* made room for them -- `bottom-nav`'s sheet, which is the whole
|
||||
* screen wide on the phone where this would otherwise auto-collapse
|
||||
* to icons. The auto-collapse is a *width* response to a narrow
|
||||
* shell, and inside a drawer the shell is not what the sidebar is
|
||||
* shell, and inside a sheet the shell is not what the sidebar is
|
||||
* sharing space with.
|
||||
*
|
||||
* It says the host owns the *box*, not only the labels: the width,
|
||||
* the scrolling and the resize handle all follow it (#71).
|
||||
*/
|
||||
@property({ type: Boolean, reflect: true })
|
||||
expanded = false;
|
||||
@@ -365,9 +407,18 @@ export class AppSidebar extends LitElement {
|
||||
* be a media query in the stylesheet.
|
||||
*/
|
||||
private applyViewportWidth() {
|
||||
const narrow =
|
||||
!this.expanded &&
|
||||
(this.narrowViewport?.matches ?? false);
|
||||
// A host that made room decides how much: `bottom-nav`'s sheet
|
||||
// is the whole screen wide, and the inline width below -- which
|
||||
// beats any rule the host could write -- would draw the old
|
||||
// 200px side drawer inside it.
|
||||
if (this.expanded) {
|
||||
this.style.width = '100%';
|
||||
this.collapsed = false;
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const narrow = this.narrowViewport?.matches ?? false;
|
||||
const width = narrow
|
||||
? MIN_WIDTH
|
||||
: this.userWidth;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -193,4 +193,119 @@ describe('bottom-nav', () => {
|
||||
expect(shadow<HTMLElement>(el, 'app-sidebar')?.hasAttribute('expanded'))
|
||||
.toBe(true);
|
||||
});
|
||||
|
||||
it('draws "More" as a sheet rising from the bottom', async () => {
|
||||
const el = await fixture<Nav>('bottom-nav');
|
||||
const drawer = shadow<HTMLElement>(el, 'wa-drawer');
|
||||
|
||||
// #71. A side drawer is a desktop shape: it opened away from the
|
||||
// thumb that asked for it and drew a 200px column of a 424px
|
||||
// screen. `placement` is the whole of the change to *where* it
|
||||
// comes from, and `without-header` is what makes it the same sheet
|
||||
// `menu-surface` draws rather than a second pattern with a title
|
||||
// bar and a close button.
|
||||
expect(drawer?.getAttribute('placement')).toBe('bottom');
|
||||
expect(drawer?.hasAttribute('without-header')).toBe(true);
|
||||
|
||||
// Named all the same: `nameDialog`'s documented aria-label path,
|
||||
// since without-header renders no heading to point at.
|
||||
expect(drawer?.getAttribute('label')).toBe('All views');
|
||||
});
|
||||
|
||||
it('leaves exactly one scroll container, and it is the sheet body', async () => {
|
||||
const el = await fixture<Nav>('bottom-nav');
|
||||
const drawer = shadow<HTMLElement & { open: boolean }>(el, 'wa-drawer');
|
||||
|
||||
if (!drawer) throw new Error('no drawer');
|
||||
|
||||
const shown = once(drawer, 'wa-after-show');
|
||||
|
||||
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
|
||||
await shown;
|
||||
await update(el, {});
|
||||
|
||||
// The report is "only part of the screen scrolls under my finger",
|
||||
// and the cause is three boxes that each scroll: the dialog, its
|
||||
// body, and the sidebar's own overflow-y host. Which one a drag
|
||||
// moves depends on where the finger landed.
|
||||
const dialog = drawer.shadowRoot?.querySelector('[part~="dialog"]');
|
||||
const body = drawer.shadowRoot?.querySelector('[part~="body"]');
|
||||
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
|
||||
|
||||
if (!dialog || !body || !sidebar) throw new Error('no sheet');
|
||||
|
||||
expect(getComputedStyle(dialog).overflowY).toBe('hidden');
|
||||
expect(getComputedStyle(body).overflowY).toBe('auto');
|
||||
expect(getComputedStyle(sidebar).overflowY).toBe('visible');
|
||||
|
||||
// And the one that does scroll keeps it to itself, or reaching the
|
||||
// end of the destinations scrolls the page behind the sheet.
|
||||
expect(getComputedStyle(body).overscrollBehaviorY).toBe('contain');
|
||||
});
|
||||
|
||||
it('gives the sheet the whole width, which the sidebar does not take', async () => {
|
||||
const el = await fixture<Nav>('bottom-nav');
|
||||
|
||||
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
|
||||
await update(el, {});
|
||||
|
||||
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
|
||||
|
||||
if (!sidebar) throw new Error('no sidebar');
|
||||
|
||||
// `app-sidebar` writes an *inline* width and caps itself at 400px,
|
||||
// which beats any rule this host could write — so "the host owns
|
||||
// the box" has to be part of what `expanded` means, or the sheet
|
||||
// draws the old 200px column inside a full-width surface.
|
||||
expect(sidebar.style.width).toBe('100%');
|
||||
expect(getComputedStyle(sidebar).maxWidth).toBe('none');
|
||||
});
|
||||
|
||||
it('sizes the sheet rows for a thumb, below the phone breakpoint', async () => {
|
||||
const el = await fixture<Nav>('bottom-nav');
|
||||
|
||||
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
|
||||
await update(el, {});
|
||||
|
||||
const sidebar = shadow<HTMLElement>(el, 'app-sidebar');
|
||||
const sheets = sidebar?.shadowRoot?.adoptedStyleSheets ?? [];
|
||||
const phoneRules: string[] = [];
|
||||
|
||||
for (const sheet of sheets) {
|
||||
for (const rule of Array.from(sheet.cssRules)) {
|
||||
if (!(rule instanceof CSSMediaRule)) continue;
|
||||
|
||||
if (!/max-width:\s*599px/.test(rule.conditionText)) continue;
|
||||
|
||||
for (const inner of Array.from(rule.cssRules)) {
|
||||
phoneRules.push(inner.cssText);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Asserted against the parsed stylesheet, like
|
||||
// `hover-affordance.test.ts` and for the same reason: this tier's
|
||||
// iframe is not 599px wide, so the rule cannot be *rendered* here —
|
||||
// but the regression worth catching is someone moving it out of the
|
||||
// query, which nothing on a desktop draws differently.
|
||||
expect(phoneRules.length).toBeGreaterThan(0);
|
||||
expect(phoneRules.some((r) => /min-height:\s*48px/.test(r))).toBe(true);
|
||||
});
|
||||
|
||||
it('takes the resize handle out of the sheet', async () => {
|
||||
const el = await fixture<Nav>('bottom-nav');
|
||||
|
||||
shadow<HTMLButtonElement>(el, '[data-testid="tab-more"]')?.click();
|
||||
await update(el, {});
|
||||
|
||||
const handle = shadow<HTMLElement>(el, 'app-sidebar')
|
||||
?.shadowRoot?.querySelector('.resize-handle');
|
||||
|
||||
if (!handle) throw new Error('no resize handle');
|
||||
|
||||
// A col-resize strip on the right edge of a touch surface: the
|
||||
// compatibility mouse events a tap synthesises reach its
|
||||
// `mousedown`, so it can start a resize nobody asked for.
|
||||
expect(getComputedStyle(handle).display).toBe('none');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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