Compare commits
8
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a113b7bd62 | ||
|
|
245647f12b | ||
|
|
3479ae8d39 | ||
|
|
e23e6f9a54 | ||
|
|
52d095e3c6 | ||
|
|
939915b1fa | ||
|
|
3aa2a434b4 | ||
|
|
944995dc3c |
@@ -4994,3 +4994,87 @@ ordinary track makes it 9.
|
||||
Filed as its own issue rather than fixed in #67's diff: it is a
|
||||
property of the shared sheet (`components/menu-surface/`), not of the
|
||||
items.
|
||||
|
||||
## The tap highlight is one inherited declaration (measured 2026-08-24)
|
||||
|
||||
`-webkit-tap-highlight-color` is an **inherited** property, and an
|
||||
inherited property crosses a shadow boundary — so `html { … :
|
||||
transparent }` in `index.css` reaches every shadow root in the app and
|
||||
no component needs a rule of its own. Measured in the running app
|
||||
(Chromium, `app-sidebar`'s `li button`, which is three shadow roots
|
||||
from the document): `rgba(0, 0, 0, 0)` with the rule, and
|
||||
`rgba(0, 0, 0, 0.18)` with it removed. That 0.18 grey over the bounding
|
||||
rect of whatever was tapped is what #54 reported.
|
||||
|
||||
The same argument was already spent once and is worth not
|
||||
re-deriving: `index.css`'s first rule is `*, *::before, *::after {
|
||||
user-select: none }`, which for the same reason already covers the
|
||||
shadow roots — #54's Findings ask for `user-select` on interactive
|
||||
surfaces and it has been done since before the issue was filed.
|
||||
|
||||
**What the highlight was, on the surfaces that had nothing else, is the
|
||||
press feedback.** Measured on a track row with the press rule removed
|
||||
and the button held down: `rgba(255, 255, 255, 0.05)` — the *hover*
|
||||
tint, arriving because the pointer is over the row, which is a
|
||||
synthesised hover on a phone and outlives the press. With the rule:
|
||||
0.12 while held, and the neighbouring row unchanged. So the press state
|
||||
is part of removing the highlight rather than a separate polish item,
|
||||
and the hover tints on those same surfaces moved behind
|
||||
`(hover: hover) and (pointer: fine)`, which is #68's gate applied to a
|
||||
tint rather than to a revealed control.
|
||||
|
||||
**`touch-action: manipulation` was considered and not taken.** The
|
||||
Findings offer it for the 300ms tap delay; this app's viewport is
|
||||
`width=device-width`, which is what removes that delay in Chrome, so
|
||||
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.
|
||||
@@ -1631,6 +1631,51 @@ sits inside which media query — and says so; the regression it exists
|
||||
for is someone hoisting a rule out of its query as a tidy-up, which
|
||||
nothing on a desktop renders differently.
|
||||
|
||||
**The web view's own tap highlight is gone, and what replaced it is a
|
||||
press state** (#54). `-webkit-tap-highlight-color` is an *inherited*
|
||||
property, so one declaration on `html` in `index.css` reaches every
|
||||
shadow root in the app and takes away the grey box a phone drew over
|
||||
the bounding rect of whatever was tapped — measured at
|
||||
`rgba(0, 0, 0, 0.18)` with the rule removed. `user-select` is the same
|
||||
argument and was already done: `index.css`'s first rule is `*, *::before,
|
||||
*::after { user-select: none }`, which reaches the shadow roots for the
|
||||
same reason.
|
||||
|
||||
Three things about it are load-bearing.
|
||||
|
||||
**Removing the highlight removes the only touch feedback several
|
||||
surfaces had**, so the press state is part of the same change rather
|
||||
than a later polish item: the four lists' rows, `bottom-nav`'s tabs,
|
||||
`app-sidebar`'s destinations (which are also the phone's "More" sheet)
|
||||
and the shared `contextMenuStyles` menu item all take
|
||||
`--yj-press-overlay` on `:active`. The cards already had one
|
||||
(`transform: scale(0.97)`) and are untouched.
|
||||
|
||||
**A press selector carries a state class or it does nothing where it
|
||||
matters.** A row is `.track-row.selected.active`, so a bare
|
||||
`.track-row:active` is one class short of it and the press is invisible
|
||||
on exactly the row a phone is most likely to press — the one it has
|
||||
just selected. The rule is last and lists `.selected:active` /
|
||||
`.active:active` beside the bare form.
|
||||
|
||||
**And the hover tints on those same surfaces moved behind
|
||||
`(hover: hover) and (pointer: fine)`**, which is #68's gate applied to
|
||||
a tint rather than to a revealed control and for the same mechanism: a
|
||||
hold synthesises a hover in the WebView, so an ungated tint arrives
|
||||
because a finger touched the row and stays there after it has gone —
|
||||
measured, since with the press rule removed a held row reads
|
||||
`rgba(255, 255, 255, 0.05)`, the hover tint, rather than nothing.
|
||||
`touch-action: manipulation` was considered and declined: the 300ms
|
||||
delay it is offered for is already absent on a `width=device-width`
|
||||
viewport, and what it would really change is the gesture stack #63
|
||||
tuned by measurement on a device this session cannot measure.
|
||||
|
||||
The split of tiers is `hover-affordance.test.ts`'s: `press-feedback.
|
||||
test.ts` reads the parsed stylesheet, because `:active` cannot be
|
||||
forced there either, and `native-touch-feel.spec.ts` *measures* — it
|
||||
holds the button down on a real row of the real list, and it is the
|
||||
only tier that loads `index.css` at all.
|
||||
|
||||
Three lists had no focused row to open a menu *from* — the queue panel
|
||||
and both playlist detail views — and gained a roving tab stop through
|
||||
`utils/roving-rows.ts`. **`track-list` deliberately does not use it**:
|
||||
@@ -1829,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
|
||||
@@ -1838,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),
|
||||
@@ -3331,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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* The web view's own tap highlight, and what replaced it (#54).
|
||||
*
|
||||
* Two halves, and each is here because no other tier can see it.
|
||||
*
|
||||
* **The highlight is killed by one declaration on `html`**, which
|
||||
* reaches the app's shadow roots because `-webkit-tap-highlight-color`
|
||||
* is inherited and inheritance crosses a shadow boundary. That is a
|
||||
* property of `index.css`, and `index.css` is loaded by the real app
|
||||
* and by nothing else — the component tier mounts a component with no
|
||||
* page stylesheet at all, which is the same reason the theme's ramps
|
||||
* are invisible to it.
|
||||
*
|
||||
* **The press state is measured rather than read.** The component tier
|
||||
* asserts the shape of the stylesheet (which rule is inside which
|
||||
* query, and that the press selector carries a state class), because
|
||||
* `:active` cannot be forced there. Here there is a real pointer: hold
|
||||
* the button down on a real row of the real list and read what the row
|
||||
* became. That is the assertion that would fail if the rule were
|
||||
* hoisted, renamed, or lost to `.selected`.
|
||||
*
|
||||
* What neither half is, is the device. Chrome 113's WebView is where
|
||||
* the grey box was reported and where a finger is; the numbers from it
|
||||
* are on the PR.
|
||||
*/
|
||||
type Page = import('@playwright/test').Page;
|
||||
|
||||
/** The phone this work was measured against, in CSS pixels. */
|
||||
const DEVICE = { width: 424, height: 439 };
|
||||
|
||||
/** The computed tap-highlight colour of a node inside a shadow root. */
|
||||
const tapHighlight = (page: Page, host: string, inner: string) =>
|
||||
page.evaluate(
|
||||
([hostSel, innerSel]) => {
|
||||
const el = document
|
||||
.querySelector(hostSel!)
|
||||
?.shadowRoot?.querySelector(innerSel!);
|
||||
|
||||
if (!el) return null;
|
||||
|
||||
return getComputedStyle(el).getPropertyValue(
|
||||
'-webkit-tap-highlight-color',
|
||||
);
|
||||
},
|
||||
[host, inner],
|
||||
);
|
||||
|
||||
test.describe('the tap highlight', () => {
|
||||
test('is transparent inside a shadow root, from one rule on html', async ({
|
||||
app,
|
||||
browserName,
|
||||
}) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'tracks',
|
||||
);
|
||||
|
||||
const row = await tapHighlight(app, 'track-list', '.track-row');
|
||||
|
||||
expect(row).not.toBeNull();
|
||||
|
||||
// The property is a WebKit extension that only iOS honours, so an
|
||||
// engine is free not to report one at all. Chromium always does —
|
||||
// measured at rgba(0, 0, 0, 0.18) with the rule removed, which is
|
||||
// the grey box the report describes — so the assertion is not
|
||||
// skippable there, and nothing this app can do makes the property
|
||||
// disappear on an engine that has it.
|
||||
test.skip(
|
||||
row === '',
|
||||
`${browserName} reports no -webkit-tap-highlight-color to read`,
|
||||
);
|
||||
|
||||
expect(row).toBe('rgba(0, 0, 0, 0)');
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('the press state that replaced it', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DEVICE);
|
||||
await app.getByTestId('tab-tracks').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'tracks',
|
||||
);
|
||||
await expect(app.locator('track-list').first()).toBeVisible();
|
||||
});
|
||||
|
||||
test.afterEach(async ({ app }) => {
|
||||
await app.mouse.up();
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
});
|
||||
|
||||
test('shows on the row being pressed, and on that row only', async ({
|
||||
app,
|
||||
}) => {
|
||||
const rows = await app.evaluate(() => {
|
||||
const found = document
|
||||
.querySelector('track-list')
|
||||
?.shadowRoot?.querySelectorAll('.track-row');
|
||||
|
||||
if (!found || found.length < 2) return null;
|
||||
|
||||
const rect = found[1]!.getBoundingClientRect();
|
||||
|
||||
return {
|
||||
x: Math.round(rect.x + rect.width / 2),
|
||||
y: Math.round(rect.y + rect.height / 2),
|
||||
};
|
||||
});
|
||||
|
||||
expect(rows).not.toBeNull();
|
||||
|
||||
const backgrounds = () =>
|
||||
app.evaluate(() => {
|
||||
const found = document
|
||||
.querySelector('track-list')!
|
||||
.shadowRoot!.querySelectorAll('.track-row');
|
||||
|
||||
return {
|
||||
pressed: getComputedStyle(found[1]!).backgroundColor,
|
||||
neighbour: getComputedStyle(found[2]!).backgroundColor,
|
||||
};
|
||||
});
|
||||
|
||||
await app.mouse.move(rows!.x, rows!.y);
|
||||
await app.mouse.down();
|
||||
|
||||
const held = await backgrounds();
|
||||
|
||||
// The press overlay, from the theme rather than from a literal in
|
||||
// a component: rgba(255, 255, 255, 0.12) on both dark ramps.
|
||||
expect(held.pressed).toBe('rgba(255, 255, 255, 0.12)');
|
||||
expect(held.neighbour).not.toBe(held.pressed);
|
||||
|
||||
await app.mouse.up();
|
||||
});
|
||||
});
|
||||
@@ -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,26 @@
|
||||
|
||||
html {
|
||||
height: 100%;
|
||||
|
||||
/* #54. The web view's own tap highlight -- the grey box a phone
|
||||
draws over the bounding rect of whatever was tapped -- gone in
|
||||
one declaration, because `-webkit-tap-highlight-color` is an
|
||||
*inherited* property and an inherited property crosses a shadow
|
||||
boundary. So this reaches every one of the app's shadow roots
|
||||
without a rule in any of them; before it, exactly one component
|
||||
(`library-status-indicator`) set it and the box appeared
|
||||
everywhere else.
|
||||
|
||||
What it costs is the only touch feedback several surfaces had,
|
||||
which is why the rows, the tab bar and the shared menu items
|
||||
grew a `:active` state in the same change: removing the wrong
|
||||
feedback and leaving none is not an improvement. The cards
|
||||
already had one (`transform: scale(0.97)`).
|
||||
|
||||
`user-select` is the same argument one rule up and was already
|
||||
done: the `*` rule at the top of this file is inherited into the
|
||||
shadow roots too. */
|
||||
-webkit-tap-highlight-color: transparent;
|
||||
}
|
||||
|
||||
body {
|
||||
|
||||
@@ -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')
|
||||
@@ -89,6 +117,17 @@ export class BottomNav extends LitElement {
|
||||
color: var(--yj-accent, #ffd43b);
|
||||
}
|
||||
|
||||
/* The press state (#54). This bar is the phone's primary
|
||||
navigation and had no feedback of its own at all -- what a
|
||||
tap produced was the web view's tap highlight, a grey box
|
||||
over the whole 48px cell, which index.css has now taken
|
||||
away. The .active rule above is which tab you are *on*; this
|
||||
is the tab being pressed, so they are a colour and a
|
||||
background rather than two colours. */
|
||||
button:active {
|
||||
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
|
||||
}
|
||||
|
||||
button:focus-visible {
|
||||
outline: 2px solid var(--yj-accent, #ffd43b);
|
||||
outline-offset: -2px;
|
||||
@@ -104,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);
|
||||
}
|
||||
`];
|
||||
|
||||
@@ -147,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
|
||||
@@ -189,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;
|
||||
};
|
||||
@@ -263,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
|
||||
|
||||
@@ -1357,8 +1357,14 @@ export class PlaylistDetails
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
/* A hover tint is for a device that hovers (#54). A hold
|
||||
synthesises a hover in the WebView, so ungated this arrives
|
||||
because a finger touched the row and stays after it has
|
||||
gone; the press state below is what a tap gets instead. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
}
|
||||
}
|
||||
|
||||
.track-item.selected {
|
||||
@@ -1378,11 +1384,13 @@ export class PlaylistDetails
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.track-item.phantom:hover {
|
||||
background-color: var(
|
||||
--yj-hover-overlay,
|
||||
rgba(255, 255, 255, 0.05)
|
||||
);
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.track-item.phantom:hover {
|
||||
background-color: var(
|
||||
--yj-hover-overlay,
|
||||
rgba(255, 255, 255, 0.05)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
.track-item.phantom.selected {
|
||||
@@ -1392,6 +1400,19 @@ export class PlaylistDetails
|
||||
);
|
||||
}
|
||||
|
||||
/* The press state (#54): the feedback a tap has now that the
|
||||
web view's own highlight box is gone (index.css). Last, and
|
||||
carrying a class, because a selected or playing row is two
|
||||
classes deep and a bare :active would lose to it. */
|
||||
.track-item.selected:active,
|
||||
.track-item.active:active,
|
||||
.track-item:active {
|
||||
background-color: var(
|
||||
--yj-press-overlay,
|
||||
rgba(255, 255, 255, 0.12)
|
||||
);
|
||||
}
|
||||
|
||||
.phantom-row {
|
||||
grid-column: 1 / -1;
|
||||
display: flex;
|
||||
|
||||
@@ -581,8 +581,14 @@ export class QueuePanel
|
||||
contain: strict;
|
||||
}
|
||||
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
/* A hover tint is for a device that hovers (#54). A hold
|
||||
synthesises a hover in the WebView, so ungated this arrives
|
||||
because a finger touched the row and stays after it has
|
||||
gone; the press state below is what a tap gets instead. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
}
|
||||
}
|
||||
|
||||
.track-item.selected {
|
||||
@@ -597,6 +603,19 @@ export class QueuePanel
|
||||
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
|
||||
}
|
||||
|
||||
/* The press state (#54): the feedback a tap has now that the
|
||||
web view's own highlight box is gone (index.css). Last, and
|
||||
carrying a class, because a selected or playing row is two
|
||||
classes deep and a bare :active would lose to it. */
|
||||
.track-item.selected:active,
|
||||
.track-item.active:active,
|
||||
.track-item:active {
|
||||
background-color: var(
|
||||
--yj-press-overlay,
|
||||
rgba(255, 255, 255, 0.12)
|
||||
);
|
||||
}
|
||||
|
||||
.track-position {
|
||||
font-size: var(--yj-text-sm);
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
|
||||
@@ -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;
|
||||
@@ -95,8 +115,15 @@ export class AppSidebar extends LitElement {
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
li button:hover {
|
||||
background-color: var(--yj-bg-elevated, #343a40);
|
||||
/* A hover tint is for a device that hovers (#54), and this
|
||||
component is on a phone too: below 600px it is what
|
||||
bottom-nav's "More" sheet mounts, where a hold
|
||||
synthesises a hover and leaves a destination looking picked
|
||||
after the finger has gone. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
li button:hover {
|
||||
background-color: var(--yj-bg-elevated, #343a40);
|
||||
}
|
||||
}
|
||||
|
||||
li button:focus-visible {
|
||||
@@ -108,6 +135,14 @@ export class AppSidebar extends LitElement {
|
||||
background-color: var(--yj-bg-overlay, #495057);
|
||||
}
|
||||
|
||||
/* The press state (#54), after the .active rule and at the same
|
||||
specificity, so pressing the destination you are already on
|
||||
still says something. It is what a tap gets now that
|
||||
index.css has taken the web view's own highlight box away. */
|
||||
li button:active {
|
||||
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
|
||||
}
|
||||
|
||||
li button p {
|
||||
margin: 0;
|
||||
white-space: nowrap;
|
||||
@@ -145,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. */
|
||||
@@ -173,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;
|
||||
@@ -350,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;
|
||||
|
||||
@@ -516,8 +516,14 @@ export class SmartPlaylistDetails
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
/* A hover tint is for a device that hovers (#54). A hold
|
||||
synthesises a hover in the WebView, so ungated this arrives
|
||||
because a finger touched the row and stays after it has
|
||||
gone; the press state below is what a tap gets instead. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.track-item:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
}
|
||||
}
|
||||
|
||||
.track-item.selected {
|
||||
@@ -533,6 +539,19 @@ export class SmartPlaylistDetails
|
||||
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
|
||||
}
|
||||
|
||||
/* The press state (#54): the feedback a tap has now that the
|
||||
web view's own highlight box is gone (index.css). Last, and
|
||||
carrying a class, because a selected or playing row is two
|
||||
classes deep and a bare :active would lose to it. */
|
||||
.track-item.selected:active,
|
||||
.track-item.active:active,
|
||||
.track-item:active {
|
||||
background-color: var(
|
||||
--yj-press-overlay,
|
||||
rgba(255, 255, 255, 0.12)
|
||||
);
|
||||
}
|
||||
|
||||
/* Phantom rows span the full grid */
|
||||
.track-item.phantom {
|
||||
display: grid;
|
||||
|
||||
@@ -1201,8 +1201,16 @@ export class TrackList
|
||||
padding-left: 6px;
|
||||
}
|
||||
|
||||
.track-row:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
/* A hover tint is for a device that hovers (#54): a hold
|
||||
synthesises a hover in the WebView, so ungated this is a
|
||||
highlight that arrives because a finger touched the row and
|
||||
then stays there after it has gone -- which reads as a
|
||||
selection the user did not make. Same gate, and the same
|
||||
mechanism, as #68's revealed controls. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.track-row:hover {
|
||||
background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05));
|
||||
}
|
||||
}
|
||||
|
||||
.track-row.selected {
|
||||
@@ -1239,6 +1247,23 @@ export class TrackList
|
||||
background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15));
|
||||
}
|
||||
|
||||
/* The press state (#54), and the only feedback a tap has now that
|
||||
the web view's tap highlight is gone (index.css).
|
||||
|
||||
**Last, and as specific as the state rules above**: a row that
|
||||
is selected and playing is .track-row.selected.active, so a
|
||||
bare .track-row:active is one class short of it and a press
|
||||
on the row a phone is most likely to press -- the one it just
|
||||
selected -- would show nothing. Instant rather than
|
||||
transitioned, because the only measured statement here about
|
||||
transitions on a list is that two card grids removed theirs
|
||||
for software-rendering repaint cost. */
|
||||
.track-row.selected:active,
|
||||
.track-row.active:active,
|
||||
.track-row:active {
|
||||
background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12));
|
||||
}
|
||||
|
||||
|
||||
.cell {
|
||||
overflow: hidden;
|
||||
|
||||
@@ -46,6 +46,17 @@ export interface ShadePalette {
|
||||
border: string;
|
||||
borderSubtle: string;
|
||||
hoverOverlay: string;
|
||||
/**
|
||||
* The tint a surface takes while it is being pressed (#54).
|
||||
*
|
||||
* Separate from `hoverOverlay` because the two answer different
|
||||
* questions and only one of them a phone can ask: a hover is a
|
||||
* pointer resting somewhere, a press is a finger on the thing it
|
||||
* is about to activate. It is deliberately the stronger of the
|
||||
* two — a press that reads the same as a hover says nothing on a
|
||||
* device where the hover is synthesised by the press itself.
|
||||
*/
|
||||
pressOverlay: string;
|
||||
selectionBg: string;
|
||||
}
|
||||
|
||||
@@ -95,6 +106,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
|
||||
border: '#333333',
|
||||
borderSubtle: '#222222',
|
||||
hoverOverlay: 'rgba(255, 255, 255, 0.05)',
|
||||
pressOverlay: 'rgba(255, 255, 255, 0.12)',
|
||||
selectionBg: 'rgba(100, 160, 255, 0.15)',
|
||||
},
|
||||
dark: {
|
||||
@@ -114,6 +126,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
|
||||
border: '#444444',
|
||||
borderSubtle: '#333333',
|
||||
hoverOverlay: 'rgba(255, 255, 255, 0.05)',
|
||||
pressOverlay: 'rgba(255, 255, 255, 0.12)',
|
||||
selectionBg: 'rgba(100, 160, 255, 0.15)',
|
||||
},
|
||||
light: {
|
||||
@@ -133,6 +146,7 @@ export const SHADE_PALETTES: Record<BackgroundShade, ShadePalette> = {
|
||||
border: '#ced4da',
|
||||
borderSubtle: '#dee2e6',
|
||||
hoverOverlay: 'rgba(0, 0, 0, 0.05)',
|
||||
pressOverlay: 'rgba(0, 0, 0, 0.12)',
|
||||
selectionBg: 'rgba(100, 160, 255, 0.15)',
|
||||
},
|
||||
};
|
||||
@@ -285,6 +299,7 @@ function deriveThemeVariables(
|
||||
|
||||
// Interactive overlays
|
||||
'--yj-hover-overlay': palette.hoverOverlay,
|
||||
'--yj-press-overlay': palette.pressOverlay,
|
||||
'--yj-selection-bg': palette.selectionBg,
|
||||
|
||||
// Semantic *fills* — the background of a solid button or badge.
|
||||
|
||||
@@ -710,10 +710,35 @@ export const contextMenuStyles = css`
|
||||
font-size: 13px;
|
||||
}
|
||||
|
||||
.context-menu-panel wa-dropdown-item:hover {
|
||||
/* A hover tint is for a device that hovers (#54).
|
||||
|
||||
Below the query is a phone, where a hold *synthesises* a hover
|
||||
in the WebView -- the same mechanism #68 gates the revealed
|
||||
controls on -- so an ungated tint is a highlight that arrives
|
||||
because a finger touched the row and then stays on it after the
|
||||
finger has gone. Which is indistinguishable from the press
|
||||
state below, and outlives it. */
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.context-menu-panel wa-dropdown-item:hover {
|
||||
background-color: var(
|
||||
--yj-hover-overlay,
|
||||
rgba(255, 255, 255, 0.1)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/* And a press state is for every device, because it is the one
|
||||
piece of feedback a tap has now that the web view's own
|
||||
highlight box is gone (index.css). Stronger than the hover tint
|
||||
on purpose, and instant rather than transitioned: the only
|
||||
measured statement this repo has about transitions on these
|
||||
surfaces is the two card grids that removed theirs because
|
||||
software rendering repaints per frame, and the phone is not
|
||||
something this session can measure. */
|
||||
.context-menu-panel wa-dropdown-item:active {
|
||||
background-color: var(
|
||||
--yj-hover-overlay,
|
||||
rgba(255, 255, 255, 0.1)
|
||||
--yj-press-overlay,
|
||||
rgba(255, 255, 255, 0.12)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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,191 @@
|
||||
/**
|
||||
* What a tap looks like now that the web view's own highlight is gone
|
||||
* (#54).
|
||||
*
|
||||
* `index.css` sets `-webkit-tap-highlight-color: transparent` on
|
||||
* `html`, which — the property being inherited — reaches every shadow
|
||||
* root in the app. That takes away the grey box a phone drew over the
|
||||
* bounding rect of whatever was tapped, and with it the only touch
|
||||
* feedback the rows, the tab bar, the sidebar's destinations and the
|
||||
* shared menu items had. So the press states below are not decoration:
|
||||
* without them this change trades wrong feedback for none.
|
||||
*
|
||||
* **Asserted against the parsed stylesheet**, on `hover-affordance`'s
|
||||
* precedent and with the same limitation stated out loud: CDP's
|
||||
* `Emulation.setEmulatedMedia` does not reach this tier's iframe, so
|
||||
* there is no way here to render a component as a phone would, and
|
||||
* `:active` cannot be forced from a test either. What the browser will
|
||||
* answer is the shape it built from the `css` literal — which rule sits
|
||||
* inside which media query, and what the press selector actually is.
|
||||
*
|
||||
* Two regressions are worth catching that way, and both are silent on a
|
||||
* desktop:
|
||||
*
|
||||
* - someone hoisting a hover tint back out of its query as a tidy-up,
|
||||
* which on a phone is a highlight that arrives because a finger
|
||||
* touched the row and stays after it has gone;
|
||||
* - someone simplifying the press selector to a bare `:active`, which
|
||||
* is one class short of `.selected` / `.active` and so does nothing
|
||||
* on the row a phone is most likely to press — the one it has just
|
||||
* selected.
|
||||
*
|
||||
* The pixels are the Android tier's, and the tap highlight itself is
|
||||
* `e2e/specs/native-touch-feel.spec.ts`, since only the real app loads
|
||||
* `index.css` at all.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/track-list/track-list';
|
||||
import '@components/queue-panel/queue-panel';
|
||||
import '@components/playlist-details/playlist-details';
|
||||
import '@components/smart-playlist-details/smart-playlist-details';
|
||||
import '@components/bottom-nav/bottom-nav';
|
||||
import '@components/sidebar/app-sidebar';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
/** Every rule in the element's own adopted stylesheets, flattened. */
|
||||
function rulesOf(host: Element): { text: string; condition: string | null }[] {
|
||||
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
|
||||
const out: { text: string; condition: string | null }[] = [];
|
||||
|
||||
for (const sheet of sheets) {
|
||||
for (const rule of Array.from(sheet.cssRules)) {
|
||||
if (rule instanceof CSSMediaRule) {
|
||||
for (const inner of Array.from(rule.cssRules)) {
|
||||
out.push({ text: inner.cssText, condition: rule.conditionText });
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
out.push({ text: rule.cssText, condition: null });
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
}
|
||||
|
||||
/** The four lists, their row selector, and the tag that draws them. */
|
||||
const LISTS: Array<[string, string]> = [
|
||||
['track-list', '.track-row'],
|
||||
['queue-panel', '.track-item'],
|
||||
['playlist-details', '.track-item'],
|
||||
['smart-playlist-details', '.track-item'],
|
||||
];
|
||||
|
||||
describe('a row says it is being pressed', () => {
|
||||
for (const [tag, row] of LISTS) {
|
||||
it(`${tag} draws a press state that survives its state classes`, async () => {
|
||||
const el = await fixture(tag, {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
// Worth nothing if it read no rules at all — the first assertion
|
||||
// icon-language.test.ts makes, for the same reason.
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const press = rules.filter(
|
||||
(r) => r.text.includes(`${row}:active`) && r.text.includes('background-color'),
|
||||
);
|
||||
|
||||
expect(press.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of press) {
|
||||
// A press is not a hover: it is the one thing a touch device
|
||||
// can say, so it must not sit behind a pointer query.
|
||||
expect(rule.condition).toBeNull();
|
||||
expect(rule.text).toContain('--yj-press-overlay');
|
||||
}
|
||||
|
||||
// The load-bearing half: the selector carries a state class, or
|
||||
// it loses to `.selected` / `.selected.active` and the press is
|
||||
// invisible on a selected or playing row.
|
||||
expect(press.some((r) => r.text.includes(`${row}.selected:active`))).toBe(true);
|
||||
});
|
||||
|
||||
it(`${tag} keeps its hover tint for devices that hover`, async () => {
|
||||
const el = await fixture(tag, {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const hover = rules.filter(
|
||||
(r) =>
|
||||
r.text.includes(`${row}:hover`) &&
|
||||
r.text.includes('--yj-hover-overlay'),
|
||||
);
|
||||
|
||||
expect(hover.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of hover) {
|
||||
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||
expect(rule.condition).toMatch(/pointer:\s*fine/);
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('the two navigations say they are being pressed', () => {
|
||||
it('the phone tab bar, which had no state of its own at all', async () => {
|
||||
const el = await fixture('bottom-nav', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const press = rules.filter((r) => r.text.startsWith('button:active'));
|
||||
|
||||
expect(press.length).toBe(1);
|
||||
expect(press[0]!.condition).toBeNull();
|
||||
expect(press[0]!.text).toContain('--yj-press-overlay');
|
||||
});
|
||||
|
||||
it("the sidebar, which is also the phone's More sheet", async () => {
|
||||
const el = await fixture('app-sidebar', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const press = rules.filter((r) => r.text.startsWith('li button:active'));
|
||||
|
||||
expect(press.length).toBe(1);
|
||||
expect(press[0]!.condition).toBeNull();
|
||||
expect(press[0]!.text).toContain('--yj-press-overlay');
|
||||
|
||||
// Its hover tint is a destination looking picked, if it is left to
|
||||
// a synthesised hover inside the More sheet.
|
||||
const hover = rules.filter((r) => r.text.startsWith('li button:hover'));
|
||||
|
||||
expect(hover.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of hover) {
|
||||
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('the shared context menu', () => {
|
||||
// One stylesheet, fourteen menus — the same reason the sheet's row
|
||||
// height lives there rather than in each host.
|
||||
it('presses its items, in the one place every menu includes', async () => {
|
||||
const el = await fixture('queue-panel', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
const press = rules.filter((r) =>
|
||||
r.text.startsWith('.context-menu-panel wa-dropdown-item:active'),
|
||||
);
|
||||
|
||||
expect(press.length).toBe(1);
|
||||
expect(press[0]!.condition).toBeNull();
|
||||
expect(press[0]!.text).toContain('--yj-press-overlay');
|
||||
|
||||
const hover = rules.filter((r) =>
|
||||
r.text.startsWith('.context-menu-panel wa-dropdown-item:hover'),
|
||||
);
|
||||
|
||||
expect(hover.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of hover) {
|
||||
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||
expect(rule.condition).toMatch(/pointer:\s*fine/);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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