Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
35c0d83819 |
@@ -1,12 +1,6 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* How far the scroll test scrolls. One constant, because the guard and
|
||||
* the assertion have to agree about it — they did not, which is #133.
|
||||
*/
|
||||
const SCROLL_TARGET = 80;
|
||||
|
||||
/**
|
||||
* Plan 007 phase 5: expanding an album shows its tracks.
|
||||
*
|
||||
@@ -110,27 +104,20 @@ test.describe('the album dropdown', () => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
|
||||
try {
|
||||
// Wait for the range the assertion below actually needs, not for
|
||||
// "scrollable at all" (#133). The guard used to be
|
||||
// `scrollHeight > clientHeight + 40` while the next line asks to
|
||||
// reach 80, so any range in 41-79 satisfied it and could not
|
||||
// satisfy the assertion — and the grid passes through exactly
|
||||
// that while it settles, because it recomputes its columns after
|
||||
// the resize rather than during it. The settled range here is
|
||||
// 330, so this waits rather than weakening anything.
|
||||
await expect
|
||||
.poll(() => scrollRange(app))
|
||||
.toMatchObject({ room: true, overflowY: 'auto' });
|
||||
await expect.poll(() => scrollRange(app)).toMatchObject({
|
||||
scrollable: true,
|
||||
overflowY: 'auto',
|
||||
});
|
||||
|
||||
await app.evaluate((target) => {
|
||||
await app.evaluate(() => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
if (sc) sc.scrollTop = target;
|
||||
}, SCROLL_TARGET);
|
||||
if (sc) sc.scrollTop = 80;
|
||||
});
|
||||
|
||||
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
|
||||
expect(await scrollTop(app)).toBe(80);
|
||||
|
||||
// And the dropdown it opens is on screen, wherever the manager
|
||||
// decides that leaves the scroll. It is *not* "the position is
|
||||
@@ -263,19 +250,16 @@ async function closeDropdown(app: Page): Promise<void> {
|
||||
|
||||
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
||||
async function scrollRange(app: Page) {
|
||||
return app.evaluate((target) => {
|
||||
return app.evaluate(() => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
return {
|
||||
// `room` is the precondition of the assertion that follows it:
|
||||
// enough range to actually reach the target. A threshold below
|
||||
// what the caller depends on is not a guard.
|
||||
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
||||
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
|
||||
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
||||
};
|
||||
}, SCROLL_TARGET);
|
||||
});
|
||||
}
|
||||
|
||||
async function scrollTop(app: Page): Promise<number> {
|
||||
|
||||
@@ -13,7 +13,6 @@ import {
|
||||
isQueueSourceNavigable,
|
||||
navigateToQueueSource,
|
||||
} from '@utils/queue-source-link';
|
||||
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||
import { PlayerController } from '@store/controllers/player-controller';
|
||||
import { creditStore } from '@store/credit-store';
|
||||
import { QueueController } from '@store/controllers/queue-controller';
|
||||
@@ -81,19 +80,6 @@ export class NowPlaying extends LitElement {
|
||||
|
||||
private reduceMotionQuery?: MediaQueryList;
|
||||
|
||||
/**
|
||||
* Phone width, from the shell's own breakpoint.
|
||||
*
|
||||
* This is in JS rather than in the stylesheet because what changes
|
||||
* is the *content*, not its appearance: the title, artist and
|
||||
* source render as plain text instead of as links, and no CSS rule
|
||||
* can take a click handler off an element.
|
||||
*/
|
||||
@state()
|
||||
private phone = false;
|
||||
|
||||
private phoneQuery?: MediaQueryList;
|
||||
|
||||
/** Whether each field is actively mid-scroll (class toggle). */
|
||||
@state()
|
||||
private titleScrolling = false;
|
||||
@@ -355,12 +341,6 @@ export class NowPlaying extends LitElement {
|
||||
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
|
||||
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
|
||||
|
||||
// Same reasoning as above: looked up here, not at module load,
|
||||
// so a test can install its own matchMedia first.
|
||||
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
|
||||
this.phone = this.phoneQuery?.matches ?? false;
|
||||
this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
|
||||
|
||||
this.resizeObserver = new ResizeObserver(() => {
|
||||
this.geometryDirty = true;
|
||||
this.requestUpdate();
|
||||
@@ -384,7 +364,6 @@ export class NowPlaying extends LitElement {
|
||||
this.attachDragListeners(false);
|
||||
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
||||
this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
|
||||
this.resizeObserver?.disconnect();
|
||||
this.stopScrollCycle('title');
|
||||
this.stopScrollCycle('artist');
|
||||
@@ -509,7 +488,7 @@ export class NowPlaying extends LitElement {
|
||||
@mouseleave=${this.handleTitleMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('title')}
|
||||
>
|
||||
<span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
||||
<span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
||||
</span>
|
||||
<span
|
||||
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
||||
@@ -519,15 +498,14 @@ export class NowPlaying extends LitElement {
|
||||
@mouseleave=${this.handleArtistMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
||||
>
|
||||
<span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
||||
<span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
||||
</span>
|
||||
${describeQueueSource(this.queue.source)
|
||||
? html`
|
||||
<span
|
||||
class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
||||
class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
||||
data-testid="now-playing-source"
|
||||
@click=${(e: MouseEvent) => {
|
||||
if (this.phone) return;
|
||||
if (!isQueueSourceNavigable(this.queue.source)) return;
|
||||
navigateToQueueSource(
|
||||
e.currentTarget as EventTarget,
|
||||
@@ -593,10 +571,6 @@ export class NowPlaying extends LitElement {
|
||||
this.reduceMotion = e.matches;
|
||||
};
|
||||
|
||||
private handlePhoneChange = (e: MediaQueryListEvent): void => {
|
||||
this.phone = e.matches;
|
||||
};
|
||||
|
||||
private shouldScroll(field: 'title' | 'artist'): boolean {
|
||||
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
||||
|
||||
@@ -632,12 +606,6 @@ export class NowPlaying extends LitElement {
|
||||
track?.artist ?? '',
|
||||
this.shouldScroll('title') ? '1' : '0',
|
||||
this.shouldScroll('artist') ? '1' : '0',
|
||||
// Crossing the breakpoint swaps a link for a bare string,
|
||||
// and a link is not guaranteed to measure the same as the
|
||||
// text inside it. The marquee travels a distance read from
|
||||
// that measurement, so this belongs in the key even though
|
||||
// the words are identical either side.
|
||||
this.phone ? '1' : '0',
|
||||
].join('\u0000');
|
||||
}
|
||||
|
||||
|
||||
@@ -11,7 +11,6 @@ import {
|
||||
import { SelectionController } from '@utils/selection-controller';
|
||||
import type { SelectionHost } from '@utils/selection-controller';
|
||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||
import {
|
||||
ContextMenuController,
|
||||
contextMenuStyles,
|
||||
@@ -106,6 +105,9 @@ const ROW_CHROME_WIDTH =
|
||||
const ROW_HEIGHT = 33;
|
||||
const PHONE_ROW_HEIGHT = 52;
|
||||
|
||||
/** The shell's phone breakpoint, as `index.css` and every component
|
||||
* stylesheet spells it. */
|
||||
const PHONE_QUERY = '(max-width: 599px)';
|
||||
|
||||
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
|
||||
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
|
||||
|
||||
@@ -1,23 +0,0 @@
|
||||
/**
|
||||
* The shell's breakpoints, where JavaScript has to agree with CSS.
|
||||
*
|
||||
* A media query inside a shadow root is answered by the viewport, so a
|
||||
* component normally states what it drops at phone width in its own
|
||||
* stylesheet and needs nothing from here. This exists for the cases
|
||||
* where the decision is not a style: `track-list` computes its grid in
|
||||
* JS from the host width, and `now-playing` renders *different content*
|
||||
* on a phone — a plain string instead of a link — which no stylesheet
|
||||
* can express.
|
||||
*
|
||||
* One breakpoint, several expressions of it. It was a private const in
|
||||
* track-list.ts when there was one; a second reader is where a copy
|
||||
* would start drifting from index.css.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Phone width. 600px rather than the sidebar's 900px because 900 is a
|
||||
* laptop: the answer there is a narrower sidebar, which is still a
|
||||
* sidebar. Below this the shell drops the sidebar column entirely and
|
||||
* bottom-nav takes over.
|
||||
*/
|
||||
export const PHONE_QUERY = '(max-width: 599px)';
|
||||
@@ -1,166 +0,0 @@
|
||||
/**
|
||||
* The mini player's links are a desktop affordance.
|
||||
*
|
||||
* `utils/explore-link.ts` makes every track and artist name navigate,
|
||||
* and `utils/queue-source-link.ts` makes "Playing from X" navigate — in
|
||||
* the bottom bar those are a few characters of text at a font size
|
||||
* chosen for a bar, which is not a touch target. Worse, explore-link
|
||||
* holds the navigation for one double-click interval and drops it if a
|
||||
* second click arrives: a gesture that exists so double-clicking a row
|
||||
* can play it, and which means nothing at all on touch.
|
||||
*
|
||||
* So below the shell's phone breakpoint the three render as plain text
|
||||
* and the whole bar's cover art opens the full-screen Now Playing view,
|
||||
* which is where the links live.
|
||||
*
|
||||
* The breakpoint is stubbed rather than emulated for the reason
|
||||
* track-list-phone.test.ts states: this tier's viewport is fixed at
|
||||
* 1280x800 by the runner, and the component reads matchMedia in
|
||||
* connectedCallback precisely so a test can answer it first.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
|
||||
import '@components/now-playing/now-playing';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, flush } from '@test/support/harness';
|
||||
import { fixture, shadow, shadowAll, text } from '@test/support/render';
|
||||
import type { TrackInfo } from '@store/player-store';
|
||||
import type { QueueTrack } from '@store/queue-store';
|
||||
|
||||
const TRACK: TrackInfo = {
|
||||
fileName: 'ashes.mp3',
|
||||
filePath: '/music/ashes.mp3',
|
||||
trackLength: 215,
|
||||
seekPosition: 0,
|
||||
state: 'playing',
|
||||
title: 'Ashes to Ashes',
|
||||
artist: 'David Bowie',
|
||||
album: 'Scary Monsters',
|
||||
coverArt: '',
|
||||
coverArtSmall: '',
|
||||
coverArtMedium: '',
|
||||
coverArtLarge: '',
|
||||
trackChangeId: 1,
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
|
||||
function queueTrack(n: number, title: string): QueueTrack {
|
||||
return {
|
||||
id: n,
|
||||
audioFileId: n,
|
||||
filePath: `/music/${n}.mp3`,
|
||||
position: n,
|
||||
title,
|
||||
artist: 'David Bowie',
|
||||
album: 'Scary Monsters',
|
||||
coverArtPath: '',
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
}
|
||||
|
||||
/** Mount the bar with the phone breakpoint answering `matches`. */
|
||||
async function mountAt(phone: boolean) {
|
||||
const real = window.matchMedia.bind(window);
|
||||
|
||||
window.matchMedia = ((q: string) =>
|
||||
q.includes('max-width: 599px')
|
||||
? {
|
||||
matches: phone,
|
||||
media: q,
|
||||
addEventListener() {},
|
||||
removeEventListener() {},
|
||||
}
|
||||
: real(q)) as typeof window.matchMedia;
|
||||
|
||||
try {
|
||||
const el = await fixture('now-playing');
|
||||
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
|
||||
emit(Events.QueueChanged, {
|
||||
tracks: [queueTrack(1, 'Ashes to Ashes')],
|
||||
currentIndex: 0,
|
||||
source: { type: 'album', id: 7, label: 'Scary Monsters' },
|
||||
});
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
return el;
|
||||
} finally {
|
||||
window.matchMedia = real;
|
||||
}
|
||||
}
|
||||
|
||||
describe('the mini player on a phone', () => {
|
||||
beforeEach(() => {
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
|
||||
});
|
||||
|
||||
it('renders the title and artist as plain text', async () => {
|
||||
const el = await mountAt(true);
|
||||
|
||||
expect(shadowAll(el, '.explore-link').length).toBe(0);
|
||||
|
||||
// The words are unchanged — this is about what they are, not about
|
||||
// hiding them. A fix that dropped the text would pass an assertion
|
||||
// about links alone.
|
||||
expect(text(el, '[data-testid="now-playing-title"]')).toContain(
|
||||
'Ashes to Ashes',
|
||||
);
|
||||
expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
|
||||
'David Bowie',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not navigate from the source line', async () => {
|
||||
const el = await mountAt(true);
|
||||
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||
|
||||
expect(source?.classList.contains('navigable')).toBe(false);
|
||||
|
||||
let navigated = false;
|
||||
el.addEventListener('navigate', () => {
|
||||
navigated = true;
|
||||
});
|
||||
|
||||
source?.click();
|
||||
|
||||
expect(navigated).toBe(false);
|
||||
});
|
||||
|
||||
it('still says where the queue came from', async () => {
|
||||
const el = await mountAt(true);
|
||||
|
||||
// Dropping the *link* is the change; dropping the information would
|
||||
// be a different and worse one.
|
||||
expect(text(el, '[data-testid="now-playing-source"]')).toBe(
|
||||
'Playing from Scary Monsters',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves the desktop bar exactly as it was', async () => {
|
||||
const el = await mountAt(false);
|
||||
|
||||
expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
|
||||
|
||||
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||
|
||||
expect(source?.classList.contains('navigable')).toBe(true);
|
||||
|
||||
let detail: unknown;
|
||||
el.addEventListener('navigate', (e) => {
|
||||
detail = (e as CustomEvent).detail;
|
||||
});
|
||||
|
||||
source?.click();
|
||||
|
||||
expect(detail).toEqual({
|
||||
view: 'explore-album-details',
|
||||
localAlbumId: 7,
|
||||
albumName: 'Scary Monsters',
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user