Compare commits

...
4 Commits
Author SHA1 Message Date
logan 979c6e83ed docs: open plan 009 and record what phase 1 found
Build & publish Arch package / arch-package (push) Successful in 2m4s
Search index maintenance / maintain-index (push) Successful in 7s
CI / check (push) Successful in 2m32s
CI / e2e (push) Successful in 6m21s
The plan's own framing was wrong in a way worth keeping: the badge was
not waiting on the download client, which had largely landed already —
it was waiting on somebody looking at a state nothing produced.
2026-08-13 14:14:39 -04:00
logan 48f7795687 test(e2e): pin the requested badge and the state it renders in
Two assertions, and the second is why this is at this tier at all.
Reaching the requested state is the only way to render the requested
icon, so the sweep that already asserts `__yjIconMisses` is empty can
finally see a name computed from state.

Both were watched failing on the pre-fix build by neutering one line
each: the badge reported `not-in-library` where `queued` was expected,
and the sweep returned `["bookmark-check"]`.

The spec gives back what it spends — the request is dropped in
`afterAll`, and cleared in `beforeAll` too, since a run that dies
between the two would otherwise fail the next one. That cleanup uses
the raw binding rather than `callBinding`: a bare `browser.newPage()`
has no init script, so the event bridge is undefined and the first
version threw where nobody was looking.

Its 60 s search budget is not paranoia either. A freshly launched app
spends ~40 s merging the core catalog artifact and Explore's search
returns nothing until it lands, including for rows staged directly
into `explore_index`.
2026-08-13 14:13:46 -04:00
logan c400f681c2 fix(icons): the "Wanted" button asked for a Pro icon
`bookmark-check` is Font Awesome **Pro**, so it was never bundled and
`window.__yjIconMisses` has held it for as long as anything could be
requested — the button rendered the missing-icon fallback in the one
state it exists to show.

`offline-icons.spec.ts` asserts that array is empty and passed anyway:
no spec had ever put the app in a state where an album is requested. A
name computed from state is only checkable from that state, which is
the case `names.txt` exists for.

Outline and solid of the same Free glyph carry the toggle instead,
which is what the vendoring script tells you to do when a name is
missing: pick one that is Free, never reach for the Pro file.
2026-08-13 14:13:38 -04:00
logan 451b46e63c fix(explore): show a requested album as queued, not absent
`library-status-indicator` has had three states since it was written
and produced two: all eight call sites were a two-way ternary between
`in-library` and `not-in-library`, so the `queued` state it styles and
labels was unreachable.

The result was the app contradicting itself on one page. An album added
to the request list showed a plus and announced "is not in your
library", forty pixels from a filled button reading "Wanted".

The rule was written at eight places, which is why none of them had all
of it, so it is `utils/library-status.ts` now: owning outranks wanting,
a satisfied request is not queued, and a request is by MBID — a track
inside a requested album is not itself requested and still says so.

