Fix/small issue batch #82

Closed
yonlu wants to merge 0 commits from fix/small-issue-batch into main
Owner

Fixed a few issues:

Fixed a few issues:
yonlu added 4 commits 2026-08-18 15:22:40 +00:00
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>
`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>
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>
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
185eb1b125
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>
yonlu added 1 commit 2026-08-18 15:22:59 +00:00
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
63ec068add
Collaborator

Cross-referencing this against the backlog, since two of these four had branches open already.

Overlap — two of the four:

Commit Issue Also in
1062b7c0 tell Lit that a track request changed something #33 #74
b3556d82 always send an art URL, even when there is no art #41 #80
bf4f3521 stop claiming a queue came from somewhere it no longer does #14
185eb1b1 let a rule set match any rule, not only all of them #35

What I have done about it, so nothing needs unpicking here:

  • #80 is closed as a duplicate. b3556d82 is the same fix and a superset (same metadataMap extraction, plus map[string]any and a third test). Nothing to port.
  • #74 is rebased down to the half this PR does not cover. Your repaint fix is the better one — it also covers the init() path, which mine did not — so that commit is gone from my branch. What is left is the other half of #33: the badge was opacity: 0 until :hover, and the report asks for it to be there unconditionally. Its test file is renamed (we had both independently written frontend/test/components/album-track-request.test.ts), and it now merges clean with this branch in either order. Verified by test-merge, along with the other six open branches.

Two small asks:

  1. Add Closes #33, Closes #41, Closes #14, Closes #35 to the description — none of the four will close on merge as it stands. (#33 wants #74 as well before it is fully done.)
  2. #14 and #35 are now assigned to you and labelled Reviewed/Confirmed; #14 and #35 were both in the roadmap (#73) as unstarted, so that is the only bookkeeping they needed.

One note on bf4f3521, worth a second pair of eyes rather than a change: q.source is also read by resolveFallback as PreviousSource, and the dynamic-mix continuation is keyed on PreviousSource.Type == "dynamicMix". Whatever now clears or rewrites the source on append should be checked against a queue that ran out while a dynamic mix was extending it, or the mix can stop treating itself as continuing and re-resolve fresh mid-run.

Cross-referencing this against the backlog, since two of these four had branches open already. **Overlap — two of the four:** | Commit | Issue | Also in | |---|---|---| | `1062b7c0` tell Lit that a track request changed something | #33 | #74 | | `b3556d82` always send an art URL, even when there is no art | #41 | #80 | | `bf4f3521` stop claiming a queue came from somewhere it no longer does | #14 | — | | `185eb1b1` let a rule set match any rule, not only all of them | #35 | — | **What I have done about it, so nothing needs unpicking here:** - **#80 is closed** as a duplicate. `b3556d82` is the same fix and a superset (same `metadataMap` extraction, plus `map[string]any` and a third test). Nothing to port. - **#74 is rebased down to the half this PR does not cover.** Your repaint fix is the better one — it also covers the `init()` path, which mine did not — so that commit is gone from my branch. What is left is the *other* half of #33: the badge was `opacity: 0` until `:hover`, and the report asks for it to be there unconditionally. Its test file is renamed (we had both independently written `frontend/test/components/album-track-request.test.ts`), and it now **merges clean with this branch in either order**. Verified by test-merge, along with the other six open branches. **Two small asks:** 1. Add `Closes #33`, `Closes #41`, `Closes #14`, `Closes #35` to the description — none of the four will close on merge as it stands. (#33 wants #74 as well before it is fully done.) 2. #14 and #35 are now assigned to you and labelled `Reviewed/Confirmed`; #14 and #35 were both in the roadmap (#73) as unstarted, so that is the only bookkeeping they needed. **One note on `bf4f3521`, worth a second pair of eyes rather than a change:** `q.source` is also read by `resolveFallback` as `PreviousSource`, and the dynamic-mix continuation is keyed on `PreviousSource.Type == "dynamicMix"`. Whatever now clears or rewrites the source on append should be checked against a queue that ran out *while* a dynamic mix was extending it, or the mix can stop treating itself as continuing and re-resolve fresh mid-run.
Collaborator

Rolled into #83 (with your four commits merged first, so the branch history and authorship are preserved) and closing here for the same reason the others are: release.yml cuts a release on every push to main, so landing these separately would produce several versions for one batch of small fixes.

#83 carries all four of your commits plus my seven, states Closes for all ten issues, and has been through the whole gate on the combined tree — make lint and make test in all three build configurations, 836 UI tests, 114 e2e specs against a seeded headless app.

Nothing of yours was changed or dropped. My #80 was the duplicate and was closed in your favour; my #74 was rebased down to the CSS half of #33 that your commit does not cover, so the two sit side by side.

The one thing worth a look after this lands is the note above about PreviousSource.Type == \"dynamicMix\" and bf4f3521.

Rolled into #83 (with your four commits merged first, so the branch history and authorship are preserved) and closing here for the same reason the others are: `release.yml` cuts a release on **every** push to `main`, so landing these separately would produce several versions for one batch of small fixes. #83 carries all four of your commits plus my seven, states `Closes` for all ten issues, and has been through the whole gate on the combined tree — `make lint` and `make test` in all three build configurations, 836 UI tests, 114 e2e specs against a seeded headless app. Nothing of yours was changed or dropped. My #80 was the duplicate and was closed in your favour; my #74 was rebased down to the CSS half of #33 that your commit does not cover, so the two sit side by side. The one thing worth a look after this lands is the note above about `PreviousSource.Type == \"dynamicMix\"` and `bf4f3521`.
logan closed this pull request 2026-08-18 15:44:33 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.