Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 logan
|
f1c066db6e | ||
|
logan
|
a1ee967323 |
@@ -3664,35 +3664,3 @@ knowing before someone "fixes" it as broken: sampled from screenshots at
|
||||
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
|
||||
row's text 242 → 133. It covers the content area only — not the sidebar
|
||||
or the transport — because the queue is not modal.
|
||||
|
||||
## No test tier can see a `hover:` media query (measured 2026-08-19)
|
||||
|
||||
Gating an affordance on `(hover: hover) and (pointer: fine)` — #68's fix
|
||||
for the play button that flashed on a long-press — is invisible to both
|
||||
browser tiers, in *different* ways, and neither of them fails.
|
||||
|
||||
- **`make ui-test`**: CDP's `Emulation.setEmulatedMedia` with a `hover`
|
||||
feature does not reach the tier's iframe. The call succeeds and
|
||||
`matchMedia('(hover: hover)')` still answers `true` afterwards. So
|
||||
there is no way to render a component as a phone would and read the
|
||||
computed style.
|
||||
- **`make e2e`**: both projects are desktop (`Desktop Chrome`,
|
||||
`Desktop Safari`), and the phone specs reach phone *width* with
|
||||
`setViewportSize`, which changes no media feature but `width`. So the
|
||||
phone specs run with `hover: hover` and the gate is never exercised.
|
||||
|
||||
What does work, and what the fix was verified with, is a second browser
|
||||
context under a device descriptor: `chromium.newContext(devices['Pixel
|
||||
5'])` reports `hover=false pointer:fine=false` and the button computes
|
||||
`display: none`, against `flex` at 1440px. That is a one-off script, not
|
||||
a spec — `isMobile` is Chromium-only, so it cannot become an e2e project
|
||||
without losing the WebKit half.
|
||||
|
||||
`hover-affordance.test.ts` therefore asserts the *parsed stylesheet* —
|
||||
that the reveal rule sits inside the media query — which catches the
|
||||
regression that actually threatens it: someone hoisting the rule back out
|
||||
as a tidy-up, a change nothing on a desktop renders differently.
|
||||
|
||||
Related: a width-gated decision **is** testable at both tiers, which is
|
||||
why #61's phone mini player is a `matchMedia` stub in the component test
|
||||
and needs nothing special.
|
||||
|
||||
@@ -82,100 +82,6 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
|
||||
// carrying in_library with a NULL local_*_id. The upsert's conflict
|
||||
// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
|
||||
// can only ever raise the flag, and this pass used to be gated on the id
|
||||
// being present — which meant nothing in the app could clear such a row,
|
||||
// ever. It is asserted for all three entity types because the gate was
|
||||
// written once and used three times, so a fix applied to one is a fix
|
||||
// that looks complete.
|
||||
//
|
||||
// The rows are seeded with raw SQL rather than through seedIndexResult
|
||||
// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
|
||||
// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
|
||||
// caught that shape and a fixture built through the upsert cannot
|
||||
// reproduce this at all. NULL is what the artifact importer and any
|
||||
// older writer leave behind, the column being nullable with no default.
|
||||
func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
si := NewSearchIndex(db, nil, nil, slog.Default())
|
||||
|
||||
// A genuinely owned artist, to prove the wider gate does not simply
|
||||
// clear everything it now looks at.
|
||||
database.InsertTestTrack(t, db, database.TestTrack{
|
||||
FilePath: "/music/owned.mp3",
|
||||
Artist: "Owned",
|
||||
})
|
||||
|
||||
artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
|
||||
if err != nil {
|
||||
t.Fatalf("read seeded artist: %v", err)
|
||||
}
|
||||
|
||||
seedIndexResult(t, db, SearchIndexResult{
|
||||
EntityType: EntityArtist,
|
||||
MBID: testMBID("owned"),
|
||||
Title: "Owned",
|
||||
ArtistName: "Owned",
|
||||
ArtistMBID: testMBID("owned"),
|
||||
InLibrary: true,
|
||||
LocalArtistID: artist.ID,
|
||||
})
|
||||
|
||||
orphans := []struct {
|
||||
name string
|
||||
entityType string
|
||||
mbid string
|
||||
}{
|
||||
{"artist", EntityArtist, "orphan-artist"},
|
||||
{"release group", EntityReleaseGroup, "orphan-release-group"},
|
||||
{"recording", EntityRecording, "orphan-recording"},
|
||||
}
|
||||
|
||||
for _, o := range orphans {
|
||||
if _, err := db.ExecContext(
|
||||
`INSERT INTO explore_index
|
||||
(entity_type, mbid, title, artist_name, artist_mbid,
|
||||
in_library,
|
||||
local_artist_id, local_release_group_id, local_recording_id)
|
||||
VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
|
||||
dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
|
||||
dbMBID(testMBID(o.mbid)),
|
||||
nil, nil, nil,
|
||||
); err != nil {
|
||||
t.Fatalf("seed %s orphan: %v", o.name, err)
|
||||
}
|
||||
}
|
||||
|
||||
si.pruneStaleLocalCrossReferences()
|
||||
|
||||
inLibrary := func(t *testing.T, mbid string) int {
|
||||
t.Helper()
|
||||
|
||||
var flag int
|
||||
if err := db.QueryRowWriter(
|
||||
"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
|
||||
).Scan(&flag); err != nil {
|
||||
t.Fatalf("read in_library for %q: %v", mbid, err)
|
||||
}
|
||||
|
||||
return flag
|
||||
}
|
||||
|
||||
for _, o := range orphans {
|
||||
if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
|
||||
t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
|
||||
}
|
||||
}
|
||||
|
||||
if got := inLibrary(t, testMBID("owned")); got != 1 {
|
||||
t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
||||
// backfill queue prioritizes artists by how many tracks the user actually
|
||||
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
||||
|
||||
@@ -2562,19 +2562,6 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
|
||||
// The row itself is left in place (it may still be part of the shipped
|
||||
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
||||
// is cleared.
|
||||
//
|
||||
// It is gated on the flag *or* the id, not on the id alone. Gated on
|
||||
// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
|
||||
// upsert can only ever raise the flag and this pass skipped such a row
|
||||
// by construction, so nothing in the app could clear it — a row claiming
|
||||
// to be owned, permanently, with no local row to check the claim
|
||||
// against. Nothing in the tree writes that shape today
|
||||
// (collectLibraryEntities sets both together), which is exactly why it
|
||||
// is worth closing now: the exposure is a database written by an older
|
||||
// version, and the next writer that sets the flag without an id, which
|
||||
// nothing structurally prevents. A NULL id fails the existence test on
|
||||
// its own, so the wider gate needs no second clause to say what "not
|
||||
// owned" means.
|
||||
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||
type prune struct {
|
||||
entityType string
|
||||
@@ -2607,8 +2594,7 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||
result, err := si.db.ExecContext(
|
||||
`UPDATE explore_index
|
||||
SET in_library = 0, `+p.column+` = NULL
|
||||
WHERE entity_type = ?
|
||||
AND (`+p.column+` IS NOT NULL OR in_library = 1)
|
||||
WHERE entity_type = ? AND `+p.column+` IS NOT NULL
|
||||
AND NOT EXISTS (`+p.exists+`)`,
|
||||
dbEntityType(p.entityType),
|
||||
)
|
||||
|
||||
@@ -161,52 +161,29 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
/*
|
||||
* The hover play button is a *hover* affordance, so it is
|
||||
* gated on the device having hover rather than on width. A
|
||||
* touch long-press synthesises a hover state in the WebView,
|
||||
* so on a phone it flashed into view during the 500ms hold
|
||||
* that utils/long-press.ts is measuring for a context menu —
|
||||
* a control appearing because you were reaching for a
|
||||
* different one. A phone user taps the album and plays from
|
||||
* the detail view, so there is nothing to replace it with.
|
||||
*
|
||||
* display:none outside the query rather than opacity:0 on
|
||||
* its own: an opacity-0 button still takes taps and is
|
||||
* still in the accessibility tree, so the invisible control
|
||||
* would keep the hit area it was never meant to have on
|
||||
* touch. Everything else stays inside, so the desktop
|
||||
* animation is unchanged.
|
||||
*/
|
||||
.play {
|
||||
display: none;
|
||||
position: absolute;
|
||||
right: 8px;
|
||||
bottom: 8px;
|
||||
width: 38px;
|
||||
height: 38px;
|
||||
border: none;
|
||||
border-radius: 50%;
|
||||
background: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-fg, #000);
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
cursor: pointer;
|
||||
opacity: 0;
|
||||
transform: translateY(6px);
|
||||
transition: opacity 0.12s ease, transform 0.12s ease;
|
||||
}
|
||||
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.play {
|
||||
position: absolute;
|
||||
right: 8px;
|
||||
bottom: 8px;
|
||||
width: 38px;
|
||||
height: 38px;
|
||||
border: none;
|
||||
border-radius: 50%;
|
||||
background: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-fg, #000);
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
cursor: pointer;
|
||||
opacity: 0;
|
||||
transform: translateY(6px);
|
||||
transition: opacity 0.12s ease, transform 0.12s ease;
|
||||
}
|
||||
|
||||
.card:hover .play,
|
||||
.card:focus-within .play {
|
||||
opacity: 1;
|
||||
transform: translateY(0);
|
||||
}
|
||||
.card:hover .play,
|
||||
.card:focus-within .play {
|
||||
opacity: 1;
|
||||
transform: translateY(0);
|
||||
}
|
||||
|
||||
.name {
|
||||
|
||||
@@ -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,85 +0,0 @@
|
||||
/**
|
||||
* A hover affordance is gated on the device having hover.
|
||||
*
|
||||
* The home page's cover cards reveal a play button on :hover. A touch
|
||||
* long-press synthesises a hover state in the WebView, so on a phone
|
||||
* that button flashed into view during the 500ms hold that
|
||||
* utils/long-press.ts is measuring for a context menu — a control
|
||||
* appearing because the user was reaching for a different one.
|
||||
*
|
||||
* This is asserted against the *parsed stylesheet* rather than by
|
||||
* emulating a touch device, and that is a limitation worth stating
|
||||
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
|
||||
* this tier's iframe — matchMedia still answers `hover: hover` after it
|
||||
* is set — so there is no way here to render the component as a phone
|
||||
* would and read the computed style. What can be checked is the shape
|
||||
* the browser actually built from the css`` literal: that the reveal
|
||||
* lives inside a hover media query and that the default is display:none.
|
||||
*
|
||||
* Which is the regression worth catching anyway. The failure mode is
|
||||
* someone hoisting the rule back out of the query for a one-line tidy —
|
||||
* a change nothing renders differently on a desktop, so every other
|
||||
* assertion in this repo passes and the phone silently regresses.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/home-view/home-view';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
/** Every rule in the element's own adopted stylesheets, flattened. */
|
||||
function rulesOf(host: Element): { text: string; condition: string | null }[] {
|
||||
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
|
||||
const out: { text: string; condition: string | null }[] = [];
|
||||
|
||||
for (const sheet of sheets) {
|
||||
for (const rule of Array.from(sheet.cssRules)) {
|
||||
if (rule instanceof CSSMediaRule) {
|
||||
for (const inner of Array.from(rule.cssRules)) {
|
||||
out.push({ text: inner.cssText, condition: rule.conditionText });
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
out.push({ text: rule.cssText, condition: null });
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('the home card play button', () => {
|
||||
it('reveals itself only where the device has hover', async () => {
|
||||
const el = await fixture('home-view', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
// The sweep is worth nothing if it read no rules at all — the same
|
||||
// first assertion icon-language.test.ts makes for the same reason.
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const reveals = rules.filter(
|
||||
(r) => r.text.includes('.play') && /opacity:\s*1/.test(r.text),
|
||||
);
|
||||
|
||||
expect(reveals.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of reveals) {
|
||||
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||
expect(rule.condition).toMatch(/pointer:\s*fine/);
|
||||
}
|
||||
});
|
||||
|
||||
it('is display:none rather than transparent where it is absent', async () => {
|
||||
const el = await fixture('home-view', {});
|
||||
|
||||
// opacity:0 alone would leave a button that still takes taps and is
|
||||
// still in the accessibility tree, so a phone would keep the hit
|
||||
// area for a control it can never see.
|
||||
const unconditional = rulesOf(el).filter(
|
||||
(r) => r.condition === null && r.text.startsWith('.play'),
|
||||
);
|
||||
|
||||
expect(unconditional.length).toBeGreaterThan(0);
|
||||
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -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',
|
||||
});
|
||||
});
|
||||
});
|
||||
+11
-6
@@ -20,14 +20,19 @@ pre-commit:
|
||||
glob: "*.go"
|
||||
run: go tool golangci-lint run --timeout 5m ./...
|
||||
|
||||
# Snapshots the tree either side of the generators and reports only
|
||||
# what moved across them. This used to be `go generate` plus a bare
|
||||
# `git diff --name-only`, which is the *whole unstaged worktree* — so
|
||||
# any unrelated edit sitting there was reported as stale generated
|
||||
# code, and `make generate` then fixed nothing. See the script.
|
||||
codegen-check:
|
||||
glob: "*.{go,sql,templ}"
|
||||
run: ./scripts/codegen-check.sh
|
||||
run: |
|
||||
go generate ./...
|
||||
if [ -n "$(git diff --name-only)" ]; then
|
||||
echo "Generated code is out of date. Run 'make generate' and stage the changes."
|
||||
# --no-pager, or this blocks forever on `less` waiting for a
|
||||
# keypress that a hook run without a tty will never get: the
|
||||
# commit hangs at exactly the moment it is trying to tell you
|
||||
# why it failed.
|
||||
git --no-pager diff --stat
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# frontend/bindings is generated by `wails3`, not `go generate`, so
|
||||
# the check above does not cover it. ~3.5s warm, ~20s on a cold
|
||||
|
||||
@@ -1,81 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# Fails when `go generate ./...` would change something that is not staged.
|
||||
#
|
||||
# The obvious spelling of this is `go generate && git diff --name-only`,
|
||||
# which is what the hook used to be, and it answers the wrong question:
|
||||
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
|
||||
# note, a plan document, the next commit's files sitting there while this
|
||||
# one lands — was reported as
|
||||
#
|
||||
# Generated code is out of date. Run 'make generate' and stage the changes.
|
||||
#
|
||||
# Running `make generate` then does nothing, because nothing generated is
|
||||
# stale, and the message sends you looking for a codegen problem that does
|
||||
# not exist. Splitting one piece of work into several commits is exactly
|
||||
# the shape that triggers it, so the workaround was a constraint on commit
|
||||
# order for no real reason.
|
||||
#
|
||||
# So the tree is snapshotted either side of the generators and only what
|
||||
# *moved across them* is reported. That is deliberately not a list of
|
||||
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
|
||||
# today's answer, a fourth generator is one `//go:generate` line away, and
|
||||
# a path list is a second place to remember it — the same reasoning that
|
||||
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
|
||||
#
|
||||
# Content, not names: a generated file that is *already* dirty and is then
|
||||
# rewritten further keeps its name in both snapshots and would otherwise
|
||||
# slip through.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
# name + worktree blob hash for every file that differs from the index.
|
||||
# A file listed but absent (a deletion) hashes as "gone" rather than
|
||||
# aborting the pipeline.
|
||||
snapshot() {
|
||||
git diff --name-only | while IFS= read -r f; do
|
||||
if [ -f "$f" ]; then
|
||||
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
|
||||
else
|
||||
printf '%s gone\n' "$f"
|
||||
fi
|
||||
done
|
||||
}
|
||||
|
||||
# A brand-new generated file is not in either diff, because it is not
|
||||
# tracked at all — the same blind spot bindings-check.sh names. Both
|
||||
# snapshots are taken before the generators run.
|
||||
before="$(snapshot)"
|
||||
before_untracked="$(git ls-files --others --exclude-standard)"
|
||||
|
||||
go generate ./...
|
||||
|
||||
after="$(snapshot)"
|
||||
after_untracked="$(git ls-files --others --exclude-standard)"
|
||||
|
||||
# Symmetric difference, and the symmetry is the whole point. Generation
|
||||
# can push a file *into* the unstaged set (it was current, now it is not)
|
||||
# or *out* of it (someone hand-edited generated output and the generator
|
||||
# put it back) — and the second is stale generated code just as much as
|
||||
# the first. Comparing one direction only reports "current" for it,
|
||||
# which is the failure this script was written to stop.
|
||||
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
|
||||
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
|
||||
|
||||
if [ -n "$moved" ]; then
|
||||
echo "codegen-check: generated code is out of date." >&2
|
||||
echo "Run 'make generate' and stage:" >&2
|
||||
printf ' %s\n' $moved >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ "$after_untracked" != "$before_untracked" ]; then
|
||||
echo "codegen-check: generation produced new files. Stage them:" >&2
|
||||
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
|
||||
<(printf '%s\n' "$after_untracked" | sort) >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "codegen-check: generated code is current"
|
||||
@@ -97,55 +97,6 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
|
||||
fi
|
||||
rm -f "$PID_FILE"
|
||||
|
||||
# ── Refuse to inherit somebody else's port ───────────────────────────
|
||||
# The PID check above only knows about *this* worktree: `make dev-stop`
|
||||
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
|
||||
# of this repo share the default port, so an app orphaned by a deleted
|
||||
# worktree goes on listening with nothing left to stop it.
|
||||
#
|
||||
# Without this check the new app starts, fails to bind, exits — and every
|
||||
# curl and playwright-cli call afterwards goes to the *other* process, so
|
||||
# the harness reports facts about an app nobody asked for. That is not a
|
||||
# quiet wrongness either: it presented as
|
||||
# "no such table: libraries" against a freshly created YJ_HOME, which
|
||||
# reads exactly like applySchema or staleshape.go having gone wrong and
|
||||
# is a frightening place to start looking.
|
||||
#
|
||||
# The startup wait below cannot catch it, because the health check is
|
||||
# satisfied by *any* app on the port — which is precisely the failure.
|
||||
# So it is refused here, before anything is launched, rather than warned
|
||||
# about. --port already exists for the legitimate second-app case.
|
||||
port_holder() {
|
||||
command -v ss >/dev/null || return 0
|
||||
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
|
||||
}
|
||||
|
||||
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
|
||||
[ -n "$(port_holder)" ]; then
|
||||
holder="$(port_holder)"
|
||||
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
|
||||
if [ -n "$holder" ]; then
|
||||
# /proc/<pid>/cwd names the checkout it belongs to, and says
|
||||
# "(deleted)" for the orphaned-worktree case that is the whole
|
||||
# reason this is worth a check.
|
||||
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
|
||||
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
|
||||
echo " pid $holder ($cmd)" >&2
|
||||
echo " cwd $cwd" >&2
|
||||
# The PID-file check above has already passed, so whatever this
|
||||
# is, `make dev-stop` does not know about it — saying otherwise
|
||||
# sends you to a command that will report success and change
|
||||
# nothing. Never `pkill -f` here either: the pattern would
|
||||
# match this script's own command line.
|
||||
echo " 'make dev-stop' will not touch it (it is not in" >&2
|
||||
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
|
||||
else
|
||||
echo " The holder could not be identified (no ss, or it belongs" >&2
|
||||
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
||||
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
||||
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
||||
@@ -249,20 +200,6 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
|
||||
sleep 0.25
|
||||
done
|
||||
|
||||
# The loop above exits on the first answer from the port, and "something
|
||||
# answered" is not "the app we started answered". The pre-launch guard
|
||||
# makes that unlikely rather than impossible — a race, or a listener
|
||||
# started in between — and the check is one signal, so it is worth making
|
||||
# here too. An empty log beside a dead pid is the "it exited immediately
|
||||
# and nothing said so" case that the original report spent its time on.
|
||||
if ! kill -0 "$APP_PID" 2>/dev/null; then
|
||||
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
|
||||
echo " $APP_PID) is gone — something else holds the port." >&2
|
||||
tail -n 30 "$LOG_FILE" >&2
|
||||
rm -f "$PID_FILE"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
cat <<EOF
|
||||
dev-headless: up
|
||||
url http://localhost:$PORT
|
||||
|
||||
+3
-27
@@ -38,10 +38,8 @@
|
||||
# Where a body is taken and no --body-file is given, it is read from stdin.
|
||||
#
|
||||
# Environment:
|
||||
# GITEA_TOKEN a PAT with write:issue. `claim` and `mine` additionally
|
||||
# need to know your username: set GITEA_USER, or give the
|
||||
# token read:user and it is looked up.
|
||||
# GITEA_USER your Gitea login. Optional; see above.
|
||||
# GITEA_TOKEN a PAT with write:issue (plus write:repository and read:user,
|
||||
# which the rest of this repo's tooling reaches for)
|
||||
# GITEA_URL defaults to https://git.ljones.me
|
||||
# GITEA_REPO defaults to yonlu/yellowjacket
|
||||
set -euo pipefail
|
||||
@@ -83,29 +81,7 @@ read_body() {
|
||||
if [ "$file" = "-" ]; then cat; else cat "$file"; fi
|
||||
}
|
||||
|
||||
# The one lookup in this script that needs a scope beyond write:issue.
|
||||
# `GET /user` requires read:user, and it is reached for exactly two reasons:
|
||||
# to name the assignee in `claim`, and to filter in `mine`. A token scoped to
|
||||
# the work this script does — write:issue — therefore failed at `claim`, which
|
||||
# is the one step the workflow requires before the first edit, so the whole
|
||||
# documented process was blocked by its own tooling.
|
||||
#
|
||||
# GITEA_USER short-circuits it, which is what lets a least-privilege token do
|
||||
# the job. The lookup stays as the fallback because it is right when the
|
||||
# scope is there and needs no setup at all.
|
||||
me() {
|
||||
if [ -n "${GITEA_USER:-}" ]; then
|
||||
printf '%s' "$GITEA_USER"
|
||||
return
|
||||
fi
|
||||
curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" |
|
||||
python3 "$py" login ||
|
||||
{
|
||||
echo "issue.sh: could not resolve your username. Set GITEA_USER, or" >&2
|
||||
echo "issue.sh: re-issue GITEA_TOKEN with read:user." >&2
|
||||
exit 1
|
||||
}
|
||||
}
|
||||
me() { curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" | python3 "$py" login; }
|
||||
|
||||
label_id() { call GET "/labels?limit=100" | python3 "$py" label-id "$1"; }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user