`explore-view` gains the `downloadStore` subscription both detail views
already had, registered `whileActive` because it is a cached view that
never unmounts. `top-results-row` needs its own: its host re-rendering
sets the same `results` array back, so Lit stops at the property and
the row never hears about a change.
2026-08-13 14:13:30 -04:00
11 changed files with 880 additions and 15 deletions
+68
View File
@@ -2196,3 +2196,71 @@ Six more things worth keeping:
hook* is what raises it — and a spec that spends the shared database
has to give it back, since the 90 specs share one backend in file
order.
## A state nothing produces is a state nobody has checked
Plan 009 phase 1: `library-status-indicator`'s third state, wired.
The generalisation: **an enum whose last value is never constructed is
not unfinished, it is wrong** — because everything around it has been
written, reviewed and tested against the two values that do occur, and
the code reads as complete from every angle except the one that
produces the third. `LibraryStatus` has had `queued` since it was
written: styled amber, given an hourglass, given the sentence "… is
queued for download". All eight call sites were a two-way ternary. So
an album on the request list rendered a plus and announced "is not in
your library" — on the same page, forty pixels from a filled button
reading "Wanted".
Nothing was going to find that. `make ui-test` and `make e2e` both
covered the badge; both asserted the states it produced. 007 phase 6
had rewritten this exact component, and the note it left behind
("when the download-client integration lands…") was itself the reason
nobody looked: it names a *future* condition for work that was already
possible, since `backend/download` was 16 541 lines and 20 bound
methods on the day it was written. **A written-down reason not to look
ages worse than the code it is about.**
Six more things worth keeping:
- **The second bug was in the screenshot of the first.** The "Wanted"
button rendered a question mark — the missing-icon fallback —
because `bookmark-check` is Font Awesome **Pro** and has never been
bundled. `offline-icons.spec.ts` asserts `__yjIconMisses` is empty
and passed the whole time: no spec had ever put the app in a state
where an album was requested. The bundled-icon design anticipated
exactly this ("twenty call sites compute their icon name from
state") and the *sweep* still could not see it, because a sweep only
sees the states it visits. Seventh regression in five plans that
only a PNG has caught, and the first found in a PNG taken of a
different bug.
- **A property that does not change does not re-render a child.**
`top-results-row` reads the request list, and its host handing back
the same `results` array means Lit stops at the property — the row
keeps its old badges while the store holds the right answer. The
virtualizer rule (`requestUpdate()` on host state) one level milder,
and the same fix: subscribe where the state is *read*.
- **A spec that gives state back has to be run twice to know it did.**
The `afterAll` cleanup called `callBinding`, which goes through
`window.__yjEvents` — installed by the `app` fixture, not by a bare
`browser.newPage()`. It threw where nothing was watching, left the
request behind and failed the *next* run with a stale `queued`. One
run proves the assertions; the second proves the teardown.
- **A freshly launched app cannot search its own catalog for ~40 s.**
The core artifact merge has to land (`core artifact: merge complete`
in `.dev/app.log`), and until it does Explore's search returns
nothing at all — *including for rows staged directly into
`explore_index` a moment earlier*, which makes it look like the
staging failed. Cost a cycle here reading as a failure of the neuter
the run was under. Budget 60 s, or wait for the log line.
- **The neuter has to be per line, not per feature.** Two fixes landed
together and each got its own one-line neuter, which is what made
the two failures distinguishable: one spec reported the wrong badge
status, the other reported `["bookmark-check"]`. Neutered together
they would both have failed and either could have been decorative.
- **The fix is where the rule is, and the rule was in eight places.**
Every one of the eight sites was individually reasonable; the third
state was missing from all of them because no site owns the
question. Same shape as `getCoverUrl()`, `track-index.ts` and
`page-header` — when a rule is written per call site, the call sites
do not disagree, they are all incomplete in the same way.
@@ -0,0 +1,215 @@
# 009 — The badge that cannot act, and the state it already had
**Status:** active — Phase 1 shipped; Phase 2 is a decision, not written yet.
**Branch:** main
**Created:** 2026-08-13
**Follows:** 008-the-last-audit
## Problem
007 phase 6 turned `library-status-indicator` from a `<button>` that did
nothing into a `role="img"` badge, on the rule that **a control which
cannot act is worse than none**, and wrote down what would change the
answer: *"when the download-client integration lands, the right change
is to make it a `<button>` again with a handler."*
Two things about that are wrong, and both were found by reading the code
and then the running app rather than the note.
**The download client has largely already landed.** `backend/download`
is 16 541 lines: a durable request model with four entity types
(`artist` / `release-group` / `release` / `recording`, `request.go`), a
reconciler, a staging importer, six provider adapters, 20 bound methods,
`downloads-view`, the `download-picker` dialog, and a working **"Want
this"** toggle on `explore-album-details`. What has not landed is the
badge.
**And the badge is not merely inert — it is wrong.** `LibraryStatus`
declares, styles and labels a third state, `queued` ("… is queued for
download"). **Zero of the eight call sites ever produce it**
(`explore-view:1839,1877`, `explore-artist-details:2116,2228,2323`,
`explore-album-details:1641,2283`, `top-results-row:258` — every one is
a two-way ternary). So an album the user has *already requested*
displays a plus and says it is not in their library.
### Reproduced, 2026-08-13, before anything was written
Against `SEED=default` with the real 900 000-row catalog:
`AddRequest({mbid: e51c54ea…, entity: 'release-group'})` for *GOLDEN* by
Jung Kook, then Explore → search "GOLDEN":
```
status not-in-library
icon plus
aria Album "GOLDEN" is not in your library
```
and on the album's **own detail page**, forty pixels apart in the same
screenshot: the button reads **"Wanted"** (filled) and the badge beside
the title reads **plus / "is not in your library"**. One component,
two surfaces, opposite answers. This is the header-badge-contradicting-
Settings failure again, and again only a PNG showed it.
The same PNG showed a second one, which is why it is in this plan:
**`bookmark-check` is not a bundled icon.** `window.__yjIconMisses`
reports exactly `["bookmark-check"]`, so the "Wanted" button renders the
fallback question-mark glyph. `e2e/specs/offline-icons.spec.ts` asserts
that array is empty and passes, because no spec has ever put the app in
a state where an album is requested — precisely the "twenty call sites
compute their icon name from state" case `names.txt` exists for.
## Ordering principle
By **what is a fact and what is a decision**.
Phase 1 is a bug: the badge contradicts the app's own state, and fixing
it needs no interaction design at all. It also produces the evidence
Phase 2 needs — once the badge can say "requested", whether it must also
*become* requestable is a question that can be looked at rather than
assumed.
Phase 2 is a decision made before any code, in the shape 008 phase 4
used, because one 20 px circle would otherwise mean three different
commitments: on an artist card a **discography subscription**
(`scope: 'future'`, `Expands()`, never satisfied), on an album a
release-group request, on a track row a recording request.
Phase 3 is whatever Phase 2 leaves. **"Album only" is a legitimate
outcome** and shrinks this plan rather than inventing work for it.
---
## Phase 1 — the badge tells the truth
**Ships:**
- `utils/library-status.ts` — one definition of the rule, since the
reason all eight sites are two-state is that the rule is written at
all eight. Owning something outranks wanting it, so `in-library` wins
over `queued`.
- The eight call sites using it.
- `explore-view` gaining the `downloadStore` subscription both detail
views already have (`init()` + `subscribe()`), through
`view-lifecycle` — it is a **cached primary view**, so a raw
`connectedCallback` subscription would live for the session.
- `bookmark-check` in `src/icons/names.txt`, and an e2e case that
reaches the state that exposes it.
**The badge stays `role="img"`.** Telling the truth is not acting.
**Watch for:** `downloadStore.init()` fetches providers, descriptors,
downloads *and* requests, so this warms a singleton on a page that
previously did not construct it — "a store with no subscriber fetches
nothing" cuts the other way here, and the cost belongs in the note.
### Phase 1 — what actually shipped
Three landings. `make ui-test` 677 → **685**; `make e2e` 90 → **92**.
- **The rule, written once.** `utils/library-status.ts`, the eight call
sites, and `explore-view`'s subscription.
- **The Pro icon.** `regular/bookmark` / `solid/bookmark`, vendored.
- **`e2e/specs/requested-badge.spec.ts`**, which is also the first spec
that reaches the state the icon sweep needed.
Pinned by `library-status.test.ts` (8) and `requested-badge.spec.ts`
(2). Both e2e cases were watched failing on the pre-fix build by
neutering one line each — the badge reported `not-in-library` where
`queued` was expected, and the sweep returned `["bookmark-check"]`.
#### Where the plan was wrong — Phase 1
Six things, and the first is the plan's own framing.
- **"When the download client lands" had already half happened, and
the note that said otherwise was written before it.** 007 phase 6
left a condition ("make it a button *with* a handler") that reads as
future work; `backend/download` was 16 541 lines and 20 bound methods
at the time it was written. The badge was not waiting on the download
client. It was waiting on somebody looking.
- **The bug was one layer below the one in the plan.** The plan says
the badge cannot act. What the reproduction says is that it could not
even *report* — three states declared, two produced, at eight sites
none of which knew about the third. "A control that cannot act" and
"a control that is wrong" are different faults and only the second
one is a lie.
- **The second bug was in the screenshot of the first.** The "Wanted"
button rendered a question mark, which is the missing-icon fallback:
`bookmark-check` is a **Pro** name. It has been that way for as long
as anything could be requested, and `offline-icons.spec.ts` — which
exists to assert exactly this — passed throughout, because it never
reached a state where an album was requested. Seventh regression in
five plans that only a PNG has caught, and the first one caught in a
PNG taken of a *different* bug.
- **A sibling component does not hear its host re-render.**
`top-results-row` takes `results` as a property; `explore-view`
re-rendering hands back the same array, so Lit stops at the property
and the row keeps its old badges. Same shape as the virtualizer rule
one level milder, and the fix is the same: subscribe where the state
is read.
- **The cleanup ran on a page that could not run it.** `afterAll` used
`callBinding`, which goes through `window.__yjEvents` — installed by
the `app` fixture and not by `browser.newPage()`. It threw where
nothing was watching, left the request behind, and failed the *next*
run of the same spec with a stale `queued`. A spec that gives state
back has to be checked by running it twice, which is what found this.
- **A freshly launched app cannot search its own catalog for ~40 s.**
The core artifact merge (`core artifact: merge complete` in
`.dev/app.log`) has to land first, and until it does Explore's search
returns nothing — *including for rows staged directly into
`explore_index` a moment earlier*, which is what makes it look like a
staging bug. It cost a cycle here reading as a failure of the neuter
it was run under.
---
## Phase 2 — what a badge click means, per entity
*(decision, before code — not started)*
What Phase 1 leaves for it, now as observations rather than guesses:
- On the album page the badge and the "Want this" button now say the
same thing twice, four centimetres apart. That is an argument for the
badge being **read-only there** and clickable only where there is no
button — or for the button going.
- A requested album shows an amber hourglass while every track in its
tracklist shows a plus, which is correct per the rule and reads as
busy. Worth deciding whether a track inside a requested album should
render *nothing* rather than a plus.
- An artist badge would mean a discography subscription, which is the
heaviest commitment in the download subsystem behind the smallest
control in the app.
---
## Phase 3 — the button
*(scope depends on Phase 2)*
---
## Deliberately not in this plan
- **Deleting the file from disk** (008 phase 4's explicit sequel). Not
refused — mis-ordered. 008's own notes record that the *reversible*
option shipped with **nothing implementing its reversibility**:
`excluded_paths` has no management surface, and "a full rescan clears
it" is the escape hatch. Shipping an irreversible delete beside a
reversible one that cannot yet be undone is backwards, and the
platform trash is a new cross-platform dependency besides.
- **`a11y.20`, deriving `_itemSize` from a measured row.** Real and
confirmed in code — `.track-row` is `height: 33px; contain: strict`
with a `rem` font size, so text scales and the box does not, across
four lists (33 / 49 / 45 / 45 px). It waits because its only honest
verification does not exist yet: both surviving comments
(`track-list.ts:349`, `queue-panel.ts:179`) say a wrong `_itemSize`
desynchronises the **native scrollbar at 20k+ rows**, and `make perf`
has no scroll-fidelity row. That measurement is its own first phase
and belongs to a plan that is about it.
## First step
Phase 1, and within it the helper rather than the call sites — the
reproduction above is already the failing case, and the point of the
helper is that there is one place for the next state to be added.
+297
View File
@@ -0,0 +1,297 @@
import { test, expect, callBinding } from '../support/fixtures.js';
/**
* The badge says an album is requested, and the button beside it agrees.
*
* `library-status-indicator` has had three states since it was written
* and produced two: every one of the eight call sites was a two-way
* ternary, so an album already on the request list showed a plus and
* said "is not in your library" — on the same page, forty pixels from a
* filled button reading "Wanted".
*
* This spec exists at this tier rather than only in the component one
* because of what it drags in with it: reaching the requested state is
* also the only way to render the requested *icon*, and the requested
* icon was `bookmark-check`, a Font Awesome **Pro** name that has never
* been bundled. `offline-icons.spec.ts` asserts `__yjIconMisses` is
* empty and passed anyway, because no spec had ever put the app in this
* state. A name computed from state is only checkable from the state.
*
* It gives back what it spends: the request is removed in `afterAll`,
* and the staged catalog rows are `INSERT OR IGNORE`d so a second run
* against the same backend is a no-op.
*/
/** A release group that exists whether or not this environment has a
* catalog — CI's `YJ_CORE_INDEX_URL` is deliberately dead. */
const MBID = 'e2e-rg-badge-0001';
const TITLE = 'Requested Album';
const ARTIST = 'Badge Artist';
let requestId = 0;
test.describe('the requested badge', () => {
test.beforeAll(async ({ browser, baseURL }) => {
const page = await browser.newPage();
await page.goto(baseURL!);
const res = await page.evaluate(
async (row) => {
const r = await fetch('/__test/sql', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
sql: `INSERT OR IGNORE INTO explore_index
(entity_type, mbid, title, artist_name, artist_mbid,
popularity, listener_count, primary_type)
VALUES ('release_group', ?, ?, ?, 'e2e-ar-badge', 10, 10, 'Album')`,
args: [row.mbid, row.title, row.artist],
}),
});
return { status: r.status, body: await r.text() };
},
{ mbid: MBID, title: TITLE, artist: ARTIST },
);
// A setup step whose failure is not checked is not setup.
expect(res.status, `staging failed: ${res.body}`).toBe(200);
// A previous run that died between adding and removing would leave
// this album requested, and the first assertion here is that it is
// not — so start from a known state rather than from the last run's
// luck. The 90 specs share one backend in file order.
await clearRequest(page);
await page.close();
});
test.afterAll(async ({ browser, baseURL }) => {
const page = await browser.newPage();
await page.goto(baseURL!);
await clearRequest(page);
await page.close();
requestId = 0;
});
test('a requested album is queued, not absent', async ({ app }) => {
const libraryId = 1;
// Before: the badge for an unrequested album.
await app.getByTestId('nav-explore').click();
await search(app, TITLE);
expect(await badgeStatus(app, TITLE)).toBe('not-in-library');
requestId = await addRequest(app, libraryId);
expect(requestId).toBeGreaterThan(0);
// The badge is told by the store, which is told by the backend —
// no navigation, no reload.
await expect
.poll(() => badgeStatus(app, TITLE), { timeout: 10_000 })
.toBe('queued');
// And it says so where it counts. The label is the whole point:
// the plus used to be accompanied by "is not in your library".
expect(await badgeLabel(app, TITLE)).toContain('queued for download');
});
test('the requested state renders a real icon', async ({ app }) => {
// `bookmark-check` is Pro, so the button rendered the fallback
// glyph for as long as anything had been requested. The sweep in
// offline-icons.spec.ts could not see it: it never reached here.
// Runs after the test above in file order, which is where the
// request comes from — but a spec that only passes as part of a
// sequence is a spec that lies when it is run alone.
if (!requestId) requestId = await addRequest(app, 1);
await app.getByTestId('nav-explore').click();
await search(app, TITLE);
await openFirstAlbum(app);
await expect
.poll(
() =>
app.evaluate(() => {
const ds = document.querySelector('explore-album-details')
?.shadowRoot;
const btn = [...(ds?.querySelectorAll('wa-button') ?? [])].find(
(b) => /Wanted/.test(b.textContent ?? ''),
);
return btn?.querySelector('wa-icon')?.getAttribute('name') ?? '';
}),
{ timeout: 10_000 },
)
.not.toBe('');
const misses = await app.evaluate(
() =>
(window as unknown as { __yjIconMisses?: string[] }).__yjIconMisses ??
[],
);
expect(misses, 'an icon name that is not bundled').toEqual([]);
});
});
function addRequest(
app: import('@playwright/test').Page,
libraryId: number,
): Promise<number> {
return callBinding<number>(app, 'download.Service.AddRequest', [
{
mbid: MBID,
entity: 'release-group',
libraryId,
artist: ARTIST,
title: TITLE,
scope: 'future',
secondary: false,
},
]);
}
/**
* Drop any request for this album, through the raw binding.
*
* Deliberately not `callBinding`: that goes through `window.__yjEvents`,
* which only exists on a page the `app` fixture created — a bare
* `browser.newPage()` has no init script, so the bridge is undefined and
* the cleanup throws where nobody is looking. The first version of this
* did exactly that and left the request behind, which failed the *next*
* run of this same spec.
*/
async function clearRequest(page: import('@playwright/test').Page) {
await page.evaluate(async (mbid) => {
const go = (
window as unknown as {
go?: {
download?: {
Service?: {
ListRequests(): Promise<{ id: number; mbid: string }[]>;
RemoveRequest(id: number): Promise<void>;
};
};
};
}
).go;
const svc = go?.download?.Service;
if (!svc) return;
const rows = (await svc.ListRequests()) ?? [];
for (const row of rows) {
if (row.mbid?.toLowerCase() === mbid.toLowerCase()) {
await svc.RemoveRequest(row.id);
}
}
}, MBID);
}
/** Type into Explore's own search box and wait for it to settle. */
async function search(app: import('@playwright/test').Page, term: string) {
// A view is a chunk, and `document.createElement` on a tag that has
// not loaded yet yields an inert element rather than throwing — so
// the box being missing reads exactly like a selector bug.
await expect
.poll(
() =>
app.evaluate(
() =>
!!document
.querySelector('explore-view')
?.shadowRoot?.querySelector('input'),
),
{ timeout: 15_000 },
)
.toBe(true);
await app.evaluate((t) => {
const sr = document.querySelector('explore-view')?.shadowRoot;
const input = sr?.querySelector('input');
if (!input) throw new Error('explore search box not found');
input.value = t;
input.dispatchEvent(new Event('input', { bubbles: true }));
}, term);
// 60 s, and it is not paranoia. A freshly launched app spends its
// first ~40 s merging the core catalog artifact, and until that lands
// Explore's search returns nothing at all — including for rows staged
// directly into `explore_index` a moment ago. A shorter budget fails
// here for a reason that has nothing to do with what is being tested,
// which is how this spec first "failed" on a build that was fine.
await expect
.poll(() => cardTitles(app), { timeout: 60_000 })
.toContain(term);
}
function cardTitles(app: import('@playwright/test').Page): Promise<string[]> {
return app.evaluate(() =>
[
...(document
.querySelector('explore-view')
?.shadowRoot?.querySelectorAll('library-status-indicator') ?? []),
].map((b) => b.getAttribute('label') ?? ''),
);
}
function badgeStatus(
app: import('@playwright/test').Page,
label: string,
): Promise<string> {
return app.evaluate((wanted) => {
const badge = [
...(document
.querySelector('explore-view')
?.shadowRoot?.querySelectorAll('library-status-indicator') ?? []),
].find((b) => b.getAttribute('label') === wanted);
return badge?.getAttribute('status') ?? '(no badge)';
}, label);
}
function badgeLabel(
app: import('@playwright/test').Page,
label: string,
): Promise<string> {
return app.evaluate((wanted) => {
const badge = [
...(document
.querySelector('explore-view')
?.shadowRoot?.querySelectorAll('library-status-indicator') ?? []),
].find((b) => b.getAttribute('label') === wanted);
return (
badge?.shadowRoot?.querySelector('.badge')?.getAttribute('aria-label') ??
'(no badge)'
);
}, label);
}
async function openFirstAlbum(app: import('@playwright/test').Page) {
await app.evaluate((wanted) => {
const badge = [
...(document
.querySelector('explore-view')
?.shadowRoot?.querySelectorAll('library-status-indicator') ?? []),
].find((b) => b.getAttribute('label') === wanted);
(badge?.closest('.album-card') as HTMLElement | null)?.click();
}, TITLE);
await expect
.poll(
() => app.evaluate(() => !!document.querySelector('explore-album-details')),
{ timeout: 15_000 },
)
.toBe(true);
}
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 384 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 64C0 28.7 28.7 0 64 0L320 0c35.3 0 64 28.7 64 64l0 417.1c0 25.6-28.5 40.8-49.8 26.6L192 412.8 49.8 507.7C28.5 521.9 0 506.6 0 481.1L0 64zM64 48c-8.8 0-16 7.2-16 16l0 387.2 117.4-78.2c16.1-10.7 37.1-10.7 53.2 0L336 451.2 336 64c0-8.8-7.2-16-16-16L64 48z"/></svg>

