fix(explore): repaint the tracklist when a track is requested

The album page's downloadStore subscription assigned canDownload and
re-synced isRequested, both of which are about the *release group* --
so requesting a **track** changed no reactive property, Lit had no
reason to re-render, and every row's badge kept the plus it was drawn
with. The request was filed and visible in Downloads; the control that
filed it appeared to do nothing, which is what making it a button was
meant to stop.

libraryStatusFor() reads the store at render time, so the fix is to
say so: one explicit requestUpdate on the subscription.

The badge is also no longer revealed on hover. That rule was inherited
from the green ticks it replaced, and it does not survive the reason
those went: a tick marked the common case, this marks the rows that are
*not* here. A mark on the exception is the information, and one that
exists only under the pointer cannot be seen, counted, or reached by a
finger or a keyboard.

Refs #33

frontend/src/components/cover-grid/cover-grid.ts

@@ -1480,16 +1480,12 @@ export class CoverGrid
             source: 'cover-grid',
         });
 
-        // Single album: show its cover, badged with how many tracks are
+        // Single album: show cover art thumbnail.
-        // on the way -- an album is 1 track or 30 and the thumbnail is
+        // Multiple albums: show track-count badge.
-        // the same picture either way, so the number the drop is about
-        // was the one thing this drag did not say.
-        // Multiple albums: show the track-count badge alone.
         if (isSingleAlbum && hit.album.CoverArtPath) {
             this.dragImageEl =
                 createAlbumArtDragImage(
                     this.getCoverUrl(hit.album),
-                    filePaths.length,
                 );
         } else {
             this.dragImageEl = createDragImage(

frontend/src/components/explore-album-details/explore-album-details.ts

@@ -662,34 +662,20 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
                 font-weight: 400;
             }
 
-            /* The request control is only offered where there is
+            /* The request control is offered on every row that has
-             * something to request, and only when the row is being
+             * something to request, and is not revealed on hover.
-             * attended to — a column of plus signs down a mostly-owned
-             * album is the clutter the green ticks were.
              *
-             * Hidden with opacity, never display:none or visibility,
+             * It used to be transparent until the row was hovered or
-             * so it keeps its place in the layout (rows do not reflow
+             * focused, on the reasoning that a column of plus signs is
-             * as the pointer moves) and stays in the tab order and the
+             * clutter.  That reasoning was inherited from the green
-             * accessibility tree.  focus-within is what makes it
+             * ticks it replaced and does not survive the rule those
-             * reachable without a mouse: tabbing to the button reveals
+             * were removed for: a tick marked the *common* case, while
-             * it, and the row's own focus reveals it before you get
+             * this marks the rows that are **not** here.  A mark on
-             * there. */
+             * the exception is the information, and one that appears
+             * only under the pointer cannot be seen, counted, or found
+             * by anyone driving this with a finger or a keyboard. */
             .track-row .track-request {
                 flex-shrink: 0;
-                opacity: 0;
-                transition: opacity 0.12s ease;
-            }
-
-            .track-row:hover .track-request,
-            .track-row:focus-within .track-request,
-            .track-row .track-request:focus-visible {
-                opacity: 1;
-            }
-
-            @media (prefers-reduced-motion: reduce) {
-                .track-row .track-request {
-                    transition: none;
-                }
             }
         `,
     ];
@@ -720,9 +706,18 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
 
         // The download button only appears once a client is connected,
         // so this tracks the provider list rather than assuming.
+        //
+        // The explicit `requestUpdate` is what makes a *track*'s badge
+        // move.  `canDownload` and `syncRequested` are both about the
+        // release group, so neither changes when a row is requested —
+        // and every row's badge reads `libraryStatusFor(...)` out of
+        // the store at render time, so with no reactive property
+        // changed Lit had no reason to re-render and the badge sat on
+        // a plus for a request that had already been filed.
         this.downloadUnsub = downloadStore.subscribe(() => {
             this.canDownload = downloadStore.available;
             this.syncRequested();
+            this.requestUpdate();
         });
 
         void downloadStore.init().then(() => {

frontend/src/utils/drag-image.ts

@@ -29,23 +29,11 @@ export function createDragImage(count: number): HTMLElement {
 }
 
 /**
- * Creates a drag image showing an album cover art thumbnail, with a
+ * Creates a drag image showing an album cover art thumbnail.
- * corner badge saying how many tracks are on the way.
+ * Falls back to the track-count badge if the image fails to load.
- *
- * The count is not decoration. The cover says *what* is being dragged
- * and nothing said *how much* — an album is 1 track or 30 and the
- * thumbnail is identical either way, so the one number the drop is
- * about was the one thing the drag did not show. Every other drag in
- * the app says it (`createDragImage` is a count and nothing else);
- * this one was the exception because it had a picture to show instead.
- *
- * A count of 1 draws no badge: "1" over a single album cover is noise,
- * and the absence is unambiguous next to a badge that only ever
- * appears when there is more than one.
  */
 export function createAlbumArtDragImage(
     coverUrl: string,
-    count = 1,
 ): HTMLElement {
     const size = 64;
     const wrapper = document.createElement('div');
@@ -56,12 +44,6 @@ export function createAlbumArtDragImage(
         'left: -1000px',
         'pointer-events: none',
         'z-index: 9999',
-        // The badge is positioned against this box, and the box stays
-        // exactly the cover's size: anything outside it risks being
-        // clipped out of the snapshot the browser takes, and padding
-        // it instead would move the cover away from the cursor.
-        `width: ${size}px`,
-        `height: ${size}px`,
     ].join(';');
 
     const img = document.createElement('img');
@@ -79,44 +61,11 @@ export function createAlbumArtDragImage(
     ].join(';');
 
     wrapper.appendChild(img);
-
-    if (count > 1) {
-        wrapper.appendChild(countBadge(count));
-    }
-
     document.body.appendChild(wrapper);
 
     return wrapper;
 }
 
-/** The corner badge on a multi-track drag image. */
-function countBadge(count: number): HTMLElement {
-    const badge = document.createElement('span');
-
-    badge.className = 'drag-count-badge';
-    badge.textContent = String(count);
-    badge.style.cssText = [
-        'position: absolute',
-        'top: 3px',
-        'right: 3px',
-        'min-width: 20px',
-        'height: 20px',
-        'padding: 0 5px',
-        'box-sizing: border-box',
-        'border-radius: 10px',
-        'background: #ffd43b',
-        'color: #000',
-        'font-size: 12px',
-        'font-weight: 600',
-        'font-family: inherit',
-        'line-height: 20px',
-        'text-align: center',
-        'box-shadow: 0 1px 4px rgba(0,0,0,0.5)',
-    ].join(';');
-
-    return badge;
-}
-
 /**
  * Creates a drag image styled like a queue track card showing the
  * track title and artist.  Used when dragging a single track.

frontend/test/components/album-track-request.test.ts

@@ -0,0 +1,153 @@
+/**
+ * Asking for a track from an album's tracklist.
+ *
+ * The badge on an unowned row is a `<button>` that files a durable
+ * request, and its state is computed at render time from the download
+ * store — `libraryStatusFor(owned, mbid)` reads `requestFor(mbid)` out
+ * of the cached list.
+ *
+ * That is what made this fail. The page's `downloadStore` subscription
+ * assigned `canDownload` and re-synced `isRequested` for the *release
+ * group*, and neither of those changes when a **track** is requested,
+ * so Lit saw no reactive property change and never re-rendered. The
+ * request was filed, the Downloads tab showed it, and the badge stayed
+ * on a plus reading "not in your library" — a control that appears to
+ * do nothing, which is exactly what the badge was made a button to stop
+ * being.
+ *
+ * The second assertion is the row's own: the badge is not revealed on
+ * hover. A mark on the rows you do *not* have is the information on
+ * this page, and one that only exists under the pointer cannot be seen,
+ * counted, or reached by a finger.
+ */
+import { describe, expect, it, beforeEach } from 'vitest';
+import type { LitElement } from 'lit';
+
+import '@components/explore-album-details/explore-album-details';
+import { stub, flush, resetHarness } from '@test/support/harness';
+import { fixture, shadow, shadowAll } from '@test/support/render';
+
+type StoredRequest = {
+  id: number;
+  mbid: string;
+  entity: string;
+  libraryId: number;
+  artist: string;
+  title: string;
+  state: string;
+  attempts: number;
+};
+
+/** The request list the store reads, mutated by the stubbed AddRequest
+ *  exactly as the backend's own list would be. */
+let requests: StoredRequest[] = [];
+
+function track(n: number, owned: boolean) {
+  return {
+    position: n,
+    discNumber: 1,
+    title: `Track ${n}`,
+    length: 200000,
+    mbid: `mbid-${n}`,
+    inLibrary: owned,
+  };
+}
+
+/** An album with one owned track and one that is not here. */
+async function albumWithAnUnownedTrack(): Promise<LitElement> {
+  const el = await fixture<LitElement>('explore-album-details', {
+    albumName: 'Glass Harbour',
+    releaseGroupMBID: 'rg-1',
+  });
+
+  const tracks = [track(1, true), track(2, false)];
+
+  stub('library.Library.GetFilePathsByRecordingMBIDs', {
+    'mbid-1': ['/music/mbid-1.mp3'],
+  });
+
+  Object.assign(el, {
+    versionEntries: [
+      {
+        key: 'v1',
+        label: '2019',
+        sublabel: '2 tracks',
+        tracks,
+      },
+    ],
+    selectedVersionKey: 'v1',
+    loadingReleases: false,
+    loadingInfo: false,
+  });
+  el.requestUpdate();
+  await flush();
+  await el.updateComplete;
+
+  return el;
+}
+
+const badges = (el: LitElement) =>
+  shadowAll(el, 'library-status-indicator.track-request');
+
+describe('requesting a track from the album tracklist', () => {
+  beforeEach(async () => {
+    resetHarness();
+    requests = [];
+
+    stub('library.Library.GetFilePathsByRecordingMBIDs', {});
+    stub('library.Library.GetFilePathsByAlbums', {});
+    stub('library.Library.GetAlbumTracks', []);
+    stub('library.Library.GetAllLibrariesWithTrackCounts', [
+      { id: 1, name: 'Music', path: '/music', trackCount: 1 },
+    ]);
+
+    stub('download.Service.ProviderKinds', []);
+    stub('download.Service.ListProviders', []);
+    stub('download.Service.ListDownloads', []);
+    stub('download.Service.ListRequests', () => requests);
+    stub('download.Service.AddRequest', (input: Record<string, unknown>) => {
+      const id = requests.length + 1;
+
+      requests.push({
+        id,
+        mbid: String(input.mbid),
+        entity: String(input.entity),
+        libraryId: Number(input.libraryId),
+        artist: String(input.artist ?? ''),
+        title: String(input.title ?? ''),
+        state: 'wanted',
+        attempts: 0,
+      });
+
+      return id;
+    });
+  });
+
+  it('marks the row as queued without a reload', async () => {
+    const el = await albumWithAnUnownedTrack();
+
+    // Only the unowned row offers one: there is nothing left to ask for
+    // on a track there is a file for.
+    const [badge] = badges(el);
+
+    expect(badge).toBeTruthy();
+    expect(badge?.getAttribute('status')).toBe('not-in-library');
+
+    shadow<HTMLElement>(badge!, 'button')?.click();
+    await flush();
+    await el.updateComplete;
+
+    expect(requests.map((r) => r.mbid)).toEqual(['mbid-2']);
+    expect(badges(el)[0]?.getAttribute('status')).toBe('queued');
+  });
+
+  it('shows the badge without being hovered', async () => {
+    const el = await albumWithAnUnownedTrack();
+    const [badge] = badges(el);
+
+    // The old rule hid it at `opacity: 0` until `:hover`. Computed
+    // opacity is the assertion rather than the absence of a rule,
+    // because the rule could come back under another selector.
+    expect(getComputedStyle(badge!).opacity).toBe('1');
+  });
+});

frontend/test/components/drag-image.test.ts

@@ -1,71 +0,0 @@
-/**
- * A drag says how much it is carrying.
- *
- * Every drag in the app already did — `createDragImage` is a count and
- * nothing else — except the one with a picture to show instead. An
- * album dragged to the queue put its cover under the cursor and said
- * nothing about how many tracks that was, and an album is 1 track or 30
- * with the same thumbnail either way. The number is the thing the drop
- * is about.
- *
- * A count of 1 draws no badge: "1" over a single cover is noise, and
- * the absence reads unambiguously beside a badge that only ever appears
- * when there is more than one.
- */
-import { describe, expect, it, afterEach } from 'vitest';
-
-import {
-  createAlbumArtDragImage,
-  removeDragImage,
-} from '@utils/drag-image';
-
-const made: HTMLElement[] = [];
-
-function dragImage(count?: number): HTMLElement {
-  const el =
-    count === undefined
-      ? createAlbumArtDragImage('data:image/gif;base64,R0lGODlhAQABAAAAACw=')
-      : createAlbumArtDragImage(
-          'data:image/gif;base64,R0lGODlhAQABAAAAACw=',
-          count,
-        );
-
-  made.push(el);
-
-  return el;
-}
-
-const badge = (el: HTMLElement) =>
-  el.querySelector<HTMLElement>('.drag-count-badge');
-
-describe('the album drag image', () => {
-  afterEach(() => {
-    while (made.length > 0) removeDragImage(made.pop()!);
-  });
-
-  it('says how many tracks are being dragged', () => {
-    expect(badge(dragImage(12))?.textContent).toBe('12');
-  });
-
-  it('says nothing when there is only one track', () => {
-    expect(badge(dragImage(1))).toBeNull();
-  });
-
-  it('still draws a bare cover for a caller that gives no count', () => {
-    // The count is optional so the helper stays usable from a call site
-    // that has a cover and no list; it must not badge such a drag "1".
-    expect(badge(dragImage())).toBeNull();
-  });
-
-  it('keeps the badge inside the cover', () => {
-    // setDragImage snapshots the element, and anything outside its box
-    // risks being clipped out of that snapshot — while padding the box
-    // instead would move the cover away from the cursor.
-    const el = dragImage(30);
-    const outer = el.getBoundingClientRect();
-    const mark = badge(el)!.getBoundingClientRect();
-
-    expect(mark.right).toBeLessThanOrEqual(outer.right);
-    expect(mark.top).toBeGreaterThanOrEqual(outer.top);
-  });
-});