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

e2e/specs/queue-toggle-state.spec.ts

@@ -1,66 +0,0 @@
-import { test, expect } from '../support/fixtures.js';
-
-/**
- * The queue button says whether the queue is open.
- *
- * It used to look identical in both states, so the only way to tell
- * what pressing it would do was to look at the other side of the window
- * and infer it — and for anyone not looking at all there was nothing to
- * infer from: no `aria-expanded`, no `aria-controls`, no pressed state.
- *
- * The state is reflected *from the panel*, not kept beside the click,
- * because the button is not the only thing that opens the queue —
- * `now-playing-view` sets the same attribute, since it hides the bar
- * this button lives in. A flag maintained by the click handler would be
- * right until something else opened the panel and then quietly wrong,
- * which is the second test here.
- */
-test.describe('the queue toggle', () => {
-  test('reports open and closed, and names what it controls', async ({
-    app,
-  }) => {
-    const toggle = app.locator('#queue-button');
-
-    await expect(toggle).toHaveAttribute('aria-controls', 'queue-panel');
-    await expect(toggle).toHaveAttribute('aria-expanded', 'false');
-
-    await toggle.click();
-    await expect(toggle).toHaveAttribute('aria-expanded', 'true');
-
-    // The state is not only in the accessibility tree: a control that
-    // announces a state it does not draw is half a fix.
-    //
-    // Background rather than colour, because the pointer is still on
-    // the button after the click and `:hover` paints it the same accent
-    // the open state does -- so a colour comparison here passes on the
-    // broken build and proves nothing.
-    const [open, closed] = await toggle.evaluate((el) => {
-      const now = getComputedStyle(el).backgroundColor;
-
-      el.setAttribute('aria-expanded', 'false');
-      const shut = getComputedStyle(el).backgroundColor;
-
-      el.setAttribute('aria-expanded', 'true');
-
-      return [now, shut];
-    });
-
-    expect(open).not.toBe(closed);
-
-    await toggle.click();
-    await expect(toggle).toHaveAttribute('aria-expanded', 'false');
-  });
-
-  test('follows the panel when something else opens it', async ({ app }) => {
-    const toggle = app.locator('#queue-button');
-
-    await expect(toggle).toHaveAttribute('aria-expanded', 'false');
-
-    // Exactly what `now-playing-view`'s queue button does.
-    await app.evaluate(() =>
-      document.getElementById('queue-panel')?.setAttribute('open', ''),
-    );
-
-    await expect(toggle).toHaveAttribute('aria-expanded', 'true');
-  });
-});

frontend/index.css

@@ -207,17 +207,6 @@ body div.sidebar {
         color: var(--yj-accent, #ffd43b);
     }
 
-    /* An open queue is a state this button can be in, and it used to
-       look exactly like the closed one -- so the only way to tell what
-       pressing it would do was to look at the other side of the window
-       and infer it. `aria-expanded` is the same fact for anyone not
-       looking at all, and it points at the panel it controls. */
-    #queue-button[aria-expanded='true'] {
-        color: var(--yj-accent, #ffd43b);
-        background: var(--yj-bg-overlay, #404040);
-        border-radius: 4px;
-    }
-
     #queue-button.drag-over {
         color: var(--yj-accent, #ffd43b);
         outline: 2px dashed var(--yj-accent, #ffd43b);

frontend/index.html

@@ -37,8 +37,7 @@
     <footer class="bottom-bar">
         <now-playing></now-playing>
         <audio-player></audio-player>
-        <button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
-            id="queue-button">
+        <button aria-label="Toggle queue" id="queue-button">
             <wa-icon name="list"></wa-icon>
         </button>
     </footer>

frontend/index.ts

@@ -521,28 +521,6 @@ if (queueButton && queuePanel) {
         }
     });
 
-    // The button says whether the panel is open, and it learns that
-    // from the panel rather than from its own click handler.
-    //
-    // It is not the only thing that opens the queue -- `now-playing-view`
-    // sets the same attribute, because it hides the bar this button
-    // lives in -- so a state kept beside the click would be right until
-    // something else opened the panel and then quietly wrong. The panel's
-    // `open` attribute is the one fact; this reflects it.
-    const reflectQueueState = () => {
-        queueButton.setAttribute(
-            'aria-expanded',
-            String(queuePanel.hasAttribute('open')),
-        );
-    };
-
-    new MutationObserver(reflectQueueState).observe(queuePanel, {
-        attributes: true,
-        attributeFilter: ['open'],
-    });
-
-    reflectQueueState();
-
     // ---------------------------------------------------------------
     // Queue button as drop target (when queue panel is closed)
     // ---------------------------------------------------------------

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

@@ -662,34 +662,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
                 font-weight: 400;
             }
 
-            /* The request control is only offered where there is
-             * something to request, and only when the row is being
-             * attended to — a column of plus signs down a mostly-owned
-             * album is the clutter the green ticks were.
+            /* The request control is offered on every row that has
+             * something to request, and is not revealed on hover.
              *
-             * Hidden with opacity, never display:none or visibility,
-             * so it keeps its place in the layout (rows do not reflow
-             * as the pointer moves) and stays in the tab order and the
-             * accessibility tree.  focus-within is what makes it
-             * reachable without a mouse: tabbing to the button reveals
-             * it, and the row's own focus reveals it before you get
-             * there. */
+             * 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 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;
-                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;
-                }
             }
         `,
     ];

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);
+  });
+});