After

Width:  |  Height:  |  Size: 566 B

@@ -24,6 +24,8 @@ import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '../library-status-indicator/library-status-indicator.js';
import { libraryStatusFor } from '@utils/library-status';
import type { LibraryStatus } from '../library-status-indicator/library-status-indicator';
import '../catalog-scope-notice/catalog-scope-notice.js';
import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js';
import '@awesome.me/webawesome/dist/components/button/button.js';
@@ -1443,7 +1445,7 @@ export class ExploreAlbumDetails extends LitElement {
* - cachedAlbums has MBID match → owned
* - any selected version has → owned (covers local-only albums
* a track marked inLibrary where releaseGroup may be null)
* - else → not owned
* - else → whatever the request list says
*
* Four different claims of decreasing confidence, OR'd together and
* reported as one tick — the last of which fires when a *single*
@@ -1452,10 +1454,11 @@ export class ExploreAlbumDetails extends LitElement {
* actions key off; this stays as it was, because the indicator's
* job is "is any of this yours" and that is what it answers.
*
* No queued state for now — that's reserved for future
* download-client integration.
* When none of them hold the answer is not automatically "no":
* the album may be on the request list, which the button directly
* below this badge has reported as "Wanted" all along.
*/
private albumLibraryStatus(): 'in-library' | 'not-in-library' {
private albumLibraryStatus(): LibraryStatus {
if (this.localAlbumId > 0) return 'in-library';
if (this.releaseGroup?.inLibrary) return 'in-library';
@@ -1477,7 +1480,11 @@ export class ExploreAlbumDetails extends LitElement {
}
}
return 'not-in-library';
// None of the five ownership claims held, so the badge falls
// through to the one thing this page already knew and never
// said: whether the album is on the request list. The button
// below it has read "Wanted" all along.
return libraryStatusFor(false, this.releaseGroupMBID);
}
/**
@@ -1882,9 +1889,14 @@ export class ExploreAlbumDetails extends LitElement {
appearance=${this.isRequested ? 'filled' : 'outlined'}
@click=${() => void this.toggleRequested(request?.id)}
>
<!-- The requested state used to ask for bookmark-check,
which is a Font Awesome *Pro* name: never bundled,
so this button has rendered the missing-icon
fallback in that state ever since. Outline and solid
of the same Free glyph carry the toggle instead. -->
<wa-icon
slot="start"
name=${this.isRequested ? 'bookmark-check' : 'bookmark'}
name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
></wa-icon>
${this.isRequested ? 'Wanted' : 'Want this'}
</wa-button>
@@ -2280,7 +2292,7 @@ export class ExploreAlbumDetails extends LitElement {
)}</span
>
<library-status-indicator
status=${track.inLibrary ? 'in-library' : 'not-in-library'}
status=${libraryStatusFor(Boolean(track.inLibrary), track.mbid)}
entity-type="track"
label=${track.title}
></library-status-indicator>
@@ -34,6 +34,7 @@ import { EventsOn } from '@runtime/runtime';
import { Events } from '../../events';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '../library-status-indicator/library-status-indicator.js';
import { libraryStatusFor } from '@utils/library-status';
import '../catalog-scope-notice/catalog-scope-notice.js';
import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js';
@@ -2113,7 +2114,7 @@ export class ExploreArtistDetails extends LitElement {
${formatListenCount(t.totalListenCount)} plays
</span>
<library-status-indicator
status=${t.inLibrary || t.localId ? 'in-library' : 'not-in-library'}
status=${libraryStatusFor(Boolean(t.inLibrary || t.localId), t.recordingMbid)}
entity-type="track"
label=${t.trackName}
></library-status-indicator>
@@ -2225,7 +2226,7 @@ export class ExploreArtistDetails extends LitElement {
${rg.date ? html`<span>${extractYear(rg.date)}</span>` : nothing}
</div>
<library-status-indicator
status=${rg.inLibrary || rg.localId ? 'in-library' : 'not-in-library'}
status=${libraryStatusFor(Boolean(rg.inLibrary || rg.localId), rg.releaseGroupMbid)}
entity-type="album"
label=${rg.title}
size="18"
@@ -2320,9 +2321,7 @@ export class ExploreArtistDetails extends LitElement {
const artURL = this.thumbnailURLs.get(rg.mbid) || '';
const year = extractYear(rg.firstReleaseDate);
const inLibrary = this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary);
const status: 'in-library' | 'not-in-library' = inLibrary
? 'in-library'
: 'not-in-library';
const status = libraryStatusFor(inLibrary, rg.mbid);
return html`
<div
@@ -1,4 +1,6 @@
import { avatarBackground } from '@utils/avatar-color';
import { libraryStatusFor } from '@utils/library-status';
import { downloadStore } from '@store/download-store';
import { LitElement, html, css, nothing } from 'lit';
import { customElement, state, query as litQuery } from 'lit/decorators.js';
import '@components/page-header/page-header';
@@ -724,6 +726,19 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) {
this.cancelIndexStatus = EventsOn(Events.IndexStatusChanged, () => {
if (this.shelves?.state !== 'ready') void this.loadShelves();
});
// The badges on every result card say whether something is
// already requested, which a background reconcile pass changes
// without this page doing anything. Registered `whileActive`
// rather than on connect: this view is cached and never
// unmounts, so a connect-time subscription would run for the
// life of the session from pages it is not on.
//
// `init()` is four fetches, and it happens on arrival for the
// same reason `loadShelves()` does — a user who never opens
// Explore should not pay for it.
this.whileActive(downloadStore.subscribe(() => this.requestUpdate()));
void downloadStore.init().then(() => this.requestUpdate());
}
/** A debounced search that lands after the user has left the page is
@@ -1836,7 +1851,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) {
${year ? html`<span>${year}</span>` : nothing}
</div>
<library-status-indicator
status=${this.libraryMBIDs.has(rg.mbid) || rg.inLibrary ? 'in-library' : 'not-in-library'}
status=${libraryStatusFor(this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary), rg.mbid)}
entity-type="album"
label=${rg.title}
></library-status-indicator>
@@ -1874,7 +1889,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) {
: nothing}
</div>
<library-status-indicator
status=${this.libraryMBIDs.has(r.mbid) || r.inLibrary ? 'in-library' : 'not-in-library'}
status=${libraryStatusFor(this.libraryMBIDs.has(r.mbid) || Boolean(r.inLibrary), r.mbid)}
entity-type="track"
label=${r.title}
></library-status-indicator>
@@ -10,6 +10,8 @@ import {
import '../library-status-indicator/library-status-indicator.js';
import type { LibraryStatus } from '../library-status-indicator/library-status-indicator.js';
import { artistLink, exploreLinkStyles } from '../../utils/explore-link';
import { libraryStatusFor } from '../../utils/library-status';
import { downloadStore } from '../../store/download-store';
/** Format milliseconds as m:ss. */
function formatDuration(ms: number | undefined): string {
@@ -50,6 +52,28 @@ export class TopResultsRow extends LitElement {
// Per-card state: cover images.
private images = new Map<string, string>();
private unsubRequests?: () => void;
/**
* The badges here say whether something is already requested, and
* this row will not hear about a change from its host: `explore-view`
* re-rendering sets the same `results` array back, so Lit stops at
* the property and never updates this element. One subscription for
* the row, not one per card.
*/
override connectedCallback(): void {
super.connectedCallback();
this.unsubRequests = downloadStore.subscribe(() =>
this.requestUpdate(),
);
}
override disconnectedCallback(): void {
this.unsubRequests?.();
this.unsubRequests = undefined;
super.disconnectedCallback();
}
static override styles = [
designTokens,
exploreLinkStyles,
@@ -255,7 +279,10 @@ export class TopResultsRow extends LitElement {
? r.year || ''
: formatDuration(r.length) || '';
const status: LibraryStatus = r.inLibrary ? 'in-library' : 'not-in-library';
const status: LibraryStatus = libraryStatusFor(
Boolean(r.inLibrary),
r.mbid,
);
const entityType: 'artist' | 'album' | 'track' =
r.entityType === 'artist'
? 'artist'
+1
View File
@@ -22,6 +22,7 @@ solid/arrow-rotate-right
solid/arrows-rotate
solid/arrow-up-short-wide
solid/backward-step
regular/bookmark
solid/bookmark
solid/box-open
solid/check
+45
View File
@@ -0,0 +1,45 @@
import { downloadStore } from '@store/download-store';
import type { LibraryStatus } from '../components/library-status-indicator/library-status-indicator';
/**
* What the tick/hourglass/plus badge should say about one entity.
*
* This exists because the rule was written at all eight call sites and
* so none of them had the whole of it: every one was a two-way ternary
* between `in-library` and `not-in-library`, and the badge's third
* state — `queued`, styled and labelled since it was written — was
* produced by nothing. An album the user had already asked for through
* the "Want this" button showed a plus and said it was not in their
* library, on the same page, forty pixels from a filled button reading
* "Wanted".
*
* Two rules decide the answer, and both are about honesty rather than
* precedence for its own sake:
*
* - **Owning outranks wanting.** A request that has been satisfied by
* any route — downloaded here, ripped, bought elsewhere — is not
* news; what the user has is.
* - **A request is by MBID, and the badge answers about the entity it
* is on.** A track inside a requested album is not itself requested,
* so it stays a plus. Saying otherwise would promise that clicking
* it later would find *that* recording.
*
* A `satisfied` request is deliberately not `queued`: nothing is coming.
* A `paused` one is, because the user did ask for it and it is still on
* the list — "queued" is a slight overstatement of a paused request and
* a much smaller one than "not in your library".
*/
export function libraryStatusFor(
owned: boolean,
mbid?: string | null,
): LibraryStatus {
if (owned) return 'in-library';
if (!mbid) return 'not-in-library';
const request = downloadStore.requestFor(mbid);
if (request && request.state !== 'satisfied') return 'queued';
return 'not-in-library';
}
@@ -0,0 +1,185 @@
/**
* Plan 009 phase 1: the badge tells the truth.
*
* `library-status-indicator` has had three states since it was written
* — a tick, an hourglass and a plus — and the hourglass was produced by
* nothing. All eight call sites were a two-way ternary, so an album the
* user had already asked for through "Want this" showed a plus and said
* it was not in their library, on the same page as a filled button
* reading "Wanted".
*
* Two tiers of assertion here, and the second is the one that would
* have failed:
*
* - the rule itself, which is now written once, and
* - a rendered Explore result whose release group is requested,
* because a helper nobody calls is a rule nobody follows.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import '@components/explore-view/explore-view';
import type { Request } from '@store/download-store';
import { libraryStatusFor } from '@utils/library-status';
import { Events } from '../../src/events';
import { emit, flush, stub } from '@test/support/harness';
import { fixture, shadow, shadowAll } from '@test/support/render';
const SEARCH = 'explore.Service.SearchLocal';
function request(overrides: Partial<Request>): Request {
return {
id: 1,
mbid: 'rg-wanted',
entity: 'release-group',
libraryId: 1,
artist: 'An Artist',
title: 'Wanted Album',
scope: 'future',
secondary: false,
state: 'wanted',
attempts: 0,
...overrides,
} as Request;
}
/** Put a request list into the store the way the backend does. */
async function withRequests(rows: Request[]): Promise<void> {
stub('download.Service.ListRequests', rows);
emit(Events.RequestsChanged);
await flush();
}
const releaseGroup = (mbid: string, title: string) => ({
mbid,
title,
artistCredit: 'An Artist',
artistMbid: 'ar-1',
primaryType: 'Album',
firstReleaseDate: '1994-05-01',
popularity: 100,
listenerCount: 10,
inLibrary: false,
secondaryTypes: [],
});
describe('libraryStatusFor', () => {
beforeEach(async () => {
await withRequests([]);
});
it('says nothing about an entity with no MBID to ask about', () => {
expect(libraryStatusFor(false, '')).toBe('not-in-library');
expect(libraryStatusFor(false, undefined)).toBe('not-in-library');
});
it('reports a request as queued', async () => {
await withRequests([request({ mbid: 'rg-wanted' })]);
expect(libraryStatusFor(false, 'rg-wanted')).toBe('queued');
});
it('lets owning outrank wanting', async () => {
// Both are true of an album that has arrived but whose request has
// not been retired yet. What the user has is not news; what they
// have is.
await withRequests([request({ mbid: 'rg-wanted' })]);
expect(libraryStatusFor(true, 'rg-wanted')).toBe('in-library');
});
it('does not call a satisfied request queued', async () => {
// Nothing is coming: the request is history. An unowned entity with
// a satisfied request is a stale row, not a download in flight.
await withRequests([request({ mbid: 'rg-done', state: 'satisfied' })]);
expect(libraryStatusFor(false, 'rg-done')).toBe('not-in-library');
});
it('does count a paused request, which the user did ask for', async () => {
await withRequests([request({ mbid: 'rg-paused', state: 'paused' })]);
expect(libraryStatusFor(false, 'rg-paused')).toBe('queued');
});
it('answers about the entity it is on, not the one containing it', () => {
// A request is by MBID. A track inside a requested album is not
// itself requested, and saying otherwise promises that clicking it
// would find that recording.
expect(libraryStatusFor(false, 'recording-inside-rg-wanted')).toBe(
'not-in-library',
);
});
});
describe('<explore-view> badges', () => {
beforeEach(async () => {
stub('explore.Service.GetThumbnails', []);
stub('explore.Service.GetThumbnail', '');
stub('explore.Service.GetArtistImageURL', '');
stub('explore.Service.GetExploreShelves', { shelves: [], state: 'ready' });
stub('library.Library.GetAllAlbums', []);
stub('library.Library.GetAllTracks', []);
await withRequests([]);
});
async function searchFor(rows: ReturnType<typeof releaseGroup>[]) {
stub(SEARCH, {
artists: [],
releaseGroups: rows,
recordings: [],
topResults: [],
});
const el = await fixture('explore-view');
(el as unknown as { viewActivated(): void }).viewActivated();
await flush();
const input = shadow<HTMLInputElement>(el, 'input');
if (input) {
input.value = 'anything';
input.dispatchEvent(new Event('input', { bubbles: true }));
}
// The search box debounces, so waiting a frame measures the input
// echoing its own character.
await new Promise((resolve) => setTimeout(resolve, 300));
await flush();
await el.updateComplete;
return el;
}
it('shows a requested album as queued rather than as absent', async () => {
await withRequests([request({ mbid: 'rg-wanted' })]);
const el = await searchFor([
releaseGroup('rg-wanted', 'Wanted Album'),
releaseGroup('rg-other', 'Some Other Album'),
]);
const badges = shadowAll(el, 'library-status-indicator');
expect(badges.length).toBeGreaterThanOrEqual(2);
expect(badges.map((b) => b.getAttribute('status'))).toEqual([
'queued',
'not-in-library',
]);
});
it('re-renders when the request list changes underneath it', async () => {
// A background reconcile pass expands an artist or retires a want
// without this page doing anything, so the badge has to be told.
const el = await searchFor([releaseGroup('rg-wanted', 'Wanted Album')]);
expect(shadow(el, 'library-status-indicator')?.getAttribute('status')).toBe(
'not-in-library',
);
await withRequests([request({ mbid: 'rg-wanted' })]);
await el.updateComplete;
expect(shadow(el, 'library-status-indicator')?.getAttribute('status')).toBe(
'queued',
);
});
});