Compare commits

..
Author SHA1 Message Date
logan e16bd245bd Merge remote-tracking branch 'origin/fix/queue-toggle-state' into integration/small-fixes
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 6m14s
2026-08-18 11:37:33 -04:00
logan 887a9324b4 Merge remote-tracking branch 'origin/fix/drag-count-badge' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan fcb484ead5 Merge remote-tracking branch 'origin/fix/album-card-year' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 48de41cd69 Merge remote-tracking branch 'origin/fix/album-tracklist-heading' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 66a6ee63ab Merge remote-tracking branch 'origin/fix/seek-bar-clock-width' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 10660c8168 Merge remote-tracking branch 'origin/fix/wanted-without-client' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 441b67daaa Merge remote-tracking branch 'origin/fix/album-track-request-badge' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 026f26bdf6 Merge remote-tracking branch 'origin/fix/small-issue-batch' into integration/small-fixes 2026-08-18 11:37:33 -04:00
logan 73dc80bdc9 fix(explore): stop hiding the request badge until the row is hovered
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Canceled after 0s
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
2026-08-18 11:31:49 -04:00
logan 760021ea5a fix(downloads): stop searching a list there is nothing to search with
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m35s
CI / e2e (pull_request) Canceled after 0s
Every pass attempted every request, each came back "no download clients
are enabled", and RecordAttempt wrote that down as an attempt and put a
retry on the clock -- so a wanted list built deliberately without a
client accrued failures and announced "next check in 6 hours" about a
check that cannot happen.

Wanting something with no way to fetch it is supported. Being told it
is being looked for is a lie, and the row says what is true instead.

Everything above the attempt still runs: an artist subscription still
expands, and a request satisfied by some other route -- ripped, bought,
copied in -- is still retired. Neither needs a provider.

TestReconcileRespectsBatchSize now installs a client that finds
nothing, because a batch size is about how many requests one pass
searches for and that only means something when there is something to
search with.

Refs #37
2026-08-18 11:26:35 -04:00
yonlu 63ec068add Merge branch 'main' into fix/small-issue-batch
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m31s
CI / e2e (pull_request) Canceled after 0s
2026-08-18 15:22:56 +00:00
logan 12e75ee24c feat(ui): badge an album drag with how many tracks it carries
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m25s
CI / e2e (pull_request) Successful in 6m5s
Dragging an album to the queue put its cover under the cursor and said
nothing about how much that was -- an album is 1 track or 30 and the
thumbnail is the same picture 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 already says it; this was the exception, because it had a picture
to show instead.

A count of 1 draws no badge: "1" over a single cover is noise, and the
absence reads clearly beside a badge that only appears above one.

The badge sits inside the cover's box rather than overhanging it,
because setDragImage snapshots the element and anything outside it
risks being clipped -- while padding the box instead would move the
cover away from the cursor.

Refs #19
2026-08-18 11:10:09 -04:00
logan 792e87298b fix(ui): stop the album grid eating the year it was sorted by
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m30s
CI / e2e (pull_request) Successful in 6m5s
The year sat inside the same ellipsis box as the title, so it was the
first thing truncation took: a card wide enough for a long album name
never showed its year, and browsing the grid *by year* showed years
only for the albums with short names. The sort said one thing and the
cards showed another.

Title and year are now a flex row where only the title gives way. A
row rather than a second line, because the card's height is what the
virtualizer measures rows by.

Refs #29
2026-08-18 11:08:37 -04:00
logan 266e7032dd fix(explore): stop labelling the album tracklist "TRACKLIST"
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m31s
CI / e2e (pull_request) Successful in 6m9s
A list of numbered titles with durations, under the album's cover, was
the one thing on the page carrying a word above it saying what it is.

What goes is the ink and not the element: the section is a landmark and
the page's heading structure runs through it, so the h3 stays and is
clipped the way sr-only clips -- never display:none, which would take
it out of the accessibility tree along with the layout.

Refs #9
2026-08-18 11:06:14 -04:00
logan d6b48fb3ac fix(player): stop the seek bar resizing as its clocks count
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 6m16s
Two different things moved it and they need different answers. Digits
in a proportional font are different widths, so 1:11 is narrower than
4:08 and the bar breathed once a second -- tabular figures fix that.
The character *count* changes too, at the hundredth minute and whenever
the right-hand clock is toggled to remaining and grows a minus sign,
which a figure width cannot fix -- so each clock reserves the widest
string this track can put in it.

The budget is per track rather than a constant: reserving six
characters on every track would push the slider in by a character at
each end to buy nothing.

Measured in the component tier: 4.5px of drift across three positions
before, none after.

Refs #13
2026-08-18 11:04:21 -04:00
yonluandClaude Opus 5 185eb1b125 feat(smartplaylist): let a rule set match any rule, not only all of them
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Canceled after 0s
CI / e2e (pull_request) Canceled after 0s
The conditions were joined with " AND " and nothing else, so a smart
playlist could only ever narrow: "jazz released after 1960" was
expressible and "jazz or blues" was not, which is most of what anyone
reaches for a second rule to say.

`RuleSet.Match` is "all" or "any", and an empty match is "all" — which
is what every playlist saved before the field existed carries, so an
upgrade cannot silently widen one. ParseRuleSet rejects anything else
rather than falling through to AND, since a playlist quietly returning
the wrong tracks is worse than one that refuses to be saved.

Under OR each condition is parenthesised and under AND it is not: AND
is the tighter operator, so an OR-join has to protect a condition
carrying a top-level AND of its own — `days_since_played less_than` is
two predicates belonging to one rule.

The editor shows the choice as a sentence with the control in the
middle, and hides it while there is one rule: with nothing to combine,
all and any are the same query.

Closes #35

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 08:06:16 -04:00
yonluandClaude Opus 5 b3556d825c fix(mediacontrols): always send an art URL, even when there is no art
Every other key in the MPRIS metadata map can be omitted safely,
because a client reading it renders a track with no title as a track
with no title. Art is different: KDE's applet treats an absent
mpris:artUrl as no news about the art and keeps drawing whatever the
last track had, so playing something without a cover left the previous
album's sleeve on screen — which reads as the wrong track playing
rather than as missing artwork.

The map's construction moves out of UpdateMetadata into a pure
metadataMap so it can be asserted on at all: everything else in this
file needs a live session bus, which is the same reason the Android
contract lives in an untagged file.

Closes #41

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 08:06:01 -04:00
yonluandClaude Opus 5 bf4f352117 fix(queue): stop claiming a queue came from somewhere it no longer does
`q.source` was written by SetQueue and cleared in exactly one place,
Clear, so no append path touched it: adding a track to a queue built
from an album left the page still offering "Playing from <that album>",
and since the source is persisted alongside the queue state the wrong
label outlived the session that earned it.

Every add and insert path drops it now. Removing and reordering
deliberately do not — a queue with a track taken out of it is still
that album, and the link still goes somewhere true. Only the arrival of
a track from elsewhere makes the claim false.

The delta event carries the source for the same reason it carries the
current index: an append emits nothing else, so the frontend would keep
the label it was last given until something forced a full state.

