From e61b7456df01dabbedc39a8ef3d93da67e172647 Mon Sep 17 00:00:00 2001
From: Logan <logan@ljones.me>
Date: Thu, 13 Aug 2026 15:17:06 -0400
Subject: [PATCH] feat(explore): make the library badge request what it is on
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

007 turned this badge from a `<button>` whose handler was a
`stopPropagation()` and a TODO into `role="img"`, on the rule that a
control which cannot act is worse than none — and wrote down what would
change the answer: a `<button>` again *with* a handler, never a handler
bolted onto something already shaped like one. This is that.

A call site opts in by passing `request-mbid`, so where a badge is
redundant it stays a badge: `explore-album-details`'s header has "Want
this" in words directly below it, and its template says so by not
opting in. An `in-library` badge is never a button either, because
there is nothing left to ask for — that is what keeps the tab stops 007
gave back from being spent on nothing.

The copy is the action, not the state, and it is deliberately about the
request list rather than the library: "Want album X" / "Cancel the
request for album X". Clicking still adds nothing to the library, which
is what made the original "Add … to library" a promise the control
could not keep.

Tracks are requestable too. `EntityRecording` is not a placeholder in
the request model — `Reconciler.tracklistFor` has a deliberate branch
for it, because one expected title is what lets filename matching score
a single-track download at all. Artists are not: there is no artist
badge anywhere, and a discography subscription belongs on the Follow
button that can say what it commits to.

The click is swallowed again, for the opposite reason to before: with
an action of its own, a click on the badge no longer means what the
card means. Enter and Space are stopped for the same reason — every
card holding one is a role=button or role=option with its own handler.
---
 .../explore-album-details.ts                  |   2 +
 .../explore-artist-details.ts                 |   6 +
 .../components/explore-view/explore-view.ts   |   4 +
 .../library-status-indicator.ts               | 173 +++++++++++++++--
 .../top-results-row/top-results-row.ts        |   2 +
 frontend/src/utils/library-status.ts          |  61 ++++++
 .../test/components/library-status.test.ts    | 180 +++++++++++++++++-
 7 files changed, 408 insertions(+), 20 deletions(-)

diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts
index 1fa086c..aa2fc0e 100644
--- a/frontend/src/components/explore-album-details/explore-album-details.ts
+++ b/frontend/src/components/explore-album-details/explore-album-details.ts
@@ -2295,6 +2295,8 @@ export class ExploreAlbumDetails extends LitElement {
                                             status=${libraryStatusFor(Boolean(track.inLibrary), track.mbid)}
                                             entity-type="track"
                                             label=${track.title}
+                                            request-mbid=${track.mbid}
+                                            request-artist=${this.artistName}
                                         ></library-status-indicator>
                                     </div>
                                 `,
diff --git a/frontend/src/components/explore-artist-details/explore-artist-details.ts b/frontend/src/components/explore-artist-details/explore-artist-details.ts
index 5b3ada4..09696ee 100644
--- a/frontend/src/components/explore-artist-details/explore-artist-details.ts
+++ b/frontend/src/components/explore-artist-details/explore-artist-details.ts
@@ -2117,6 +2117,8 @@ export class ExploreArtistDetails extends LitElement {
                                                         status=${libraryStatusFor(Boolean(t.inLibrary || t.localId), t.recordingMbid)}
                                                         entity-type="track"
                                                         label=${t.trackName}
+                                                        request-mbid=${t.recordingMbid}
+                                                        request-artist=${t.artistName ?? ''}
                                                     ></library-status-indicator>
                                                 </div>
                                             `,
@@ -2229,6 +2231,8 @@ export class ExploreArtistDetails extends LitElement {
                             status=${libraryStatusFor(Boolean(rg.inLibrary || rg.localId), rg.releaseGroupMbid)}
                             entity-type="album"
                             label=${rg.title}
+                            request-mbid=${rg.releaseGroupMbid}
+                            request-artist=${this.artist?.name ?? ''}
                             size="18"
                         ></library-status-indicator>
                     </div>
@@ -2358,6 +2362,8 @@ export class ExploreArtistDetails extends LitElement {
                         status=${status}
                         entity-type="album"
                         label=${rg.title}
+                        request-mbid=${rg.mbid}
+                        request-artist=${this.artist?.name ?? ''}
                     ></library-status-indicator>
                 </div>
             </div>
diff --git a/frontend/src/components/explore-view/explore-view.ts b/frontend/src/components/explore-view/explore-view.ts
index fddda97..ca360ce 100644
--- a/frontend/src/components/explore-view/explore-view.ts
+++ b/frontend/src/components/explore-view/explore-view.ts
@@ -1854,6 +1854,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) {
                                         status=${libraryStatusFor(this.libraryMBIDs.has(rg.mbid) || Boolean(rg.inLibrary), rg.mbid)}
                                         entity-type="album"
                                         label=${rg.title}
+                                        request-mbid=${rg.mbid}
+                                        request-artist=${rg.artistCredit ?? ''}
                                     ></library-status-indicator>
                                 </div>
                             </div>
@@ -1892,6 +1894,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) {
                                     status=${libraryStatusFor(this.libraryMBIDs.has(r.mbid) || Boolean(r.inLibrary), r.mbid)}
                                     entity-type="track"
                                     label=${r.title}
+                                    request-mbid=${r.mbid}
+                                    request-artist=${r.artistCredit ?? ''}
                                 ></library-status-indicator>
                             </div>
                         `,
diff --git a/frontend/src/components/library-status-indicator/library-status-indicator.ts b/frontend/src/components/library-status-indicator/library-status-indicator.ts
index 2a87de9..6bc4c9f 100644
--- a/frontend/src/components/library-status-indicator/library-status-indicator.ts
+++ b/frontend/src/components/library-status-indicator/library-status-indicator.ts
@@ -1,6 +1,9 @@
 import { LitElement, html, css, nothing } from 'lit';
-import { customElement, property } from 'lit/decorators.js';
+import { customElement, property, state } from 'lit/decorators.js';
 import '@awesome.me/webawesome/dist/components/icon/icon.js';
+import { toggleRequest } from '@utils/library-status';
+import { notificationStore } from '@store/notification-store';
+import { describeError } from '@utils/describe-error';
 
 /**
  * Library status for an entity (artist, album, or track).
@@ -17,18 +20,23 @@ export type LibraryStatus = 'in-library' | 'queued' | 'not-in-library';
  * Tri-state library status indicator: a small circular badge embedded
  * in track rows, album cards, and artist cards.
  *
- * **It is a badge, not a control.**  It was a `<button>` whose click
- * handler was a `stopPropagation()` and a comment saying to wire up
- * the download client later — so an Explore results page offered 20
- * keyboard stops (of 66) that promised an action and performed none,
- * and every one of them announced itself as a button.  A control that
- * cannot act is worse than no control: it costs the keyboard user the
- * tab stop *and* the expectation.
+ * **It is a button only where it can act, and a badge everywhere
+ * else.**  It used to be a `<button>` whose click handler was a
+ * `stopPropagation()` and a comment saying to wire up the download
+ * client later, so an Explore results page offered 20 keyboard stops
+ * (of 66) that promised an action and performed none.  007 made it
+ * `role="img"` for that reason and wrote down what would change the
+ * answer: a `<button>` again *with* a handler, never a handler bolted
+ * onto something already shaped like one.
  *
- * So it is `role="img"` with a label, until there is something to
- * click.  When the download-client integration lands, the right change
- * is to make it a `<button>` again *with a handler* — not to add the
- * handler to something already shaped like a button.
+ * A call site opts in by passing `request-mbid`.  Where it does, this
+ * is a `<button>` that toggles a durable **request** — and the copy
+ * says so, because clicking still adds nothing to the library.  Where
+ * it does not (`explore-album-details`'s header, which has "Want this"
+ * in words directly below it) it stays exactly what it was.
+ *
+ * An `in-library` badge is never a button under either: there is
+ * nothing left to ask for.
  *
  * Colours and glyphs:
  *  - in-library    → green circle, check mark
@@ -65,6 +73,31 @@ export class LibraryStatusIndicator extends LitElement {
     @property({ type: Number })
     size = 20;
 
+    /**
+     * MBID to request when this is clicked.  Supplying it is what makes
+     * this a control; omitting it leaves a badge.  Only `album` and
+     * `track` are requestable — see `utils/library-status.ts`.
+     */
+    @property({ type: String, attribute: 'request-mbid' })
+    requestMbid = '';
+
+    /** Display-cache artist for the request list. Matching is by MBID. */
+    @property({ type: String, attribute: 'request-artist' })
+    requestArtist = '';
+
+    @state()
+    private busy = false;
+
+    /** True when this can act: a call site opted in, and there is
+     *  something left to ask for. */
+    private get actionable(): boolean {
+        return (
+            this.requestMbid !== '' &&
+            this.status !== 'in-library' &&
+            this.entityType !== 'artist'
+        );
+    }
+
     static override styles = css`
         :host {
             display: inline-flex;
@@ -96,7 +129,8 @@ export class LibraryStatusIndicator extends LitElement {
             /* A <button> gets box-sizing: border-box from the UA
              * stylesheet and a <span> does not, so dropping the button
              * grew the badge by its 1px border on each side — 36px to
-             * 38px, caught by the stored screenshot. */
+             * 38px, caught by the stored screenshot. Set explicitly so
+             * the two branches of render() are the same size. */
             box-sizing: border-box;
             width: var(--indicator-size);
             height: var(--indicator-size);
@@ -116,6 +150,29 @@ export class LibraryStatusIndicator extends LitElement {
         wa-icon {
             font-size: calc(var(--indicator-size) * 0.55);
             line-height: 1;
+            pointer-events: none;
+        }
+
+        button.badge {
+            cursor: pointer;
+            font: inherit;
+        }
+
+        button.badge:hover:not(:disabled) {
+            filter: brightness(1.25);
+        }
+
+        button.badge:disabled {
+            cursor: default;
+            opacity: 0.6;
+        }
+
+        /* The card underneath draws its own focus ring, and this sits
+         * inside it — so the badge needs one of its own or a keyboard
+         * user cannot tell which of the two has focus. */
+        button.badge:focus-visible {
+            outline: 2px solid var(--yj-accent, #ffd43b);
+            outline-offset: 2px;
         }
 
         /* Prevent the button from intercepting drag gestures on album
@@ -145,18 +202,81 @@ export class LibraryStatusIndicator extends LitElement {
                   : 'track';
         const name = this.label ? ` "${this.label}"` : '';
 
+        // A control is named after what activating it does; a badge is
+        // named after what it is. Both are still deliberately about the
+        // *request list* rather than the library — clicking this adds a
+        // row to one and nothing to the other, and "Add … to library"
+        // was the old button's promise written into the copy.
+        if (this.actionable) {
+            return this.status === 'queued'
+                ? `Cancel the request for ${kind}${name}`
+                : `Want ${kind}${name}`;
+        }
+
         switch (this.status) {
             case 'in-library':
                 return `${capitalize(kind)}${name} is in your library`;
             case 'queued':
                 return `${capitalize(kind)}${name} is queued for download`;
             default:
-                // Not "Add … to library": nothing here adds anything.
-                // The old copy was the button's promise written out.
                 return `${capitalize(kind)}${name} is not in your library`;
         }
     }
 
+    /**
+     * Toggle the request.
+     *
+     * The click is swallowed, which it was before too — but for the
+     * opposite reason. 007 removed a `stopPropagation()` that guarded
+     * nothing, on the rule that with no action of its own the badge is
+     * part of its card and a click on it should mean what the card
+     * means. Now it has one, so it does not.
+     */
+    private async onActivate(event: Event) {
+        event.stopPropagation();
+        event.preventDefault();
+
+        if (this.busy || !this.actionable) return;
+
+        this.busy = true;
+
+        try {
+            await toggleRequest({
+                mbid: this.requestMbid,
+                entity: this.entityType === 'album' ? 'album' : 'track',
+                title: this.label,
+                artist: this.requestArtist,
+            });
+        } catch (err) {
+            console.error('could not update the request list', err);
+
+            // Transient: the badge visibly stayed where it was, so
+            // there is nothing for the user to do about it that they
+            // are not already doing.
+            notificationStore.transient({
+                text: describeError(err, 'That request could not be updated.'),
+                tone: 'error',
+            });
+        } finally {
+            this.busy = false;
+        }
+    }
+
+    /**
+     * Keep Enter and Space from reaching the card underneath.
+     *
+     * A `<button>` fires `click` on both by itself, so this only has to
+     * stop the keydown propagating — every card holding one of these is
+     * a `role="button"` or `role="option"` with its own Enter/Space
+     * handler, and without this a keyboard activation would both file
+     * the request and open the page.
+     */
+    private onKeydown(event: KeyboardEvent) {
+        if (event.key === 'Enter' || event.key === ' ') {
+            event.stopPropagation();
+        }
+    }
+
     override render() {
         // Sync the host CSS variable with the configured size.
         if (this.size && this.size !== 20) {
@@ -164,12 +284,29 @@ export class LibraryStatusIndicator extends LitElement {
         }
 
         const title = this.tooltip();
+        const icon = this.iconName()
+            ? html`<wa-icon name=${this.iconName()} aria-hidden="true"></wa-icon>`
+            : nothing;
+
+        if (this.actionable) {
+            return html`
+                <button
+                    class="badge"
+                    type="button"
+                    title=${title}
+                    aria-label=${title}
+                    ?disabled=${this.busy}
+                    @click=${this.onActivate}
+                    @keydown=${this.onKeydown}
+                >
+                    ${icon}
+                </button>
+            `;
+        }
 
         return html`
             <span class="badge" role="img" title=${title} aria-label=${title}>
-                ${this.iconName()
-                    ? html`<wa-icon name=${this.iconName()} aria-hidden="true"></wa-icon>`
-                    : nothing}
+                ${icon}
             </span>
         `;
     }
diff --git a/frontend/src/components/top-results-row/top-results-row.ts b/frontend/src/components/top-results-row/top-results-row.ts
index 3d97b10..7259d5c 100644
--- a/frontend/src/components/top-results-row/top-results-row.ts
+++ b/frontend/src/components/top-results-row/top-results-row.ts
@@ -341,6 +341,8 @@ export class TopResultsRow extends LitElement {
                         status=${status}
                         entity-type=${entityType}
                         label=${r.name}
+                        request-mbid=${r.mbid}
+                        request-artist=${r.artistCredit ?? ''}
                         size="22"
                     ></library-status-indicator>`}
             </div>
diff --git a/frontend/src/utils/library-status.ts b/frontend/src/utils/library-status.ts
index 8c6326e..50be773 100644
--- a/frontend/src/utils/library-status.ts
+++ b/frontend/src/utils/library-status.ts
@@ -1,4 +1,6 @@
 import { downloadStore } from '@store/download-store';
+import { libraryStore } from '@store/library-store';
+import type { download } from '@go/models';
 import type { LibraryStatus } from '../components/library-status-indicator/library-status-indicator';
 
 /**
@@ -43,3 +45,62 @@ export function libraryStatusFor(
 
     return 'not-in-library';
 }
+
+/** What a badge can ask for. Artists are deliberately absent: a
+ *  discography subscription is `explore-artist-details`'s Follow
+ *  button, which can say what it is committing to. */
+export type RequestableEntity = 'album' | 'track';
+
+const ENTITY: Record<RequestableEntity, string> = {
+    album: 'release-group',
+    track: 'recording',
+};
+
+/**
+ * Add or drop a request for one entity, and report which way it went.
+ *
+ * The counterpart to `libraryStatusFor`, here rather than in the badge
+ * because the badge is one of several things that can ask —
+ * `explore-album-details`'s "Want this" button is the other, and two
+ * implementations of "what does wanting something mean" is exactly what
+ * phase 1 was about.
+ *
+ * Returns `'wanted'` or `'cancelled'` so a caller can announce what
+ * happened; throws if the backend refused, because a badge that
+ * silently does nothing is what this whole plan is about.
+ */
+export async function toggleRequest(input: {
+    mbid: string;
+    entity: RequestableEntity;
+    title: string;
+    artist?: string;
+}): Promise<'wanted' | 'cancelled'> {
+    const existing = downloadStore.requestFor(input.mbid);
+
+    if (existing) {
+        await downloadStore.removeRequest(existing.id);
+
+        return 'cancelled';
+    }
+
+    // A request belongs to a library because that is where its files
+    // will land. There is always at least one by the time anything is
+    // on screen — the first-run wizard blocks every pointer event until
+    // there is — but an explicit failure beats a request filed against
+    // library 0, which no import would ever match.
+    const libraryId = await libraryStore.getDefaultLibraryId();
+
+    if (!libraryId) throw new Error('no library to add this to');
+
+    await downloadStore.addRequest({
+        mbid: input.mbid,
+        entity: ENTITY[input.entity],
+        libraryId,
+        artist: input.artist ?? '',
+        title: input.title,
+        scope: 'future',
+        secondary: false,
+    } as download.RequestInput);
+
+    return 'wanted';
+}
diff --git a/frontend/test/components/library-status.test.ts b/frontend/test/components/library-status.test.ts
index c567b5a..af069b9 100644
--- a/frontend/test/components/library-status.test.ts
+++ b/frontend/test/components/library-status.test.ts
@@ -21,8 +21,16 @@ 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';
+import { notificationStore } from '@store/notification-store';
+import {
+  calls,
+  emit,
+  flush,
+  lastArgs,
+  stub,
+  stubFailure,
+} from '@test/support/harness';
+import { fixture, shadow, shadowAll, update } from '@test/support/render';
 
 const SEARCH = 'explore.Service.SearchLocal';
 
@@ -183,3 +191,171 @@ describe('<explore-view> badges', () => {
     );
   });
 });
+
+/**
+ * Plan 009 phase 3: the badge becomes a button where it can act.
+ *
+ * 007 made it `role="img"` because a control that cannot act is worse
+ * than none, and wrote down what would change the answer: a `<button>`
+ * *with* a handler. Both halves of that are asserted here — the badge
+ * branch is still a badge (the tests in `chrome.test.ts` pin it, and
+ * they pass unchanged because a call site has to opt in), and the
+ * button branch actually files a request.
+ */
+describe('<library-status-indicator> as a control', () => {
+  beforeEach(async () => {
+    stub('download.Service.ListRequests', []);
+    stub('download.Service.AddRequest', 7);
+    stub('download.Service.RemoveRequest', null);
+    stub('library.Library.GetAllLibrariesWithTrackCounts', [
+      { id: 3, name: 'Music' },
+    ]);
+    notificationStore.clear();
+    await withRequests([]);
+  });
+
+  const badge = (props: Record<string, unknown> = {}) =>
+    fixture('library-status-indicator', {
+      entityType: 'album',
+      label: 'Abbey Road',
+      ...props,
+    });
+
+  it('is a button only where a call site opted in', async () => {
+    const inert = await badge();
+    const control = await badge({ requestMbid: 'rg-1' });
+
+    expect(shadow(inert, 'button')).toBeNull();
+    expect(shadow(inert, '.badge')?.getAttribute('role')).toBe('img');
+    expect(shadow(control, 'button')).not.toBeNull();
+  });
+
+  it('is never a button for something already owned', async () => {
+    // There is nothing left to ask for, so the tab stop would cost the
+    // keyboard user exactly what 007 gave back.
+    const el = await badge({ requestMbid: 'rg-1', status: 'in-library' });
+
+    expect(shadow(el, 'button')).toBeNull();
+  });
+
+  it('is named after what activating it does', async () => {
+    const el = await badge({ requestMbid: 'rg-1' });
+
+    expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
+      'Want album "Abbey Road"',
+    );
+
+    await update(el, { status: 'queued' });
+
+    expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
+      'Cancel the request for album "Abbey Road"',
+    );
+  });
+
+  it('still describes rather than offers where it cannot act', async () => {
+    const el = await badge();
+
+    expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
+      'Album "Abbey Road" is not in your library',
+    );
+  });
+
+  it('files a request for the entity it is on', async () => {
+    const el = await badge({ requestMbid: 'rg-1', requestArtist: 'The Beatles' });
+
+    shadow<HTMLElement>(el, 'button')?.click();
+    await flush();
+
+    expect(lastArgs('download.Service.AddRequest')?.[0]).toMatchObject({
+      mbid: 'rg-1',
+      entity: 'release-group',
+      title: 'Abbey Road',
+      artist: 'The Beatles',
+      libraryId: 3,
+    });
+  });
+
+  it('asks for a recording when it is on a track', async () => {
+    const el = await badge({
+      entityType: 'track',
+      label: 'Come Together',
+      requestMbid: 'rec-1',
+    });
+
+    shadow<HTMLElement>(el, 'button')?.click();
+    await flush();
+
+    expect(lastArgs('download.Service.AddRequest')?.[0]).toMatchObject({
+      entity: 'recording',
+    });
+  });
+
+  it('cancels a request it already made', async () => {
+    await withRequests([request({ id: 42, mbid: 'rg-1' })]);
+
+    const el = await badge({ requestMbid: 'rg-1', status: 'queued' });
+
+    shadow<HTMLElement>(el, 'button')?.click();
+    await flush();
+
+    expect(lastArgs('download.Service.RemoveRequest')).toEqual([42]);
+    expect(calls('download.Service.AddRequest')).toEqual([]);
+  });
+
+  it('keeps its click off the card it sits on', async () => {
+    // The inverse of the badge branch, and for the opposite reason:
+    // with an action of its own, a click on it no longer means what
+    // the card means.
+    const el = await badge({ requestMbid: 'rg-1' });
+    const button = shadow<HTMLElement>(el, 'button');
+    let bubbled = 0;
+
+    el.addEventListener('click', () => {
+      bubbled += 1;
+    });
+
+    button?.click();
+    await flush();
+
+    // Both halves, because "nothing bubbled" is free on a build with
+    // no button to click: `?.click()` on null is a silent no-op and
+    // this passed on the neutered build until it also asserted that
+    // the click did the thing it was swallowed for.
+    expect([button !== null, bubbled, calls('download.Service.AddRequest')
+      .length]).toEqual([true, 0, 1]);
+  });
+
+  it('keeps Enter and Space off it too', async () => {
+    // Every card holding one of these is a role=button or role=option
+    // with its own Enter/Space handler, so without this a keyboard
+    // activation would file the request *and* open the page.
+    const el = await badge({ requestMbid: 'rg-1' });
+    const seen: string[] = [];
+
+    el.addEventListener('keydown', (e) => seen.push((e as KeyboardEvent).key));
+
+    for (const key of ['Enter', ' ', 'ArrowDown']) {
+      shadow<HTMLElement>(el, 'button')?.dispatchEvent(
+        new KeyboardEvent('keydown', { key, bubbles: true, composed: true }),
+      );
+    }
+
+    // ArrowDown is not ours: the grid still moves by it.
+    expect(seen).toEqual(['ArrowDown']);
+  });
+
+  it('says so when the request could not be filed', async () => {
+    stubFailure('download.Service.AddRequest', 'nope');
+
+    const el = await badge({ requestMbid: 'rg-1' });
+
+    shadow<HTMLElement>(el, 'button')?.click();
+    await flush();
+
+    expect(notificationStore.getAll().map((n) => n.level)).toEqual([
+      'transient',
+    ]);
+    // The badge is where it was, which is why the toast is transient.
+    expect(shadow(el, 'button')).not.toBeNull();
+  });
+});