Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
12e75ee24c |
@@ -1480,12 +1480,16 @@ export class CoverGrid
|
||||
source: 'cover-grid',
|
||||
});
|
||||
|
||||
// Single album: show cover art thumbnail.
|
||||
// Multiple albums: show track-count badge.
|
||||
// Single album: show its cover, badged with how many tracks are
|
||||
// on the way -- an album is 1 track or 30 and the thumbnail is
|
||||
// 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(
|
||||
|
||||
@@ -662,20 +662,34 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
font-weight: 400;
|
||||
}
|
||||
|
||||
/* The request control is offered on every row that has
|
||||
* something to request, and is not revealed on hover.
|
||||
/* 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.
|
||||
*
|
||||
* 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. */
|
||||
* 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. */
|
||||
.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;
|
||||
}
|
||||
}
|
||||
`,
|
||||
];
|
||||
@@ -706,18 +720,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(() => {
|
||||
|
||||
@@ -29,11 +29,23 @@ export function createDragImage(count: number): HTMLElement {
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a drag image showing an album cover art thumbnail.
|
||||
* Falls back to the track-count badge if the image fails to load.
|
||||
* Creates a drag image showing an album cover art thumbnail, with a
|
||||
* corner badge saying how many tracks are on the way.
|
||||
*
|
||||
* 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');
|
||||
@@ -44,6 +56,12 @@ 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');
|
||||
@@ -61,11 +79,44 @@ 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.
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,71 @@
|
||||
/**
|
||||
* 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user