Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
12e75ee24c |
@@ -1480,12 +1480,16 @@ export class CoverGrid
|
|||||||
source: 'cover-grid',
|
source: 'cover-grid',
|
||||||
});
|
});
|
||||||
|
|
||||||
// Single album: show cover art thumbnail.
|
// Single album: show its cover, badged with how many tracks are
|
||||||
// Multiple albums: show track-count badge.
|
// 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) {
|
if (isSingleAlbum && hit.album.CoverArtPath) {
|
||||||
this.dragImageEl =
|
this.dragImageEl =
|
||||||
createAlbumArtDragImage(
|
createAlbumArtDragImage(
|
||||||
this.getCoverUrl(hit.album),
|
this.getCoverUrl(hit.album),
|
||||||
|
filePaths.length,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
this.dragImageEl = createDragImage(
|
this.dragImageEl = createDragImage(
|
||||||
|
|||||||
@@ -662,20 +662,34 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
font-weight: 400;
|
font-weight: 400;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* The request control is offered on every row that has
|
/* The request control is only offered where there is
|
||||||
* something to request, and is not revealed on hover.
|
* 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
|
* Hidden with opacity, never display:none or visibility,
|
||||||
* focused, on the reasoning that a column of plus signs is
|
* so it keeps its place in the layout (rows do not reflow
|
||||||
* clutter. That reasoning was inherited from the green
|
* as the pointer moves) and stays in the tab order and the
|
||||||
* ticks it replaced and does not survive the rule those
|
* accessibility tree. focus-within is what makes it
|
||||||
* were removed for: a tick marked the *common* case, while
|
* reachable without a mouse: tabbing to the button reveals
|
||||||
* this marks the rows that are **not** here. A mark on
|
* it, and the row's own focus reveals it before you get
|
||||||
* the exception is the information, and one that appears
|
* there. */
|
||||||
* only under the pointer cannot be seen, counted, or found
|
|
||||||
* by anyone driving this with a finger or a keyboard. */
|
|
||||||
.track-row .track-request {
|
.track-row .track-request {
|
||||||
flex-shrink: 0;
|
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,
|
// The download button only appears once a client is connected,
|
||||||
// so this tracks the provider list rather than assuming.
|
// 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.downloadUnsub = downloadStore.subscribe(() => {
|
||||||
this.canDownload = downloadStore.available;
|
this.canDownload = downloadStore.available;
|
||||||
this.syncRequested();
|
this.syncRequested();
|
||||||
this.requestUpdate();
|
|
||||||
});
|
});
|
||||||
|
|
||||||
void downloadStore.init().then(() => {
|
void downloadStore.init().then(() => {
|
||||||
|
|||||||
@@ -29,11 +29,23 @@ export function createDragImage(count: number): HTMLElement {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates a drag image showing an album cover art thumbnail.
|
* Creates a drag image showing an album cover art thumbnail, with a
|
||||||
* Falls back to the track-count badge if the image fails to load.
|
* 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(
|
export function createAlbumArtDragImage(
|
||||||
coverUrl: string,
|
coverUrl: string,
|
||||||
|
count = 1,
|
||||||
): HTMLElement {
|
): HTMLElement {
|
||||||
const size = 64;
|
const size = 64;
|
||||||
const wrapper = document.createElement('div');
|
const wrapper = document.createElement('div');
|
||||||
@@ -44,6 +56,12 @@ export function createAlbumArtDragImage(
|
|||||||
'left: -1000px',
|
'left: -1000px',
|
||||||
'pointer-events: none',
|
'pointer-events: none',
|
||||||
'z-index: 9999',
|
'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(';');
|
].join(';');
|
||||||
|
|
||||||
const img = document.createElement('img');
|
const img = document.createElement('img');
|
||||||
@@ -61,11 +79,44 @@ export function createAlbumArtDragImage(
|
|||||||
].join(';');
|
].join(';');
|
||||||
|
|
||||||
wrapper.appendChild(img);
|
wrapper.appendChild(img);
|
||||||
|
|
||||||
|
if (count > 1) {
|
||||||
|
wrapper.appendChild(countBadge(count));
|
||||||
|
}
|
||||||
|
|
||||||
document.body.appendChild(wrapper);
|
document.body.appendChild(wrapper);
|
||||||
|
|
||||||
return 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
|
* Creates a drag image styled like a queue track card showing the
|
||||||
* track title and artist. Used when dragging a single track.
|
* 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