fix(explore): stop hiding the request badge until the row is hovered

The badge on a row you do not own was transparent until the row was
hovered or focused. 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, while this marks the rows that are not here. A mark
on the exception is the information on this page, and one that appears
only under the pointer cannot be seen, counted, or reached by anyone
driving the app with a finger.

The repaint half of #33 is fixed in #82; this is only the visibility,
rebased to leave that alone.

Refs #33

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

@@ -666,14 +666,15 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
              * something to request, and is not revealed on hover.
              *
              * It used to be transparent until the row was hovered or
-             * focused, on the reasoning that a column of plus signs is
-             * clutter.  That reasoning was inherited from the green
-             * ticks it replaced and does not survive the rule those
-             * were removed for: a tick marked the *common* case, while
-             * this marks the rows that are **not** here.  A mark on
-             * 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. */
+             * focused, on the reasoning that a column of plus signs
+             * down a mostly-owned album is clutter. That reasoning was
+             * inherited from the green ticks it replaced and does not
+             * survive the rule those were removed for: a tick marked
+             * the *common* case, while this marks the rows that are
+             * **not** here. A mark on the exception is the information
+             * on this page — and one that appears only under the
+             * pointer cannot be seen, counted, or reached by anyone
+             * driving this with a finger. */
             .track-row .track-request {
                 flex-shrink: 0;
             }
@@ -706,18 +707,9 @@ 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/test/components/album-request-badge-visibility.test.ts

@@ -0,0 +1,97 @@
+/**
+ * The request badge on an unowned row is there without being hovered.
+ *
+ * It used to be transparent until the row was hovered or focused, on
+ * the reasoning that a column of plus signs down a mostly-owned album
+ * is clutter. That reasoning came from the green ticks it replaced and
+ * does not survive the rule those were removed for: a tick marked the
+ * **common** case, while this marks the rows that are *not* here. A
+ * mark on the exception is the information on this page, and one that
+ * exists only under the pointer cannot be seen, counted, or reached by
+ * anyone driving the app with a finger.
+ *
+ * That the badge *repaints* when clicked is the other half of #33 and
+ * is covered by `album-track-request.test.ts`.
+ */
+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, shadowAll } from '@test/support/render';
+
+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',
+  });
+
+  stub('library.Library.GetFilePathsByRecordingMBIDs', {
+    'mbid-1': ['/music/mbid-1.mp3'],
+  });
+
+  Object.assign(el, {
+    versionEntries: [
+      {
+        key: 'v1',
+        label: '2019',
+        sublabel: '2 tracks',
+        tracks: [track(1, true), track(2, false)],
+      },
+    ],
+    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('the tracklist’s request badge', () => {
+  beforeEach(() => {
+    resetHarness();
+    stub('library.Library.GetFilePathsByRecordingMBIDs', {});
+    stub('library.Library.GetFilePathsByAlbums', {});
+    stub('library.Library.GetAlbumTracks', []);
+    stub('library.Library.GetAllLibrariesWithTrackCounts', []);
+    stub('download.Service.ProviderKinds', []);
+    stub('download.Service.ListProviders', []);
+    stub('download.Service.ListDownloads', []);
+    stub('download.Service.ListRequests', []);
+  });
+
+  it('is visible without a pointer anywhere near it', async () => {
+    const el = await albumWithAnUnownedTrack();
+    const [badge] = badges(el);
+
+    expect(badge).toBeTruthy();
+    // Computed opacity rather than the absence of a rule, because the
+    // rule could come back under a different selector.
+    expect(getComputedStyle(badge!).opacity).toBe('1');
+  });
+
+  it('is still only on the rows with something to request', async () => {
+    // Always-visible is not the same as everywhere: an owned track has
+    // nothing left to ask for, and a badge on it would be the column of
+    // green ticks this page deliberately stopped drawing.
+    expect(badges(await albumWithAnUnownedTrack())).toHaveLength(1);
+  });
+});

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

@@ -1,153 +0,0 @@
-/**
- * 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');
-  });
-});