Closes #14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 08:05:43 -04:00
yonluandClaude Opus 5 1062b7c0bc fix(explore): tell Lit that a track request changed something
The album page's tracklist badges read `libraryStatusFor(false,
track.mbid)` at render time, which is a dependency on `downloadStore`
that Lit cannot see. The page did subscribe to that store, but its
callback only assigned `canDownload` and `isRequested` — neither of
which a *track* request changes — so no reactive field moved and the
component never re-rendered. The request was filed, the plus stayed a
plus, and clicking again cancelled it.

The other three hosts rendering these badges have always asked for the
repaint in the same place, which is what made this one look correct on
inspection.

Closes #33

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 08:04:39 -04:00
26 changed files with 1642 additions and 87 deletions
+21 -1
View File
@@ -310,6 +310,25 @@ func (r *Reconciler) run(ctx context.Context, force bool) (Summary, error) {
summary.Synced = r.syncExternalLists(ctx)
// Nothing is searched for when there is nothing to search with, and
// the point is what that *does not* do to the list.
//
// Attempting anyway is not merely wasted work: every request comes
// back "no download clients are enabled", which RecordAttempt writes
// down as an attempt and schedules a retry for -- so a user who has
// deliberately built a wanted list with no client watched their
// requests accrue failures and announce "next check in 6 hours"
// about a check that cannot happen. Wanting something without a way
// to fetch it is a supported thing to do; being told it is being
// looked for is a lie.
//
// Everything above this line still runs: an artist subscription
// still expands, and a request the user satisfied by some other
// route -- ripped, bought, copied in -- is still retired, because
// neither needs a provider.
summary.NoProviders = len(r.manager.enabledProviders()) == 0
if !summary.NoProviders {
attempted, started, err := r.attemptDue(ctx, force)
if err != nil {
return summary, err
@@ -317,8 +336,9 @@ func (r *Reconciler) run(ctx context.Context, force bool) (Summary, error) {
summary.Attempted = attempted
summary.Started = started
}
summary.Waiting = r.countWaiting(ctx)
summary.NoProviders = len(r.manager.enabledProviders()) == 0
r.logger.Info(
"reconciled request list",
+78
View File
@@ -453,6 +453,15 @@ func TestReconcileRespectsBatchSize(t *testing.T) {
f := newReconcileFixture(t)
ctx := context.Background()
// A client that searches and finds nothing. The batch size is about
// how many requests one pass *searches for*, which only means
// anything when there is something to search with -- a pass with no
// provider now attempts nothing at all, deliberately.
f.manager.installProvider(
Config{ID: 1, Priority: 50},
NewFakeProvider(1, "finds-nothing", Caps{CanSearch: true}),
)
f.reconciler.SetBatch(2)
for _, mbid := range []string{"rg-1", "rg-2", "rg-3", "rg-4"} {
@@ -593,3 +602,72 @@ func TestSummaryReportsNoProviders(t *testing.T) {
t.Error("summary did not report that no download client is enabled")
}
}
// ...and it does not search, which is the part the user sees.
//
// Attempting with no provider fails every request with "no download
// clients are enabled", and RecordAttempt writes that down as an
// attempt and schedules a retry -- so a wanted list built deliberately
// without a client accrued failures and announced "next check in 6
// hours" about a check that cannot happen. Wanting something with no
// way to fetch it is supported; being told it is being looked for is
// a lie.
func TestNoProvidersMeansNoAttempt(t *testing.T) {
t.Parallel()
f := newReconcileFixture(t)
ctx := context.Background()
id, err := f.store.AddRequest(ctx, Request{
MBID: "rg-1",
Entity: EntityReleaseGroup,
LibraryID: 1,
Title: "OK Computer",
})
if err != nil {
t.Fatalf("AddRequest: %v", err)
}
f.catalog.tracklists["rg-1"] = fourTrackDownload().Expected
summary, err := f.reconciler.RunNow(ctx)
if err != nil {
t.Fatalf("RunNow: %v", err)
}
if summary.Attempted != 0 {
t.Errorf("attempted %d requests with no client to search with, want 0",
summary.Attempted)
}
// The list still knows what is on it: "nothing happened" has to be
// reportable as "nothing was searched for, of the one thing you
// want" rather than as silence.
if summary.Waiting != 1 {
t.Errorf("summary reported %d waiting, want 1", summary.Waiting)
}
req, err := f.store.GetRequest(ctx, id)
if err != nil {
t.Fatalf("GetRequest: %v", err)
}
if req.Attempts != 0 {
t.Errorf("attempts = %d, want 0: a pass that could not search did not",
req.Attempts)
}
if req.LastError != "" {
t.Errorf("lastError = %q, want empty: the request did not fail, it "+
"was never tried", req.LastError)
}
// A new request is due immediately (next_try_at is set to now on
// insert), so the fault is not the presence of a time -- it is a
// time pushed into the future by a failed attempt, which is what the
// UI renders as "next check in 6 hours".
if req.NextTryAt.After(time.Now().Add(time.Minute)) {
t.Errorf("next try scheduled for %v: a check that cannot happen was "+
"put on the clock", req.NextTryAt)
}
}
+41 -10
View File
@@ -279,17 +279,19 @@ func (h *MPRISHandler) enqueue(fn func()) {
}
}
// UpdateMetadata pushes track metadata to D-Bus.
func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
h.mu.Lock()
h.trackID++
tid := h.trackID
h.mu.Unlock()
m := map[string]interface{}{
// metadataMap builds the org.mpris.MediaPlayer2.Player Metadata value
// for one track.
//
// It is separated from UpdateMetadata, which needs a live D-Bus
// connection, so the map's contents can be asserted on: this file is
// behind a build tag and everything in it that touches h is reachable
// only from a session bus, which is the same reason the Android
// contract lives in an untagged androidpayload.go.
func metadataMap(meta Metadata, trackID uint64) map[string]any {
m := map[string]any{
"mpris:trackid": dbus.ObjectPath(
fmt.Sprintf(
"/org/yellowjacket/Track/%d", tid,
"/org/yellowjacket/Track/%d", trackID,
),
),
}
@@ -306,16 +308,45 @@ func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
m["xesam:album"] = meta.Album
}
// Always present, even with nothing to point at.
//
// Every other key here can be omitted safely because a client
// reading the map sees a track with no title or no album and
// renders it that way. Art is different: KDE's applet (and
// others) treat an *absent* mpris:artUrl as "no news about the
// art" and keep drawing whatever the last track had, so playing
// something with no cover left the previous album's sleeve on
// screen — which reads as the wrong track playing rather than as
// missing artwork.
//
// An empty string is the honest answer and is what the spec's
// "URI" type degrades to; a client that cannot load it falls back
// to its own placeholder, which is the behaviour wanted.
artURL := ""
if meta.ArtFilePath != "" {
m["mpris:artUrl"] = "file://" + meta.ArtFilePath
artURL = "file://" + meta.ArtFilePath
}
m["mpris:artUrl"] = artURL
if meta.DurationSec > 0 {
m["mpris:length"] = int64(
meta.DurationSec,
) * usPerSec
}
return m
}
// UpdateMetadata pushes track metadata to D-Bus.
func (h *MPRISHandler) UpdateMetadata(meta Metadata) {
h.mu.Lock()
h.trackID++
tid := h.trackID
h.mu.Unlock()
m := metadataMap(meta, tid)
h.enqueue(func() {
h.props.SetMust(playerIf, "Metadata", m)
})
+86
View File
@@ -0,0 +1,86 @@
//go:build linux && !android
package mediacontrols
import "testing"
// The one key that must be present even when it is empty.
//
// Everything else in the map may be omitted, because a client reading
// it renders a track with no title as a track with no title. Art is
// different: KDE's applet treats an *absent* mpris:artUrl as no news
// about the art and keeps drawing the last one it saw, so a track with
// no cover wore the previous album's sleeve — which reads as the wrong
// track playing rather than as missing artwork.
func TestMetadataMapAlwaysCarriesArtURL(t *testing.T) {
t.Parallel()
tests := []struct {
name string
meta Metadata
want string
}{
{
name: "no art at all",
meta: Metadata{Title: "Blue in Green"},
want: "",
},
{
name: "art on disk",
meta: Metadata{
Title: "Blue in Green",
ArtFilePath: "/covers/kind-of-blue_lg.jpg",
},
want: "file:///covers/kind-of-blue_lg.jpg",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
m := metadataMap(tt.meta, 1)
got, ok := m["mpris:artUrl"]
if !ok {
t.Fatal("mpris:artUrl is absent; it must always be sent")
}
if got != tt.want {
t.Errorf("mpris:artUrl = %v, want %q", got, tt.want)
}
})
}
}
// The trackid has to change between tracks or a client is entitled to
// treat the metadata as describing the same track it already has.
func TestMetadataMapTrackIDVaries(t *testing.T) {
t.Parallel()
first := metadataMap(Metadata{Title: "A"}, 1)["mpris:trackid"]
second := metadataMap(Metadata{Title: "B"}, 2)["mpris:trackid"]
if first == second {
t.Errorf("trackid did not change: %v", first)
}
}
// The optional keys stay optional — this is what makes artUrl's
// always-present treatment a deliberate exception rather than drift.
func TestMetadataMapOmitsEmptyOptionalFields(t *testing.T) {
t.Parallel()
m := metadataMap(Metadata{}, 1)
for _, key := range []string{
"xesam:title",
"xesam:artist",
"xesam:album",
"mpris:length",
} {
if _, ok := m[key]; ok {
t.Errorf("%s is present for an empty Metadata", key)
}
}
}
+1
View File
@@ -81,6 +81,7 @@ func (q *Queue) emitTracksModified(
Index: index,
Positions: positions,
CurrentIndex: q.currentIndex,
Source: q.source,
},
)
}
+50
View File
@@ -219,6 +219,56 @@ func TestEmit_AddTrackSendsDeltaNotSnapshot(t *testing.T) {
}
}
// The append clears the source, and the delta is the only event those
// paths emit — so if it does not carry the source, the frontend keeps
// the label it was last given and goes on offering a link back to an
// album the queue no longer holds until something forces a full state.
func TestEmit_AppendDeltaCarriesClearedSource(t *testing.T) {
t.Parallel()
q, db, rec := setupRecordedQueue(t)
paths := seedAudioFiles(t, db, 4)
q.SetQueue(
paths[:3], 0, false,
Source{Type: "album", ID: 1, Label: "Abbey Road"},
)
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
}
rec.Reset()
q.AddTrack(paths[3])
if got := modifiedOf(t, rec).Source; got != (Source{}) {
t.Errorf("delta source = %+v, want zero value", got)
}
}
// And a delta that did not clear it still reports the source it has,
// or the frontend would drop a perfectly good label on every removal.
func TestEmit_NonAppendDeltaCarriesSource(t *testing.T) {
t.Parallel()
q, db, rec := setupRecordedQueue(t)
paths := seedAudioFiles(t, db, 4)
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
q.SetQueue(paths, 0, false, album)
if _, ok := rec.Wait(events.QueueChanged, waitFor); !ok {
t.Fatalf("no QueueChanged after SetQueue; got %v", rec.Names())
}
rec.Reset()
q.RemoveTrack(3)
if got := modifiedOf(t, rec).Source; got != album {
t.Errorf("delta source = %+v, want %+v", got, album)
}
}
func TestEmit_RemoveTracksReportsPositions(t *testing.T) {
t.Parallel()
+44
View File
@@ -156,12 +156,21 @@ type PlaybackFailure struct {
}
// TracksModified is the payload for the QueueTracksModified event.
//
// Source is carried because an append is exactly what can *invalidate*
// it: a queue built from one album stops being that album the moment a
// track from somewhere else is added to it. The delta is the only event
// those paths emit, so without this the frontend would keep the label
// it was last given and go on saying "Playing from" an album that is no
// longer what is queued — an event carrying what its consumer needs, so
// nothing has to invalidate anything.
type TracksModified struct {
Action string `json:"action"`
Tracks []Track `json:"tracks,omitempty"`
Index int `json:"index"`
Positions []int `json:"positions,omitempty"`
CurrentIndex int `json:"currentIndex"`
Source Source `json:"source"`
}
// Queue manages an ordered list of tracks for playback.
@@ -455,6 +464,8 @@ func (q *Queue) AddTrack(filePath string) {
q.generateShuffleOrder()
}
q.dropSource()
q.persistAddTrack(track)
q.persistState()
q.emitTracksModified(
@@ -505,6 +516,8 @@ func (q *Queue) AddTracks(filePaths []string) {
q.generateShuffleOrder()
}
q.dropSource()
q.persistAddTracks(newTracks)
q.persistState()
q.emitTracksModified(
@@ -563,6 +576,8 @@ func (q *Queue) InsertNextTracks(filePaths []string) {
q.generateShuffleOrder()
}
q.dropSource()
q.persistInsertTracks(newTracks, insertPos)
q.persistState()
q.emitTracksModified(
@@ -613,6 +628,8 @@ func (q *Queue) InsertNext(filePath string) {
q.generateShuffleOrder()
}
q.dropSource()
q.persistInsertTracks([]Track{track}, insertPos)
q.persistState()
q.emitTracksModified(
@@ -680,6 +697,8 @@ func (q *Queue) InsertTracksAt(filePaths []string, index int) {
q.generateShuffleOrder()
}
q.dropSource()
q.persistInsertTracks(newTracks, index)
q.persistState()
q.emitTracksModified(
@@ -1537,6 +1556,31 @@ func (q *Queue) reindexPositions() {
}
}
// dropSource forgets which collection the queue was built from.
//
// A Source is a claim that everything queued came from one album,
// playlist, genre or artist, and the frontend renders it as a
// "Playing from X" link back to that page. Adding or inserting a track
// makes the claim false — the queue is now that album *plus* something
// else — so every path that does so calls this.
//
// It was set by SetQueue and cleared in exactly one place, Clear, so a
// label survived every append. It is persisted too (source_type /
// source_id / source_label on the queue state row), which is what made
// a wrong label outlive the session that earned it: an album queued on
// Monday, added to on Tuesday, still offered a link back to that album
// on Friday.
//
// Removing, reordering and shuffling deliberately do not call this. A
// queue with a track taken out of it, or played in another order, is
// still that album — the link still goes somewhere true. Only the
// arrival of a track from elsewhere makes it a lie.
//
// The caller must hold q.mu.
func (q *Queue) dropSource() {
q.source = Source{}
}
// commitMutation persists the current queue state after a mutation.
// When reindex is true, track positions are renumbered first.
// The caller must hold q.mu.
+111
View File
@@ -126,6 +126,117 @@ func TestClear_ResetsSource(t *testing.T) {
}
}
// A queue built from one album stops being that album the moment a
// track from somewhere else joins it, so every path that adds one
// drops the source. Before this, SetQueue was the only writer and
// Clear the only clearer, so "Playing from Abbey Road" outlived every
// append — and, being persisted, every restart too.
func TestAppendPathsDropSource(t *testing.T) {
t.Parallel()
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
tests := []struct {
name string
append func(q *Queue, paths []string)
}{
{
name: "AddTrack",
append: func(q *Queue, paths []string) {
q.AddTrack(paths[5])
},
},
{
name: "AddTracks",
append: func(q *Queue, paths []string) {
q.AddTracks(paths[5:7])
},
},
{
name: "InsertNext",
append: func(q *Queue, paths []string) {
q.InsertNext(paths[5])
},
},
{
name: "InsertNextTracks",
append: func(q *Queue, paths []string) {
q.InsertNextTracks(paths[5:7])
},
},
{
name: "InsertTracksAt",
append: func(q *Queue, paths []string) {
q.InsertTracksAt(paths[5:7], 1)
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 8)
q.SetQueue(paths[:5], 0, false, album)
if got := q.GetState().Source; got != album {
t.Fatalf("source before append: got %+v, want %+v", got, album)
}
tt.append(q, paths)
if got := q.GetState().Source; got != (Source{}) {
t.Errorf(
"source after %s: got %+v, want zero value",
tt.name, got,
)
}
})
}
}
// Removing and reordering deliberately do not drop it: a queue with a
// track taken out of it is still that album, and the link still goes
// somewhere true.
func TestRemoveAndMoveKeepSource(t *testing.T) {
t.Parallel()
album := Source{Type: "album", ID: 1, Label: "Abbey Road"}
t.Run("RemoveTrack", func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 5)
q.SetQueue(paths, 0, false, album)
q.RemoveTrack(3)
if got := q.GetState().Source; got != album {
t.Errorf("source after RemoveTrack: got %+v, want %+v", got, album)
}
})
t.Run("MoveQueueTracks", func(t *testing.T) {
t.Parallel()
q, db := setupTestQueue(t)
paths := seedAudioFiles(t, db, 5)
q.SetQueue(paths, 0, false, album)
q.MoveQueueTracks([]int{0}, 3)
if got := q.GetState().Source; got != album {
t.Errorf(
"source after MoveQueueTracks: got %+v, want %+v",
got, album,
)
}
})
}
func TestSetQueue_WithStartIndex(t *testing.T) {
t.Parallel()
+72 -3
View File
@@ -28,6 +28,7 @@ var (
errUnsupportedOp = errors.New("unsupported operator")
errInvalidSortField = errors.New("invalid sort field: not in allowed field list")
errNotNumeric = errors.New("value must be numeric")
errInvalidMatch = errors.New("match must be \"all\" or \"any\"")
)
// Rule represents a single filter condition for a smart playlist.
@@ -37,10 +38,42 @@ type Rule struct {
Value string `json:"value"`
}
// MatchType decides how a rule set's conditions combine.
//
// The rules used to be joined with " AND " and nothing else, so a
// playlist could only ever narrow: "jazz released after 1960" was
// expressible and "jazz or blues" was not, which is most of what
// anyone reaches for a second rule to say.
type MatchType string
const (
// MatchAll requires every rule to hold — the historical behaviour,
// and what an empty match means so that every rule set written
// before this existed keeps the meaning it was saved with.
MatchAll MatchType = "all"
// MatchAny requires at least one rule to hold.
MatchAny MatchType = "any"
)
// joiner returns the SQL keyword that combines two conditions.
// An unrecognised value cannot reach here — ParseRuleSet rejects one
// — so the default is about the empty string, which is every rule set
// saved before this field existed.
func (m MatchType) joiner() string {
if m == MatchAny {
return " OR "
}
return " AND "
}
// RuleSet holds the complete filter configuration for a smart
// playlist, including optional sort and limit.
type RuleSet struct {
Rules []Rule `json:"rules"`
// Match is "all" or "any"; empty means "all". It is omitempty so
// an untouched playlist's stored JSON does not change shape.
Match MatchType `json:"match,omitempty"`
Limit int `json:"limit,omitempty"`
SortField string `json:"sort_field,omitempty"`
SortDir string `json:"sort_dir,omitempty"`
@@ -116,7 +149,12 @@ const genreDelimiter = "||"
// slice of rules. It is a pure function — no database access needed.
// Returns the clause (without the leading "WHERE"), the parameter
// args, and any validation error.
func BuildWhereClause(rules []Rule) (string, []any, error) {
//
// match decides how the conditions combine; an empty match is MatchAll,
// which is what every rule set saved before the field existed means.
func BuildWhereClause(
rules []Rule, match MatchType,
) (string, []any, error) {
if len(rules) == 0 {
return "", nil, nil
}
@@ -179,7 +217,28 @@ func BuildWhereClause(rules []Rule) (string, []any, error) {
args = append(args, condArgs...)
}
return strings.Join(conditions, " AND "), args, nil
// Under OR, each condition is parenthesised; under AND it is not.
//
// The asymmetry is deliberate rather than an omission. AND is the
// tighter operator in SQL, so an OR-join has to protect any
// condition that contains a top-level AND of its own or the halves
// come apart: `days_since_played less_than` is
// `last_played IS NOT NULL AND <expr> < ?`, which read without
// brackets under an OR-join happens to still parse correctly and
// would stop doing so the moment a condition grows a top-level OR.
// Bracketing under AND would be a no-op semantically and would
// rewrite the clause every existing test pins, so the brackets go
// exactly where they change something.
if match == MatchAny {
bracketed := make([]string, len(conditions))
for i, cond := range conditions {
bracketed[i] = "(" + cond + ")"
}
conditions = bracketed
}
return strings.Join(conditions, match.joiner()), args, nil
}
// validateOperator checks that the operator is valid for the field
@@ -599,7 +658,7 @@ func Evaluate(
start := time.Now()
logger := db.Logger()
where, args, err := BuildWhereClause(ruleSet.Rules)
where, args, err := BuildWhereClause(ruleSet.Rules, ruleSet.Match)
if err != nil {
return nil, fmt.Errorf(
"smart playlist rule error: %w", err,
@@ -1036,6 +1095,16 @@ func ParseRuleSet(jsonStr string) (RuleSet, error) {
)
}
// A match nobody recognises would otherwise fall through to AND,
// which is a playlist quietly returning the wrong tracks rather
// than refusing to be saved. This is the only place a rule set
// enters the backend, so it is the only place that has to ask.
if rs.Match != "" && rs.Match != MatchAll && rs.Match != MatchAny {
return RuleSet{}, fmt.Errorf(
"%w: %q", errInvalidMatch, rs.Match,
)
}
return rs, nil
}
+203 -24
View File
@@ -1,6 +1,7 @@
package smartplaylist
import (
"errors"
"strings"
"testing"
@@ -170,7 +171,7 @@ func TestBuildWhereClause_TextIs(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -189,7 +190,7 @@ func TestBuildWhereClause_TextIsNot(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is_not", Value: "Queen"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -209,7 +210,7 @@ func TestBuildWhereClause_TextContains(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "contains", Value: "Black"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -231,7 +232,7 @@ func TestBuildWhereClause_TextDoesNotContain(t *testing.T) {
Field: "title", Operator: "does_not_contain",
Value: "Black",
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -251,7 +252,7 @@ func TestBuildWhereClause_TextStartsWith(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "starts_with", Value: "Back"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -270,7 +271,7 @@ func TestBuildWhereClause_TextEndsWith(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "title", Operator: "ends_with", Value: "Black"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -292,7 +293,7 @@ func TestBuildWhereClause_TextIsAnyOf(t *testing.T) {
Field: "artist", Operator: "is_any_of",
Value: `["Queen","AC/DC"]`,
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -312,7 +313,7 @@ func TestBuildWhereClause_NumericIs(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "is", Value: "1980"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -331,7 +332,7 @@ func TestBuildWhereClause_NumericIsNot(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "is_not", Value: "1980"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -350,7 +351,7 @@ func TestBuildWhereClause_NumericGreaterThan(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "greater_than", Value: "2000"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -369,7 +370,7 @@ func TestBuildWhereClause_NumericLessThan(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "less_than", Value: "1980"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -391,7 +392,7 @@ func TestBuildWhereClause_NumericBetween(t *testing.T) {
Field: "year", Operator: "between",
Value: "1975,1985",
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -414,7 +415,7 @@ func TestBuildWhereClause_NumericBetweenJSON(t *testing.T) {
Field: "year", Operator: "between",
Value: `["1975","1985"]`,
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -434,7 +435,7 @@ func TestBuildWhereClause_GenreIsProducesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "is", Value: "Rock"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -466,7 +467,7 @@ func TestBuildWhereClause_GenreIsNotProducesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "is_not", Value: "Rock"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -495,7 +496,7 @@ func TestBuildWhereClause_GenreIsAnyOfProducesSubquery(t *testing.T) {
Field: "genre", Operator: "is_any_of",
Value: `["Rock","Pop"]`,
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -524,7 +525,7 @@ func TestBuildWhereClause_GenreContainsUsesSubquery(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "genre", Operator: "contains", Value: "Rock"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -557,7 +558,7 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -572,6 +573,110 @@ func TestBuildWhereClause_MultipleRulesAND(t *testing.T) {
}
}
func TestBuildWhereClause_MultipleRulesOR(t *testing.T) {
t.Parallel()
clause, args, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"},
}, MatchAny)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := "(artist_name = ? COLLATE NOCASE) OR (year > ?)"
if clause != want {
t.Errorf("clause = %q, want %q", clause, want)
}
if len(args) != 2 || args[0] != "Queen" || args[1] != int64(1975) {
t.Errorf("args = %v, want [Queen 1975]", args)
}
}
// An empty match is what every rule set saved before the field existed
// carries, and it has to keep meaning AND — a playlist silently
// widening to OR on upgrade is the whole risk of adding this field.
func TestBuildWhereClause_EmptyMatchIsAll(t *testing.T) {
t.Parallel()
rules := []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1975"},
}
empty, _, err := BuildWhereClause(rules, "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
all, _, err := BuildWhereClause(rules, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if empty != all {
t.Errorf("empty match = %q, want the same as MatchAll %q",
empty, all)
}
}
// A condition carrying its own top-level AND is what makes the
// bracketing under OR load-bearing: `days_since_played less_than`
// is two predicates, and both belong to the same rule.
func TestBuildWhereClause_ORBracketsCompoundCondition(t *testing.T) {
t.Parallel()
clause, _, err := BuildWhereClause([]Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{
Field: "days_since_played",
Operator: "less_than",
Value: "30",
},
}, MatchAny)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !strings.Contains(clause, "(last_played IS NOT NULL AND") {
t.Errorf(
"compound condition is not bracketed under OR: %q",
clause,
)
}
}
func TestParseRuleSet_RejectsUnknownMatch(t *testing.T) {
t.Parallel()
_, err := ParseRuleSet(`{"rules":[],"match":"either"}`)
if err == nil {
t.Fatal("expected an error for an unknown match type")
}
if !errors.Is(err, errInvalidMatch) {
t.Errorf("err = %v, want errInvalidMatch", err)
}
}
func TestParseRuleSet_AcceptsAnyAndAll(t *testing.T) {
t.Parallel()
for _, want := range []MatchType{MatchAll, MatchAny} {
rs, err := ParseRuleSet(
`{"rules":[],"match":"` + string(want) + `"}`,
)
if err != nil {
t.Fatalf("match %q: unexpected error: %v", want, err)
}
if rs.Match != want {
t.Errorf("match = %q, want %q", rs.Match, want)
}
}
}
func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
t.Parallel()
@@ -581,7 +686,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
Field: "genre", Operator: "does_not_contain",
Value: "Punk",
},
})
}, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -609,7 +714,7 @@ func TestBuildWhereClause_SameFieldMultipleTimes(t *testing.T) {
func TestBuildWhereClause_EmptyRules(t *testing.T) {
t.Parallel()
clause, args, err := BuildWhereClause(nil)
clause, args, err := BuildWhereClause(nil, MatchAll)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -631,7 +736,7 @@ func TestBuildWhereClause_InvalidField(t *testing.T) {
Field: "nonexistent", Operator: "is",
Value: "anything",
},
})
}, MatchAll)
if err == nil {
t.Fatal("expected error for invalid field, got nil")
}
@@ -654,7 +759,7 @@ func TestBuildWhereClause_InvalidOperatorForNumeric(t *testing.T) {
_, _, err := BuildWhereClause([]Rule{
{Field: "year", Operator: "contains", Value: "1980"},
})
}, MatchAll)
if err == nil {
t.Fatal(
"expected error for text operator on numeric field",
@@ -676,7 +781,7 @@ func TestBuildWhereClause_InvalidOperatorForText(t *testing.T) {
Field: "artist", Operator: "greater_than",
Value: "Queen",
},
})
}, MatchAll)
if err == nil {
t.Fatal(
"expected error for numeric operator on text field",
@@ -723,6 +828,80 @@ func TestEvaluate_TextIs(t *testing.T) {
}
}
// Two rules that share no track at all: under AND this is empty, and
// under OR it is the union. Before Match existed only the first was
// expressible, so a playlist could only ever narrow — "jazz or blues"
// had no way to be said.
func TestEvaluate_MatchAnyUnionsWhereMatchAllIntersects(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
seedSmartPlaylistData(t, db)
// Queen has two tracks; Beyoncé has one; no track is by both.
rules := []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "artist", Operator: "is", Value: "Beyoncé"},
}
all, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAll})
if err != nil {
t.Fatalf("Evaluate(all): %v", err)
}
if len(all) != 0 {
t.Errorf("match=all returned %d tracks, want 0", len(all))
}
either, err := Evaluate(db, RuleSet{Rules: rules, Match: MatchAny})
if err != nil {
t.Fatalf("Evaluate(any): %v", err)
}
if len(either) != 3 {
t.Fatalf("match=any returned %d tracks, want 3", len(either))
}
for _, tr := range either {
if tr.ArtistName != "Queen" && tr.ArtistName != "Beyoncé" {
t.Errorf(
"track %q has artist %q, want Queen or Beyoncé",
tr.TrackName, tr.ArtistName,
)
}
}
}
// An empty match is what every playlist saved before the field existed
// carries, and it has to keep meaning AND all the way through Evaluate
// — a stored playlist silently widening on upgrade is the only real
// risk in adding this.
func TestEvaluate_EmptyMatchStillIntersects(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
seedSmartPlaylistData(t, db)
tracks, err := Evaluate(db, RuleSet{
Rules: []Rule{
{Field: "artist", Operator: "is", Value: "Queen"},
{Field: "year", Operator: "greater_than", Value: "1979"},
},
})
if err != nil {
t.Fatalf("Evaluate: %v", err)
}
// Only "Another One Bites the Dust" (Queen, 1980) satisfies both.
if len(tracks) != 1 {
t.Fatalf("got %d tracks, want 1", len(tracks))
}
if want := "Another One Bites the Dust"; tracks[0].TrackName != want {
t.Errorf("got %q, want %q", tracks[0].TrackName, want)
}
}
// TestEvaluate_ArtworkEnrichment verifies the presentation-only
// cover-art and MusicBrainz-ID fields are attached to matched tracks
// by the batched fetchArtwork pass (they are no longer part of the
@@ -1340,7 +1519,7 @@ func TestSQLInjection_FieldName(t *testing.T) {
Field: "title; DROP TABLE playlists",
Operator: "is", Value: "x",
},
})
}, MatchAll)
if err == nil {
t.Fatal(
"expected error for injected field name, got nil",
@@ -68,6 +68,29 @@ export class SeekBar extends LitElement {
align-items: center;
}
/* The clocks must not resize as they count.
Two things move them, and they need different answers. Digits in
a proportional font are different widths, so 1:11 is narrower
than 4:08 and the bar breathed once a second -- that is what
tabular figures fix. The character *count* changes too, at the
hundredth minute and whenever the right-hand clock is toggled to
remaining and grows a minus sign, and a figure width cannot fix
that -- so each clock also reserves the widest string this track
can put in it. The budget is per track rather than a constant
because reserving six characters on every track would push the
slider in by a character at each end for nothing. */
#seek-bar-container small,
.time-toggle {
font-variant-numeric: tabular-nums;
flex: 0 0 auto;
min-width: calc(var(--yj-clock-chars, 5) * 1ch);
}
#seek-bar-container small {
text-align: left;
}
.time-toggle {
background: none;
border: none;
@@ -76,6 +99,9 @@ export class SeekBar extends LitElement {
font: inherit;
font-size: var(--wa-font-size-s, 0.875rem);
cursor: pointer;
/* One more for the minus sign the remaining form carries. */
min-width: calc((var(--yj-clock-chars, 5) + 1) * 1ch);
text-align: right;
}
.time-toggle:hover,
@@ -219,8 +245,19 @@ export class SeekBar extends LitElement {
: formatSeconds(this.trackLength);
const rightTime = this.hasTrack ? rightLabel : '--:--';
// The widest string either clock can hold for *this* track. The
// duration is the longest elapsed value there can be, so its length
// is the budget; `--:--` is five, which is also the floor.
const clockChars = Math.max(
5,
this.hasTrack ? formatSeconds(this.trackLength).length : 0,
);
return html`
<div id="seek-bar-container">
<div
id="seek-bar-container"
style="--yj-clock-chars: ${clockChars}"
>
<small data-testid="elapsed-time">${elapsedTime}</small>
<wa-slider
label="Seek"
@@ -111,13 +111,32 @@ const gridStyles = css`
scale: 0.95;
}
/* Title and year on one line, and only the title truncates.
The year used to be part of the same run of text, so it was the
first thing an ellipsis ate: a card wide enough for a long album
name never showed its year, and browsing by year showed years
only for the albums with short names -- the sort said one thing
and the cards showed another.
A flex row rather than a second line, because the card's height
is what the virtualizer measures rows by. */
.album-name {
font-size: var(--album-name-font, 14px);
font-weight: 400;
color: var(--yj-text-primary, #fff);
display: flex;
justify-content: center;
align-items: baseline;
gap: 0.35em;
min-width: 0;
}
.album-title {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
min-width: 0;
}
.artist-name {
@@ -131,6 +150,8 @@ const gridStyles = css`
.album-year {
color: var(--yj-text-tertiary, #888);
flex: 0 0 auto;
white-space: nowrap;
}
/* ========================================
@@ -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(
@@ -1898,10 +1902,10 @@ export class CoverGrid
class="album-name"
title="${album.Name}"
>
${album.Name}${album.Year
? html`
<span class="album-year">
(${album.Year})</span
<span class="album-title">${album.Name}</span
>${album.Year
? html`<span class="album-year"
>(${album.Year})</span
>`
: nothing}
</div>
@@ -512,7 +512,9 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
${request.artist ? `${request.artist}` : ''}${request.title ||
request.mbid}
</div>
<div class="detail">${requestDetail(request, this.nowMs)}</div>
<div class="detail">
${requestDetail(request, this.nowMs, this.canDownload)}
</div>
</div>
<div class="actions">
${request.state === 'satisfied'
@@ -706,10 +708,20 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
* looked for rather than as an error, because that is what it is — the
* retry is already scheduled and there is nothing for the user to do.
*/
function requestDetail(request: Request, nowMs: number): string {
function requestDetail(
request: Request,
nowMs: number,
canDownload: boolean,
): string {
if (request.state === 'satisfied') return 'In your library';
if (request.state === 'paused') return 'Paused — not being looked for';
// With no client there is no search and no retry clock — the
// backend stopped scheduling one — so a row must not imply either.
// "Queued" and "next check in 6 hours" are both promises nothing is
// in a position to keep.
if (!canDownload) return 'On your list — no download client to search with';
if (request.attempts === 0) return 'Queued — not searched for yet';
const tries = `Searched ${request.attempts} time${request.attempts === 1 ? '' : 's'}`;
@@ -2,6 +2,7 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { srOnly } from '../../styles/sr-only.css';
import {
LookupReleaseGroup,
BrowseReleases,
@@ -289,6 +290,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
designTokens,
exploreLinkStyles,
contextMenuStyles,
srOnly,
css`
:host {
display: flex;
@@ -662,34 +664,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;
}
}
`,
];
@@ -720,14 +709,28 @@ 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 `requestUpdate` is what makes the *tracklist's* badges
// move. Both assignments below are reactive fields, so Lit
// repaints when either changes — but a track request changes
// neither: `canDownload` is about providers and `isRequested`
// is about this album's own release group. Each row's badge
// reads `libraryStatusFor(false, track.mbid)` at render time,
// which is a dependency on the store that Lit cannot see, so
// clicking one filed the request and left the plus exactly
// where it was. The other three hosts rendering these badges
// (`explore-artist-details`, `explore-view`, `top-results-row`)
// have always asked for the repaint here; this one did not.
this.downloadUnsub = downloadStore.subscribe(() => {
this.canDownload = downloadStore.available;
this.syncRequested();
this.requestUpdate();
});
void downloadStore.init().then(() => {
this.canDownload = downloadStore.available;
this.syncRequested();
this.requestUpdate();
});
void this.resolveTargetLibraryId();
@@ -3033,11 +3036,21 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
/* ── Tracklist ── */
/**
* The heading is there and is not drawn.
*
* A list of numbered titles with durations under an album's cover
* does not need a word above it saying what it is — it was the
* only thing on this page labelling something already obvious. But
* the section is a landmark and the page's heading structure runs
* through it, so what goes is the *ink*, not the element: a reader
* jumping by heading still finds the tracklist.
*/
private renderTracklist() {
if (this.loadingReleases) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="section-loading">Loading tracks\u2026</div>
</section>
`;
@@ -3050,7 +3063,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
if (!current) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="section-error">
<wa-icon name="triangle-exclamation"></wa-icon>
No release data available.
@@ -3063,7 +3076,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
if (tracks.length === 0) {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div
style="color: var(--yj-text-tertiary, #888); font-size: var(--yj-text-md)"
>
@@ -3079,7 +3092,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
return html`
<section>
<h3 class="section-header">Tracklist</h3>
<h3 class="sr-only">Tracklist</h3>
<div class="tracklist">
${discNumbers.map((discNum) => {
const discTracks = discMap.get(discNum) ?? [];
@@ -193,6 +193,10 @@ export class SmartPlaylistEditor extends LitElement {
// ── Internal state ──────────────────────────────────────────────
@state() private ruleRows: RuleRow[] = [emptyRule()];
/** Whether every rule must hold or any one of them. Mirrors the
* backend's `match`; 'all' is the default and the only thing a
* playlist saved before this existed can have meant. */
@state() private matchType: 'all' | 'any' = 'all';
@state() private limit = 0;
@state() private sortField = 'random';
@state() private sortDir = '';
@@ -224,6 +228,19 @@ export class SmartPlaylistEditor extends LitElement {
flex-shrink: 0;
}
.match-row {
display: flex;
align-items: center;
gap: 6px;
font-size: var(--yj-text-sm);
color: var(--yj-text-secondary, #b3b3b3);
flex-wrap: wrap;
}
.match-select {
min-width: 72px;
}
.rule-row {
display: grid;
grid-template-columns: 160px 140px 1fr 28px;
@@ -511,6 +528,10 @@ export class SmartPlaylistEditor extends LitElement {
);
this.ruleRows = rows.length > 0 ? rows : [emptyRule()];
// A playlist saved before this field existed has no match
// and means "all" — the backend reads an empty match the
// same way, so an upgrade cannot widen anyone's playlist.
this.matchType = parsed.match === 'any' ? 'any' : 'all';
this.limit = parsed.limit ?? 0;
this.sortField = parsed.sort_field || 'random';
this.sortDir = parsed.sort_dir ?? '';
@@ -551,6 +572,7 @@ export class SmartPlaylistEditor extends LitElement {
return JSON.stringify({
rules,
match: this.matchType,
limit: this.limit || 0,
sort_field: this.sortField || '',
sort_dir: this.sortDir || '',
@@ -630,6 +652,11 @@ export class SmartPlaylistEditor extends LitElement {
this.onRulesChanged();
}
private updateMatchType(value: string) {
this.matchType = value === 'any' ? 'any' : 'all';
this.onRulesChanged();
}
private updateSortField(value: string) {
this.sortField = value;
if (!value) this.sortDir = '';
@@ -731,6 +758,7 @@ export class SmartPlaylistEditor extends LitElement {
override render() {
return html`
<div class="rule-rows">
${this.renderMatchType()}
${this.ruleRows.map((row, index) =>
this.renderRuleRow(row, index),
)}
@@ -743,6 +771,46 @@ export class SmartPlaylistEditor extends LitElement {
`;
}
/**
* Whether every rule has to hold, or any one of them.
*
* It is a sentence with a control in the middle rather than a
* labelled field, because the two readings differ by one word and
* that word is the whole of the setting — "Match **all** of the
* following rules" says what the list below it means in a way a
* select labelled "Match" beside a list does not.
*
* Hidden while there is one rule: with nothing to combine, all and
* any are the same query, and a control whose two settings cannot
* differ is a question the user has no way to answer wrongly and
* no reason to answer at all.
*/
private renderMatchType() {
if (this.ruleRows.length < 2) return nothing;
return html`
<div class="match-row">
<span>Match</span>
<select
class="match-select"
aria-label="Match all or any of the following rules"
@change=${(e: Event) =>
this.updateMatchType(
(e.target as HTMLSelectElement).value,
)}
>
<option value="all" ?selected=${this.matchType === 'all'}>
all
</option>
<option value="any" ?selected=${this.matchType === 'any'}>
any
</option>
</select>
<span>of the following rules</span>
</div>
`;
}
private renderRuleRow(row: RuleRow, index: number) {
const isBetween = row.operator === 'between';
const operators = row.field ? getOperatorsForField(row.field) : [];
+7
View File
@@ -57,6 +57,12 @@ interface TracksModified {
index: number;
positions?: number[];
currentIndex: number;
/** The queue's source *after* the mutation. An append clears it
* backend-side — a queue built from one album is not that album
* once a track from elsewhere joins it — and this delta is the
* only event those paths emit, so the label would otherwise keep
* pointing at a collection the queue no longer holds. */
source?: QueueSource;
}
type Subscriber = () => void;
@@ -198,6 +204,7 @@ class QueueStore {
}
this.state.currentIndex = delta.currentIndex;
this.state.source = delta.source ?? EMPTY_QUEUE_SOURCE;
}
// ===================================================================
+53 -2
View File
@@ -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.
@@ -0,0 +1,87 @@
/**
* The year on an album card survives a long album name.
*
* The year used to be part of the same run of text as the title, inside
* one `text-overflow: ellipsis` box — so it was the first thing the
* ellipsis ate. A card wide enough for a long name never showed its
* year at all, which means sorting the grid *by year* showed years only
* for the albums with short names: the sort said one thing and the
* cards showed another.
*
* The fix is a flex row in which only the title truncates, rather than
* a second line, because the card's height is what the virtualizer
* measures rows by.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/cover-grid/cover-grid';
import { emit, stub, flush, resetHarness } from '@test/support/harness';
import { Events } from '../../src/events';
import { fixture, shadowAll } from '@test/support/render';
const LONG =
'The Rise and Fall of a Midwest Princess in the Key of Everything';
/** Long names throughout: the fault only shows on a card under
* pressure, and a grid of "Album 3" proves nothing. */
const ALBUMS = Array.from({ length: 12 }, (_, i) => ({
ID: i + 1,
Name: `${LONG} ${i + 1}`,
ArtistName: 'Aurora Fields',
Year: 2019 + (i % 5),
}));
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
function sized(el: HTMLElement): void {
el.style.display = 'block';
el.style.height = '600px';
el.style.width = '900px';
}
async function settle(el: LitElement): Promise<void> {
await flush();
await el.updateComplete;
await new Promise((r) => setTimeout(r, 80));
}
describe('the album cards year', () => {
beforeEach(() => {
resetHarness();
stub('library.Library.GetAlbums', ALBUMS);
stub('library.Library.GetTracks', []);
emit(Events.LibraryScanComplete);
});
it('is rendered on every card, however long the name', async () => {
const el = await fixture<LitElement>('cover-grid');
sized(el);
await settle(el);
const cards = shadowAll(el, '.album-card');
const years = shadowAll(el, '.album-year');
expect(cards.length).toBeGreaterThan(0);
expect(years).toHaveLength(cards.length);
expect(years.every((y) => /^\(\d{4}\)$/.test(y.textContent!.trim()))).toBe(
true,
);
});
it('is not what the ellipsis eats', async () => {
const el = await fixture<LitElement>('cover-grid');
sized(el);
await settle(el);
const year = shadowAll(el, '.album-year')[0]!;
const title = shadowAll(el, '.album-title')[0]!;
// The title is the box that gives way...
expect(title.scrollWidth).toBeGreaterThan(title.clientWidth);
// ...and the year keeps every pixel it asked for.
expect(year.clientWidth).toBeGreaterThan(0);
expect(year.scrollWidth).toBeLessThanOrEqual(year.clientWidth + 1);
});
});
@@ -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 tracklists 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);
});
});
@@ -0,0 +1,149 @@
/**
* Issue #33: "Want track" filed the request and left the badge alone.
*
* The tracklist's badges read `libraryStatusFor(false, track.mbid)` at
* render time, which is a dependency on `downloadStore` that Lit cannot
* see. `explore-album-details` did subscribe to that store, but its
* callback only assigned `canDownload` and `isRequested` — neither of
* which a *track* request changes — so nothing in the component's
* reactive state moved and the page never re-rendered. The request was
* real, the plus stayed a plus, and clicking again cancelled it.
*
* The other three hosts rendering these badges (`explore-artist-
* details`, `explore-view`, `top-results-row`) have always asked for
* the repaint in the same place, which is what made this one look
* correct on inspection.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-album-details/explore-album-details';
import type { Request } from '@store/download-store';
import { Events } from '../../src/events';
import { stub, flush, resetHarness, emit } from '@test/support/harness';
import { fixture, shadowAll } from '@test/support/render';
type RequestOverrides = Partial<Omit<Request, 'state' | 'entity'>> & {
state?: `${Request['state']}`;
entity?: `${Request['entity']}`;
};
function request(overrides: RequestOverrides): Request {
return {
id: 1,
mbid: 'mbid-1',
entity: 'recording',
libraryId: 1,
artist: 'An Artist',
title: 'Track 1',
scope: 'future',
secondary: false,
state: 'wanted',
attempts: 0,
...overrides,
} as Request;
}
/** Put a request list into the store the way the backend does. */
async function withRequests(rows: Request[]): Promise<void> {
stub('download.Service.ListRequests', rows);
emit(Events.RequestsChanged);
await flush();
}
function track(n: number) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `mbid-${n}`,
inLibrary: false,
};
}
/** An unowned catalog tracklist, which is the only case with badges:
* an owned row renders none, there being nothing left to ask for. */
async function withTracklist(count: number): Promise<LitElement> {
const el = await fixture<LitElement>('explore-album-details', {
albumName: 'Glass Harbour',
});
Object.assign(el, {
versionEntries: [
{
key: 'v1',
label: '2019',
sublabel: `${count} tracks`,
tracks: Array.from({ length: count }, (_, i) => track(i + 1)),
},
],
selectedVersionKey: 'v1',
loadingReleases: false,
loadingInfo: false,
});
el.requestUpdate();
await flush();
await el.updateComplete;
return el;
}
/** The status of each track badge, in tracklist order. */
function badgeStatuses(el: LitElement): string[] {
return shadowAll(el, 'library-status-indicator.track-request').map(
(b) => b.getAttribute('status') ?? '',
);
}
describe('the album tracklists request badges', () => {
beforeEach(async () => {
resetHarness();
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
stub('library.Library.GetFilePathsByAlbums', {});
stub('library.Library.GetAlbumTracks', []);
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
await withRequests([]);
});
it('starts as a plus on every unowned row', async () => {
const el = await withTracklist(3);
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'not-in-library',
'not-in-library',
]);
});
it('repaints the row whose track has been requested', async () => {
const el = await withTracklist(3);
await withRequests([request({ mbid: 'mbid-2' })]);
await el.updateComplete;
// Only the requested row moves. A request is by MBID, so the two
// rows either side of it are still a plus.
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'queued',
'not-in-library',
]);
});
it('repaints again when the request is cancelled', async () => {
const el = await withTracklist(3);
await withRequests([request({ mbid: 'mbid-2' })]);
await el.updateComplete;
await withRequests([]);
await el.updateComplete;
expect(badgeStatuses(el)).toEqual([
'not-in-library',
'not-in-library',
'not-in-library',
]);
});
});
@@ -0,0 +1,89 @@
/**
* The tracklist's own heading.
*
* A list of numbered titles with durations, under the album's cover, is
* the one thing on this page that did not need a word above it saying
* what it was — "TRACKLIST" labelled the only thing already obvious.
*
* What goes is the *ink*, not the element. The section is a landmark
* and the page's heading structure runs through it, so a reader moving
* by heading still has to be able to find it, and it is hidden the way
* `sr-only` hides things: `clip-path`, never `display: none`, which
* would take it out of the accessibility tree along with the layout.
*/
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) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `mbid-${n}`,
inLibrary: true,
};
}
async function albumPage(): Promise<LitElement> {
const el = await fixture<LitElement>('explore-album-details', {
albumName: 'Glass Harbour',
releaseGroupMBID: 'rg-1',
});
Object.assign(el, {
versionEntries: [
{
key: 'v1',
label: '2019',
sublabel: '2 tracks',
tracks: [track(1), track(2)],
},
],
selectedVersionKey: 'v1',
loadingReleases: false,
loadingInfo: false,
});
el.requestUpdate();
await flush();
await el.updateComplete;
return el;
}
const tracklistHeading = (el: LitElement) =>
shadowAll(el, 'h3').find((h) => h.textContent?.trim() === 'Tracklist');
describe('the album tracklist heading', () => {
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 still in the tree', async () => {
expect(tracklistHeading(await albumPage())).toBeTruthy();
});
it('takes up no room on the page', async () => {
const heading = tracklistHeading(await albumPage())!;
const box = heading.getBoundingClientRect();
expect(box.width).toBeLessThanOrEqual(1);
expect(box.height).toBeLessThanOrEqual(1);
// Hidden by clipping, not by removal: display:none and
// visibility:hidden both take it out of the accessibility tree.
expect(getComputedStyle(heading).display).not.toBe('none');
expect(getComputedStyle(heading).visibility).not.toBe('hidden');
});
});
@@ -0,0 +1,90 @@
/**
* What a wanted list says when there is nothing to search with.
*
* Wanting something without a download client is a supported thing to
* do — the list is kept, and it starts moving when a client is added.
* What was not supported was the app *claiming to be looking*: every
* pass attempted each request, failed it with "no download clients are
* enabled", recorded that as an attempt and scheduled a retry, so a row
* read "Searched 3 times, no download clients are enabled · next check
* in 6 hours" about a check that could not happen.
*
* The backend half is `TestNoProvidersMeansNoAttempt`. This is the row.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/downloads-view/downloads-view';
import { stub, emit, flush, resetHarness } from '@test/support/harness';
import { Events } from '../../src/events';
import { fixture, shadowAll } from '@test/support/render';
/** A request that has been tried and is waiting on a retry — the shape
* a list with a client in it produces. */
const WAITING = {
id: 1,
mbid: 'rg-1',
entity: 'release-group',
libraryId: 1,
artist: 'Aurora Fields',
title: 'Glass Harbour',
state: 'wanted',
attempts: 3,
lastError: 'no source has it yet',
nextTryAt: new Date(Date.now() + 6 * 3600_000).toISOString(),
};
const PROVIDER = {
id: 1,
kind: 'slskd',
name: 'Sound',
enabled: true,
priority: 50,
};
/**
* The download store is a singleton whose `init()` runs once per
* session, so a second mount does not re-read the provider list. The
* event is how the app itself learns a client was added, and is what
* makes this test independent of which case ran first.
*/
async function view(providers: unknown[]): Promise<LitElement> {
stub('download.Service.ListProviders', providers);
const el = await fixture<LitElement>('downloads-view');
emit(Events.DownloadProvidersChanged);
await flush();
await el.updateComplete;
return el;
}
const details = (el: LitElement) =>
shadowAll(el, '.detail').map((d) => d.textContent!.trim());
describe('a request row with no download client', () => {
beforeEach(() => {
resetHarness();
stub('download.Service.ProviderKinds', []);
stub('download.Service.ListProviders', []);
stub('download.Service.ListDownloads', []);
stub('download.Service.ListRequests', [WAITING]);
});
it('does not promise a check that cannot happen', async () => {
const el = await view([]);
expect(details(el)).toHaveLength(1);
expect(details(el)[0]).toBe(
'On your list — no download client to search with',
);
expect(details(el)[0]).not.toMatch(/next check/);
});
it('reports the retry schedule again once a client exists', async () => {
const el = await view([PROVIDER]);
expect(details(el)[0]).toMatch(/next check/);
});
});
@@ -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);
});
});
@@ -227,6 +227,41 @@ describe('<seek-bar>', () => {
expect(text(el, '[data-testid="remaining-time"]')).toBe('01:30');
});
it('keeps the slider still as the clocks count', async () => {
const el = await fixture('seek-bar');
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 31 });
await flush();
await el.updateComplete;
const slider = () =>
shadow(el, 'wa-slider')!.getBoundingClientRect();
const before = slider();
// 1:11 against 4:08 is the reported jitter: different digits, and
// in a proportional font different widths. Toggling the right-hand
// clock is the other half -- the minus sign is a whole character.
for (const positionSeconds of [8, 71, 88]) {
emit(Events.PlaybackPositionChanged, {
positionSeconds,
trackLength: 90,
trackChangeId: 31,
seq: positionSeconds,
playing: true,
});
await flush();
await el.updateComplete;
expect(slider().width).toBeCloseTo(before.width, 1);
expect(slider().left).toBeCloseTo(before.left, 1);
}
await click(el, '[data-testid="remaining-time"]');
await el.updateComplete;
expect(slider().width).toBeCloseTo(before.width, 1);
});
it('renders the position the backend reports rather than its own count', async () => {
const el = await fixture('seek-bar');
+55
View File
@@ -198,6 +198,61 @@ describe('queue store: move', () => {
});
});
/**
* "Playing from X" is a claim that everything queued came from X, and
* appending a track from anywhere else makes it false. The backend
* clears the source on every add/insert path — but the delta is the
* only event those paths emit, so the label corrects itself here or
* not at all.
*/
describe('queue store: the source travels on the delta', () => {
function syncWithAlbum(): void {
emit(Events.QueueChanged, {
tracks: [track(1), track(2)],
currentIndex: 0,
shuffleMode: false,
repeatMode: 'off',
source: { type: 'album', id: 7, label: 'Abbey Road' },
});
}
beforeEach(() => {
syncWithAlbum();
});
it('drops the label when an append clears it backend-side', () => {
emit(Events.QueueTracksModified, {
action: 'add',
tracks: [track(3)],
index: 2,
currentIndex: 0,
source: { type: '', id: 0, label: '' },
});
expect(queueStore.getState().source).toEqual({
type: '',
id: 0,
label: '',
});
});
it('keeps a label the backend still reports, as on a removal', () => {
emit(Events.QueueTracksModified, {
action: 'remove',
positions: [1],
index: 0,
currentIndex: 0,
source: { type: 'album', id: 7, label: 'Abbey Road' },
});
expect(queueStore.getState().source).toEqual({
type: 'album',
id: 7,
label: 'Abbey Road',
});
});
});
describe('queue store: mode deltas', () => {
beforeEach(() => {
sync([track(1)], 0);