Compare commits
28
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
5fae61cdf1 | ||
|
|
1f43234b80 | ||
|
|
ca00f8a803 | ||
|
|
0be7b4fdc8 | ||
|
|
18f10e966b | ||
|
|
e897364a73 | ||
|
|
5fa68fcf43 | ||
|
|
b1368bbc7e | ||
|
|
9432f68c8b | ||
|
|
4c921ed1ba | ||
|
|
f8800ca1f8 | ||
|
|
dddc8aaf55 | ||
|
|
f6e9df2f68 | ||
|
|
47f65dad89 | ||
|
|
f5dae71050 | ||
|
|
b0bda625e0 | ||
|
|
19ba5f0394 | ||
|
|
439a6cd77b | ||
|
|
a5515d1d9f | ||
|
|
7b90633456 | ||
|
|
7838f45ed4 | ||
|
|
2453d717cf | ||
|
|
49445ded77 | ||
|
|
b9e60bdb0a | ||
|
|
bcf3856b6f | ||
|
|
d225f922fb | ||
|
|
dfb338fc37 | ||
|
|
26251badda |
@@ -42,11 +42,16 @@ strings and identical specs produce different bytes on different builds.
|
|||||||
playback and then clicks pause races the track ending and fails
|
playback and then clicks pause races the track ending and fails
|
||||||
against a correct UI. Use `LONG_TRACK` (90 s, `edge-lengths`) exported
|
against a correct UI. Use `LONG_TRACK` (90 s, `edge-lengths`) exported
|
||||||
from `e2e/support/fixtures.ts`.
|
from `e2e/support/fixtures.ts`.
|
||||||
- **WAV tracks scan in untitled.** `backend/tagwriter` writes WAV tags
|
- **WAV tracks scan like every other format.** #104 added
|
||||||
into a RIFF `id3 ` chunk and `dhowden/tag` has no RIFF parser, so
|
`backend/riff`, so the scan reads the `id3 ` chunk `backend/tagwriter`
|
||||||
there is no "Field Recordings" artist in the Artists view. This is a
|
writes and both WAVs come in fully tagged: "Field Recordings" is an
|
||||||
known open bug pinned by `TestWAVTagsAreNotReadableYet`; do not
|
ordinary artist in the Artists view, with a "Test Tones" album and a
|
||||||
"fix" a spec by asserting the broken behaviour elsewhere.
|
cover. They are therefore not an example of an untitled or albumless
|
||||||
|
track — the only two tracks with no album are
|
||||||
|
`unsorted/no-tags-at-all.mp3` and `unsorted/title-only.mp3`. Prose
|
||||||
|
written before #104 says the opposite and names
|
||||||
|
`TestWAVTagsAreNotReadableYet`, a test that change deleted; that is
|
||||||
|
dated history rather than a description of the app.
|
||||||
|
|
||||||
## Seeds
|
## Seeds
|
||||||
|
|
||||||
|
|||||||
@@ -81,6 +81,24 @@ rules. Never "go fix it" — the leg contract is in this file.
|
|||||||
| escalation | `yj-loop.escalate` | go/kimi-k3 (T3) | same leg re-run, seeded with failure summary |
|
| escalation | `yj-loop.escalate` | go/kimi-k3 (T3) | same leg re-run, seeded with failure summary |
|
||||||
| prose (PR body, commit msgs, journal) | `yj-loop.scribe` | go/mimo-v2.5 (T0) | text only, from supplied facts |
|
| prose (PR body, commit msgs, journal) | `yj-loop.scribe` | go/mimo-v2.5 (T0) | text only, from supplied facts |
|
||||||
|
|
||||||
|
**Model fallback on quota exhaustion.** The pinned models are the
|
||||||
|
intent, not a guarantee. The qwen token plan is a weekly pool and has
|
||||||
|
run dry mid-tick (`429 … 1-week quota exhausted`). When a leg's launch
|
||||||
|
fails with a 429, re-run it with a per-run `model` override one rung
|
||||||
|
down and journal the substitution — never spend the T3 escalation
|
||||||
|
model on a quota substitution. The qwen-pinned legs (`work`,
|
||||||
|
`diffreview`) fall back `qwen/deepseek-v4-pro-0813` → `go/deepseek-v4-pro`
|
||||||
|
→ `go/glm-5.3-flash`. Do **not** use the `deepseek/...` provider: it has
|
||||||
|
no models, only catalog overrides, and fails silently (empty artifact,
|
||||||
|
no session) — the model lives on the `go` gateway.
|
||||||
|
|
||||||
|
**Launch legs in the foreground.** The async subagent runner has died
|
||||||
|
without persisting a child session (nothing to resume) and emits
|
||||||
|
spurious "needs attention" nudges on runs that are already complete.
|
||||||
|
Foreground `subagent` calls are the reliable mode here. A worker that
|
||||||
|
dies mid-leg leaves uncommitted work: inspect the tree, then relaunch
|
||||||
|
to *complete* — never to re-implement.
|
||||||
|
|
||||||
Orchestrator-only legs: **claim** (`issue.sh claim --branch` — atomic,
|
Orchestrator-only legs: **claim** (`issue.sh claim --branch` — atomic,
|
||||||
refuses if held), **ship's PR/CI polling** (REST API below — `gitea_ci`
|
refuses if held), **ship's PR/CI polling** (REST API below — `gitea_ci`
|
||||||
job_logs 404s on this Gitea; the REST endpoints are the way), **merge**
|
job_logs 404s on this Gitea; the REST endpoints are the way), **merge**
|
||||||
@@ -163,7 +181,26 @@ Merge when, and only when, **all** hold:
|
|||||||
- the protection contexts `CI / check` and `CI / e2e` are green on the
|
- the protection contexts `CI / check` and `CI / e2e` are green on the
|
||||||
PR's head, read from the API, not from the PR page's badge;
|
PR's head, read from the API, not from the PR page's badge;
|
||||||
- the PR reports mergeable;
|
- the PR reports mergeable;
|
||||||
- the critique leg ran and no open blocker stands.
|
- the critique leg ran and no open blocker stands;
|
||||||
|
- the branch is **not behind `origin/main`** — the protection's
|
||||||
|
`block_on_outdated_branch: true` refuses it anyway; never
|
||||||
|
`force_manually_merged` around it.
|
||||||
|
|
||||||
|
**Refresh before every merge.** In the loop worktree: `git fetch origin`
|
||||||
|
in the same breath, then `git merge origin/main` on the PR branch,
|
||||||
|
push. The fetch must be immediate — a cached `origin/main` merges
|
||||||
|
against the wrong base, CI goes green on it, and the merge comes back
|
||||||
|
405 "behind base", one whole CI cycle wasted (measured on the adoption
|
||||||
|
wave). A textual conflict
|
||||||
|
stops the leg there — as diff text, not as a failed merge click: hunks
|
||||||
|
the loop authored are resolved by the loop; anything else is left with
|
||||||
|
`⟦loop⟧` comment for a human, never forced. After any refresh push,
|
||||||
|
re-poll the PR's own required contexts on the **new head** before
|
||||||
|
merging.
|
||||||
|
|
||||||
|
**Merges happen one at a time**, each re-reading state — the previous
|
||||||
|
merge moved `main`, and the next PR's mergeability is recomputed at
|
||||||
|
its own turn.
|
||||||
|
|
||||||
```
|
```
|
||||||
curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
|
curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
|
||||||
@@ -172,12 +209,20 @@ curl -sS -X POST -H "Authorization: token $GITEA_TOKEN" \
|
|||||||
-d '{"Do":"merge","merge_message_field":"default","force_manually_merged":false}'
|
-d '{"Do":"merge","merge_message_field":"default","force_manually_merged":false}'
|
||||||
```
|
```
|
||||||
|
|
||||||
Afterwards: `scripts/issue.sh list --state open` and check the footer
|
**Afterwards watch the `push` run on `main`** — the CI the merge
|
||||||
took. Close stragglers with `issue.sh close`, naming the merge commit.
|
started. A red main after a loop merge is a **halt**: comment what is
|
||||||
`unclaim.yml` handles the label; it is not instant; reopening does not
|
known on the offending PR, mark the state file, stop taking new issues.
|
||||||
restore it. Merging fans out to nothing (releases are the manual
|
That run is the only thing between a clean textual merge of
|
||||||
`release.yml`, which the loop never runs) — the criticism stands before
|
independently-written PRs and a self-contradicting main; no
|
||||||
the merge because nothing stands after it.
|
mergeability check sees it. Only a green main lets the tick proceed (to
|
||||||
|
footer verification, below).
|
||||||
|
|
||||||
|
Footer verification: `scripts/issue.sh list --state open` and check
|
||||||
|
the footer took. Close stragglers with `issue.sh close`, naming the
|
||||||
|
merge commit. `unclaim.yml` handles the label; it is not instant;
|
||||||
|
reopening does not restore it. Merging fans out to nothing (releases
|
||||||
|
are the manual `release.yml`, which the loop never runs) — the
|
||||||
|
criticism stands before the merge because nothing stands after it.
|
||||||
|
|
||||||
## Rails — the loop's absolute rules
|
## Rails — the loop's absolute rules
|
||||||
|
|
||||||
@@ -224,7 +269,10 @@ AVD), then `make android-emulator` per session.
|
|||||||
|
|
||||||
- **Worktree:** `git worktree add ~/.paseo/worktrees/loop/jumpy-hound
|
- **Worktree:** `git worktree add ~/.paseo/worktrees/loop/jumpy-hound
|
||||||
origin/main` (from any clone; branch from origin/main in the loop
|
origin/main` (from any clone; branch from origin/main in the loop
|
||||||
tree, never `git checkout main`).
|
tree, never `git checkout main`). **Provision it once before the
|
||||||
|
first push:** `make build-frontend` and `make testdata` — the pre-push
|
||||||
|
`go-test` hook needs `frontend/dist` (the `//go:embed` in `main.go`)
|
||||||
|
and the fixture library, and refuses the push without them.
|
||||||
- **Session:** pi in that worktree, `/name loop`. Add the job via
|
- **Session:** pi in that worktree, `/name loop`. Add the job via
|
||||||
`/schedule-prompt` (name `yj-loop`, cron
|
`/schedule-prompt` (name `yj-loop`, cron
|
||||||
`0 0 10-18 * * 1-5`, prompt: "Read `.pi/skills/yj-loop/SKILL.md` and
|
`0 0 10-18 * * 1-5`, prompt: "Read `.pi/skills/yj-loop/SKILL.md` and
|
||||||
@@ -247,4 +295,12 @@ AVD), then `make android-emulator` per session.
|
|||||||
which leg it really is.
|
which leg it really is.
|
||||||
- The job did not fire — the scheduler fires only while a session is
|
- The job did not fire — the scheduler fires only while a session is
|
||||||
open in its directory (documented); "the loop is off" is the correct
|
open in its directory (documented); "the loop is off" is the correct
|
||||||
reading, not a bug.
|
reading, not a bug.
|
||||||
|
- `error: object file … is empty` / `unpack-objects failed` / `bad
|
||||||
|
object refs/heads/…` during a fetch or checkout — the shared object
|
||||||
|
store was corrupted (a killed fetch leaves 0-byte object files, and a
|
||||||
|
local ref can end up pointing at the dead sha1). **Halt and report**;
|
||||||
|
do not retry, the churn only deepens it. Human repair: delete the
|
||||||
|
0-byte objects, `git fetch origin --prune`, delete any ref that
|
||||||
|
still dangles (`git update-ref -d refs/heads/<b>`), re-checkout the
|
||||||
|
worktree at `origin/main`, then `git fsck --full`.
|
||||||
@@ -112,7 +112,9 @@ in the job's directory — that limitation is the switch:
|
|||||||
|
|
||||||
- **Worktree:** `git worktree add` a dedicated clone at
|
- **Worktree:** `git worktree add` a dedicated clone at
|
||||||
`~/.paseo/worktrees/loop/jumpy-hound`. Loop edits happen only there; a
|
`~/.paseo/worktrees/loop/jumpy-hound`. Loop edits happen only there; a
|
||||||
dirty tree there is the loop's business and nobody else's.
|
dirty tree there is the loop's business and nobody else's. **Provision
|
||||||
|
it once before its first push:** `make build-frontend` + `make testdata`
|
||||||
|
— the pre-push `go-test` hook needs both and refuses without them.
|
||||||
- **Session:** pi in that worktree, `/name loop`. The job is bound to that
|
- **Session:** pi in that worktree, `/name loop`. The job is bound to that
|
||||||
session, so another pi elsewhere in the same directory does not
|
session, so another pi elsewhere in the same directory does not
|
||||||
double-fire it.
|
double-fire it.
|
||||||
@@ -140,6 +142,20 @@ from a concurrent session is caught before the first edit.
|
|||||||
|
|
||||||
- **Only PRs the loop opened.** A collaborator's PR is never merged, never
|
- **Only PRs the loop opened.** A collaborator's PR is never merged, never
|
||||||
commented on for pressure, never touched.
|
commented on for pressure, never touched.
|
||||||
|
- **Every branch is refreshed against main before its merge**, in the
|
||||||
|
loop worktree — the refresh is where a textual conflict surfaces, as
|
||||||
|
diff text: hunks the loop authored are resolved there, anything else
|
||||||
|
is left to a human with a `⟦loop⟧` comment. The protection's
|
||||||
|
`block_on_outdated_branch` makes the refresh mandatory for adopted
|
||||||
|
(pre-loop) branches: behind `main`, a PR cannot merge at all.
|
||||||
|
Required contexts are re-polled on the refreshed head.
|
||||||
|
- **Merges are one at a time**, each re-reading state — the previous
|
||||||
|
merge moved `main`, and the next PR's mergeability is recomputed at
|
||||||
|
its own turn.
|
||||||
|
- **Post-merge, the `push` run on `main` is watched.** A red main after
|
||||||
|
a loop merge halts the loop. That run is the only guard against the
|
||||||
|
class no mergeability check sees: two PRs touching the same file,
|
||||||
|
merging cleanly, contradicting each other.
|
||||||
- The gate is the protection rule itself, read from the API: contexts
|
- The gate is the protection rule itself, read from the API: contexts
|
||||||
`CI / check*` and `CI / e2e*` green, PR mergeable. (Required approvals
|
`CI / check*` and `CI / e2e*` green, PR mergeable. (Required approvals
|
||||||
is 0 today; if a second person changes protection rules, the merge
|
is 0 today; if a second person changes protection rules, the merge
|
||||||
|
|||||||
@@ -172,7 +172,7 @@ make ui-test # Vitest component/store suite in a real browser (no app)
|
|||||||
make ui-visual # Same, including toMatchScreenshot comparisons
|
make ui-visual # Same, including toMatchScreenshot comparisons
|
||||||
make ui-setup # Install the Vitest provider's own Chromium (once)
|
make ui-setup # Install the Vitest provider's own Chromium (once)
|
||||||
make bindings-check # Fail if frontend/bindings is stale vs the Go bindings
|
make bindings-check # Fail if frontend/bindings is stale vs the Go bindings
|
||||||
make skill-check # Fail if .pi/ documents a make target that doesn't exist
|
make skill-check # Fail if a doc names a make target that doesn't exist
|
||||||
make commit-check # Fail if a commit subject is not a Conventional Commit
|
make commit-check # Fail if a commit subject is not a Conventional Commit
|
||||||
make lint # golangci-lint v2 (strict), all three build configurations
|
make lint # golangci-lint v2 (strict), all three build configurations
|
||||||
make test # All tests with race detector, all three build configurations
|
make test # All tests with race detector, all three build configurations
|
||||||
@@ -673,6 +673,28 @@ rather than renaming them.
|
|||||||
one of the shell's rows, which is what the skip link is absolutely
|
one of the shell's rows, which is what the skip link is absolutely
|
||||||
positioned to avoid.
|
positioned to avoid.
|
||||||
- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments.
|
- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments.
|
||||||
|
|
||||||
|
**A setter that can reject its argument puts the old value back**, and
|
||||||
|
that is a correctness rule rather than hygiene (#231). `Save()`
|
||||||
|
validates the *whole* config, so a value left behind by a failed write
|
||||||
|
does not merely fail its own call: it fails every later save, of every
|
||||||
|
unrelated setting — theme, launch page, shortcuts, libraries — for the
|
||||||
|
rest of the session. Nothing reaches disk, so a restart clears it,
|
||||||
|
which is exactly what makes the fault invisible and unreportable. One
|
||||||
|
rejected track-list column list was enough to stop the app saving
|
||||||
|
anything at all.
|
||||||
|
|
||||||
|
Two shapes are safe and a third is the trap. A setter that assigns and
|
||||||
|
*then* validates snapshots the field first and restores it on the
|
||||||
|
error path — seven do. `SetLibraryDirectory` is the better shape where
|
||||||
|
the value can be built on its own: it validates a candidate *before*
|
||||||
|
assigning, so there is nothing to undo. And a setter whose argument no
|
||||||
|
validation inspects needs neither — the bools, the favourites playlist
|
||||||
|
id and the shortcut bindings, plus `SetViewVisible`, which refuses an
|
||||||
|
unknown, non-hideable or launch-page view up front so
|
||||||
|
`GeneralConfig.Validate` never sees one it would fail on. Which set a
|
||||||
|
new setter joins is decided by whether its own `Validate` can reject
|
||||||
|
it, not by preference.
|
||||||
- `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists.
|
- `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists.
|
||||||
- `mediacontrols` — OS media controls behind one `Handler`: MPRIS over
|
- `mediacontrols` — OS media controls behind one `Handler`: MPRIS over
|
||||||
D-Bus on desktop Linux, a MediaSession on Android, a no-op stub
|
D-Bus on desktop Linux, a MediaSession on Android, a no-op stub
|
||||||
@@ -3286,6 +3308,27 @@ its own duplicates apart) — and changing either is invisible against an
|
|||||||
existing `YJ_HOME`, whose `config.toml` already holds the old list, so
|
existing `YJ_HOME`, whose `config.toml` already holds the old list, so
|
||||||
`make sandbox-seed NAME=default` before believing the app.
|
`make sandbox-seed NAME=default` before believing the app.
|
||||||
|
|
||||||
|
**And the *valid* columns are declared twice too, which is the pair
|
||||||
|
that drifted.** `tracklist.AllColumnIDs` is what the backend accepts;
|
||||||
|
`COLUMN_DEFS` is what the frontend knows how to draw, and they are not
|
||||||
|
the same set — `titleArtist` is a definition and not a choice, since it
|
||||||
|
is the phone's stacked column and is picked by width in
|
||||||
|
`PHONE_COLUMN_IDS`. Settings built its list from `Object.keys(
|
||||||
|
COLUMN_DEFS)` and so offered it: **two rows both called "Track Name"**
|
||||||
|
(#197), the second unselectable, because ticking it sends a column set
|
||||||
|
Go rejects with `unknown track-list column ID` and `config-page`
|
||||||
|
swallows that into a `console.error`. `CONFIGURABLE_COLUMN_IDS` is what
|
||||||
|
the configurator reads now, derived from a `configurable` flag on the
|
||||||
|
definition, and `settings-column-list.test.ts` reads Go's own list out
|
||||||
|
of the source rather than writing it down a third time — the rule being
|
||||||
|
about every column, so checking one checks nothing.
|
||||||
|
|
||||||
|
One thing it does **not** fix, because it is reachable from any invalid
|
||||||
|
input rather than from that row: `SetTrackListColumns` assigns before it
|
||||||
|
validates, so a rejected list stays in memory and `Save()` validates the
|
||||||
|
whole config — one tick and **no setting saves for the rest of the
|
||||||
|
session**, silently. That is #231.
|
||||||
|
|
||||||
**Event-driven communication**: Backend emits events via Wails runtime; frontend stores subscribe to them. Event names are constants in `backend/events/`.
|
**Event-driven communication**: Backend emits events via Wails runtime; frontend stores subscribe to them. Event names are constants in `backend/events/`.
|
||||||
|
|
||||||
`frontend/src/events.ts` is **generated** from `backend/events/events.go`
|
`frontend/src/events.ts` is **generated** from `backend/events/events.go`
|
||||||
|
|||||||
@@ -192,7 +192,7 @@ css-check: ## Fail on a css`` literal ended early by a backtick, or a nested rul
|
|||||||
# Every command in them is a make target on purpose, so this is
|
# Every command in them is a make target on purpose, so this is
|
||||||
# checkable. It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
|
# checkable. It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
|
||||||
# two harnesses cannot drift onto two descriptions of one project.
|
# two harnesses cannot drift onto two descriptions of one project.
|
||||||
skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md is not a symlink
|
skill-check: ## Fail if the docs name a missing make target, or AGENTS.md is not a symlink
|
||||||
@./scripts/skill-check.sh
|
@./scripts/skill-check.sh
|
||||||
|
|
||||||
# Conventional Commits, which CLAUDE.md claimed CI enforced for a long
|
# Conventional Commits, which CLAUDE.md claimed CI enforced for a long
|
||||||
|
|||||||
@@ -303,6 +303,24 @@ func (c *Config) GetLibraryDirectory() string {
|
|||||||
return string(c.Library.DirectoryPath)
|
return string(c.Library.DirectoryPath)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A rejected setter puts the old value back, and that is not tidiness
|
||||||
|
// (#231). Save validates the *whole* config, so a value left behind by
|
||||||
|
// a failed write does not merely fail its own call: it fails every
|
||||||
|
// later save, of every unrelated setting, silently and for the rest of
|
||||||
|
// the session. Nothing reaches disk, so a restart clears it -- which
|
||||||
|
// is exactly what makes the fault hard to see and impossible to report.
|
||||||
|
//
|
||||||
|
// The setters below that assign and then validate therefore snapshot
|
||||||
|
// the field first and restore it on the error path. SetLibraryDirectory
|
||||||
|
// is the other safe shape and the better one where the value can be
|
||||||
|
// built on its own: it validates a candidate *before* assigning
|
||||||
|
// anything, so there is nothing to undo.
|
||||||
|
//
|
||||||
|
// Not every setter needs either. A bool, an int64 and the shortcut
|
||||||
|
// bindings pass through no validation that can reject them, and
|
||||||
|
// SetViewVisible refuses an unknown, non-hideable or launch-page view
|
||||||
|
// up front, so GeneralConfig.Validate never sees one it would fail on.
|
||||||
|
|
||||||
// SetLibraryDirectory validates and saves a new library directory,
|
// SetLibraryDirectory validates and saves a new library directory,
|
||||||
// then emits the LibraryConfigChanged event so listeners (e.g. the
|
// then emits the LibraryConfigChanged event so listeners (e.g. the
|
||||||
// Library scanner) can react.
|
// Library scanner) can react.
|
||||||
@@ -360,11 +378,14 @@ func (c *Config) SetScanConcurrency(mode string) error {
|
|||||||
c.Library.ApplyDefaults()
|
c.Library.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.Library.ScanConcurrency
|
||||||
c.Library.ScanConcurrency = library.ScanConcurrency(
|
c.Library.ScanConcurrency = library.ScanConcurrency(
|
||||||
mode,
|
mode,
|
||||||
)
|
)
|
||||||
|
|
||||||
if err := c.Library.Validate(); err != nil {
|
if err := c.Library.Validate(); err != nil {
|
||||||
|
c.Library.ScanConcurrency = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid scan concurrency mode: %w", err,
|
"invalid scan concurrency mode: %w", err,
|
||||||
)
|
)
|
||||||
@@ -455,9 +476,12 @@ func (c *Config) SetThemeAccentColor(
|
|||||||
c.Theme.ApplyDefaults()
|
c.Theme.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.Theme.AccentColor
|
||||||
c.Theme.AccentColor = color
|
c.Theme.AccentColor = color
|
||||||
|
|
||||||
if err := c.Theme.Validate(); err != nil {
|
if err := c.Theme.Validate(); err != nil {
|
||||||
|
c.Theme.AccentColor = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid theme accent color: %w", err,
|
"invalid theme accent color: %w", err,
|
||||||
)
|
)
|
||||||
@@ -488,9 +512,12 @@ func (c *Config) SetThemeBackgroundShade(
|
|||||||
c.Theme.ApplyDefaults()
|
c.Theme.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.Theme.BackgroundShade
|
||||||
c.Theme.BackgroundShade = theme.BackgroundShade(shade)
|
c.Theme.BackgroundShade = theme.BackgroundShade(shade)
|
||||||
|
|
||||||
if err := c.Theme.Validate(); err != nil {
|
if err := c.Theme.Validate(); err != nil {
|
||||||
|
c.Theme.BackgroundShade = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid theme background shade: %w", err,
|
"invalid theme background shade: %w", err,
|
||||||
)
|
)
|
||||||
@@ -544,9 +571,12 @@ func (c *Config) SetDefaultPage(page string) error {
|
|||||||
c.General.ApplyDefaults()
|
c.General.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.General.DefaultPage
|
||||||
c.General.DefaultPage = View(page)
|
c.General.DefaultPage = View(page)
|
||||||
|
|
||||||
if err := c.General.Validate(); err != nil {
|
if err := c.General.Validate(); err != nil {
|
||||||
|
c.General.DefaultPage = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid default page: %w", err,
|
"invalid default page: %w", err,
|
||||||
)
|
)
|
||||||
@@ -591,9 +621,12 @@ func (c *Config) SetQueueFallback(mode string) error {
|
|||||||
c.General.ApplyDefaults()
|
c.General.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.General.QueueFallback
|
||||||
c.General.QueueFallback = QueueFallback(mode)
|
c.General.QueueFallback = QueueFallback(mode)
|
||||||
|
|
||||||
if err := c.General.Validate(); err != nil {
|
if err := c.General.Validate(); err != nil {
|
||||||
|
c.General.QueueFallback = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid queue fallback: %w", err,
|
"invalid queue fallback: %w", err,
|
||||||
)
|
)
|
||||||
@@ -801,9 +834,12 @@ func (c *Config) SetTrackListColumns(
|
|||||||
c.TrackList = &tracklist.Config{}
|
c.TrackList = &tracklist.Config{}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.TrackList.Columns
|
||||||
c.TrackList.Columns = columns
|
c.TrackList.Columns = columns
|
||||||
|
|
||||||
if err := c.TrackList.Validate(); err != nil {
|
if err := c.TrackList.Validate(); err != nil {
|
||||||
|
c.TrackList.Columns = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid track-list columns: %w", err,
|
"invalid track-list columns: %w", err,
|
||||||
)
|
)
|
||||||
@@ -901,9 +937,12 @@ func (c *Config) SetFavoritesIconStyle(
|
|||||||
c.Favorites.ApplyDefaults()
|
c.Favorites.ApplyDefaults()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
previous := c.Favorites.IconStyle
|
||||||
c.Favorites.IconStyle = favorites.IconStyle(style)
|
c.Favorites.IconStyle = favorites.IconStyle(style)
|
||||||
|
|
||||||
if err := c.Favorites.Validate(); err != nil {
|
if err := c.Favorites.Validate(); err != nil {
|
||||||
|
c.Favorites.IconStyle = previous
|
||||||
|
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"invalid favorites icon style: %w", err,
|
"invalid favorites icon style: %w", err,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,262 @@
|
|||||||
|
package config
|
||||||
|
|
||||||
|
import (
|
||||||
|
"log/slog"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/library"
|
||||||
|
"yellowjacket/backend/tracklist"
|
||||||
|
)
|
||||||
|
|
||||||
|
// newSavableConfig builds a loaded, valid config in a temp directory,
|
||||||
|
// so Save() writes rather than refusing with errSaveBeforeLoad.
|
||||||
|
//
|
||||||
|
// The library directory is real and set, because Config.Validate only
|
||||||
|
// validates the Library section when DirectoryPath is non-empty -- an
|
||||||
|
// empty one would hide a poisoned ScanConcurrency from the whole-config
|
||||||
|
// save that is the symptom under test.
|
||||||
|
func newSavableConfig(t *testing.T) *Config {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
c := &Config{
|
||||||
|
logger: slog.Default(),
|
||||||
|
filePath: filepath.Join(t.TempDir(), "config.toml"),
|
||||||
|
Library: &library.Config{
|
||||||
|
DirectoryPath: library.Directory(t.TempDir()),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
c.applyDefaults()
|
||||||
|
|
||||||
|
if err := c.Load(); err != nil {
|
||||||
|
t.Fatalf("Load() error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.Save(); err != nil {
|
||||||
|
t.Fatalf("Save() on a fresh config error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSetterRejectionDoesNotPoisonTheConfig is the whole of #231.
|
||||||
|
//
|
||||||
|
// Every setter here assigns to the in-memory config and then validates.
|
||||||
|
// When the validation rejects the argument, the rejected value has to go
|
||||||
|
// back -- not because the caller sees it (it gets an error either way),
|
||||||
|
// but because Config.Save() validates the *whole* config. A value left
|
||||||
|
// behind by a failed setter therefore fails every later save, of every
|
||||||
|
// unrelated setting, silently and for the rest of the session.
|
||||||
|
//
|
||||||
|
// So each case asserts three things in order: the setter reports the
|
||||||
|
// error, the getter still reports the old value, and an unrelated save
|
||||||
|
// still works. The third is the one the user feels.
|
||||||
|
func TestSetterRejectionDoesNotPoisonTheConfig(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
// reject calls the setter with an argument its own Validate
|
||||||
|
// refuses.
|
||||||
|
reject func(*Config) error
|
||||||
|
// read reports the value the setter writes, so the rollback is
|
||||||
|
// asserted on the config rather than only on the save.
|
||||||
|
read func(*Config) string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "scan concurrency",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetScanConcurrency("telepathy")
|
||||||
|
},
|
||||||
|
read: (*Config).GetScanConcurrency,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "theme accent colour",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetThemeAccentColor("not-a-hex")
|
||||||
|
},
|
||||||
|
read: (*Config).GetThemeAccentColor,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "theme background shade",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetThemeBackgroundShade("chartreuse")
|
||||||
|
},
|
||||||
|
read: (*Config).GetThemeBackgroundShade,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "default page",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetDefaultPage("nowhere")
|
||||||
|
},
|
||||||
|
read: (*Config).GetDefaultPage,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "queue fallback",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetQueueFallback("improvise")
|
||||||
|
},
|
||||||
|
read: (*Config).GetQueueFallback,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "favorites icon style",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
return c.SetFavoritesIconStyle("asterisk")
|
||||||
|
},
|
||||||
|
read: (*Config).GetFavoritesIconStyle,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "track-list columns",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
// titleArtist is a drawing definition, not a
|
||||||
|
// configurable column (#197), so it is exactly what
|
||||||
|
// the frontend used to be able to send.
|
||||||
|
return c.SetTrackListColumns([]tracklist.Column{
|
||||||
|
{ID: "titleArtist"},
|
||||||
|
})
|
||||||
|
},
|
||||||
|
read: func(c *Config) string {
|
||||||
|
return columnIDs(c.GetTrackListColumns())
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "track-list columns, duplicated",
|
||||||
|
reject: func(c *Config) error {
|
||||||
|
// The route #197 closed was one invalid id; a
|
||||||
|
// duplicate is the one still reachable from a client
|
||||||
|
// that assembles the list itself.
|
||||||
|
return c.SetTrackListColumns([]tracklist.Column{
|
||||||
|
{ID: tracklist.ColTrackName},
|
||||||
|
{ID: tracklist.ColTrackName},
|
||||||
|
})
|
||||||
|
},
|
||||||
|
read: func(c *Config) string {
|
||||||
|
return columnIDs(c.GetTrackListColumns())
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := newSavableConfig(t)
|
||||||
|
before := tc.read(c)
|
||||||
|
|
||||||
|
if err := tc.reject(c); err == nil {
|
||||||
|
t.Fatal("setter accepted an invalid value, want an error")
|
||||||
|
}
|
||||||
|
|
||||||
|
if after := tc.read(c); after != before {
|
||||||
|
t.Errorf(
|
||||||
|
"value after a rejected write = %q, want the previous %q",
|
||||||
|
after, before,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The symptom: an unrelated setting can no longer be saved.
|
||||||
|
if err := c.SetPopupVolume(true); err != nil {
|
||||||
|
t.Errorf("an unrelated setter failed after a rejected write: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.Save(); err != nil {
|
||||||
|
t.Errorf("Save() failed after a rejected write: %v", err)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRejectedSetterLeavesNothingOnDisk pairs with the sweep above: the
|
||||||
|
// rollback must not be undone by what the file already holds, so a
|
||||||
|
// config reloaded from disk after a rejected write agrees with memory.
|
||||||
|
func TestRejectedSetterLeavesNothingOnDisk(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := newSavableConfig(t)
|
||||||
|
|
||||||
|
if err := c.SetThemeAccentColor("#123456"); err != nil {
|
||||||
|
t.Fatalf("SetThemeAccentColor() error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.SetThemeAccentColor("not-a-hex"); err == nil {
|
||||||
|
t.Fatal("SetThemeAccentColor accepted a non-colour, want an error")
|
||||||
|
}
|
||||||
|
|
||||||
|
reloaded := &Config{logger: slog.Default(), filePath: c.filePath}
|
||||||
|
reloaded.applyDefaults()
|
||||||
|
|
||||||
|
if err := reloaded.Load(); err != nil {
|
||||||
|
t.Fatalf("Load() error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := reloaded.GetThemeAccentColor(); got != "#123456" {
|
||||||
|
t.Errorf("accent colour on disk = %q, want %q", got, "#123456")
|
||||||
|
}
|
||||||
|
|
||||||
|
if c.GetThemeAccentColor() != reloaded.GetThemeAccentColor() {
|
||||||
|
t.Errorf(
|
||||||
|
"in-memory accent %q disagrees with disk %q after a rejected write",
|
||||||
|
c.GetThemeAccentColor(), reloaded.GetThemeAccentColor(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSetLibraryDirectoryValidatesBeforeAssigning pins the precedent the
|
||||||
|
// seven rolled-back setters follow: this one has always built and
|
||||||
|
// validated a candidate before assigning, so a bad path never reaches
|
||||||
|
// the config at all.
|
||||||
|
func TestSetLibraryDirectoryValidatesBeforeAssigning(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := newSavableConfig(t)
|
||||||
|
before := c.GetLibraryDirectory()
|
||||||
|
|
||||||
|
if err := c.SetLibraryDirectory(filepath.Join(t.TempDir(), "no-such-dir")); err == nil {
|
||||||
|
t.Fatal("SetLibraryDirectory accepted a missing directory, want an error")
|
||||||
|
}
|
||||||
|
|
||||||
|
if after := c.GetLibraryDirectory(); after != before {
|
||||||
|
t.Errorf("library directory = %q, want the previous %q", after, before)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.Save(); err != nil {
|
||||||
|
t.Errorf("Save() failed after a rejected library directory: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSetViewVisibleRefusesBeforeAssigning covers the other setter left
|
||||||
|
// out of the rollback pass: it guards its own argument up front, so
|
||||||
|
// GeneralConfig.Validate never sees a view it would reject.
|
||||||
|
func TestSetViewVisibleRefusesBeforeAssigning(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := newSavableConfig(t)
|
||||||
|
|
||||||
|
if err := c.SetViewVisible("no-such-view", false); err == nil {
|
||||||
|
t.Fatal("SetViewVisible accepted an unknown view, want an error")
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.SetViewVisible(c.GetDefaultPage(), false); err == nil {
|
||||||
|
t.Fatal("SetViewVisible hid the launch page, want an error")
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := c.Save(); err != nil {
|
||||||
|
t.Errorf("Save() failed after a refused view visibility change: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// columnIDs renders a column list for comparison in the table above.
|
||||||
|
func columnIDs(cols []tracklist.Column) string {
|
||||||
|
ids := make([]byte, 0, len(cols)*8)
|
||||||
|
|
||||||
|
for i, col := range cols {
|
||||||
|
if i > 0 {
|
||||||
|
ids = append(ids, ',')
|
||||||
|
}
|
||||||
|
|
||||||
|
ids = append(ids, col.ID...)
|
||||||
|
}
|
||||||
|
|
||||||
|
return string(ids)
|
||||||
|
}
|
||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"path/filepath"
|
"path/filepath"
|
||||||
"runtime"
|
"runtime"
|
||||||
"strings"
|
"strings"
|
||||||
|
"syscall"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -17,6 +18,21 @@ import (
|
|||||||
|
|
||||||
// stubYtDlp writes an executable script that echoes the given stdout
|
// stubYtDlp writes an executable script that echoes the given stdout
|
||||||
// and returns it as a provider config binary path.
|
// and returns it as a provider config binary path.
|
||||||
|
//
|
||||||
|
// The write is held under syscall.ForkLock, and that is not tidiness:
|
||||||
|
// the kernel refuses to exec a file that is open for writing anywhere
|
||||||
|
// in the process, and these tests are parallel, so a *sibling* test's
|
||||||
|
// fork can duplicate this descriptor in the moment it is open and
|
||||||
|
// carry it past our close — the exec a moment later then fails with
|
||||||
|
// ETXTBSY, "text file busy". That is #146, seen once in CI and once
|
||||||
|
// locally, on trees containing no Go at all. Closing sooner is not
|
||||||
|
// available (os.WriteFile has already closed the file before anything
|
||||||
|
// execs it) and O_CLOEXEC does not help, because the window is between
|
||||||
|
// another goroutine's fork and its own exec. ForkLock is the lock
|
||||||
|
// syscall.forkExec takes across that fork, so holding it here means no
|
||||||
|
// child can exist while the descriptor does. Measured on this helper
|
||||||
|
// under 12 concurrent writers: 176-189 of 2400 execs refused without
|
||||||
|
// it, 0 of 2400 with it.
|
||||||
func stubYtDlp(t *testing.T, script string) string {
|
func stubYtDlp(t *testing.T, script string) string {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
@@ -26,9 +42,11 @@ func stubYtDlp(t *testing.T, script string) string {
|
|||||||
|
|
||||||
path := filepath.Join(t.TempDir(), "yt-dlp")
|
path := filepath.Join(t.TempDir(), "yt-dlp")
|
||||||
|
|
||||||
if err := os.WriteFile(
|
syscall.ForkLock.Lock()
|
||||||
path, []byte("#!/bin/sh\n"+script), 0o700,
|
err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700)
|
||||||
); err != nil {
|
syscall.ForkLock.Unlock()
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
t.Fatalf("write stub: %v", err)
|
t.Fatalf("write stub: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -42,10 +42,10 @@ const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
|
|||||||
* because the number this issue is about (a button 48px wider than the
|
* because the number this issue is about (a button 48px wider than the
|
||||||
* box holding it) is not in the accessibility tree at all.
|
* box holding it) is not in the accessibility tree at all.
|
||||||
*/
|
*/
|
||||||
const headerFit = (page: import('@playwright/test').Page) =>
|
const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
|
||||||
page.evaluate(() => {
|
page.evaluate((tag) => {
|
||||||
const root = document
|
const root = document
|
||||||
.querySelector('[data-testid="main-content"] playlist-view')
|
.querySelector(`[data-testid="main-content"] ${tag}`)
|
||||||
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
|
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
|
||||||
|
|
||||||
if (!root) return null;
|
if (!root) return null;
|
||||||
@@ -76,7 +76,7 @@ const headerFit = (page: import('@playwright/test').Page) =>
|
|||||||
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
|
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
|
||||||
].map((i) => i.textContent?.trim() ?? ''),
|
].map((i) => i.textContent?.trim() ?? ''),
|
||||||
};
|
};
|
||||||
});
|
}, view);
|
||||||
|
|
||||||
test.describe('the page header never clips an action', () => {
|
test.describe('the page header never clips an action', () => {
|
||||||
test.beforeEach(async ({ app }) => {
|
test.beforeEach(async ({ app }) => {
|
||||||
@@ -316,3 +316,58 @@ test.describe('the page header never clips an action', () => {
|
|||||||
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
|
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The Tracks header carries the play-all/shuffle-all pair (#31), so
|
||||||
|
* the promise above has to hold for it too — the same per-button
|
||||||
|
* measurement, one view over. Its two actions are the whole of the
|
||||||
|
* header's declared set, and the pair is what plays the list the row
|
||||||
|
* is in, so a button rendered 20px of its 90px is a queue of nothing.
|
||||||
|
*/
|
||||||
|
const TRACK_ACTIONS = ['Play all', 'Shuffle all'];
|
||||||
|
|
||||||
|
test.describe('the Tracks header never clips an action', () => {
|
||||||
|
test.beforeEach(async ({ app }) => {
|
||||||
|
await app.getByTestId('nav-tracks').click();
|
||||||
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
|
'data-active-view',
|
||||||
|
'tracks',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test.afterEach(async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: 1280, height: 800 });
|
||||||
|
});
|
||||||
|
|
||||||
|
for (const vp of VIEWPORTS) {
|
||||||
|
test(`every action is reachable at ${vp.name}`, async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: vp.width, height: vp.height });
|
||||||
|
|
||||||
|
await expect
|
||||||
|
.poll(async () => (await headerFit(app, 'track-list'))?.clipped)
|
||||||
|
.toEqual([]);
|
||||||
|
|
||||||
|
const fit = (await headerFit(app, 'track-list'))!;
|
||||||
|
|
||||||
|
expect(fit.overflow).toBeLessThanOrEqual(0);
|
||||||
|
|
||||||
|
// Between them, buttons and menu account for both actions —
|
||||||
|
// not "it fits" but "nothing was dropped to make it fit".
|
||||||
|
expect([...fit.buttons, ...fit.menu].sort()).toEqual(
|
||||||
|
[...TRACK_ACTIONS].sort(),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The pair's names, through the accessibility tree — a shadow query
|
||||||
|
* measures, but it cannot say what a screen reader is offered.
|
||||||
|
*/
|
||||||
|
test('both actions are named controls', async ({ app }) => {
|
||||||
|
for (const label of TRACK_ACTIONS) {
|
||||||
|
await expect(
|
||||||
|
app.getByRole('button', { name: label, exact: true }),
|
||||||
|
).toBeVisible();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -0,0 +1,220 @@
|
|||||||
|
import { test, expect, callBinding, resetEvents, waitForEvent } from '../support/fixtures.js';
|
||||||
|
|
||||||
|
type Page = import('@playwright/test').Page;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Play-all/Shuffle-all, asserted on what the backend queued rather than
|
||||||
|
* on playback pixels.
|
||||||
|
*
|
||||||
|
* `SetQueue` reports the queue through `QueueChanged`, and `GetState`
|
||||||
|
* says exactly what it holds: the tracks in order, whether shuffle is
|
||||||
|
* on, and the `Source` the "Playing from" link is built from. That is
|
||||||
|
* the honest contract here — the buttons are only as good as the queue
|
||||||
|
* they build, and the queue is only as good as the source it names.
|
||||||
|
*/
|
||||||
|
|
||||||
|
interface QueueState {
|
||||||
|
tracks: { filePath: string; title: string }[];
|
||||||
|
currentIndex: number;
|
||||||
|
shuffleMode: boolean;
|
||||||
|
source: { type: string; id: number; label: string };
|
||||||
|
}
|
||||||
|
|
||||||
|
const TRACKS_SOURCE = { type: 'tracks', id: 0, label: 'All Tracks' };
|
||||||
|
|
||||||
|
const getQueue = (app: Page) =>
|
||||||
|
callBinding<QueueState>(app, 'queue.Queue.GetState');
|
||||||
|
|
||||||
|
/** The track paths a rendered track list shows, in row order. */
|
||||||
|
function displayedPaths(app: Page, scope: string): Promise<string[]> {
|
||||||
|
return app
|
||||||
|
.locator(`${scope} [data-testid="track-row"]`)
|
||||||
|
.evaluateAll((els) =>
|
||||||
|
els.map((el) => el.getAttribute('data-file-path') ?? ''),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Leave shuffle in a known state. The mode persists across specs in
|
||||||
|
* one backend process, so a test that asserts on it has to set it. */
|
||||||
|
async function setShuffleMode(app: Page, on: boolean): Promise<void> {
|
||||||
|
const state = await getQueue(app);
|
||||||
|
|
||||||
|
if (state.shuffleMode !== on) {
|
||||||
|
await resetEvents(app);
|
||||||
|
await callBinding(app, 'queue.Queue.ToggleShuffle');
|
||||||
|
await waitForEvent(app, 'QueueModeChanged');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test.describe('play-all/shuffle-all on the track list', () => {
|
||||||
|
test.beforeEach(async ({ app }) => {
|
||||||
|
await callBinding(app, 'queue.Queue.Clear').catch(() => {
|
||||||
|
/* the queue is clearable on every build these specs run against */
|
||||||
|
});
|
||||||
|
await setShuffleMode(app, false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('Tracks Play all queues the displayed list with an honest source', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await app.getByTestId('nav-tracks').click();
|
||||||
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
|
'data-active-view',
|
||||||
|
'tracks',
|
||||||
|
);
|
||||||
|
await expect(
|
||||||
|
app.locator('track-list [data-testid="track-row"]').first(),
|
||||||
|
).toBeVisible();
|
||||||
|
|
||||||
|
const paths = await displayedPaths(app, 'track-list');
|
||||||
|
|
||||||
|
await resetEvents(app);
|
||||||
|
await app.getByTestId('page-action-play-all').click();
|
||||||
|
await waitForEvent(app, 'QueueChanged');
|
||||||
|
|
||||||
|
const state = await getQueue(app);
|
||||||
|
|
||||||
|
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||||
|
expect(state.currentIndex).toBe(0);
|
||||||
|
expect(state.shuffleMode).toBe(false);
|
||||||
|
expect(state.source).toEqual(TRACKS_SOURCE);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('Tracks Shuffle all turns shuffle on and keeps the source', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await app.getByTestId('nav-tracks').click();
|
||||||
|
await expect(
|
||||||
|
app.locator('track-list [data-testid="track-row"]').first(),
|
||||||
|
).toBeVisible();
|
||||||
|
|
||||||
|
const paths = await displayedPaths(app, 'track-list');
|
||||||
|
|
||||||
|
await resetEvents(app);
|
||||||
|
await app.getByTestId('page-action-shuffle-all').click();
|
||||||
|
await waitForEvent(app, 'QueueChanged');
|
||||||
|
|
||||||
|
const state = await getQueue(app);
|
||||||
|
|
||||||
|
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||||
|
expect(state.shuffleMode).toBe(true);
|
||||||
|
expect(state.source).toEqual(TRACKS_SOURCE);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test.describe('play-all on an embedded track list', () => {
|
||||||
|
test.beforeEach(async ({ app }) => {
|
||||||
|
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
|
||||||
|
await setShuffleMode(app, false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a genre page queues the genre with its name as the source', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await app.getByTestId('nav-genres').click();
|
||||||
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
|
'data-active-view',
|
||||||
|
'genres',
|
||||||
|
);
|
||||||
|
|
||||||
|
const first = app.locator('genres-view .genre-card').first();
|
||||||
|
|
||||||
|
await expect(first).toBeVisible();
|
||||||
|
await first.click();
|
||||||
|
|
||||||
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
|
'data-active-view',
|
||||||
|
'genre-details',
|
||||||
|
);
|
||||||
|
await expect(
|
||||||
|
app.locator('genre-details [data-testid="track-row"]').first(),
|
||||||
|
).toBeVisible();
|
||||||
|
|
||||||
|
const genreName = (await app
|
||||||
|
.locator('genre-details .genre-title')
|
||||||
|
.textContent())?.trim();
|
||||||
|
const paths = await displayedPaths(app, 'genre-details');
|
||||||
|
|
||||||
|
await resetEvents(app);
|
||||||
|
await app
|
||||||
|
.locator('genre-details [data-testid="page-action-play-all"]')
|
||||||
|
.click();
|
||||||
|
await waitForEvent(app, 'QueueChanged');
|
||||||
|
|
||||||
|
const state = await getQueue(app);
|
||||||
|
|
||||||
|
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||||
|
expect(state.currentIndex).toBe(0);
|
||||||
|
expect(state.source).toEqual({ type: 'genre', id: 0, label: genreName });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test.describe('play-all on the library artist page', () => {
|
||||||
|
test.beforeEach(async ({ app }) => {
|
||||||
|
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
|
||||||
|
await setShuffleMode(app, false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an artist page queues album paths in album order with the artist source', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
const artists = await callBinding<{ ID: number; Name: string }[]>(
|
||||||
|
app,
|
||||||
|
'library.Library.GetArtists',
|
||||||
|
[0],
|
||||||
|
);
|
||||||
|
const first = artists[0]!;
|
||||||
|
|
||||||
|
await app.evaluate(
|
||||||
|
([id, name]) => {
|
||||||
|
document.dispatchEvent(
|
||||||
|
new CustomEvent('navigate', {
|
||||||
|
detail: {
|
||||||
|
view: 'artist-details',
|
||||||
|
artistId: id,
|
||||||
|
artistName: name,
|
||||||
|
},
|
||||||
|
bubbles: true,
|
||||||
|
composed: true,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
[first.ID, first.Name] as const,
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
|
'data-active-view',
|
||||||
|
'artist-details',
|
||||||
|
);
|
||||||
|
await expect(app.getByTestId('artist-play-all')).toBeEnabled();
|
||||||
|
|
||||||
|
const albums = await callBinding<{ ID: number }[]>(
|
||||||
|
app,
|
||||||
|
'library.Library.GetAlbumsByArtist',
|
||||||
|
[first.Name, 0],
|
||||||
|
);
|
||||||
|
const byAlbum = await callBinding<Record<string, string[]>>(
|
||||||
|
app,
|
||||||
|
'library.Library.GetFilePathsByAlbums',
|
||||||
|
[albums.map((a) => a.ID), 0],
|
||||||
|
);
|
||||||
|
const expected: string[] = [];
|
||||||
|
|
||||||
|
for (const album of albums) {
|
||||||
|
expected.push(...(byAlbum[String(album.ID)] ?? []));
|
||||||
|
}
|
||||||
|
|
||||||
|
await resetEvents(app);
|
||||||
|
await app.getByTestId('artist-play-all').click();
|
||||||
|
await waitForEvent(app, 'QueueChanged');
|
||||||
|
|
||||||
|
const state = await getQueue(app);
|
||||||
|
|
||||||
|
expect(state.tracks.map((t) => t.filePath)).toEqual(expected);
|
||||||
|
expect(state.source).toEqual({
|
||||||
|
type: 'artist',
|
||||||
|
id: first.ID,
|
||||||
|
label: first.Name,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -137,7 +137,7 @@ test.describe('queue', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('shuffle and repeat toggles report their state', async ({ app }) => {
|
test('shuffle and repeat toggles report their state', async ({ app }) => {
|
||||||
const shuffle = app.getByRole('button', { name: 'Shuffle' });
|
const shuffle = app.getByRole('button', { name: 'Shuffle', exact: true });
|
||||||
|
|
||||||
await resetEvents(app);
|
await resetEvents(app);
|
||||||
await shuffle.click();
|
await shuffle.click();
|
||||||
|
|||||||
@@ -11,11 +11,22 @@ import {
|
|||||||
GetArtistImageCachedPath,
|
GetArtistImageCachedPath,
|
||||||
GetArtistMBID,
|
GetArtistMBID,
|
||||||
} from '@go/explore/service.js';
|
} from '@go/explore/service.js';
|
||||||
|
import { GetFilePathsByAlbums } from '@go/library/library.js';
|
||||||
|
import { libraryStore } from '@store/library-store';
|
||||||
|
import { notificationStore } from '@store/notification-store';
|
||||||
|
import { dict } from '@utils/binding';
|
||||||
|
import { playAll } from '@utils/play-all';
|
||||||
|
import { describeError } from '@utils/describe-error';
|
||||||
|
import { ICON_PLAY, ICON_SHUFFLE } from '@utils/icon-language';
|
||||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||||
import '@components/cover-grid/cover-grid.js';
|
import '@components/cover-grid/cover-grid.js';
|
||||||
|
import '../notifications/inline-notice';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import { backButton } from '../../styles/back-button.css';
|
import { backButton } from '../../styles/back-button.css';
|
||||||
|
|
||||||
|
/** The region the artist header's own failures are rendered in. */
|
||||||
|
const ArtistRegion = 'library-artist';
|
||||||
|
|
||||||
@customElement('artist-details')
|
@customElement('artist-details')
|
||||||
export class ArtistDetails extends LitElement {
|
export class ArtistDetails extends LitElement {
|
||||||
@property({ type: Number, attribute: 'artist-id' })
|
@property({ type: Number, attribute: 'artist-id' })
|
||||||
@@ -131,6 +142,39 @@ export class ArtistDetails extends LitElement {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.header-actions {
|
||||||
|
margin-left: auto;
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 8px;
|
||||||
|
flex-shrink: 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
.header-action {
|
||||||
|
background: none;
|
||||||
|
border: 1px solid var(--yj-border-subtle, #555);
|
||||||
|
border-radius: 4px;
|
||||||
|
color: var(--yj-text-primary, #fff);
|
||||||
|
padding: 6px 12px;
|
||||||
|
font-size: var(--yj-text-md, 13px);
|
||||||
|
font-family: inherit;
|
||||||
|
cursor: pointer;
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 6px;
|
||||||
|
white-space: nowrap;
|
||||||
|
}
|
||||||
|
|
||||||
|
.header-action:hover {
|
||||||
|
border-color: var(--yj-accent, #ffd43b);
|
||||||
|
color: var(--yj-accent-text, #ffd43b);
|
||||||
|
}
|
||||||
|
|
||||||
|
.header-action:disabled {
|
||||||
|
opacity: 0.5;
|
||||||
|
cursor: default;
|
||||||
|
}
|
||||||
|
|
||||||
/* ====================================
|
/* ====================================
|
||||||
* Content
|
* Content
|
||||||
* ==================================== */
|
* ==================================== */
|
||||||
@@ -145,6 +189,23 @@ export class ArtistDetails extends LitElement {
|
|||||||
height: 100%;
|
height: 100%;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Phone widths: the header's flex row squeezed .artist-info to
|
||||||
|
* nothing, so the title ellipsised away entirely and the
|
||||||
|
* actions clipped against the host's own overflow — the album
|
||||||
|
* page's fault one detail view over (#66). The pair takes its
|
||||||
|
* own row instead. Written last, because a media query adds no
|
||||||
|
* specificity and a rule placed above the plain ones it
|
||||||
|
* overrides is silently dead. */
|
||||||
|
@media (max-width: 599px) {
|
||||||
|
.artist-header {
|
||||||
|
flex-wrap: wrap;
|
||||||
|
}
|
||||||
|
|
||||||
|
.header-actions {
|
||||||
|
flex-basis: 100%;
|
||||||
|
margin-left: 0;
|
||||||
|
}
|
||||||
|
}
|
||||||
`];
|
`];
|
||||||
|
|
||||||
override connectedCallback() {
|
override connectedCallback() {
|
||||||
@@ -302,6 +363,45 @@ export class ArtistDetails extends LitElement {
|
|||||||
return name.charAt(0).toUpperCase();
|
return name.charAt(0).toUpperCase();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Play every track on this artist's albums, in album order.
|
||||||
|
*
|
||||||
|
* One `GetFilePathsByAlbums` call returns the paths grouped by
|
||||||
|
* album id; the caller owns the ordering, so they are flattened in
|
||||||
|
* `this.albums` order rather than by id.
|
||||||
|
*/
|
||||||
|
private async playAllTracks(shuffle: boolean): Promise<void> {
|
||||||
|
if (this.albums.length === 0) return;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const libId = libraryStore.getSelectedLibraryId() ?? 0;
|
||||||
|
const ids = this.albums.map((a) => a.ID);
|
||||||
|
const byAlbum = await dict(
|
||||||
|
GetFilePathsByAlbums(ids, libId),
|
||||||
|
);
|
||||||
|
const paths: string[] = [];
|
||||||
|
|
||||||
|
for (const id of ids) {
|
||||||
|
paths.push(...(byAlbum[id] ?? []));
|
||||||
|
}
|
||||||
|
|
||||||
|
playAll(
|
||||||
|
paths,
|
||||||
|
{
|
||||||
|
type: 'artist',
|
||||||
|
id: this.artistId,
|
||||||
|
label: this.artistName,
|
||||||
|
},
|
||||||
|
shuffle,
|
||||||
|
);
|
||||||
|
} catch (error) {
|
||||||
|
console.error('Could not play artist:', error);
|
||||||
|
notificationStore.inline(ArtistRegion, {
|
||||||
|
text: describeError(error, 'Could not play this artist’s tracks.'),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/* ================================================================
|
/* ================================================================
|
||||||
* Rendering
|
* Rendering
|
||||||
* ================================================================ */
|
* ================================================================ */
|
||||||
@@ -351,12 +451,38 @@ export class ArtistDetails extends LitElement {
|
|||||||
`
|
`
|
||||||
: ''}
|
: ''}
|
||||||
</div>
|
</div>
|
||||||
|
<div class="header-actions">
|
||||||
|
<button
|
||||||
|
class="header-action"
|
||||||
|
data-testid="artist-play-all"
|
||||||
|
?disabled=${this.albums.length === 0}
|
||||||
|
@click=${() =>
|
||||||
|
void this.playAllTracks(false)}
|
||||||
|
>
|
||||||
|
<wa-icon name=${ICON_PLAY}></wa-icon>
|
||||||
|
Play all
|
||||||
|
</button>
|
||||||
|
<button
|
||||||
|
class="header-action"
|
||||||
|
data-testid="artist-shuffle-all"
|
||||||
|
?disabled=${this.albums.length === 0}
|
||||||
|
@click=${() =>
|
||||||
|
void this.playAllTracks(true)}
|
||||||
|
>
|
||||||
|
<wa-icon name=${ICON_SHUFFLE}></wa-icon>
|
||||||
|
Shuffle all
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="content">
|
<div class="content">
|
||||||
<cover-grid
|
<cover-grid
|
||||||
.externalAlbums=${this.albums}
|
.externalAlbums=${this.albums}
|
||||||
></cover-grid>
|
></cover-grid>
|
||||||
</div>
|
</div>
|
||||||
|
<inline-notice
|
||||||
|
region=${ArtistRegion}
|
||||||
|
testid="artist-play-message"
|
||||||
|
></inline-notice>
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -51,7 +51,7 @@ import type { BackgroundShade } from '@store/theme-store';
|
|||||||
import type { IconStyle } from '@store/favorites-store';
|
import type { IconStyle } from '@store/favorites-store';
|
||||||
import {
|
import {
|
||||||
COLUMN_DEFS,
|
COLUMN_DEFS,
|
||||||
ALL_COLUMN_IDS,
|
CONFIGURABLE_COLUMN_IDS,
|
||||||
} from '@components/track-list/columns';
|
} from '@components/track-list/columns';
|
||||||
|
|
||||||
import './config-field';
|
import './config-field';
|
||||||
@@ -1640,7 +1640,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
|||||||
...this.trackListCtrl.columnIds,
|
...this.trackListCtrl.columnIds,
|
||||||
];
|
];
|
||||||
|
|
||||||
const disabledIds = ALL_COLUMN_IDS.filter(
|
const disabledIds = CONFIGURABLE_COLUMN_IDS.filter(
|
||||||
(id) => !enabledIds.includes(id),
|
(id) => !enabledIds.includes(id),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ import type * as autotagservice from '@go/autotagservice/models.js';
|
|||||||
import { confirmAction } from '../confirm-dialog/confirm-dialog';
|
import { confirmAction } from '../confirm-dialog/confirm-dialog';
|
||||||
import { queueStore } from '../../store/queue-store';
|
import { queueStore } from '../../store/queue-store';
|
||||||
import type { QueueSource } from '../../store/queue-store';
|
import type { QueueSource } from '../../store/queue-store';
|
||||||
|
import { playAll } from '@utils/play-all';
|
||||||
import { notificationStore } from '../../store/notification-store';
|
import { notificationStore } from '../../store/notification-store';
|
||||||
import '../notifications/inline-notice';
|
import '../notifications/inline-notice';
|
||||||
import {
|
import {
|
||||||
@@ -2771,20 +2772,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
|
|
||||||
/** Play what the user owns of this release, optionally shuffled. */
|
/** Play what the user owns of this release, optionally shuffled. */
|
||||||
private playOwned(shuffle: boolean): void {
|
private playOwned(shuffle: boolean): void {
|
||||||
const paths = this.ownedFilePaths();
|
|
||||||
|
|
||||||
// The button is only rendered when there is something to play,
|
// The button is only rendered when there is something to play,
|
||||||
// so an empty set here is not a state the user can reach.
|
// so an empty set here is not a state the user can reach. The
|
||||||
if (paths.length === 0) return;
|
// shuffle-mode semantics live in `playAll`, shared with the
|
||||||
|
// play-all/shuffle-all pair on every track list.
|
||||||
// `shuffleStart` only picks a random first track when shuffle
|
playAll(this.ownedFilePaths(), this.queueSource(), shuffle);
|
||||||
// mode is *already* on — it does not turn it on — so the mode
|
|
||||||
// has to be set before the queue, not after.
|
|
||||||
if (shuffle && !queueStore.getState().shuffleMode) {
|
|
||||||
queueStore.toggleShuffle();
|
|
||||||
}
|
|
||||||
|
|
||||||
queueStore.setQueue(paths, 0, shuffle, this.queueSource());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Append what the user owns of this release to the queue. */
|
/** Append what the user owns of this release to the queue. */
|
||||||
|
|||||||
@@ -2,12 +2,14 @@ import { LitElement, html, css, nothing } from 'lit';
|
|||||||
import { customElement, state, query } from 'lit/decorators.js';
|
import { customElement, state, query } from 'lit/decorators.js';
|
||||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||||
|
import { EventsOn } from '@runtime/runtime';
|
||||||
import {
|
import {
|
||||||
AddLibrary,
|
AddLibrary,
|
||||||
GetAllLibrariesWithTrackCounts,
|
GetAllLibrariesWithTrackCounts,
|
||||||
} from '@go/library/library.js';
|
} from '@go/library/library.js';
|
||||||
import { describeError, explainError } from '@utils/describe-error';
|
import { describeError, explainError } from '@utils/describe-error';
|
||||||
import { nameDialogsIn } from '@utils/name-dialog';
|
import { nameDialogsIn } from '@utils/name-dialog';
|
||||||
|
import { Events } from '../../events';
|
||||||
import { pickDirectory } from '../../utils/pick-directory';
|
import { pickDirectory } from '../../utils/pick-directory';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -19,6 +21,13 @@ import { pickDirectory } from '../../utils/pick-directory';
|
|||||||
* prompting the user to pick their music folder, registers it through the
|
* prompting the user to pick their music folder, registers it through the
|
||||||
* library CRUD API, and dismisses itself. AddLibrary emits LibraryAdded
|
* library CRUD API, and dismisses itself. AddLibrary emits LibraryAdded
|
||||||
* and kicks off the initial scan automatically.
|
* and kicks off the initial scan automatically.
|
||||||
|
*
|
||||||
|
* **The dismissal follows the library existing, not the button being
|
||||||
|
* pressed.** `AddLibrary` emits `LibraryAdded` whoever calls it, so the
|
||||||
|
* wizard waits on the state it exists to wait for rather than on a step
|
||||||
|
* in its own flow — a library arriving by any other route (Settings, a
|
||||||
|
* direct call) leaves a full-screen modal up otherwise, intercepting
|
||||||
|
* every pointer event.
|
||||||
*/
|
*/
|
||||||
@customElement('first-run-wizard')
|
@customElement('first-run-wizard')
|
||||||
export class FirstRunWizard extends LitElement {
|
export class FirstRunWizard extends LitElement {
|
||||||
@@ -37,9 +46,18 @@ export class FirstRunWizard extends LitElement {
|
|||||||
/** Error message from a failed pick/save, if any. */
|
/** Error message from a failed pick/save, if any. */
|
||||||
@state() private errorMessage = '';
|
@state() private errorMessage = '';
|
||||||
|
|
||||||
|
/** Unsubscribe from LibraryAdded, while this element is connected. */
|
||||||
|
private cancelLibraryAdded?: () => void;
|
||||||
|
|
||||||
override async connectedCallback(): Promise<void> {
|
override async connectedCallback(): Promise<void> {
|
||||||
super.connectedCallback();
|
super.connectedCallback();
|
||||||
|
|
||||||
|
// Subscribed before the read below, so a library arriving while
|
||||||
|
// that call is in flight is not answered with a stale empty list.
|
||||||
|
this.cancelLibraryAdded = EventsOn(Events.LibraryAdded, () => {
|
||||||
|
this.dismiss();
|
||||||
|
});
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const existing = await GetAllLibrariesWithTrackCounts();
|
const existing = await GetAllLibrariesWithTrackCounts();
|
||||||
|
|
||||||
@@ -54,6 +72,8 @@ export class FirstRunWizard extends LitElement {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (this.finished) return;
|
||||||
|
|
||||||
this.active = true;
|
this.active = true;
|
||||||
|
|
||||||
await this.updateComplete;
|
await this.updateComplete;
|
||||||
@@ -61,6 +81,13 @@ export class FirstRunWizard extends LitElement {
|
|||||||
if (this.dialog) this.dialog.open = true;
|
if (this.dialog) this.dialog.open = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override disconnectedCallback(): void {
|
||||||
|
this.cancelLibraryAdded?.();
|
||||||
|
this.cancelLibraryAdded = undefined;
|
||||||
|
|
||||||
|
super.disconnectedCallback();
|
||||||
|
}
|
||||||
|
|
||||||
static override styles = css`
|
static override styles = css`
|
||||||
wa-dialog {
|
wa-dialog {
|
||||||
--width: 480px;
|
--width: 480px;
|
||||||
@@ -239,6 +266,20 @@ export class FirstRunWizard extends LitElement {
|
|||||||
if (!this.finished) e.preventDefault();
|
if (!this.finished) e.preventDefault();
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Close, and stay closed: a library exists, so setup is over.
|
||||||
|
*
|
||||||
|
* `finished` is set first, or `preventClose` cancels the hide this
|
||||||
|
* asks for.
|
||||||
|
*/
|
||||||
|
private dismiss(): void {
|
||||||
|
this.finished = true;
|
||||||
|
|
||||||
|
if (this.dialog) this.dialog.open = false;
|
||||||
|
|
||||||
|
this.active = false;
|
||||||
|
}
|
||||||
|
|
||||||
private handleChoose = async (): Promise<void> => {
|
private handleChoose = async (): Promise<void> => {
|
||||||
this.errorMessage = '';
|
this.errorMessage = '';
|
||||||
|
|
||||||
@@ -264,11 +305,7 @@ export class FirstRunWizard extends LitElement {
|
|||||||
try {
|
try {
|
||||||
await AddLibrary(this.selectedDirectory);
|
await AddLibrary(this.selectedDirectory);
|
||||||
|
|
||||||
this.finished = true;
|
this.dismiss();
|
||||||
|
|
||||||
if (this.dialog) this.dialog.open = false;
|
|
||||||
|
|
||||||
this.active = false;
|
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
this.errorMessage = explainError(
|
this.errorMessage = explainError(
|
||||||
err,
|
err,
|
||||||
|
|||||||
@@ -85,7 +85,9 @@ import {
|
|||||||
ICON_PLAYLIST,
|
ICON_PLAYLIST,
|
||||||
ICON_QUEUE,
|
ICON_QUEUE,
|
||||||
ICON_REMOVE,
|
ICON_REMOVE,
|
||||||
|
ICON_SHUFFLE,
|
||||||
} from '@utils/icon-language';
|
} from '@utils/icon-language';
|
||||||
|
import { playAll } from '@utils/play-all';
|
||||||
|
|
||||||
/** One playlist row: the track and its position in the *playlist*,
|
/** One playlist row: the track and its position in the *playlist*,
|
||||||
* which is not its position in the filtered view. */
|
* which is not its position in the filtered view. */
|
||||||
@@ -355,14 +357,32 @@ export class PlaylistDetails
|
|||||||
// Track interactions
|
// Track interactions
|
||||||
// =================================================================
|
// =================================================================
|
||||||
|
|
||||||
private handlePlayAll() {
|
private playableFilePaths(): string[] {
|
||||||
const filePaths = this.tracks
|
return this.tracks
|
||||||
.filter((t) => !t.Phantom)
|
.filter((t) => !t.Phantom)
|
||||||
.map((t) => t.FilePath);
|
.map((t) => t.FilePath);
|
||||||
|
}
|
||||||
|
|
||||||
if (filePaths.length === 0) return;
|
private handlePlayAll() {
|
||||||
|
// Start at the first row, not at a random one: the old `true`
|
||||||
|
// was `shuffleStart`, which only picks a random first track
|
||||||
|
// when shuffle mode is already on — so "Play All" quietly did
|
||||||
|
// "play from the top" while leaving the mode as it was. The
|
||||||
|
// mode semantics now live in `playAll`, shared with the other
|
||||||
|
// track lists.
|
||||||
|
playAll(
|
||||||
|
this.playableFilePaths(),
|
||||||
|
{ type: 'playlist', id: this.playlistId, label: this.playlistName },
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
queueStore.setQueue(filePaths, 0, true, { type: 'playlist', id: this.playlistId, label: this.playlistName });
|
private handleShuffleAll() {
|
||||||
|
playAll(
|
||||||
|
this.playableFilePaths(),
|
||||||
|
{ type: 'playlist', id: this.playlistId, label: this.playlistName },
|
||||||
|
true,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
private handleTrackClick(
|
private handleTrackClick(
|
||||||
@@ -1599,9 +1619,16 @@ export class PlaylistDetails
|
|||||||
class="play-all-button"
|
class="play-all-button"
|
||||||
@click=${() => this.handlePlayAll()}
|
@click=${() => this.handlePlayAll()}
|
||||||
>
|
>
|
||||||
<wa-icon name="play"></wa-icon>
|
<wa-icon name=${ICON_PLAY}></wa-icon>
|
||||||
Play All
|
Play All
|
||||||
</button>
|
</button>
|
||||||
|
<button
|
||||||
|
class="play-all-button"
|
||||||
|
@click=${() => this.handleShuffleAll()}
|
||||||
|
>
|
||||||
|
<wa-icon name=${ICON_SHUFFLE}></wa-icon>
|
||||||
|
Shuffle All
|
||||||
|
</button>
|
||||||
</div>
|
</div>
|
||||||
<div class="track-header">
|
<div class="track-header">
|
||||||
<div class="header-cell col-number">#</div>
|
<div class="header-cell col-number">#</div>
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import {
|
|||||||
import { EventsOn } from '@runtime/runtime';
|
import { EventsOn } from '@runtime/runtime';
|
||||||
import { Events } from '../../events';
|
import { Events } from '../../events';
|
||||||
import { queueStore } from '@store/queue-store';
|
import { queueStore } from '@store/queue-store';
|
||||||
|
import { playAll } from '@utils/play-all';
|
||||||
import { creditStore } from '@store/credit-store';
|
import { creditStore } from '@store/credit-store';
|
||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
import { SearchController } from '@store/controllers/search-controller';
|
import { SearchController } from '@store/controllers/search-controller';
|
||||||
@@ -771,24 +772,29 @@ export class SmartPlaylistDetails
|
|||||||
// Actions
|
// Actions
|
||||||
// =================================================================
|
// =================================================================
|
||||||
|
|
||||||
private handlePlay() {
|
private playableFilePaths(): string[] {
|
||||||
const filePaths = this.tracks
|
return this.tracks
|
||||||
.filter((t) => !t.Phantom)
|
.filter((t) => !t.Phantom)
|
||||||
.map((t) => t.FilePath);
|
.map((t) => t.FilePath);
|
||||||
|
}
|
||||||
|
|
||||||
if (filePaths.length === 0) return;
|
private handlePlay() {
|
||||||
|
playAll(
|
||||||
queueStore.setQueue(filePaths, 0, false, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
|
this.playableFilePaths(),
|
||||||
|
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
|
||||||
|
false,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
private handleShuffle() {
|
private handleShuffle() {
|
||||||
const filePaths = this.tracks
|
// This used to be a no-op when shuffle mode was off: it passed
|
||||||
.filter((t) => !t.Phantom)
|
// `shuffleStart` without turning the mode on, so the queue
|
||||||
.map((t) => t.FilePath);
|
// started at track 1 in order. `playAll` sets the mode first.
|
||||||
|
playAll(
|
||||||
if (filePaths.length === 0) return;
|
this.playableFilePaths(),
|
||||||
|
{ type: 'smartPlaylist', id: this.playlistId, label: this.playlistName },
|
||||||
queueStore.setQueue(filePaths, 0, true, { type: 'smartPlaylist', id: this.playlistId, label: this.playlistName });
|
true,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
private async handleRefresh() {
|
private async handleRefresh() {
|
||||||
|
|||||||
@@ -32,6 +32,18 @@ export interface ColumnDef {
|
|||||||
id: string;
|
id: string;
|
||||||
/** Human-readable header label. */
|
/** Human-readable header label. */
|
||||||
label: string;
|
label: string;
|
||||||
|
/**
|
||||||
|
* Whether Settings may offer this column. Defaults to true.
|
||||||
|
*
|
||||||
|
* A definition is not the same thing as a *choice*. `titleArtist`
|
||||||
|
* is the phone's stacked column, picked by width in
|
||||||
|
* `PHONE_COLUMN_IDS`, and `tracklist.AllColumnIDs` in Go does not
|
||||||
|
* list it — so a tick in the configurator sends a column set the
|
||||||
|
* backend rejects with `unknown track-list column ID`, the tick
|
||||||
|
* reverts on the next render, and the only trace is a
|
||||||
|
* `console.error` (#197).
|
||||||
|
*/
|
||||||
|
configurable?: boolean;
|
||||||
/** Extracts the display value from a track. */
|
/** Extracts the display value from a track. */
|
||||||
accessor: (track: library.Track) => string;
|
accessor: (track: library.Track) => string;
|
||||||
/** Default CSS width (used when no saved width exists). */
|
/** Default CSS width (used when no saved width exists). */
|
||||||
@@ -98,11 +110,16 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
|
|||||||
},
|
},
|
||||||
titleArtist: {
|
titleArtist: {
|
||||||
id: 'titleArtist',
|
id: 'titleArtist',
|
||||||
// Named for what it sorts by, since that is the only place the
|
// Named for what it sorts by. That label is drawn nowhere
|
||||||
// label is user-visible: the phone has no column headers, and
|
// today: the phone has no column headers, and the page header's
|
||||||
// the page header's sort list is built from the *configured*
|
// sort list is built from the *configured* columns, which this
|
||||||
// columns rather than the drawn ones.
|
// one can never be — see `configurable` below.
|
||||||
label: 'Track Name',
|
label: 'Track Name',
|
||||||
|
// Chosen by width, never by the user, and rejected by the
|
||||||
|
// backend if it ever were. #197: Settings listed it anyway, so
|
||||||
|
// there were two rows called "Track Name" and the second one
|
||||||
|
// could not be selected.
|
||||||
|
configurable: false,
|
||||||
accessor: (t) => t.TrackName,
|
accessor: (t) => t.TrackName,
|
||||||
defaultWidth: '1fr',
|
defaultWidth: '1fr',
|
||||||
comparator: (a, b) => compareStr(a.TrackName, b.TrackName),
|
comparator: (a, b) => compareStr(a.TrackName, b.TrackName),
|
||||||
@@ -266,10 +283,15 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* All column IDs in default display order.
|
* The column IDs Settings may offer, in default display order.
|
||||||
* Used by the settings UI to list available columns.
|
*
|
||||||
|
* Not every definition is one: a column the user cannot choose has no
|
||||||
|
* row in the configurator, because a checkbox that cannot change
|
||||||
|
* anything is worse than an absent one — see `ColumnDef.configurable`.
|
||||||
*/
|
*/
|
||||||
export const ALL_COLUMN_IDS: string[] = Object.keys(COLUMN_DEFS);
|
export const CONFIGURABLE_COLUMN_IDS: string[] = Object.keys(
|
||||||
|
COLUMN_DEFS,
|
||||||
|
).filter((id) => COLUMN_DEFS[id]?.configurable !== false);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Column IDs that are always searched regardless of visibility.
|
* Column IDs that are always searched regardless of visibility.
|
||||||
|
|||||||
@@ -26,7 +26,10 @@ import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller
|
|||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
import { SearchController } from '@store/controllers/search-controller';
|
import { SearchController } from '@store/controllers/search-controller';
|
||||||
import '@components/page-header/page-header';
|
import '@components/page-header/page-header';
|
||||||
import type { SortOption } from '@components/page-header/page-header';
|
import type {
|
||||||
|
SortOption,
|
||||||
|
PageAction,
|
||||||
|
} from '@components/page-header/page-header';
|
||||||
import { TrackListController } from '@store/controllers/tracklist-controller';
|
import { TrackListController } from '@store/controllers/tracklist-controller';
|
||||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||||
import { queueStore } from '@store/queue-store';
|
import { queueStore } from '@store/queue-store';
|
||||||
@@ -87,7 +90,9 @@ import {
|
|||||||
ICON_PLAYLIST,
|
ICON_PLAYLIST,
|
||||||
ICON_PLAY_NEXT,
|
ICON_PLAY_NEXT,
|
||||||
ICON_QUEUE,
|
ICON_QUEUE,
|
||||||
|
ICON_SHUFFLE,
|
||||||
} from '@utils/icon-language';
|
} from '@utils/icon-language';
|
||||||
|
import { playAll } from '@utils/play-all';
|
||||||
|
|
||||||
const COLUMN_STORAGE_KEY = 'track-list-column-widths';
|
const COLUMN_STORAGE_KEY = 'track-list-column-widths';
|
||||||
const SORT_FIELD_KEY = 'track-list-sort-field';
|
const SORT_FIELD_KEY = 'track-list-sort-field';
|
||||||
@@ -2393,6 +2398,27 @@ export class TrackList
|
|||||||
.map((c) => ({ id: c.id, label: c.label })),
|
.map((c) => ({ id: c.id, label: c.label })),
|
||||||
];
|
];
|
||||||
|
|
||||||
|
const hasTracks = this.cachedSortedTracks.length > 0;
|
||||||
|
|
||||||
|
const actions: PageAction[] = [
|
||||||
|
{
|
||||||
|
id: 'play-all',
|
||||||
|
label: 'Play all',
|
||||||
|
icon: ICON_PLAY,
|
||||||
|
priority: 1,
|
||||||
|
disabled: !hasTracks,
|
||||||
|
onSelect: this.handlePlayAll,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'shuffle-all',
|
||||||
|
label: 'Shuffle all',
|
||||||
|
icon: ICON_SHUFFLE,
|
||||||
|
priority: 0,
|
||||||
|
disabled: !hasTracks,
|
||||||
|
onSelect: this.handleShuffleAll,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
return html`
|
return html`
|
||||||
<page-header
|
<page-header
|
||||||
heading=${this.externalTracks === undefined ? 'Tracks' : ''}
|
heading=${this.externalTracks === undefined ? 'Tracks' : ''}
|
||||||
@@ -2404,11 +2430,29 @@ export class TrackList
|
|||||||
sort-field=${this.sortField ?? ''}
|
sort-field=${this.sortField ?? ''}
|
||||||
sort-direction=${this.sortDirection}
|
sort-direction=${this.sortDirection}
|
||||||
search-term=${this.searchCtrl.term}
|
search-term=${this.searchCtrl.term}
|
||||||
|
.actions=${actions}
|
||||||
@sort-change=${this.onPageHeaderSort}
|
@sort-change=${this.onPageHeaderSort}
|
||||||
></page-header>
|
></page-header>
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The queue is the list as displayed, in the order the user sees. */
|
||||||
|
private handlePlayAll = (): void => {
|
||||||
|
playAll(
|
||||||
|
this.cachedSortedTracks.map((t) => t.FilePath),
|
||||||
|
this.effectiveQueueSource,
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
private handleShuffleAll = (): void => {
|
||||||
|
playAll(
|
||||||
|
this.cachedSortedTracks.map((t) => t.FilePath),
|
||||||
|
this.effectiveQueueSource,
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
private onPageHeaderSort = (
|
private onPageHeaderSort = (
|
||||||
e: CustomEvent<{ field: string; direction: 'asc' | 'desc' }>,
|
e: CustomEvent<{ field: string; direction: 'asc' | 'desc' }>,
|
||||||
) => {
|
) => {
|
||||||
|
|||||||
@@ -0,0 +1,26 @@
|
|||||||
|
import { queueStore } from '@store/queue-store';
|
||||||
|
import type { QueueSource } from '@store/queue-store';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Queue a list and start it, optionally shuffled.
|
||||||
|
*
|
||||||
|
* This is the one place that owns what "shuffle this collection" means.
|
||||||
|
* `SetQueue`'s `shuffleStart` only picks a random first track when
|
||||||
|
* shuffle mode is *already* on — it does not turn it on — so the mode
|
||||||
|
* has to be set before the queue, not after. The album page used to
|
||||||
|
* carry that rule privately; the play-all/shuffle-all pair on every
|
||||||
|
* track list now shares it.
|
||||||
|
*/
|
||||||
|
export function playAll(
|
||||||
|
paths: string[],
|
||||||
|
source: QueueSource | undefined,
|
||||||
|
shuffle: boolean,
|
||||||
|
): void {
|
||||||
|
if (paths.length === 0) return;
|
||||||
|
|
||||||
|
if (shuffle && !queueStore.getState().shuffleMode) {
|
||||||
|
queueStore.toggleShuffle();
|
||||||
|
}
|
||||||
|
|
||||||
|
queueStore.setQueue(paths, 0, shuffle, source);
|
||||||
|
}
|
||||||
@@ -0,0 +1,123 @@
|
|||||||
|
/**
|
||||||
|
* #175: the first-run wizard's dismissal follows the library existing,
|
||||||
|
* not its own button being pressed.
|
||||||
|
*
|
||||||
|
* The wizard is a modal that blocks every pointer event, so a library
|
||||||
|
* arriving by another route — Settings, a direct call — used to leave
|
||||||
|
* it up over an app that was already set up. `LibraryAdded` is emitted
|
||||||
|
* by `AddLibrary` whoever calls it, which is what makes one
|
||||||
|
* subscription the whole fix.
|
||||||
|
*/
|
||||||
|
import { beforeEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { emit, stub } from '../support/harness';
|
||||||
|
import { fixture, shadow, shadowAll } from '../support/render';
|
||||||
|
import { wails } from '../support/wails-fake';
|
||||||
|
|
||||||
|
import '@components/first-run-wizard/first-run-wizard';
|
||||||
|
|
||||||
|
import type { FirstRunWizard } from '@components/first-run-wizard/first-run-wizard';
|
||||||
|
|
||||||
|
/** A library row, as `GetAllLibrariesWithTrackCounts` returns one. */
|
||||||
|
const aLibrary = {
|
||||||
|
id: 1,
|
||||||
|
name: 'Music',
|
||||||
|
path: '/home/logan/Music',
|
||||||
|
trackCount: 9,
|
||||||
|
};
|
||||||
|
|
||||||
|
/** Mount the wizard on a fresh install: no libraries yet. */
|
||||||
|
async function wizardOnAFreshInstall(): Promise<FirstRunWizard> {
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||||
|
|
||||||
|
return fixture<FirstRunWizard>('first-run-wizard');
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Whether the wizard is rendering its modal at all. */
|
||||||
|
function isShowing(el: FirstRunWizard): boolean {
|
||||||
|
return shadow(el, 'wa-dialog') !== null;
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
stub('library.Library.AddLibrary', aLibrary);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('first-run-wizard', () => {
|
||||||
|
it('shows on a fresh install and stays up until a library exists', async () => {
|
||||||
|
const el = await wizardOnAFreshInstall();
|
||||||
|
|
||||||
|
expect(isShowing(el)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('stays hidden when a library is already configured', async () => {
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', [aLibrary]);
|
||||||
|
|
||||||
|
const el = await fixture<FirstRunWizard>('first-run-wizard');
|
||||||
|
|
||||||
|
expect(isShowing(el)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('dismisses when a library appears by another route', async () => {
|
||||||
|
const el = await wizardOnAFreshInstall();
|
||||||
|
|
||||||
|
expect(isShowing(el)).toBe(true);
|
||||||
|
|
||||||
|
emit(Events.LibraryAdded, aLibrary);
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(isShowing(el)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not raise itself when a library arrives while it is asking', async () => {
|
||||||
|
// The read is still in flight when the event lands, so its
|
||||||
|
// answer — an empty list — is stale by the time it returns.
|
||||||
|
let answer: (libraries: unknown[]) => void = () => {};
|
||||||
|
|
||||||
|
stub(
|
||||||
|
'library.Library.GetAllLibrariesWithTrackCounts',
|
||||||
|
() =>
|
||||||
|
new Promise((resolve) => {
|
||||||
|
answer = resolve;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const el = await fixture<FirstRunWizard>('first-run-wizard');
|
||||||
|
|
||||||
|
emit(Events.LibraryAdded, aLibrary);
|
||||||
|
answer([]);
|
||||||
|
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(isShowing(el)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still dismisses through its own Get Started button', async () => {
|
||||||
|
stub('frontendutil.FrontendUtil.HasNativeDirectoryPicker', true);
|
||||||
|
stub('frontendutil.FrontendUtil.DirectoryPicker', '/home/logan/Music');
|
||||||
|
|
||||||
|
const { resetDirectoryPickerCache } = await import(
|
||||||
|
'@utils/pick-directory'
|
||||||
|
);
|
||||||
|
|
||||||
|
resetDirectoryPickerCache();
|
||||||
|
|
||||||
|
const el = await wizardOnAFreshInstall();
|
||||||
|
const [choose, finish] = shadowAll<HTMLButtonElement>(el, '.btn');
|
||||||
|
|
||||||
|
choose?.click();
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
finish?.click();
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(
|
||||||
|
wails.calls.filter((c) => c.path === 'library.Library.AddLibrary'),
|
||||||
|
).toHaveLength(1);
|
||||||
|
expect(isShowing(el)).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,373 @@
|
|||||||
|
/**
|
||||||
|
* The play-all/shuffle-all pair on every page that lists tracks.
|
||||||
|
*
|
||||||
|
* The pair is driven by one helper (`utils/play-all`) that owns the
|
||||||
|
* one rule the album page already carried: `shuffleStart` does not turn
|
||||||
|
* shuffle on, it only picks a random first track once the mode is on —
|
||||||
|
* so the mode has to be set *before* the queue, not after. The hosts
|
||||||
|
* differ only in where their paths come from and what `Source` they
|
||||||
|
* hand over.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/track-list/track-list';
|
||||||
|
import '@components/artist-details/artist-details';
|
||||||
|
import '@components/playlist-details/playlist-details';
|
||||||
|
import '@components/smart-playlist-details/smart-playlist-details';
|
||||||
|
import {
|
||||||
|
stub,
|
||||||
|
flush,
|
||||||
|
resetHarness,
|
||||||
|
calls,
|
||||||
|
lastArgs,
|
||||||
|
emit,
|
||||||
|
} from '@test/support/harness';
|
||||||
|
import {
|
||||||
|
fixture,
|
||||||
|
shadowAll,
|
||||||
|
deepShadow,
|
||||||
|
} from '@test/support/render';
|
||||||
|
|
||||||
|
/** The action button rendered by `<page-header>`, through the nested
|
||||||
|
* shadow roots (track-list → page-header). */
|
||||||
|
function pageAction(
|
||||||
|
host: LitElement,
|
||||||
|
id: string,
|
||||||
|
): HTMLElement | null {
|
||||||
|
return deepShadow<HTMLElement>(host, `[data-testid="page-action-${id}"]`);
|
||||||
|
}
|
||||||
|
|
||||||
|
function setShuffleMode(on: boolean): void {
|
||||||
|
emit('QueueModeChanged', { shuffleMode: on, repeatMode: 'off' });
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The queue's `SetQueue` args, with the shuffle flag and source. */
|
||||||
|
function queued(): {
|
||||||
|
paths: string[];
|
||||||
|
startIndex: number;
|
||||||
|
shuffleStart: boolean;
|
||||||
|
source: unknown;
|
||||||
|
} {
|
||||||
|
const args = lastArgs('queue.Queue.SetQueue');
|
||||||
|
|
||||||
|
if (!args) throw new Error('nothing was queued');
|
||||||
|
|
||||||
|
return {
|
||||||
|
paths: args[0] as string[],
|
||||||
|
startIndex: args[1] as number,
|
||||||
|
shuffleStart: args[2] as boolean,
|
||||||
|
source: args[3],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// =====================================================================
|
||||||
|
// The track list (Tracks, and every embedding detail view)
|
||||||
|
// =====================================================================
|
||||||
|
|
||||||
|
const PATHS = Array.from({ length: 12 }, (_, i) => `/music/track-${i}.mp3`);
|
||||||
|
|
||||||
|
const LIST = PATHS.map((FilePath, i) => ({
|
||||||
|
FilePath,
|
||||||
|
TrackName: `Track ${i}`,
|
||||||
|
ArtistName: 'An Artist',
|
||||||
|
Album: 'An Album',
|
||||||
|
Duration: 180,
|
||||||
|
}));
|
||||||
|
|
||||||
|
const GENRE_SOURCE = { type: 'genre', id: 0, label: 'Dream Pop' };
|
||||||
|
|
||||||
|
async function embeddedTrackList(): Promise<LitElement> {
|
||||||
|
resetHarness();
|
||||||
|
localStorage.removeItem('track-list-column-widths');
|
||||||
|
|
||||||
|
const el = await fixture<LitElement>('track-list', {
|
||||||
|
externalTracks: LIST,
|
||||||
|
queueSource: GENRE_SOURCE,
|
||||||
|
});
|
||||||
|
|
||||||
|
// Say which order is being asserted rather than inheriting a
|
||||||
|
// persisted sort. See play-in-context.test.ts for the same trap.
|
||||||
|
(el as unknown as { sortField: string | null }).sortField = null;
|
||||||
|
|
||||||
|
el.style.display = 'block';
|
||||||
|
el.style.height = '600px';
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 60));
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the track-list play-all/shuffle-all pair', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
localStorage.removeItem('track-list-column-widths');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders in the primary Tracks header and is disabled while empty', async () => {
|
||||||
|
const el = await fixture<LitElement>('track-list', {});
|
||||||
|
|
||||||
|
const play = pageAction(el, 'play-all');
|
||||||
|
const shuffle = pageAction(el, 'shuffle-all');
|
||||||
|
|
||||||
|
expect(play).not.toBeNull();
|
||||||
|
expect(shuffle).not.toBeNull();
|
||||||
|
expect(play?.hasAttribute('disabled')).toBe(true);
|
||||||
|
expect(shuffle?.hasAttribute('disabled')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders in an embedded track-list header too', async () => {
|
||||||
|
const el = await embeddedTrackList();
|
||||||
|
|
||||||
|
expect(pageAction(el, 'play-all')).not.toBeNull();
|
||||||
|
expect(pageAction(el, 'shuffle-all')).not.toBeNull();
|
||||||
|
expect(pageAction(el, 'play-all')?.hasAttribute('disabled')).toBe(false);
|
||||||
|
expect(pageAction(el, 'shuffle-all')?.hasAttribute('disabled')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Play all queues the displayed list in order, unshuffled', async () => {
|
||||||
|
const el = await embeddedTrackList();
|
||||||
|
|
||||||
|
pageAction(el, 'play-all')!.click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: PATHS,
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: false,
|
||||||
|
source: GENRE_SOURCE,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Shuffle all turns shuffle on before queueing when the mode is off', async () => {
|
||||||
|
const el = await embeddedTrackList();
|
||||||
|
|
||||||
|
setShuffleMode(false);
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
pageAction(el, 'shuffle-all')!.click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
const all = calls();
|
||||||
|
const toggle = all.findLastIndex(
|
||||||
|
(c) => c.path === 'queue.Queue.ToggleShuffle',
|
||||||
|
);
|
||||||
|
const setQueue = all.findLastIndex(
|
||||||
|
(c) => c.path === 'queue.Queue.SetQueue',
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(toggle).toBeGreaterThanOrEqual(0);
|
||||||
|
expect(toggle).toBeLessThan(setQueue);
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: PATHS,
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: true,
|
||||||
|
source: GENRE_SOURCE,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Shuffle all does not toggle when the mode is already on', async () => {
|
||||||
|
const el = await embeddedTrackList();
|
||||||
|
|
||||||
|
setShuffleMode(true);
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
pageAction(el, 'shuffle-all')!.click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
expect(calls('queue.Queue.ToggleShuffle')).toHaveLength(0);
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: PATHS,
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: true,
|
||||||
|
source: GENRE_SOURCE,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// =====================================================================
|
||||||
|
// The library artist page
|
||||||
|
// =====================================================================
|
||||||
|
|
||||||
|
const ALBUMS = [
|
||||||
|
{ ID: 3, Name: 'Third', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
|
||||||
|
{ ID: 1, Name: 'First', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
|
||||||
|
{ ID: 2, Name: 'Second', ArtistName: 'Aurora Fields', ArtistMBID: '', MBID: '', CoverArtPath: '', CoverArtSmall: '', CoverArtMedium: '', CoverArtLarge: '', Year: 0, ReleaseYear: 0 },
|
||||||
|
];
|
||||||
|
|
||||||
|
describe('the artist page play-all pair', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('explore.Service.GetArtistMBID', '');
|
||||||
|
stub('library.Library.GetAlbumsByArtist', ALBUMS);
|
||||||
|
stub('library.Library.GetFilePathsByAlbums', {
|
||||||
|
'3': ['/a3-1', '/a3-2'],
|
||||||
|
'1': ['/a1'],
|
||||||
|
'2': ['/a2-1', '/a2-2', '/a2-3'],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('flattens album paths in the album list order', async () => {
|
||||||
|
const el = await fixture<LitElement>('artist-details', {
|
||||||
|
artistId: 7,
|
||||||
|
artistName: 'Aurora Fields',
|
||||||
|
artistMBID: '',
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
const play = shadowAll<HTMLElement>(el, '[data-testid="artist-play-all"]')[0];
|
||||||
|
|
||||||
|
play!.click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: ['/a3-1', '/a3-2', '/a1', '/a2-1', '/a2-2', '/a2-3'],
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: false,
|
||||||
|
source: { type: 'artist', id: 7, label: 'Aurora Fields' },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// =====================================================================
|
||||||
|
// A smart playlist
|
||||||
|
// =====================================================================
|
||||||
|
|
||||||
|
function smartPlaylistTracks(n: number) {
|
||||||
|
return Array.from({ length: n }, (_, i) => ({
|
||||||
|
ID: i + 1,
|
||||||
|
FilePath: `/music/track-${i}.mp3`,
|
||||||
|
Title: `Track ${i}`,
|
||||||
|
Artist: 'An Artist',
|
||||||
|
Album: 'An Album',
|
||||||
|
Duration: 180000,
|
||||||
|
CoverArtSmall: `/covers/${i}_sm.jpg`,
|
||||||
|
CoverArtMedium: `/covers/${i}_md.jpg`,
|
||||||
|
CoverArtPath: `/covers/${i}.jpg`,
|
||||||
|
Phantom: false,
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the smart-playlist play-all pair', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('playlist.Service.GetSmartPlaylistTracks', smartPlaylistTracks(8));
|
||||||
|
stub('playlist.Service.GetSmartPlaylistRules', '{"rules":[]}');
|
||||||
|
stub('playlist.Service.GetAllPlaylists', []);
|
||||||
|
});
|
||||||
|
|
||||||
|
/** The details header's own action row, not a page-header action. */
|
||||||
|
function actionButton(el: LitElement, label: string): HTMLElement {
|
||||||
|
const button = shadowAll<HTMLElement>(el, '.action-button').find(
|
||||||
|
(b) => b.textContent?.trim() === label,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!button) throw new Error(`no "${label}" action button rendered`);
|
||||||
|
|
||||||
|
return button;
|
||||||
|
}
|
||||||
|
|
||||||
|
it('Shuffle turns the mode on before queueing, so the queue starts shuffled', async () => {
|
||||||
|
const el = await fixture<LitElement>('smart-playlist-details', {
|
||||||
|
playlistId: 1,
|
||||||
|
playlistName: 'A smart playlist',
|
||||||
|
});
|
||||||
|
|
||||||
|
el.style.display = 'block';
|
||||||
|
el.style.height = '600px';
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 60));
|
||||||
|
|
||||||
|
setShuffleMode(false);
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
actionButton(el, 'Shuffle').click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
// The issue's headline fix. `SetQueue`'s `shuffleStart` only picks
|
||||||
|
// a random first track when shuffle mode is already on — it does
|
||||||
|
// not turn it on — so reverting the mode toggle puts the queue back
|
||||||
|
// to track 1 in order while every assertion about the queue's
|
||||||
|
// contents still passes. Order of the calls is the assertion.
|
||||||
|
const all = calls();
|
||||||
|
const toggle = all.findLastIndex(
|
||||||
|
(c) => c.path === 'queue.Queue.ToggleShuffle',
|
||||||
|
);
|
||||||
|
const setQueue = all.findLastIndex(
|
||||||
|
(c) => c.path === 'queue.Queue.SetQueue',
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(toggle).toBeGreaterThanOrEqual(0);
|
||||||
|
expect(toggle).toBeLessThan(setQueue);
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: true,
|
||||||
|
source: { type: 'smartPlaylist', id: 1, label: 'A smart playlist' },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// =====================================================================
|
||||||
|
// A regular playlist
|
||||||
|
// =====================================================================
|
||||||
|
|
||||||
|
function playlistTracks(n: number) {
|
||||||
|
return Array.from({ length: n }, (_, i) => ({
|
||||||
|
ID: i + 1,
|
||||||
|
FilePath: `/music/track-${i}.mp3`,
|
||||||
|
Title: `Track ${i}`,
|
||||||
|
Artist: 'An Artist',
|
||||||
|
Album: 'An Album',
|
||||||
|
Duration: 180000,
|
||||||
|
Phantom: false,
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the playlist play-all pair', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('playlist.Service.GetPlaylistTracks', playlistTracks(8));
|
||||||
|
stub('playlist.Service.GetAllPlaylists', []);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('offers Shuffle All beside Play All, both through the helper', async () => {
|
||||||
|
const el = await fixture<LitElement>('playlist-details', {
|
||||||
|
playlistId: 1,
|
||||||
|
playlistName: 'A playlist',
|
||||||
|
});
|
||||||
|
|
||||||
|
el.style.display = 'block';
|
||||||
|
el.style.height = '600px';
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 60));
|
||||||
|
|
||||||
|
const buttons = shadowAll<HTMLElement>(el, '.play-all-button');
|
||||||
|
|
||||||
|
expect(buttons.map((b) => b.textContent?.trim())).toEqual([
|
||||||
|
'Play All',
|
||||||
|
'Shuffle All',
|
||||||
|
]);
|
||||||
|
|
||||||
|
setShuffleMode(false);
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
buttons[1]!.click();
|
||||||
|
await flush();
|
||||||
|
|
||||||
|
expect(queued()).toEqual({
|
||||||
|
paths: Array.from({ length: 8 }, (_, i) => `/music/track-${i}.mp3`),
|
||||||
|
startIndex: 0,
|
||||||
|
shuffleStart: true,
|
||||||
|
source: { type: 'playlist', id: 1, label: 'A playlist' },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,190 @@
|
|||||||
|
/**
|
||||||
|
* Settings offers the columns the backend will accept, and no others.
|
||||||
|
*
|
||||||
|
* The list is built from `COLUMN_DEFS`, which is the *drawing* table:
|
||||||
|
* every definition the track list knows how to render, including
|
||||||
|
* `titleArtist` — the phone's stacked column, chosen by width in
|
||||||
|
* `PHONE_COLUMN_IDS` and never by a person. `tracklist.AllColumnIDs` in
|
||||||
|
* Go does not list that id, so the configurator offered a nineteenth
|
||||||
|
* row that could not be ticked:
|
||||||
|
*
|
||||||
|
* ```
|
||||||
|
* validate = unknown track-list column ID: "titleArtist"
|
||||||
|
* titleArtist valid = false
|
||||||
|
* ```
|
||||||
|
*
|
||||||
|
* What a user saw was **two rows both called "Track Name"** (#197), one
|
||||||
|
* of which did nothing — and a screen reader heard "Show the Track Name
|
||||||
|
* column" twice with nothing to tell them apart, which is `a11y.32`'s
|
||||||
|
* complaint inside the list that was fixed for exactly that.
|
||||||
|
*
|
||||||
|
* It is worse than an inert control, which is why the duplicate name
|
||||||
|
* was not the thing to fix. `SetTrackListColumns` assigns before it
|
||||||
|
* validates, so a rejected list stays in memory and `Save()` validates
|
||||||
|
* the whole config:
|
||||||
|
*
|
||||||
|
* ```
|
||||||
|
* later, unrelated SetThemeAccentColor = could not save config: invalid
|
||||||
|
* config: ... unknown track-list column ID: "titleArtist"
|
||||||
|
* ```
|
||||||
|
*
|
||||||
|
* — one tick and no setting saves for the rest of the session. That
|
||||||
|
* half is filed separately; this file keeps the row from being offered.
|
||||||
|
*
|
||||||
|
* The last test is the one that would have caught it when the column
|
||||||
|
* was added: the two lists are in different languages, so nothing but a
|
||||||
|
* sweep can hold them together.
|
||||||
|
*/
|
||||||
|
import { beforeEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import '@components/config-page/config-page';
|
||||||
|
|
||||||
|
import {
|
||||||
|
COLUMN_DEFS,
|
||||||
|
CONFIGURABLE_COLUMN_IDS,
|
||||||
|
} from '@components/track-list/columns';
|
||||||
|
import { flush, stub } from '@test/support/harness';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
/** Go's own list of column ids, as text. */
|
||||||
|
const GO_CONFIG = Object.values(
|
||||||
|
import.meta.glob<string>('../../../backend/tracklist/config.go', {
|
||||||
|
eager: true,
|
||||||
|
query: '?raw',
|
||||||
|
import: 'default',
|
||||||
|
}),
|
||||||
|
)[0];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The ids `tracklist.AllColumnIDs` actually contains.
|
||||||
|
*
|
||||||
|
* Read out of the source rather than written down here, because a
|
||||||
|
* third copy of this list is a third thing to forget — which is the
|
||||||
|
* defect, one copy earlier.
|
||||||
|
*/
|
||||||
|
function goColumnIDs(source: string): string[] {
|
||||||
|
const constants = new Map<string, string>();
|
||||||
|
const constBlock = /const \(([\s\S]*?)\n\)/.exec(source)?.[1] ?? '';
|
||||||
|
|
||||||
|
for (const [, name, id] of constBlock.matchAll(
|
||||||
|
/(\w+)\s+ColumnID\s*=\s*"([^"]+)"/g,
|
||||||
|
)) {
|
||||||
|
constants.set(name!, id!);
|
||||||
|
}
|
||||||
|
|
||||||
|
const listBlock =
|
||||||
|
/var AllColumnIDs = \[\]ColumnID\{([\s\S]*?)\n\}/.exec(source)?.[1] ?? '';
|
||||||
|
|
||||||
|
return [...listBlock.matchAll(/(\w+),/g)]
|
||||||
|
.map(([, name]) => constants.get(name!))
|
||||||
|
.filter((id): id is string => id !== undefined);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The column rows, and only those.
|
||||||
|
*
|
||||||
|
* Settings’ view-visibility list (#25) is drawn with the same two
|
||||||
|
* classes, so a bare `.column-label` sweeps 29 rows across two
|
||||||
|
* sections — and "Albums" the destination sitting beside "Album" the
|
||||||
|
* column is not the fault this file is about. The `for`/`id` prefix is
|
||||||
|
* what tells them apart.
|
||||||
|
*/
|
||||||
|
const COLUMN_ROW_LABEL = 'label.column-label[for^="column-"]';
|
||||||
|
const COLUMN_ROW_BOX = 'input.column-toggle[id^="column-"]';
|
||||||
|
|
||||||
|
/** The rows the configurator draws, by their visible name. */
|
||||||
|
async function columnRowNames(): Promise<string[]> {
|
||||||
|
const page = await fixture('config-page');
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await page.updateComplete;
|
||||||
|
|
||||||
|
// Every section renders collapsed, and a collapsed body is `hidden`.
|
||||||
|
for (const section of shadowAll<HTMLElement>(page, 'config-section')) {
|
||||||
|
section.shadowRoot
|
||||||
|
?.querySelector<HTMLButtonElement>('button[aria-expanded="false"]')
|
||||||
|
?.click();
|
||||||
|
}
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await page.updateComplete;
|
||||||
|
|
||||||
|
return shadowAll<HTMLElement>(page, COLUMN_ROW_LABEL).map(
|
||||||
|
(label) => label.textContent?.trim() ?? '',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the Settings column list', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
for (const path of [
|
||||||
|
'library.Library.GetAllLibrariesWithTrackCounts',
|
||||||
|
'jobs.Service.GetJobs',
|
||||||
|
'download.Service.ListProviders',
|
||||||
|
'download.Service.ProviderKinds',
|
||||||
|
]) {
|
||||||
|
stub(path, []);
|
||||||
|
}
|
||||||
|
|
||||||
|
stub('config.Config.GetShortcuts', {});
|
||||||
|
stub('config.Config.GetDownloadPreferences', {});
|
||||||
|
stub('config.Config.GetThemeAccentColor', '#ffd43b');
|
||||||
|
stub('config.Config.GetThemeBackgroundShade', 'dark');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names each row once', async () => {
|
||||||
|
const names = await columnRowNames();
|
||||||
|
|
||||||
|
// A sweep over nothing passes.
|
||||||
|
expect(names.length, 'the page draws column rows').toBeGreaterThan(5);
|
||||||
|
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const duplicated = names.filter((name) => !seen.add(name));
|
||||||
|
|
||||||
|
expect(duplicated).toEqual([]);
|
||||||
|
expect(names.filter((n) => n === 'Track Name')).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('gives each checkbox a name that identifies it', async () => {
|
||||||
|
// The visible half above is what was reported; this is the half a
|
||||||
|
// screen reader gets, and it is the one `config-page` computes
|
||||||
|
// from the same string.
|
||||||
|
const page = await fixture('config-page');
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await page.updateComplete;
|
||||||
|
|
||||||
|
const labels = shadowAll<HTMLInputElement>(page, COLUMN_ROW_BOX).map(
|
||||||
|
(box) => box.getAttribute('aria-label') ?? '',
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(labels.length, 'the page draws column checkboxes').toBeGreaterThan(5);
|
||||||
|
expect(new Set(labels).size).toBe(labels.length);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('the column table', () => {
|
||||||
|
it('offers no column the backend would reject', async () => {
|
||||||
|
const accepted = goColumnIDs(GO_CONFIG ?? '');
|
||||||
|
|
||||||
|
// Two non-vacuity guards: a glob that stopped matching, and a
|
||||||
|
// parse that stopped finding the list it names.
|
||||||
|
expect(GO_CONFIG, 'backend/tracklist/config.go is readable').toBeTruthy();
|
||||||
|
expect(accepted.length, 'AllColumnIDs was parsed').toBeGreaterThan(10);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
CONFIGURABLE_COLUMN_IDS.filter((id) => !accepted.includes(id)),
|
||||||
|
).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still knows how to draw every column it offers', async () => {
|
||||||
|
// The filter must not have taken a column *out* of the drawing
|
||||||
|
// table: `configurable` says what Settings may list, not what the
|
||||||
|
// list may render.
|
||||||
|
expect(
|
||||||
|
CONFIGURABLE_COLUMN_IDS.filter((id) => COLUMN_DEFS[id] === undefined),
|
||||||
|
).toEqual([]);
|
||||||
|
expect(CONFIGURABLE_COLUMN_IDS).not.toContain('titleArtist');
|
||||||
|
expect(COLUMN_DEFS['titleArtist'], 'the phone still has its column')
|
||||||
|
.toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
+7
-3
@@ -38,10 +38,14 @@ pre-commit:
|
|||||||
glob: "*.go"
|
glob: "*.go"
|
||||||
run: ./scripts/bindings-check.sh
|
run: ./scripts/bindings-check.sh
|
||||||
|
|
||||||
# .pi/ documents make targets; a stale one sends an agent off a
|
# The docs document make targets; a stale one sends an agent — or a
|
||||||
# cliff with total confidence. Instant.
|
# contributor reading CONTRIBUTING.md — off a cliff with total
|
||||||
|
# confidence. The glob is the script's own scanned set, because a
|
||||||
|
# hook that does not fire on a file the check reads is the drift the
|
||||||
|
# check exists to prevent: it was `{Makefile,.pi/**/*.md}` while the
|
||||||
|
# script already read CLAUDE.md. Instant.
|
||||||
skill-check:
|
skill-check:
|
||||||
glob: "{Makefile,.pi/**/*.md}"
|
glob: "{Makefile,.pi/**/*.md,AGENTS.md,CLAUDE.md,README.md,CONTRIBUTING.md}"
|
||||||
run: ./scripts/skill-check.sh
|
run: ./scripts/skill-check.sh
|
||||||
|
|
||||||
frontend-typecheck:
|
frontend-typecheck:
|
||||||
|
|||||||
+21
-4
@@ -14,6 +14,11 @@
|
|||||||
# missing: CLAUDE.md names 27 targets and nothing verified one of them,
|
# missing: CLAUDE.md names 27 targets and nothing verified one of them,
|
||||||
# so the file the agents trust most was the file least checked.
|
# so the file the agents trust most was the file least checked.
|
||||||
#
|
#
|
||||||
|
# README.md and CONTRIBUTING.md are in it too, and the header sentence
|
||||||
|
# above is why: a person who has *not* read the Makefile goes looking in
|
||||||
|
# the contributor-facing doc, so a renamed target sends them off the
|
||||||
|
# same cliff it sends an agent off. CONTRIBUTING.md names 21 targets.
|
||||||
|
#
|
||||||
# **AGENTS.md is a symlink to CLAUDE.md.** This repo is worked on by
|
# **AGENTS.md is a symlink to CLAUDE.md.** This repo is worked on by
|
||||||
# two agent harnesses that read different files by convention — Claude
|
# two agent harnesses that read different files by convention — Claude
|
||||||
# Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
|
# Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
|
||||||
@@ -43,7 +48,19 @@ if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
[ -d .pi ] || exit 0
|
# The scan is over the docs that are actually there: a checkout without
|
||||||
|
# .pi/ still has README.md and CONTRIBUTING.md to check, and gating the
|
||||||
|
# whole run on .pi/ would have made the human-facing half conditional on
|
||||||
|
# the agent-facing one. This list is used twice — once to read the
|
||||||
|
# mentions out and once to say which file a missing target came from —
|
||||||
|
# because a second list is a second thing to forget.
|
||||||
|
# `ls` exits non-zero when *any* of its arguments is missing while still
|
||||||
|
# printing the ones that are there, and under `set -e` that would sink
|
||||||
|
# the assignment rather than scanning what exists, so swallow it.
|
||||||
|
docs="$({ find .pi -name '*.md' 2>/dev/null
|
||||||
|
ls CLAUDE.md README.md CONTRIBUTING.md 2>/dev/null || true; })"
|
||||||
|
|
||||||
|
[ -n "$docs" ] || exit 0
|
||||||
|
|
||||||
# `make -pq` prints the database including every rule, without running
|
# `make -pq` prints the database including every rule, without running
|
||||||
# anything. It exits non-zero when a target is out of date, and under
|
# anything. It exits non-zero when a target is out of date, and under
|
||||||
@@ -68,7 +85,7 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
|
|||||||
# AGENTS.md is deliberately not in this list: it is a symlink to
|
# AGENTS.md is deliberately not in this list: it is a symlink to
|
||||||
# CLAUDE.md, asserted above, so scanning it would report every failure
|
# CLAUDE.md, asserted above, so scanning it would report every failure
|
||||||
# twice under two names.
|
# twice under two names.
|
||||||
mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
|
mentioned="$(printf '%s\n' "$docs" |
|
||||||
xargs awk '
|
xargs awk '
|
||||||
FNR == 1 { fence = 0 }
|
FNR == 1 { fence = 0 }
|
||||||
/^```/ { fence = !fence; next }
|
/^```/ { fence = !fence; next }
|
||||||
@@ -93,10 +110,10 @@ for t in $mentioned; do
|
|||||||
done
|
done
|
||||||
|
|
||||||
if [ -n "$missing" ]; then
|
if [ -n "$missing" ]; then
|
||||||
echo "skill-check: the agent docs name make targets that do not exist:" >&2
|
echo "skill-check: the docs name make targets that do not exist:" >&2
|
||||||
for t in $missing; do
|
for t in $missing; do
|
||||||
echo " make $t" >&2
|
echo " make $t" >&2
|
||||||
grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/ /' >&2
|
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
|
||||||
done
|
done
|
||||||
echo "Fix the docs, or restore the target." >&2
|
echo "Fix the docs, or restore the target." >&2
|
||||||
exit 1
|
exit 1
|
||||||
|
|||||||
Reference in New Issue
Block a user