Compare commits
19
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
065a879190 | ||
|
|
89882b4863 | ||
|
|
18a08daa91 | ||
|
|
aa59773d22 | ||
|
|
a4777f26b6 | ||
|
|
92faa9741b | ||
|
|
bf0a53e64c | ||
|
|
7be4a02e31 | ||
|
|
ad9c25a5a2 | ||
|
|
a83a127e31 | ||
|
|
4b9114fd8d | ||
|
|
e049a71458 | ||
|
|
0c944f2382 | ||
|
|
75525b67e4 | ||
|
|
85768dc489 | ||
|
|
1a221a40d3 | ||
|
|
0821deb877 | ||
|
|
31ada14111 | ||
|
|
20139394f3 |
@@ -0,0 +1,107 @@
|
|||||||
|
name: Unclaim
|
||||||
|
|
||||||
|
# A `Closes #N` footer in a commit body closes the issue on merge — and
|
||||||
|
# leaves `Status/In Progress` on it, because Gitea's auto-close touches
|
||||||
|
# state and nothing else. So #100 was closed and simultaneously marked
|
||||||
|
# as being actively worked on, and `scripts/issue.sh close` (which does
|
||||||
|
# drop the label) is exactly the thing the footer exists to avoid
|
||||||
|
# calling.
|
||||||
|
#
|
||||||
|
# **This hooks the close, not the merge.** Stripping the label in the
|
||||||
|
# PR would work and would be a per-PR habit; habits are what the footer
|
||||||
|
# removed. `issues: [closed]` covers every path an issue can close by —
|
||||||
|
# the footer on merge, `issue.sh close`, someone clicking Close in the
|
||||||
|
# web UI — and asks nothing of anyone at any of them.
|
||||||
|
#
|
||||||
|
# **Reopening deliberately does not restore it.** Reopening says the
|
||||||
|
# work was not finished, not that somebody is at a keyboard doing it
|
||||||
|
# now; the claim gets re-made by whoever picks it up.
|
||||||
|
#
|
||||||
|
# **This is not instant, and should not be described as it.** The
|
||||||
|
# runner has capacity 1 and is shared with an index build that can hold
|
||||||
|
# it for three hours, so a label tweak can queue behind one. Stale for
|
||||||
|
# an afternoon beats stale forever, which is what it was.
|
||||||
|
#
|
||||||
|
# The audit that answers "is this still firing" stays in CLAUDE.md and
|
||||||
|
# is one command:
|
||||||
|
#
|
||||||
|
# ./scripts/issue.sh list --state closed --label "Status/In Progress"
|
||||||
|
#
|
||||||
|
# A workflow that silently stops working is the failure mode this whole
|
||||||
|
# area has already produced once.
|
||||||
|
|
||||||
|
on:
|
||||||
|
issues:
|
||||||
|
types: [closed]
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
unclaim:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
container:
|
||||||
|
image: ubuntu:24.04
|
||||||
|
|
||||||
|
steps:
|
||||||
|
- name: Drop the claim label
|
||||||
|
# **Inside a container the act runner selects `sh`, not bash**, so
|
||||||
|
# `set -o pipefail` fails the job on its second line with "Illegal
|
||||||
|
# option" and the step never reaches the API. `homebrew-formula.yml`
|
||||||
|
# carries the same `set -euo pipefail` without trouble because it
|
||||||
|
# runs with **no container**, on the host image where bash is the
|
||||||
|
# default — so "another workflow does it" is not evidence here.
|
||||||
|
shell: bash
|
||||||
|
env:
|
||||||
|
# The automatic Actions token, as release.yml uses for the
|
||||||
|
# floor tag. It needs no more than write access to this repo.
|
||||||
|
TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||||
|
API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
|
||||||
|
ISSUE: ${{ github.event.issue.number }}
|
||||||
|
run: |
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
# `ca-certificates` is named because `--no-install-recommends`
|
||||||
|
# skips it, and `ubuntu:24.04` ships no CA bundle of its own —
|
||||||
|
# so curl comes up unable to verify TLS against our own Gitea
|
||||||
|
# and fails with "error setting certificate file" (exit 77).
|
||||||
|
# Every other containerised workflow here spells it out for the
|
||||||
|
# same reason; this one did not, and cost a release cycle.
|
||||||
|
apt-get update -qq
|
||||||
|
apt-get install -y -qq --no-install-recommends \
|
||||||
|
ca-certificates curl jq >/dev/null
|
||||||
|
|
||||||
|
label_id=$(
|
||||||
|
curl -sSf -H "Authorization: token $TOKEN" "$API/labels?limit=100" |
|
||||||
|
jq -r '.[] | select(.name == "Status/In Progress") | .id'
|
||||||
|
)
|
||||||
|
|
||||||
|
# The label not existing is a repo somebody reorganised, not a
|
||||||
|
# failure of this run — say so and stop, rather than failing a
|
||||||
|
# job on every close from then on.
|
||||||
|
if [ -z "$label_id" ]; then
|
||||||
|
echo "unclaim: no 'Status/In Progress' label in this repo; nothing to do"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
# DELETE is idempotent here: an issue that never carried the
|
||||||
|
# label answers the same as one that did, which is what makes
|
||||||
|
# this safe to run on *every* close rather than only the ones
|
||||||
|
# that were claimed.
|
||||||
|
# The body is captured, not discarded, so a refusal is
|
||||||
|
# diagnosable from this log alone. Whether the automatic
|
||||||
|
# token carries issue-write scope is still unproven, and
|
||||||
|
# "DELETE returned 403" without Gitea's own sentence costs
|
||||||
|
# another merge to find out which of the two it is.
|
||||||
|
body=$(mktemp)
|
||||||
|
code=$(
|
||||||
|
curl -sS -o "$body" -w '%{http_code}' -X DELETE \
|
||||||
|
-H "Authorization: token $TOKEN" \
|
||||||
|
"$API/issues/$ISSUE/labels/$label_id"
|
||||||
|
)
|
||||||
|
|
||||||
|
case "$code" in
|
||||||
|
204) echo "unclaim: #$ISSUE is closed and unclaimed" ;;
|
||||||
|
*)
|
||||||
|
echo "unclaim: DELETE returned $code for #$ISSUE" >&2
|
||||||
|
cat "$body" >&2
|
||||||
|
exit 1
|
||||||
|
;;
|
||||||
|
esac
|
||||||
-118
@@ -1,118 +0,0 @@
|
|||||||
# Work log
|
|
||||||
|
|
||||||
Temporal memory: what happened and what's next. Structure lives in
|
|
||||||
`CLAUDE.md`, operational instructions in `.pi/skills/yellowjacket-dev/`,
|
|
||||||
measured discoveries in `.planning/NOTES.md`. Don't duplicate those here.
|
|
||||||
|
|
||||||
## Current state
|
|
||||||
|
|
||||||
Plan 005 (agent development harness) is **complete — all seven
|
|
||||||
phases**. Everything from phase 1 onward is still **uncommitted**: one
|
|
||||||
large but coherent working-tree diff, nothing pushed.
|
|
||||||
|
|
||||||
All four tiers verified green from a cold, cleaned state:
|
|
||||||
`make ui-test` 313 passed, `make lint` 0 issues × 3 configurations,
|
|
||||||
`make test` green × 3 passes, `make e2e` 19 passed. Both CI jobs
|
|
||||||
verified green in a bare `ubuntu:24.04` container, including 19/19 on
|
|
||||||
WebKit.
|
|
||||||
|
|
||||||
**Committed and pushed** as `5ca6cad` (the harness) + `ccacd67` (a CI
|
|
||||||
fix), and **green on the real runner**: job `check` ~4 min, job `e2e`
|
|
||||||
~3 min with 19/19 chromium *and* 19/19 webkit. One commit rather than
|
|
||||||
seven because the working tree was the end state, not per-phase
|
|
||||||
snapshots — `Makefile`, `CLAUDE.md` and `lefthook.yml` are touched by
|
|
||||||
nearly every phase, so a split would have been fabricated history.
|
|
||||||
|
|
||||||
Still unverified, because no run has failed yet: the
|
|
||||||
`actions/upload-artifact` step (`continue-on-error`, so it cannot mask
|
|
||||||
a real failure) and whether pnpm honours `npm_config_store_dir` for
|
|
||||||
store caching. Worth checking the next time a spec legitimately fails.
|
|
||||||
|
|
||||||
- [ ] `gitea_ci`'s `job_logs` returns 404 on Gitea 1.27.1 — the endpoint
|
|
||||||
is not exposed. Logs come from the VPS instead: `zstdcat` the file
|
|
||||||
under `gitea/actions_log/<owner>/<repo>/<xx>/<task_id>.log.zst`,
|
|
||||||
and note `zstdcat` is not in the gitea container, so
|
|
||||||
`docker cp` it out first. Job status is `action_run_job.status`
|
|
||||||
(1 success, 2 failure, 4 skipped, 5 waiting, 6 running).
|
|
||||||
Probably belongs in the `gitea` skill, not here.
|
|
||||||
|
|
||||||
Open items deliberately not fixed: WAV tags are write-only
|
|
||||||
(`TestWAVTagsAreNotReadableYet`), `themeStore.loadFromBackend`'s failure
|
|
||||||
handler cannot recover, `backend/playlist` has no CRUD suite.
|
|
||||||
|
|
||||||
## Log
|
|
||||||
|
|
||||||
### 2026-08-11 — cold skill run, then phase 7 (CI)
|
|
||||||
|
|
||||||
- **Followed the skill cold first**, as the last session asked. It
|
|
||||||
works: app up from a wiped `.dev/`, an undocumented flow driven
|
|
||||||
(queue panel + shuffle, asserted on `QueueModeChanged`), stopped —
|
|
||||||
~1 minute, no dead ends. One real config bug: `outputDir` in
|
|
||||||
`.playwright/cli.config.json` resolves against **cwd**, not the
|
|
||||||
config file's directory (only `initScript` does that), so snapshots
|
|
||||||
were landing above the repo and a *stale* one from the previous
|
|
||||||
session answered `ls -t` instead. That cost a DOM walk to disprove a
|
|
||||||
regression that did not exist. Four smaller doc gaps fixed
|
|
||||||
(`sandbox-seed` already runs `testdata`; `ui-setup`/`e2e-setup` were
|
|
||||||
undocumented prerequisites; `snapshot` prints a path; `dev-stop`
|
|
||||||
leaves the browser open), plus `dev-headless.sh`'s own banner, which
|
|
||||||
was suggesting the bare `window.go` call its next paragraph warns
|
|
||||||
against.
|
|
||||||
- **Built both CI jobs as container scripts before writing any YAML**,
|
|
||||||
then transcribed the YAML back out and re-ran it to prove the
|
|
||||||
transcription. Push-and-see is a bad loop on a self-hosted runner.
|
|
||||||
- **It found a real bug immediately**: `make lint` omitted
|
|
||||||
`webkit2_41` on all three passes, so it was linting configurations
|
|
||||||
nothing builds. Invisible on Arch (which still ships
|
|
||||||
`webkit2gtk-4.0.pc`), fatal on Ubuntu 24.04. Tag sets now match
|
|
||||||
`make test`.
|
|
||||||
- **Both open decisions settled by measurement**: ALSA `null` PCM for
|
|
||||||
audio (no daemon; the elapsed clock really advances), dead-address
|
|
||||||
stub for the explore artifact (and setting it for the *app* run, not
|
|
||||||
just seeding, is worth 8x on suite wall clock). **WebKit is a
|
|
||||||
required step** — it had never been run anywhere, so one throwaway
|
|
||||||
container run replaced a coin flip with 19/19 at +11 s.
|
|
||||||
|
|
||||||
### 2026-08-10 — phase 6, pi affordances
|
|
||||||
|
|
||||||
- Added `.pi/skills/yellowjacket-dev/` as a directory rather than a flat
|
|
||||||
file: only the description is always in context, so `SKILL.md` stays
|
|
||||||
short enough that reading it whole is never a decision, and the deeper
|
|
||||||
material sits in `references/{harness,fixtures,ui-tier,schema-change}.md`.
|
|
||||||
- Settled the CLAUDE.md-vs-skill split **grammatically, not topically**,
|
|
||||||
because a topical split is what rots — every new fact gets two
|
|
||||||
plausible homes. Three docs, three tenses: NOTES.md is past
|
|
||||||
(measured, dated, append-only), CLAUDE.md is present (what the system
|
|
||||||
is), the skill is imperative (what to run). A new paragraph's tense
|
|
||||||
decides where it goes.
|
|
||||||
- The five gotchas (binding timeouts, first-run wizard, `pkill -f`,
|
|
||||||
seeds-by-running, WebKit-is-CI-only) went **inline in SKILL.md**, not
|
|
||||||
into a reference: you need them before the failure, not after.
|
|
||||||
- Trimmed CLAUDE.md's "Fixtures and the headless harness" section by
|
|
||||||
about half — the command sequences and gotchas it was carrying are now
|
|
||||||
the skill's, and leaving both would have created exactly the duplicate
|
|
||||||
description this repo has a standing rule against.
|
|
||||||
- Added `make skill-check` / `scripts/skill-check.sh` + a pre-commit
|
|
||||||
hook: every command in `.pi/**/*.md` must be a real `make` target, so
|
|
||||||
the Makefile stays the source of truth for invocation and a renamed
|
|
||||||
target fails a commit instead of misleading an agent later. Verified
|
|
||||||
it fails (it caught its own not-yet-created target) and passes.
|
|
||||||
- Added the `/e2e` prompt template: promoting a hand-driven
|
|
||||||
`playwright-cli` session into a spec is a transcription with four
|
|
||||||
fixed substitutions (refs → testids, sleeps → `waitForEvent`, raw
|
|
||||||
`window.go` → `callBinding`, short fixture → `LONG_TRACK`), plus three
|
|
||||||
runs — pass, pass again, pass after a DB restore — because the usual
|
|
||||||
failure is a spec depending on state the hand-driving left behind.
|
|
||||||
- One shell trap: under `set -euo pipefail`, `x="$(make -pqRr | …)"`
|
|
||||||
fails the whole assignment, because `make -q` exits non-zero when a
|
|
||||||
target is out of date and `pipefail` propagates it.
|
|
||||||
|
|
||||||
### Earlier
|
|
||||||
|
|
||||||
Phases 1–5 of plan 005: fixture generator and manifest, headless launch
|
|
||||||
and seeds, the event bridge + `data-testid` pass + `backend/testctl` +
|
|
||||||
`e2e/`, the Vitest component tier + `make bindings-check`, and the
|
|
||||||
`events.Emit` wrapper with its in-process service-event tests. Recaps
|
|
||||||
and the five "verified end to end" blocks are in
|
|
||||||
`.planning/plans/active/005-agent-development-harness.md`; the lessons
|
|
||||||
are in `.planning/NOTES.md`.
|
|
||||||
@@ -3483,3 +3483,54 @@ public tap.
|
|||||||
|
|
||||||
A guard added today does not protect a tag that points at yesterday. When
|
A guard added today does not protect a tag that points at yesterday. When
|
||||||
re-pointing a tag, check what the workflows looked like *there*.
|
re-pointing a tag, check what the workflows looked like *there*.
|
||||||
|
|
||||||
|
## A tag reader looks at exactly one spelling of "total" (measured 2026-08-18)
|
||||||
|
|
||||||
|
Writing #16's totals means matching the reader, which is
|
||||||
|
`dhowden/tag`, and it is narrower than the specs are:
|
||||||
|
|
||||||
|
- **Vorbis (FLAC, OGG): `TRACKTOTAL` and `DISCTOTAL` only.**
|
||||||
|
`vorbis.go`'s `Track()` reads `tracknumber` and `tracktotal` and
|
||||||
|
nothing else, so `TOTALTRACKS` — which several taggers write and
|
||||||
|
which xiph lists — and a `1/12` packed into `TRACKNUMBER` both read
|
||||||
|
back as *no total*. They write successfully. Nothing errors.
|
||||||
|
- **ID3v2 (MP3): `TRCK`/`TPOS` as `n/N`**, via `parseXofN`. That is one
|
||||||
|
frame carrying two facts, which is why `applyPositionFrame` reads the
|
||||||
|
existing frame before writing either half.
|
||||||
|
- **WAV: nothing at all.** There is no RIFF reader in the module, so a
|
||||||
|
WAV's `id3 ` chunk is invisible to `metadata.ExtractTags` — every
|
||||||
|
field, not just the totals. Filed as #104.
|
||||||
|
|
||||||
|
The general shape, and the reason this is written down: a tag written
|
||||||
|
under a name the reader does not look at is indistinguishable from one
|
||||||
|
never written. So the tests assert the round trip through
|
||||||
|
`metadata.ExtractTags` — the reader the *scan* uses — rather than
|
||||||
|
through the bytes the writer produced.
|
||||||
|
|
||||||
|
## The published catalog artifact predates `total_tracks` (measured 2026-08-18)
|
||||||
|
|
||||||
|
```
|
||||||
|
$ curl -sSI .../generic/yellowjacket-core-index/latest/core-index.db.zst
|
||||||
|
last-modified: Mon, 10 Aug 2026 04:38:16 GMT
|
||||||
|
content-length: 75417037
|
||||||
|
|
||||||
|
$ sqlite3 core-index.db \
|
||||||
|
"SELECT COUNT(*) FROM pragma_table_info('explore_index') WHERE name='total_tracks';"
|
||||||
|
0
|
||||||
|
$ sqlite3 core-index.db "SELECT COUNT(*) FROM explore_index;"
|
||||||
|
1079667
|
||||||
|
```
|
||||||
|
|
||||||
|
The column landed in the schema on 2026-08-16; the artifact is from
|
||||||
|
08-10, and `index-artifact.yml` is a weekly cron, not a push trigger.
|
||||||
|
So `completenessAnswer()`'s catalog fallback answers 0 for **every**
|
||||||
|
user today — the machinery is correct and `artifactHasTotals()` is
|
||||||
|
doing precisely its job, there is just no data behind it. Same position
|
||||||
|
the credit tables are in; both ride on the next publish (#88).
|
||||||
|
|
||||||
|
The general point, which is why this is written down rather than just
|
||||||
|
fixed: **a probe that makes a column optional also makes its absence
|
||||||
|
silent.** `artifactHasTotals` and `artifactHasCredits` are both correct
|
||||||
|
and both mean a feature can ship, pass every test, and produce nothing
|
||||||
|
for anybody without a single failure anywhere. Checking the *published
|
||||||
|
file* is one query and is not implied by any tick in CI.
|
||||||
|
|||||||
@@ -53,14 +53,50 @@ reinvention:
|
|||||||
- **Hard blockers are real Gitea dependencies**, which render on the
|
- **Hard blockers are real Gitea dependencies**, which render on the
|
||||||
issue itself, and the blocked issue carries `Status/Blocked`.
|
issue itself, and the blocked issue carries `Status/Blocked`.
|
||||||
- **A PR body carries a commit-to-issue table, the verification
|
- **A PR body carries a commit-to-issue table, the verification
|
||||||
actually run, and a `Closes` list** — PR #83 is the shape.
|
actually run, and a `Closes` list** — PR #83 is the shape. That list
|
||||||
|
is for whoever reads the PR; what actually closes an issue is the
|
||||||
|
footer below.
|
||||||
|
|
||||||
**And the `Closes` list does not reliably close anything.** #83 listed
|
**The closing keyword goes in the commit body, one issue per line.**
|
||||||
ten and five of them stayed open, shipped in `main`, for a fortnight.
|
|
||||||
So closing is a step you take and check, not a keyword you trust:
|
```
|
||||||
`./scripts/issue.sh close <n>` after the merge, with a comment naming
|
docs: delete four documents that contradict the code
|
||||||
the commit that shipped it. `close` also drops `Status/In Progress`,
|
|
||||||
because a claim outlives the work if nothing takes the label off.
|
<body>
|
||||||
|
|
||||||
|
Closes #98
|
||||||
|
```
|
||||||
|
|
||||||
|
**Gitea parses commit messages that reach `main`; it does not parse the
|
||||||
|
PR body**, which only closes anything if the merge happens to copy it
|
||||||
|
into the merge commit. Both halves of that were measured. #83's merge
|
||||||
|
commit carried `Closes #9, #13, #14, …` and closed **five of ten** — a
|
||||||
|
comma list is partially matched. #93's merge commit body was one
|
||||||
|
`Reviewed-on:` trailer, so #92 stayed open behind a perfectly correct
|
||||||
|
`Closes` line in the PR description.
|
||||||
|
|
||||||
|
A footer costs nothing elsewhere: Conventional Commits allows one,
|
||||||
|
`scripts/commit-check.sh` only regexes the subject, and
|
||||||
|
semantic-release reads the type from the subject — so this changes no
|
||||||
|
release decision. The rule that the issue number stays out of the
|
||||||
|
**subject** is unaffected, and was never about the body.
|
||||||
|
|
||||||
|
**Check it anyway.** A squash, or a merge message edited by hand,
|
||||||
|
still drops the footer. `./scripts/issue.sh list --state open` after a
|
||||||
|
merge, looking for what you just shipped; `./scripts/issue.sh close
|
||||||
|
<n>` for whatever did not take, with a comment naming the commit.
|
||||||
|
|
||||||
|
**Unclaiming is automatic, and it is hooked to the close rather than
|
||||||
|
to the merge.** Gitea's auto-close changes state and nothing else, so a
|
||||||
|
footer left `Status/In Progress` on a closed issue — #100 was closed
|
||||||
|
and marked as being actively worked on at the same time.
|
||||||
|
`.gitea/workflows/unclaim.yml` runs on `issues: [closed]`, which covers
|
||||||
|
the footer, `issue.sh close` and a click in the web UI alike; stripping
|
||||||
|
the label in the PR instead would have been a per-PR habit, and habits
|
||||||
|
are what the footer removed. It is not instant — the runner has
|
||||||
|
capacity 1 — and reopening deliberately does not restore the label.
|
||||||
|
`./scripts/issue.sh list --state closed --label "Status/In Progress"`
|
||||||
|
is how you find out it has stopped firing.
|
||||||
|
|
||||||
## Planning
|
## Planning
|
||||||
|
|
||||||
@@ -1412,6 +1448,52 @@ is therefore **reported at runtime** to `window.__yjIconMisses` and
|
|||||||
drawn as a fallback — an e2e sweep asserts there are none — since a
|
drawn as a fallback — an e2e sweep asserts there are none — since a
|
||||||
missing icon used to be impossible, the CDN having had everything.
|
missing icon used to be impossible, the CDN having had everything.
|
||||||
|
|
||||||
|
**What each icon *means* is a second table, and it is
|
||||||
|
`utils/icon-language.ts`.** Bundling answers "does this name resolve";
|
||||||
|
nothing answered "does this name mean what the one next to it means",
|
||||||
|
and a wrong-but-real icon renders perfectly. So `plus` came to mean add
|
||||||
|
to the queue, add to a playlist, make a new playlist **and** you do not
|
||||||
|
own this — the first two *adjacent in the same context menu* — while
|
||||||
|
`list` meant the queue, the Playlists destination and adding to the
|
||||||
|
queue.
|
||||||
|
|
||||||
|
The rule the table is built on: **an icon names the noun it acts on,
|
||||||
|
not the verb.** "Add to queue" and "add to playlist" are one verb on
|
||||||
|
two nouns, so the noun is what has to differ — which is why adding to a
|
||||||
|
playlist wears the Playlists destination's own icon, and why the queue
|
||||||
|
got `bars-staggered` and stopped wearing Playlists'. `plus` keeps the
|
||||||
|
one meaning it is unambiguous about, making something that is not there
|
||||||
|
yet.
|
||||||
|
|
||||||
|
Four things about it are load-bearing:
|
||||||
|
|
||||||
|
- **The request toggle is one glyph in two weights**
|
||||||
|
(`regular/bookmark` → `solid/bookmark`), because two states of a
|
||||||
|
toggle have to read as each other's opposite and a plus against a
|
||||||
|
bookmark does not. The pair was *already in the app and already
|
||||||
|
right* on `explore-album-details`'s "Request this" button while the
|
||||||
|
badge forty pixels away showed a plus — `utils/library-status.ts`'s
|
||||||
|
fault one layer down, having made the two agree on what wanting means
|
||||||
|
and left them disagreeing on what it looks like.
|
||||||
|
- **Downloads keeps the solid bookmark, deliberately.** That is the
|
||||||
|
same word twice, not two words: the badge says "this is on your
|
||||||
|
list" and the nav item is that list.
|
||||||
|
- **`icon-language.test.ts` sweeps the source**, because the rule is
|
||||||
|
about every call site and checking one checks nothing — the same
|
||||||
|
shape as `TestNoDirectRuntimeEmits`. It reads every `src/**/*.ts` as
|
||||||
|
raw text and fails on a literal `name="plus"` or `icon: 'list'`
|
||||||
|
outside the table, and its **first assertion is that it read
|
||||||
|
anything at all**, since a sweep over an empty glob passes.
|
||||||
|
- **It also asserts every `ICON_*` is bundled**, which closes the loop
|
||||||
|
the runtime cannot: `bookmark-check` is Font Awesome **Pro** and sat
|
||||||
|
on `explore-artist-details`'s Follow button, drawn for every followed
|
||||||
|
artist as a circled question mark. `offline-icons.spec.ts` sweeps
|
||||||
|
`__yjIconMisses` and could not see it, because no spec had ever
|
||||||
|
followed an artist — the same fault `requested-badge.spec.ts` was
|
||||||
|
written for, one component over, still live. A name computed from
|
||||||
|
state was only checkable from the state; now it is checkable from the
|
||||||
|
table.
|
||||||
|
|
||||||
**An album page says how much of the album is yours.**
|
**An album page says how much of the album is yours.**
|
||||||
`explore-album-details` is a *catalog* page and there is no
|
`explore-album-details` is a *catalog* page and there is no
|
||||||
library-side album detail page at all, so the album on it may be
|
library-side album detail page at all, so the album on it may be
|
||||||
@@ -1511,11 +1593,55 @@ shape as the encoding probe beside it.
|
|||||||
|
|
||||||
What neither side can give is *which* tracks are missing, only how many
|
What neither side can give is *which* tracks are missing, only how many
|
||||||
— so an incomplete album still browses, and that is now the exception
|
— so an incomplete album still browses, and that is now the exception
|
||||||
rather than every album load. Two smaller consequences: existing databases
|
rather than every album load. One smaller consequence: existing databases
|
||||||
read "unknown" until a rescan repopulates the column (which degrades to
|
read "unknown" until a rescan repopulates the column, which degrades to
|
||||||
exactly the old behaviour, so nothing breaks), and our own `tagwriter`
|
exactly the old behaviour, so nothing breaks.
|
||||||
writes track and disc *numbers* but not totals, so autotagging a folder
|
|
||||||
currently degrades the field this rests on.
|
**And our own writers declare the total, because for a long time they
|
||||||
|
did not.** `tagwriter` wrote track and disc *numbers* and dropped the
|
||||||
|
totals, so autotagging an album actively **erased** the evidence this
|
||||||
|
rests on: the release became MBID-matched — a green tick — while the
|
||||||
|
field `GetAlbumCompleteness` reads stayed absent, which is exactly the
|
||||||
|
"2 of 10 tracks, reported as in your library" the report described.
|
||||||
|
`FieldTotalTracks` / `FieldTotalDiscs` are written by the autotag apply
|
||||||
|
pass and by the download importer, and `dbsync` persists the track
|
||||||
|
total to the row so the album page agrees with the file without waiting
|
||||||
|
for a rescan.
|
||||||
|
|
||||||
|
Five things about it are load-bearing, and four of them fail silently:
|
||||||
|
|
||||||
|
- **The total is per *disc*, not per release**, because that is what
|
||||||
|
the tag form declares and what `GetAlbumCompleteness` **sums** per
|
||||||
|
disc — a release total written on every file multiplies a two-disc
|
||||||
|
album's expectation by two, and no library can then satisfy it.
|
||||||
|
`backend/tagtotals` is that derivation, once, because the two callers
|
||||||
|
must not import each other or the writer.
|
||||||
|
- **The Vorbis names are `TRACKTOTAL` and `DISCTOTAL` and no other
|
||||||
|
spelling.** `dhowden/tag`'s Vorbis reader looks at exactly those two
|
||||||
|
keys, so a perfectly reasonable `TOTALTRACKS`, or a `1/12` inside
|
||||||
|
`TRACKNUMBER`, is written successfully and reads back as no total at
|
||||||
|
all. The tests assert the round trip through the reader the *scan*
|
||||||
|
uses rather than through the bytes, for that reason.
|
||||||
|
- **ID3's number and total share one frame**, so writing either alone
|
||||||
|
has to read the other off the existing tag or it silently discards
|
||||||
|
it. A total with no number is not written: `/12` is what a reader
|
||||||
|
parses as track 0.
|
||||||
|
- **The totals are written unconditionally, not on a diff.** The case
|
||||||
|
this exists for is a file that declares *no* total, which compares
|
||||||
|
equal to nothing and is exactly what a "only if it changed" guard
|
||||||
|
skips.
|
||||||
|
- **A single-track download must not be totalled.** A `RecordingMBID`
|
||||||
|
anchor resolves `Expected` to that one track, so the same code would
|
||||||
|
tag a track off a twelve-track album "1 of 1" — and a declared total
|
||||||
|
outranks the catalog total that would otherwise have answered
|
||||||
|
correctly. Confidently wrong is worse than absent here, which is the
|
||||||
|
same rule `Known` exists for.
|
||||||
|
|
||||||
|
One gap this did not close, and it is older: **`dhowden/tag` has no
|
||||||
|
RIFF reader**, so nothing the tag writer puts in a WAV's `id3 ` chunk
|
||||||
|
is visible to `metadata.ExtractTags` — not the totals and not the title
|
||||||
|
either. `wav_test.go` reads that chunk itself, which is why no test
|
||||||
|
ever noticed.
|
||||||
|
|
||||||
**The absence is what gets marked, not the presence.** The tracklist
|
**The absence is what gets marked, not the presence.** The tracklist
|
||||||
put a green tick against every owned track and a legend underneath
|
put a green tick against every owned track and a legend underneath
|
||||||
@@ -1543,6 +1669,32 @@ side-effect worth knowing: this is what finally makes `ownership()`
|
|||||||
say something true here, since counting the displayed tracklist of a
|
say something true here, since counting the displayed tracklist of a
|
||||||
library-only entry could only ever produce "9 of 9".
|
library-only entry could only ever produce "9 of 9".
|
||||||
|
|
||||||
|
**And it can be asked, because the rule alone reaches too few albums.**
|
||||||
|
That guard depends on two inputs the user does not control: the files
|
||||||
|
declaring a per-disc total, and the catalog's own `total_tracks`. Where
|
||||||
|
neither says — which is a great deal of any library, and *every* library
|
||||||
|
until an artifact carrying the column is published — a partly-owned
|
||||||
|
album showed only the tracks on disk with nothing to say the rest
|
||||||
|
existed. `renderTracklistScope()` is the explicit route: a
|
||||||
|
"Show the whole album" switch that flips the synthetic "Your Library"
|
||||||
|
entry between the local files and the release, which is the rendering
|
||||||
|
the page could already do and could only be *triggered* automatically.
|
||||||
|
|
||||||
|
Three things about it are load-bearing. **`showFullTracklist` is a
|
||||||
|
tri-state**, `null` meaning "follow the automatic rule": the rule is
|
||||||
|
right when it fires and the switch has to be able to agree with the page
|
||||||
|
it sits on rather than starting out contradicting it, which a plain
|
||||||
|
boolean would need recomputed every time the completeness answer moved
|
||||||
|
underneath it. **`fullReleaseCluster()` falls back to the
|
||||||
|
highest-scoring cluster**, because `findLibraryCluster` is a guess over
|
||||||
|
the `inLibrary` flags and returns *nothing* when none are set — which is
|
||||||
|
exactly the untagged library the switch exists for, so without the
|
||||||
|
fallback the control would be absent precisely where it is needed. And
|
||||||
|
**it is shown only where it can change what is on screen**: against the
|
||||||
|
library entry, with a release to switch to, and only when the two
|
||||||
|
tracklists differ — the same test the version dropdown answers, one
|
||||||
|
control over.
|
||||||
|
|
||||||
**A dropdown is only a choice if the choices differ.** The version
|
**A dropdown is only a choice if the choices differ.** The version
|
||||||
selector tested `versionEntries.length`, but a release group routinely
|
selector tested `versionEntries.length`, but a release group routinely
|
||||||
has several releases — reissues, regional pressings, a remaster — whose
|
has several releases — reissues, regional pressings, a remaster — whose
|
||||||
@@ -2150,10 +2302,11 @@ Pre-commit runs vet, lint, codegen check, and frontend typecheck in parallel. Pr
|
|||||||
|
|
||||||
## CI
|
## CI
|
||||||
|
|
||||||
Seven workflows in `.gitea/workflows/`. Five of them package and
|
Eight workflows in `.gitea/workflows/`. Five of them package and
|
||||||
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
|
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
|
||||||
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
|
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
|
||||||
those run at all; only `ci.yml` gates, and it is the one to look at when
|
those run at all; `unclaim.yml` is housekeeping on the tracker and
|
||||||
|
touches no code; only `ci.yml` gates, and it is the one to look at when
|
||||||
deciding whether a push was healthy.
|
deciding whether a push was healthy.
|
||||||
|
|
||||||
**`release.yml` is the entry point for all of it.** On every push to
|
**`release.yml` is the entry point for all of it.** On every push to
|
||||||
|
|||||||
@@ -106,5 +106,8 @@ make dev # run with hot-reload
|
|||||||
make build-prod # produce a release binary
|
make build-prod # produce a release binary
|
||||||
```
|
```
|
||||||
|
|
||||||
More detail for contributors lives in
|
More detail for contributors lives in [`CLAUDE.md`](./CLAUDE.md) — the
|
||||||
[`docs/dev/overview.md`](./docs/dev/overview.md) and [`CLAUDE.md`](./CLAUDE.md).
|
architecture, the conventions and the reasons behind them. What is
|
||||||
|
being worked on is [the issue
|
||||||
|
tracker](https://git.ljones.me/yonlu/yellowjacket/issues); #73 is the
|
||||||
|
roadmap.
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
|
|
||||||
"yellowjacket/backend/database/sql/sqlcgen"
|
"yellowjacket/backend/database/sql/sqlcgen"
|
||||||
|
"yellowjacket/backend/tagtotals"
|
||||||
)
|
)
|
||||||
|
|
||||||
// TagChanges mirrors tagwriter.TagChanges — redefined here so the
|
// TagChanges mirrors tagwriter.TagChanges — redefined here so the
|
||||||
@@ -28,6 +29,8 @@ const (
|
|||||||
FieldYear = "year"
|
FieldYear = "year"
|
||||||
FieldTrackNumber = "track_number"
|
FieldTrackNumber = "track_number"
|
||||||
FieldDiscNumber = "disc_number"
|
FieldDiscNumber = "disc_number"
|
||||||
|
FieldTotalTracks = "total_tracks"
|
||||||
|
FieldTotalDiscs = "total_discs"
|
||||||
FieldCoverArt = "cover_art"
|
FieldCoverArt = "cover_art"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -418,5 +421,32 @@ func buildChanges(
|
|||||||
changes[FieldDiscNumber] = track.DiscNumber
|
changes[FieldDiscNumber] = track.DiscNumber
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The totals are what says "2 of 10" rather than a bare tick, and
|
||||||
|
// dropping them here is what made autotagging an album *erase* the
|
||||||
|
// evidence: the release becomes MBID-matched while the field
|
||||||
|
// GetAlbumCompleteness reads stays absent.
|
||||||
|
//
|
||||||
|
// They are written unconditionally where the candidate has a
|
||||||
|
// tracklist, not only when they differ from the local value, because
|
||||||
|
// the common case is a file that declares no total at all -- which
|
||||||
|
// compares equal to nothing and would be skipped by a diff guard.
|
||||||
|
if tracks, discs := tagtotals.For(
|
||||||
|
candidatePositions(cand), track.DiscNumber,
|
||||||
|
); tracks > 0 {
|
||||||
|
changes[FieldTotalTracks] = tracks
|
||||||
|
changes[FieldTotalDiscs] = discs
|
||||||
|
}
|
||||||
|
|
||||||
return changes
|
return changes
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// candidatePositions is the candidate's tracklist as bare positions.
|
||||||
|
func candidatePositions(cand Candidate) []tagtotals.Position {
|
||||||
|
out := make([]tagtotals.Position, 0, len(cand.Tracks))
|
||||||
|
|
||||||
|
for _, t := range cand.Tracks {
|
||||||
|
out = append(out, tagtotals.Position{Disc: t.DiscNumber, Track: t.Position})
|
||||||
|
}
|
||||||
|
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,90 @@
|
|||||||
|
package autotag
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// Autotagging an album used to *erase* the evidence that says "2 of 10":
|
||||||
|
// the release became MBID-matched while the totals the files declared
|
||||||
|
// went unwritten, so the album page showed a plain tick. These pin the
|
||||||
|
// two halves of the fix that are easy to get wrong silently.
|
||||||
|
func TestBuildChanges_Totals(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
twoDiscs := Candidate{
|
||||||
|
Tracks: []CandidateTrack{
|
||||||
|
{DiscNumber: 1, Position: 1},
|
||||||
|
{DiscNumber: 1, Position: 2},
|
||||||
|
{DiscNumber: 2, Position: 1},
|
||||||
|
{DiscNumber: 2, Position: 2},
|
||||||
|
{DiscNumber: 2, Position: 3},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
cand Candidate
|
||||||
|
local LocalTrack
|
||||||
|
track CandidateTrack
|
||||||
|
wantTracks any
|
||||||
|
wantDiscs any
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
// The common case, and the one a diff guard would skip: the
|
||||||
|
// file declares no total at all, so the total "has not
|
||||||
|
// changed" and would never be written.
|
||||||
|
name: "a file with no total gets one",
|
||||||
|
cand: Candidate{Tracks: []CandidateTrack{
|
||||||
|
{Position: 1}, {Position: 2}, {Position: 3},
|
||||||
|
}},
|
||||||
|
local: LocalTrack{TrackNumber: 1},
|
||||||
|
track: CandidateTrack{Position: 1},
|
||||||
|
wantTracks: 3,
|
||||||
|
wantDiscs: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// 5 here would be the release's track count. Summed once
|
||||||
|
// per disc by GetAlbumCompleteness that claims a ten-track
|
||||||
|
// expectation for a five-track album, which no library can
|
||||||
|
// ever satisfy.
|
||||||
|
name: "a multi-disc release totals the track's own disc",
|
||||||
|
cand: twoDiscs,
|
||||||
|
local: LocalTrack{},
|
||||||
|
track: CandidateTrack{DiscNumber: 2, Position: 1},
|
||||||
|
wantTracks: 3,
|
||||||
|
wantDiscs: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "the other disc gets its own total",
|
||||||
|
cand: twoDiscs,
|
||||||
|
local: LocalTrack{},
|
||||||
|
track: CandidateTrack{DiscNumber: 1, Position: 1},
|
||||||
|
wantTracks: 2,
|
||||||
|
wantDiscs: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// A candidate with no tracklist knows nothing, and writing
|
||||||
|
// a zero would claim it did.
|
||||||
|
name: "a candidate with no tracklist writes no total",
|
||||||
|
cand: Candidate{},
|
||||||
|
local: LocalTrack{},
|
||||||
|
track: CandidateTrack{Position: 1},
|
||||||
|
wantTracks: nil,
|
||||||
|
wantDiscs: nil,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
changes := buildChanges(tc.local, tc.cand, tc.track)
|
||||||
|
|
||||||
|
if got := changes[FieldTotalTracks]; got != tc.wantTracks {
|
||||||
|
t.Errorf("%s: got %v, want %v", FieldTotalTracks, got, tc.wantTracks)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := changes[FieldTotalDiscs]; got != tc.wantDiscs {
|
||||||
|
t.Errorf("%s: got %v, want %v", FieldTotalDiscs, got, tc.wantDiscs)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,38 @@
|
|||||||
|
package autotagservice
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/autotag"
|
||||||
|
"yellowjacket/backend/tagwriter"
|
||||||
|
)
|
||||||
|
|
||||||
|
// twAdapter passes the diff map through unchanged, so autotag's field
|
||||||
|
// constants and tagwriter's are the same keys written down twice --
|
||||||
|
// deliberately, to keep autotag out of the write pipeline's import
|
||||||
|
// graph. A key that drifts does not fail to compile and does not fail
|
||||||
|
// to write: the writer simply finds no entry under the name it looks
|
||||||
|
// for, and the field is silently dropped. That is what this pins, and
|
||||||
|
// this package is the one place that imports both.
|
||||||
|
func TestAutotagAndTagwriterAgreeOnFieldNames(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
pairs := map[string][2]string{
|
||||||
|
"title": {autotag.FieldTitle, tagwriter.FieldTitle},
|
||||||
|
"artist": {autotag.FieldArtist, tagwriter.FieldArtist},
|
||||||
|
"album": {autotag.FieldAlbum, tagwriter.FieldAlbum},
|
||||||
|
"album artist": {autotag.FieldAlbumArtist, tagwriter.FieldAlbumArtist},
|
||||||
|
"year": {autotag.FieldYear, tagwriter.FieldYear},
|
||||||
|
"track number": {autotag.FieldTrackNumber, tagwriter.FieldTrackNumber},
|
||||||
|
"disc number": {autotag.FieldDiscNumber, tagwriter.FieldDiscNumber},
|
||||||
|
"total tracks": {autotag.FieldTotalTracks, tagwriter.FieldTotalTracks},
|
||||||
|
"total discs": {autotag.FieldTotalDiscs, tagwriter.FieldTotalDiscs},
|
||||||
|
"cover art": {autotag.FieldCoverArt, tagwriter.FieldCoverArt},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, pair := range pairs {
|
||||||
|
if pair[0] != pair[1] {
|
||||||
|
t.Errorf("%s: autotag says %q, tagwriter says %q", name, pair[0], pair[1])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -12,6 +12,7 @@ import (
|
|||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
"yellowjacket/backend/tagtotals"
|
||||||
"yellowjacket/backend/tagwriter"
|
"yellowjacket/backend/tagwriter"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -275,6 +276,25 @@ func (i *Importer) tagFile(p plannedFile, dl Download) error {
|
|||||||
changes[tagwriter.FieldDiscNumber] = p.Track.DiscNumber
|
changes[tagwriter.FieldDiscNumber] = p.Track.DiscNumber
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An imported file should arrive knowing how much of the album it
|
||||||
|
// is one of, or the album reads as "in your library" from its first
|
||||||
|
// imported track onward.
|
||||||
|
//
|
||||||
|
// A *track* download is the case this must not touch: a
|
||||||
|
// RecordingMBID anchor resolves Expected to exactly that one track,
|
||||||
|
// so totalling it would write "1 of 1" onto a track off a
|
||||||
|
// twelve-track album -- a confident lie, and one that outranks the
|
||||||
|
// catalog's own total, which is the fallback that would otherwise
|
||||||
|
// have answered correctly.
|
||||||
|
if dl.RecordingMBID == "" {
|
||||||
|
if tracks, discs := tagtotals.For(
|
||||||
|
expectedPositions(dl.Expected), p.Track.DiscNumber,
|
||||||
|
); tracks > 0 {
|
||||||
|
changes[tagwriter.FieldTotalTracks] = tracks
|
||||||
|
changes[tagwriter.FieldTotalDiscs] = discs
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if err := i.tags.WriteUntrackedFileTags(p.Source, changes); err != nil {
|
if err := i.tags.WriteUntrackedFileTags(p.Source, changes); err != nil {
|
||||||
return fmt.Errorf("write tags: %w", err)
|
return fmt.Errorf("write tags: %w", err)
|
||||||
}
|
}
|
||||||
@@ -282,6 +302,18 @@ func (i *Importer) tagFile(p plannedFile, dl Download) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// expectedPositions is the download's resolved tracklist as bare
|
||||||
|
// positions.
|
||||||
|
func expectedPositions(expected []ExpectedTrack) []tagtotals.Position {
|
||||||
|
out := make([]tagtotals.Position, 0, len(expected))
|
||||||
|
|
||||||
|
for _, t := range expected {
|
||||||
|
out = append(out, tagtotals.Position{Disc: t.DiscNumber, Track: t.Position})
|
||||||
|
}
|
||||||
|
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
// destinationFor computes a file's library path from the template.
|
// destinationFor computes a file's library path from the template.
|
||||||
func (i *Importer) destinationFor(
|
func (i *Importer) destinationFor(
|
||||||
p plannedFile,
|
p plannedFile,
|
||||||
|
|||||||
@@ -446,3 +446,77 @@ func keysOf(m map[string]tagwriter.TagChanges) []string {
|
|||||||
|
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An imported album should arrive knowing its own size, or the album
|
||||||
|
// page reads "in your library" from its first imported track onward --
|
||||||
|
// which is the badge complaint this exists to answer.
|
||||||
|
func TestImportWritesTheAlbumTotals(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
f := newImportFixture(t,
|
||||||
|
"01 - Airbag.flac",
|
||||||
|
"02 - Paranoid Android.flac",
|
||||||
|
"03 - Subterranean Homesick Alien.flac",
|
||||||
|
"04 - Exit Music (For a Film).flac",
|
||||||
|
)
|
||||||
|
|
||||||
|
if _, err := f.importer.Import(
|
||||||
|
context.Background(),
|
||||||
|
fourTrackDownload(),
|
||||||
|
Result{Dir: f.dir, Files: f.files},
|
||||||
|
ImportOptions{LibraryRoot: f.root, WriteTags: true},
|
||||||
|
); err != nil {
|
||||||
|
t.Fatalf("Import: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
changes := f.tags.writes["01 - Airbag.flac"]
|
||||||
|
if changes == nil {
|
||||||
|
t.Fatal("no tag write recorded for the first track")
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := changes[tagwriter.FieldTotalTracks]; got != 4 {
|
||||||
|
t.Errorf("%s: got %v, want 4", tagwriter.FieldTotalTracks, got)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := changes[tagwriter.FieldTotalDiscs]; got != 1 {
|
||||||
|
t.Errorf("%s: got %v, want 1", tagwriter.FieldTotalDiscs, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A RecordingMBID anchor resolves Expected to exactly the one track it
|
||||||
|
// asked for, so totalling it would tag a track off a twelve-track album
|
||||||
|
// as "1 of 1" -- worse than saying nothing, because a declared total
|
||||||
|
// outranks the catalog total that would have answered correctly.
|
||||||
|
func TestImportWritesNoTotalsForATrackDownload(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
f := newImportFixture(t, "01 - Airbag.flac")
|
||||||
|
|
||||||
|
dl := Download{
|
||||||
|
ID: "dl-track",
|
||||||
|
LibraryID: 1,
|
||||||
|
RecordingMBID: "mbid-recording",
|
||||||
|
Artist: "Radiohead",
|
||||||
|
Album: "OK Computer",
|
||||||
|
Expected: []ExpectedTrack{{Position: 1, Title: "Airbag"}},
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err := f.importer.Import(
|
||||||
|
context.Background(),
|
||||||
|
dl,
|
||||||
|
Result{Dir: f.dir, Files: f.files},
|
||||||
|
ImportOptions{LibraryRoot: f.root, WriteTags: true},
|
||||||
|
); err != nil {
|
||||||
|
t.Fatalf("Import: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
changes := f.tags.writes["01 - Airbag.flac"]
|
||||||
|
if changes == nil {
|
||||||
|
t.Fatal("no tag write recorded")
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := changes[tagwriter.FieldTotalTracks]; ok {
|
||||||
|
t.Errorf("%s written for a single-track download: %v",
|
||||||
|
tagwriter.FieldTotalTracks, changes[tagwriter.FieldTotalTracks])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,56 @@
|
|||||||
|
// Package tagtotals derives the totals a tag's "5/12" form declares.
|
||||||
|
//
|
||||||
|
// It exists because the two writers that know a release's full
|
||||||
|
// tracklist -- the autotag apply pass and the download importer --
|
||||||
|
// must not import each other or the tag writer, and because getting
|
||||||
|
// the denominator wrong is invisible: a total that is too large marks
|
||||||
|
// a complete album incomplete forever, and nothing fails.
|
||||||
|
package tagtotals
|
||||||
|
|
||||||
|
// Position is one track's place in a release. A zero Disc means the
|
||||||
|
// release did not say, which is disc 1.
|
||||||
|
type Position struct {
|
||||||
|
Disc int
|
||||||
|
Track int
|
||||||
|
}
|
||||||
|
|
||||||
|
// For returns the totals to write on a file sitting on disc `disc`:
|
||||||
|
// how many tracks that disc has, and how many discs the release has.
|
||||||
|
//
|
||||||
|
// The track total is **per disc** and not the release's track count,
|
||||||
|
// because that is what the tag form means and what
|
||||||
|
// GetAlbumCompleteness sums -- summing a release total once per disc
|
||||||
|
// would multiply a two-disc album's expectation by two.
|
||||||
|
//
|
||||||
|
// Tracks are counted by distinct position rather than by row: a
|
||||||
|
// tracklist that lists a position twice is a defect in the source, and
|
||||||
|
// counting it twice would put an album permanently out of reach of its
|
||||||
|
// own total.
|
||||||
|
func For(all []Position, disc int) (tracks, discs int) {
|
||||||
|
disc = normaliseDisc(disc)
|
||||||
|
|
||||||
|
seenTracks := make(map[int]struct{}, len(all))
|
||||||
|
seenDiscs := make(map[int]struct{}, 1)
|
||||||
|
|
||||||
|
for _, p := range all {
|
||||||
|
d := normaliseDisc(p.Disc)
|
||||||
|
seenDiscs[d] = struct{}{}
|
||||||
|
|
||||||
|
if d != disc || p.Track <= 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
seenTracks[p.Track] = struct{}{}
|
||||||
|
}
|
||||||
|
|
||||||
|
return len(seenTracks), len(seenDiscs)
|
||||||
|
}
|
||||||
|
|
||||||
|
// normaliseDisc treats an undeclared disc as disc 1.
|
||||||
|
func normaliseDisc(d int) int {
|
||||||
|
if d <= 0 {
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
|
return d
|
||||||
|
}
|
||||||
@@ -0,0 +1,92 @@
|
|||||||
|
package tagtotals_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/tagtotals"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFor(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
singleDisc := []tagtotals.Position{
|
||||||
|
{Disc: 0, Track: 1}, {Disc: 0, Track: 2}, {Disc: 0, Track: 3},
|
||||||
|
}
|
||||||
|
|
||||||
|
twoDiscs := []tagtotals.Position{
|
||||||
|
{Disc: 1, Track: 1},
|
||||||
|
{Disc: 1, Track: 2},
|
||||||
|
{Disc: 2, Track: 1},
|
||||||
|
{Disc: 2, Track: 2},
|
||||||
|
{Disc: 2, Track: 3},
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
all []tagtotals.Position
|
||||||
|
disc int
|
||||||
|
wantTracks int
|
||||||
|
wantDiscs int
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "a single-disc release totals its own tracks",
|
||||||
|
all: singleDisc, disc: 0, wantTracks: 3, wantDiscs: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// An undeclared disc is disc 1, on both sides of the
|
||||||
|
// question -- a file tagged "disc 1" and a tracklist that
|
||||||
|
// declares no disc describe the same disc.
|
||||||
|
name: "an undeclared disc is disc 1",
|
||||||
|
all: singleDisc, disc: 1, wantTracks: 3, wantDiscs: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// The whole point: 5 here would be the release's track
|
||||||
|
// count, which summed once per disc claims a ten-track
|
||||||
|
// expectation for a five-track album.
|
||||||
|
name: "a multi-disc release totals the file's own disc",
|
||||||
|
all: twoDiscs, disc: 2, wantTracks: 3, wantDiscs: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "the other disc gets its own total",
|
||||||
|
all: twoDiscs, disc: 1, wantTracks: 2, wantDiscs: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// A disc the tracklist does not mention cannot be totalled,
|
||||||
|
// and 0 is how the caller is told to write nothing.
|
||||||
|
name: "a disc with no tracks totals nothing",
|
||||||
|
all: twoDiscs, disc: 3, wantTracks: 0, wantDiscs: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "an empty tracklist totals nothing",
|
||||||
|
all: nil, disc: 1, wantTracks: 0, wantDiscs: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// A source that lists a position twice would otherwise put
|
||||||
|
// the album permanently one track short of its own total.
|
||||||
|
name: "a repeated position counts once",
|
||||||
|
all: []tagtotals.Position{
|
||||||
|
{Disc: 1, Track: 1}, {Disc: 1, Track: 1}, {Disc: 1, Track: 2},
|
||||||
|
},
|
||||||
|
disc: 1, wantTracks: 2, wantDiscs: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a track with no position is not counted",
|
||||||
|
all: []tagtotals.Position{
|
||||||
|
{Disc: 1, Track: 0}, {Disc: 1, Track: 1},
|
||||||
|
},
|
||||||
|
disc: 1, wantTracks: 1, wantDiscs: 1,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tracks, discs := tagtotals.For(tc.all, tc.disc)
|
||||||
|
if tracks != tc.wantTracks || discs != tc.wantDiscs {
|
||||||
|
t.Errorf("For(%v, %d) = (%d, %d), want (%d, %d)",
|
||||||
|
tc.all, tc.disc, tracks, discs, tc.wantTracks, tc.wantDiscs)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -183,6 +183,15 @@ func syncDatabase(
|
|||||||
discNum = toNullInt64(v)
|
discNum = toNullInt64(v)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The completeness evidence. Without this the row keeps whatever
|
||||||
|
// the last scan read while the file on disk now declares a total,
|
||||||
|
// so the album stays "unknown" until a full rescan -- which is the
|
||||||
|
// state the report describes.
|
||||||
|
totalTracks := old.TotalTracks
|
||||||
|
if v, ok := asInt(params.changes[FieldTotalTracks]); ok {
|
||||||
|
totalTracks = toNullInt64(v)
|
||||||
|
}
|
||||||
|
|
||||||
composer := old.Composer
|
composer := old.Composer
|
||||||
if v, ok := params.changes[FieldComposer].(string); ok {
|
if v, ok := params.changes[FieldComposer].(string); ok {
|
||||||
composer = v
|
composer = v
|
||||||
@@ -207,7 +216,7 @@ func syncDatabase(
|
|||||||
AlbumID: albumID,
|
AlbumID: albumID,
|
||||||
TrackNumber: trackNum,
|
TrackNumber: trackNum,
|
||||||
DiscNumber: discNum,
|
DiscNumber: discNum,
|
||||||
TotalTracks: old.TotalTracks,
|
TotalTracks: totalTracks,
|
||||||
Year: year,
|
Year: year,
|
||||||
Composer: composer,
|
Composer: composer,
|
||||||
Comment: old.Comment,
|
Comment: old.Comment,
|
||||||
|
|||||||
@@ -101,6 +101,11 @@ func applyFlacTextChanges(cmt *flacvorbis.MetaDataBlockVorbisComment, changes Ta
|
|||||||
{FieldYear, flacvorbis.FIELD_DATE, true},
|
{FieldYear, flacvorbis.FIELD_DATE, true},
|
||||||
{FieldTrackNumber, flacvorbis.FIELD_TRACKNUMBER, true},
|
{FieldTrackNumber, flacvorbis.FIELD_TRACKNUMBER, true},
|
||||||
{FieldDiscNumber, "DISCNUMBER", true},
|
{FieldDiscNumber, "DISCNUMBER", true},
|
||||||
|
// TRACKTOTAL/DISCTOTAL and no other spelling: dhowden/tag's
|
||||||
|
// Vorbis reader looks at exactly these two keys, so TOTALTRACKS
|
||||||
|
// or a "1/12" inside TRACKNUMBER reads back as no total at all.
|
||||||
|
{FieldTotalTracks, "TRACKTOTAL", true},
|
||||||
|
{FieldTotalDiscs, "DISCTOTAL", true},
|
||||||
{FieldComposer, "COMPOSER", false},
|
{FieldComposer, "COMPOSER", false},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+63
-11
@@ -6,6 +6,7 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
|
||||||
id3v2 "github.com/bogem/id3v2/v2"
|
id3v2 "github.com/bogem/id3v2/v2"
|
||||||
|
|
||||||
@@ -66,17 +67,10 @@ func applyTextChanges(tag *id3v2.Tag, changes TagChanges) {
|
|||||||
tag.SetYear(strconv.Itoa(v))
|
tag.SetYear(strconv.Itoa(v))
|
||||||
}
|
}
|
||||||
|
|
||||||
if v, ok := asInt(changes[FieldTrackNumber]); ok {
|
applyPositionFrame(tag, "Track number/Position in set", changes,
|
||||||
trckID := tag.CommonID("Track number/Position in set")
|
FieldTrackNumber, FieldTotalTracks)
|
||||||
tag.DeleteFrames(trckID)
|
applyPositionFrame(tag, "Part of a set", changes,
|
||||||
tag.AddTextFrame(trckID, id3v2.EncodingUTF8, strconv.Itoa(v))
|
FieldDiscNumber, FieldTotalDiscs)
|
||||||
}
|
|
||||||
|
|
||||||
if v, ok := asInt(changes[FieldDiscNumber]); ok {
|
|
||||||
tposID := tag.CommonID("Part of a set")
|
|
||||||
tag.DeleteFrames(tposID)
|
|
||||||
tag.AddTextFrame(tposID, id3v2.EncodingUTF8, strconv.Itoa(v))
|
|
||||||
}
|
|
||||||
|
|
||||||
if v, ok := changes[FieldComposer].(string); ok {
|
if v, ok := changes[FieldComposer].(string); ok {
|
||||||
tag.DeleteFrames("TCOM")
|
tag.DeleteFrames("TCOM")
|
||||||
@@ -90,6 +84,64 @@ func applyTextChanges(tag *id3v2.Tag, changes TagChanges) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// applyPositionFrame writes an ID3v2 position frame (TRCK or TPOS) in
|
||||||
|
// the "n/N" form the readers parse.
|
||||||
|
//
|
||||||
|
// The number and the total are separate diff entries and either may be
|
||||||
|
// absent, so the frame's *existing* value is the base: writing a total
|
||||||
|
// alone must not discard the number that is already there, and writing
|
||||||
|
// a number alone must not discard a total the file already declared.
|
||||||
|
// A total with no number at all is not written, since "/12" says
|
||||||
|
// nothing a reader can use.
|
||||||
|
func applyPositionFrame(
|
||||||
|
tag *id3v2.Tag, description string, changes TagChanges, numKey, totalKey string,
|
||||||
|
) {
|
||||||
|
_, hasNum := changes[numKey]
|
||||||
|
_, hasTotal := changes[totalKey]
|
||||||
|
|
||||||
|
if !hasNum && !hasTotal {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
frameID := tag.CommonID(description)
|
||||||
|
|
||||||
|
num, total := parseXofN(
|
||||||
|
strings.TrimRight(tag.GetTextFrame(frameID).Text, "\x00 \t\n\r"),
|
||||||
|
)
|
||||||
|
|
||||||
|
if v, ok := asInt(changes[numKey]); ok {
|
||||||
|
num = v
|
||||||
|
}
|
||||||
|
|
||||||
|
if v, ok := asInt(changes[totalKey]); ok {
|
||||||
|
total = v
|
||||||
|
}
|
||||||
|
|
||||||
|
if num <= 0 {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
value := strconv.Itoa(num)
|
||||||
|
if total > 0 {
|
||||||
|
value += "/" + strconv.Itoa(total)
|
||||||
|
}
|
||||||
|
|
||||||
|
tag.DeleteFrames(frameID)
|
||||||
|
tag.AddTextFrame(frameID, id3v2.EncodingUTF8, value)
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseXofN splits an ID3v2 "n/N" position value. A bare "n" yields a
|
||||||
|
// zero total, and anything unparseable yields zeros — the same reading
|
||||||
|
// dhowden/tag gives the frame.
|
||||||
|
func parseXofN(s string) (int, int) {
|
||||||
|
numText, totalText, _ := strings.Cut(s, "/")
|
||||||
|
|
||||||
|
num, _ := strconv.Atoi(strings.TrimSpace(numText))
|
||||||
|
total, _ := strconv.Atoi(strings.TrimSpace(totalText))
|
||||||
|
|
||||||
|
return num, total
|
||||||
|
}
|
||||||
|
|
||||||
// applyCoverArtChanges handles the FieldCoverArt entry in the diff map.
|
// applyCoverArtChanges handles the FieldCoverArt entry in the diff map.
|
||||||
//
|
//
|
||||||
// - []byte with len > 0: embed the given image as front cover.
|
// - []byte with len > 0: embed the given image as front cover.
|
||||||
|
|||||||
@@ -166,6 +166,8 @@ var oggFieldMappings = []struct { //nolint:gochecknoglobals // field mapping tab
|
|||||||
{FieldYear, "DATE", true},
|
{FieldYear, "DATE", true},
|
||||||
{FieldTrackNumber, "TRACKNUMBER", true},
|
{FieldTrackNumber, "TRACKNUMBER", true},
|
||||||
{FieldDiscNumber, "DISCNUMBER", true},
|
{FieldDiscNumber, "DISCNUMBER", true},
|
||||||
|
{FieldTotalTracks, "TRACKTOTAL", true},
|
||||||
|
{FieldTotalDiscs, "DISCTOTAL", true},
|
||||||
{FieldComposer, "COMPOSER", false},
|
{FieldComposer, "COMPOSER", false},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -333,3 +333,31 @@ func TestWriteTrackTags_DBSync(t *testing.T) {
|
|||||||
t.Error("expected FTS5 result for 'New Title'")
|
t.Error("expected FTS5 result for 'New Title'")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The row is what the album page reads, and it is only refreshed by a
|
||||||
|
// scan. Leaving total_tracks at whatever the last scan saw means an
|
||||||
|
// album autotagged just now stays "unknown" -- a plain tick on an album
|
||||||
|
// the user holds two tracks of -- until a full rescan happens to run.
|
||||||
|
func TestWriteTrackTags_PersistsTheTotal(t *testing.T) {
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
dir := t.TempDir()
|
||||||
|
trackID := seedTestTrack(t, db, createPipelineTestMP3(t, dir))
|
||||||
|
|
||||||
|
tw := NewTagWriter(testLogger(), db, &mockPlayer{}, &mockPipelineLocker{})
|
||||||
|
|
||||||
|
if err := tw.WriteTrackTags(trackID, TagChanges{
|
||||||
|
FieldTrackNumber: 2,
|
||||||
|
FieldTotalTracks: 10,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("WriteTrackTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
af, err := db.Queries.GetAudioFile(context.Background(), trackID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("get audio file: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !af.TotalTracks.Valid || af.TotalTracks.Int64 != 10 {
|
||||||
|
t.Errorf("total_tracks: got %v, want 10", af.TotalTracks)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -26,6 +26,15 @@ const (
|
|||||||
FieldDiscNumber = "disc_number"
|
FieldDiscNumber = "disc_number"
|
||||||
FieldComposer = "composer"
|
FieldComposer = "composer"
|
||||||
FieldCoverArt = "cover_art" // []byte for set, nil for clear
|
FieldCoverArt = "cover_art" // []byte for set, nil for clear
|
||||||
|
|
||||||
|
// FieldTotalTracks is how many tracks are on *this file's disc*, not
|
||||||
|
// in the whole release. That is what the "5/12" form declares and
|
||||||
|
// what GetAlbumCompleteness sums per disc; a release total written
|
||||||
|
// here would multiply the expectation by the number of discs.
|
||||||
|
FieldTotalTracks = "total_tracks"
|
||||||
|
|
||||||
|
// FieldTotalDiscs is how many discs the release has.
|
||||||
|
FieldTotalDiscs = "total_discs"
|
||||||
)
|
)
|
||||||
|
|
||||||
// AudioFormat represents a supported audio file format.
|
// AudioFormat represents a supported audio file format.
|
||||||
|
|||||||
@@ -0,0 +1,199 @@
|
|||||||
|
package tagwriter
|
||||||
|
|
||||||
|
import (
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/metadata"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The totals are the evidence GetAlbumCompleteness reads, and every way
|
||||||
|
// of getting them wrong is silent: a tag written under a name the
|
||||||
|
// reader does not look at reads back as no total at all, which is
|
||||||
|
// indistinguishable from never having written one. So these assert the
|
||||||
|
// round trip through the *reader the scan uses*, not the bytes.
|
||||||
|
//
|
||||||
|
// WAV is the exception and it is not this change's: dhowden/tag has no
|
||||||
|
// RIFF reader at all, so metadata.ExtractTags cannot see a WAV's ID3
|
||||||
|
// chunk -- which is why every other test here reads that chunk itself.
|
||||||
|
func TestWriteTotals_RoundTripsInEveryFormat(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
changes := TagChanges{
|
||||||
|
FieldTitle: "Some Song",
|
||||||
|
FieldTrackNumber: 2,
|
||||||
|
FieldTotalTracks: 10,
|
||||||
|
FieldDiscNumber: 1,
|
||||||
|
FieldTotalDiscs: 2,
|
||||||
|
}
|
||||||
|
|
||||||
|
viaScanner := func(t *testing.T, path string) *metadata.TrackMetadata {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
meta, err := metadata.ExtractTags(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExtractTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return meta
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
write func(t *testing.T, dir string) string
|
||||||
|
read func(t *testing.T, path string) *metadata.TrackMetadata
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "mp3",
|
||||||
|
read: viaScanner,
|
||||||
|
write: func(t *testing.T, dir string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := createTestMP3(t, dir, "totals.mp3", nil)
|
||||||
|
if err := writeMp3Tags(testLogger(), path, changes); err != nil {
|
||||||
|
t.Fatalf("writeMp3Tags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return path
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "flac",
|
||||||
|
read: viaScanner,
|
||||||
|
write: func(t *testing.T, dir string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := filepath.Join(dir, "totals.flac")
|
||||||
|
makeMinimalFLAC(t, path)
|
||||||
|
|
||||||
|
if err := writeFlacTags(testLogger(), path, changes); err != nil {
|
||||||
|
t.Fatalf("writeFlacTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return path
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "ogg",
|
||||||
|
read: viaScanner,
|
||||||
|
write: func(t *testing.T, dir string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := filepath.Join(dir, "totals.ogg")
|
||||||
|
createTestOGG(t, path)
|
||||||
|
|
||||||
|
if err := writeOggTags(testLogger(), path, changes); err != nil {
|
||||||
|
t.Fatalf("writeOggTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return path
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "wav",
|
||||||
|
read: readWavID3Tags,
|
||||||
|
write: func(t *testing.T, dir string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := createTestWAV(t, dir, "totals.wav", nil)
|
||||||
|
|
||||||
|
if err := writeWavTags(testLogger(), path, changes); err != nil {
|
||||||
|
t.Fatalf("writeWavTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return path
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
meta := tc.read(t, tc.write(t, t.TempDir()))
|
||||||
|
|
||||||
|
assertIntField(t, "TrackNumber", meta.TrackNumber, 2)
|
||||||
|
assertIntField(t, "TotalTracks", meta.TotalTracks, 10)
|
||||||
|
assertIntField(t, "DiscNumber", meta.DiscNumber, 1)
|
||||||
|
assertIntField(t, "TotalDiscs", meta.TotalDiscs, 2)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A number and a total are separate diff entries, so writing one must
|
||||||
|
// not discard the other. For ID3v2 they share a single "n/N" frame,
|
||||||
|
// which is the only place this can go wrong -- and it goes wrong by
|
||||||
|
// silently zeroing a total the file already declared.
|
||||||
|
func TestWriteMp3Totals_PartialUpdateKeepsTheOtherHalf(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
t.Run("writing the number keeps the total", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := createTestMP3(t, dir, "seeded.mp3", TagChanges{
|
||||||
|
FieldTrackNumber: 2,
|
||||||
|
FieldTotalTracks: 10,
|
||||||
|
})
|
||||||
|
|
||||||
|
if err := writeMp3Tags(testLogger(), path, TagChanges{
|
||||||
|
FieldTrackNumber: 4,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("writeMp3Tags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
meta, err := metadata.ExtractTags(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExtractTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
assertIntField(t, "TrackNumber", meta.TrackNumber, 4)
|
||||||
|
assertIntField(t, "TotalTracks", meta.TotalTracks, 10)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("writing the total keeps the number", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := createTestMP3(t, dir, "seeded.mp3", TagChanges{
|
||||||
|
FieldTrackNumber: 7,
|
||||||
|
})
|
||||||
|
|
||||||
|
if err := writeMp3Tags(testLogger(), path, TagChanges{
|
||||||
|
FieldTotalTracks: 12,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("writeMp3Tags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
meta, err := metadata.ExtractTags(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExtractTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
assertIntField(t, "TrackNumber", meta.TrackNumber, 7)
|
||||||
|
assertIntField(t, "TotalTracks", meta.TotalTracks, 12)
|
||||||
|
})
|
||||||
|
|
||||||
|
// "/12" says nothing a reader can use, and dhowden/tag reads it as
|
||||||
|
// track 0 -- which the scan would store as a real track number.
|
||||||
|
t.Run("a total with no number writes nothing", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := createTestMP3(t, dir, "bare.mp3", nil)
|
||||||
|
|
||||||
|
if err := writeMp3Tags(testLogger(), path, TagChanges{
|
||||||
|
FieldTotalTracks: 12,
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("writeMp3Tags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
meta, err := metadata.ExtractTags(path)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExtractTags: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
assertIntField(t, "TrackNumber", meta.TrackNumber, 0)
|
||||||
|
assertIntField(t, "TotalTracks", meta.TotalTracks, 0)
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -522,19 +522,20 @@ func readWavID3Tags(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Track number (TRCK).
|
// Track number and total (TRCK), disc number and total (TPOS).
|
||||||
|
// Both carry the "n/N" form, so they are read the way a reader
|
||||||
|
// reads them rather than with Atoi -- which sees "2/10" as 0.
|
||||||
trckID := parsed.CommonID("Track number/Position in set")
|
trckID := parsed.CommonID("Track number/Position in set")
|
||||||
if frames := parsed.GetFrames(trckID); len(frames) > 0 {
|
if frames := parsed.GetFrames(trckID); len(frames) > 0 {
|
||||||
if tf, ok := frames[0].(id3v2.TextFrame); ok {
|
if tf, ok := frames[0].(id3v2.TextFrame); ok {
|
||||||
meta.TrackNumber = atoiSafe(tf.Text)
|
meta.TrackNumber, meta.TotalTracks = parseXofN(tf.Text)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Disc number (TPOS).
|
|
||||||
tposID := parsed.CommonID("Part of a set")
|
tposID := parsed.CommonID("Part of a set")
|
||||||
if frames := parsed.GetFrames(tposID); len(frames) > 0 {
|
if frames := parsed.GetFrames(tposID); len(frames) > 0 {
|
||||||
if tf, ok := frames[0].(id3v2.TextFrame); ok {
|
if tf, ok := frames[0].(id3v2.TextFrame); ok {
|
||||||
meta.DiscNumber = atoiSafe(tf.Text)
|
meta.DiscNumber, meta.TotalDiscs = parseXofN(tf.Text)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,194 +0,0 @@
|
|||||||
# Config Improvement Suggestions
|
|
||||||
|
|
||||||
Remaining suggestions for improving the configuration system in YellowJacket.
|
|
||||||
|
|
||||||
## 2. Thread Safety Concerns
|
|
||||||
|
|
||||||
The current `Config` struct lacks synchronization:
|
|
||||||
- `Load()` and `Save()` can race with concurrent reads
|
|
||||||
- `handleConfigUpdate()` in library mutates `l.conf.DirectoryPath` without locks
|
|
||||||
|
|
||||||
**Suggestion:** Add a `sync.RWMutex` to protect config access, especially if config is read during scans.
|
|
||||||
|
|
||||||
```go
|
|
||||||
type Config struct {
|
|
||||||
mu sync.RWMutex
|
|
||||||
ctx context.Context
|
|
||||||
logger *slog.Logger
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
|
|
||||||
func (c *Config) Load() error {
|
|
||||||
c.mu.Lock()
|
|
||||||
defer c.mu.Unlock()
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 3. Nil Safety in Validation
|
|
||||||
|
|
||||||
In `config.go`, validation only runs if `c.Library != nil`, but `handleConfigPost` dereferences `postedConfig.Library` without checking for nil:
|
|
||||||
|
|
||||||
```go
|
|
||||||
if postedConfig.Library != nil {
|
|
||||||
c.Library = postedConfig.Library
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Status:** Partially addressed in the event refactor, but consider adding explicit nil checks in `Validate()` as well.
|
|
||||||
|
|
||||||
## 4. Inconsistent Error Handling on HTTP Responses
|
|
||||||
|
|
||||||
In `httphandler.go:28-31`, `WriteHeader` is called *after* rendering the error template, which won't work as expected (headers must be set before writing body):
|
|
||||||
|
|
||||||
```go
|
|
||||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
|
||||||
w.WriteHeader(http.StatusInternalServerError) // Too late!
|
|
||||||
```
|
|
||||||
|
|
||||||
**Fix:** Set the status code before rendering:
|
|
||||||
|
|
||||||
```go
|
|
||||||
w.WriteHeader(http.StatusInternalServerError)
|
|
||||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
|
||||||
```
|
|
||||||
|
|
||||||
## 5. Make `scanWorkerCount` Configurable
|
|
||||||
|
|
||||||
There's a TODO at `library.go:289`:
|
|
||||||
```go
|
|
||||||
// TODO: make configurable via Config.
|
|
||||||
var scanWorkerCount = goruntime.NumCPU()
|
|
||||||
```
|
|
||||||
|
|
||||||
**Suggestion:** Add this to `library.Config`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type Config struct {
|
|
||||||
DirectoryPath Directory `form:"Directory" schema:"directory,required"`
|
|
||||||
ScanWorkers int `form:"ScanWorkers" schema:"scan_workers"`
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Then in `NewLibrary()` or `Scan()`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
workers := l.conf.ScanWorkers
|
|
||||||
if workers <= 0 {
|
|
||||||
workers = goruntime.NumCPU()
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 6. Consider Config Defaults
|
|
||||||
|
|
||||||
Currently if no config exists, an empty one is saved. Consider providing sensible defaults (e.g., common music directories like `~/Music`).
|
|
||||||
|
|
||||||
```go
|
|
||||||
func (c *Config) setDefaults() {
|
|
||||||
if c.Library == nil {
|
|
||||||
c.Library = &library.Config{}
|
|
||||||
}
|
|
||||||
if c.Library.DirectoryPath == "" {
|
|
||||||
// Try common music directories
|
|
||||||
home, _ := os.UserHomeDir()
|
|
||||||
musicDir := filepath.Join(home, "Music")
|
|
||||||
if info, err := os.Stat(musicDir); err == nil && info.IsDir() {
|
|
||||||
c.Library.DirectoryPath = library.Directory(musicDir)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 7. Config Reload/Watch Capability
|
|
||||||
|
|
||||||
The config is only loaded at startup. Consider adding:
|
|
||||||
- File watcher for external config changes (using `fsnotify`)
|
|
||||||
- Explicit reload method callable from UI
|
|
||||||
|
|
||||||
```go
|
|
||||||
func (c *Config) Watch() error {
|
|
||||||
watcher, err := fsnotify.NewWatcher()
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
go func() {
|
|
||||||
for event := range watcher.Events {
|
|
||||||
if event.Op&fsnotify.Write == fsnotify.Write {
|
|
||||||
c.Load()
|
|
||||||
// Emit event for listeners
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
|
|
||||||
return watcher.Add(c.filePath)
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 8. Validation Should Return Structured Errors
|
|
||||||
|
|
||||||
Currently validation returns combined errors. Consider returning a structured validation result that the UI can map to specific fields for better user feedback.
|
|
||||||
|
|
||||||
```go
|
|
||||||
type ValidationError struct {
|
|
||||||
Field string
|
|
||||||
Message string
|
|
||||||
}
|
|
||||||
|
|
||||||
type ValidationResult struct {
|
|
||||||
Valid bool
|
|
||||||
Errors []ValidationError
|
|
||||||
}
|
|
||||||
|
|
||||||
func (c *Config) ValidateStructured() ValidationResult {
|
|
||||||
var result ValidationResult
|
|
||||||
result.Valid = true
|
|
||||||
|
|
||||||
if c.Library != nil {
|
|
||||||
if err := c.Library.Validate(); err != nil {
|
|
||||||
result.Valid = false
|
|
||||||
result.Errors = append(result.Errors, ValidationError{
|
|
||||||
Field: "Library.DirectoryPath",
|
|
||||||
Message: err.Error(),
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return result
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 9. Use Standard Library for Config Paths
|
|
||||||
|
|
||||||
The path construction in `system/userdata.go` doesn't respect `$XDG_CONFIG_HOME` on Linux or use the standard Go `os.UserConfigDir()`.
|
|
||||||
|
|
||||||
**Current implementation:**
|
|
||||||
```go
|
|
||||||
case "linux":
|
|
||||||
return fmt.Sprintf("/home/%s/%s/yellowjacket", username, unixSubdirs[dt]), nil
|
|
||||||
```
|
|
||||||
|
|
||||||
**Suggested improvement:**
|
|
||||||
```go
|
|
||||||
func GetUserConfigDirPath() (string, error) {
|
|
||||||
baseDir, err := os.UserConfigDir() // Respects XDG_CONFIG_HOME
|
|
||||||
if err != nil {
|
|
||||||
return "", fmt.Errorf("could not get user config directory: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
path := filepath.Join(baseDir, "yellowjacket")
|
|
||||||
|
|
||||||
if err := os.MkdirAll(path, 0o755); err != nil {
|
|
||||||
return "", fmt.Errorf("could not create config directory: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return path, nil
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
This approach:
|
|
||||||
- Respects `$XDG_CONFIG_HOME` on Linux
|
|
||||||
- Uses proper macOS paths (`~/Library/Application Support`)
|
|
||||||
- Uses `%AppData%` on Windows
|
|
||||||
- Is more portable and follows platform conventions
|
|
||||||
@@ -1,53 +0,0 @@
|
|||||||
# Development Overview
|
|
||||||
|
|
||||||
YellowJacket is a moderately complex application. This document gives an overview of how development of it works.
|
|
||||||
|
|
||||||
## Logical Breakdown
|
|
||||||
|
|
||||||
YellowJacket can be thought about in a heirarchy of logical modules and components. The borders of these logical sections are mostly represented in the code and directory structure as well.
|
|
||||||
|
|
||||||
- Frontend
|
|
||||||
- UI Components (see [Lit](###lit-web-components))
|
|
||||||
- Backend
|
|
||||||
- App
|
|
||||||
- Asset Handler
|
|
||||||
- Logging
|
|
||||||
- System
|
|
||||||
- Player
|
|
||||||
- Library
|
|
||||||
- Config
|
|
||||||
- Database
|
|
||||||
- Queries (see [sqlc](###sqlc))
|
|
||||||
|
|
||||||
## Dependencies
|
|
||||||
|
|
||||||
YellowJacket uses many tools and libraries to provide its functionality.
|
|
||||||
This section lists each of these dependencies and explains how they are used.
|
|
||||||
|
|
||||||
### [Wails](https://wails.io)
|
|
||||||
|
|
||||||
Used to create desktop apps with Go and web technologies.
|
|
||||||
|
|
||||||
### [SQLite](https://github.com/mattn/go-sqlite3?tab=readme-ov-file#go-sqlite3)
|
|
||||||
|
|
||||||
Used for local database.
|
|
||||||
|
|
||||||
### [sqlc](https://sqlc.dev/)
|
|
||||||
|
|
||||||
Used to generate Go code from SQL.
|
|
||||||
|
|
||||||
### [Templ](https://templ.guide/)
|
|
||||||
|
|
||||||
Used to generate HTML templates with Go code.
|
|
||||||
|
|
||||||
### [Beep](https://github.com/gopxl/beep?tab=readme-ov-file#beep)
|
|
||||||
|
|
||||||
Used for audio playback.
|
|
||||||
|
|
||||||
### [Lit Web Components](https://lit.dev/)
|
|
||||||
|
|
||||||
Used for dynamic/reactive frontend components.
|
|
||||||
|
|
||||||
### [HTMX](https://htmx.org/)
|
|
||||||
|
|
||||||
Used for requesting HTML fragments from the backend and rendering them on the frontend.
|
|
||||||
-1648
File diff suppressed because it is too large
Load Diff
+4
-1
@@ -39,7 +39,10 @@
|
|||||||
<audio-player></audio-player>
|
<audio-player></audio-player>
|
||||||
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
||||||
id="queue-button">
|
id="queue-button">
|
||||||
<wa-icon name="list"></wa-icon>
|
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
|
||||||
|
because this file has no module scope. It was `list`,
|
||||||
|
which is the Playlists destination's icon. -->
|
||||||
|
<wa-icon name="bars-staggered"></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
</footer>
|
</footer>
|
||||||
<!-- The phone's primary navigation, hidden above 600px by
|
<!-- The phone's primary navigation, hidden above 600px by
|
||||||
|
|||||||
@@ -0,0 +1 @@
|
|||||||
|
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 96C0 78.3 14.3 64 32 64l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L32 128C14.3 128 0 113.7 0 96zM64 256c0-17.7 14.3-32 32-32l384 0c17.7 0 32 14.3 32 32s-14.3 32-32 32L96 288c-17.7 0-32-14.3-32-32zM448 416c0 17.7-14.3 32-32 32L32 448c-17.7 0-32-14.3-32-32s14.3-32 32-32l384 0c17.7 0 32 14.3 32 32z"/></svg>
|
||||||
|
After Width: | Height: | Size: 609 B |
@@ -37,6 +37,10 @@ import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'
|
|||||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||||
import '@components/playlist-picker/playlist-picker.js';
|
import '@components/playlist-picker/playlist-picker.js';
|
||||||
import { dict, list } from '@utils/binding';
|
import { dict, list } from '@utils/binding';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/** Pixels to change card width per scroll tick. */
|
/** Pixels to change card width per scroll tick. */
|
||||||
const ZOOM_STEP = 16;
|
const ZOOM_STEP = 16;
|
||||||
@@ -1371,7 +1375,7 @@ export class ArtistsView
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -1407,7 +1411,7 @@ export class ArtistsView
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import type WaDrawer from '@awesome.me/webawesome/dist/components/drawer/drawer.
|
|||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import '../sidebar/app-sidebar.js';
|
import '../sidebar/app-sidebar.js';
|
||||||
import { nameDialog } from '@utils/name-dialog';
|
import { nameDialog } from '@utils/name-dialog';
|
||||||
|
import { ICON_PLAYLIST } from '@utils/icon-language';
|
||||||
|
|
||||||
type View = 'home' | 'albums' | 'tracks' | 'playlists';
|
type View = 'home' | 'albums' | 'tracks' | 'playlists';
|
||||||
|
|
||||||
@@ -138,7 +139,7 @@ export class BottomNav extends LitElement {
|
|||||||
{ id: 'home', label: 'Home', icon: 'house' },
|
{ id: 'home', label: 'Home', icon: 'house' },
|
||||||
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
|
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
|
||||||
{ id: 'tracks', label: 'Tracks', icon: 'music' },
|
{ id: 'tracks', label: 'Tracks', icon: 'music' },
|
||||||
{ id: 'playlists', label: 'Playlists', icon: 'list' },
|
{ id: 'playlists', label: 'Playlists', icon: ICON_PLAYLIST },
|
||||||
];
|
];
|
||||||
|
|
||||||
override connectedCallback() {
|
override connectedCallback() {
|
||||||
|
|||||||
@@ -76,6 +76,10 @@ import type {
|
|||||||
SortDirection,
|
SortDirection,
|
||||||
} from './cover-grid-types.js';
|
} from './cover-grid-types.js';
|
||||||
import { list } from '@utils/binding';
|
import { list } from '@utils/binding';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
@customElement('cover-grid')
|
@customElement('cover-grid')
|
||||||
export class CoverGrid
|
export class CoverGrid
|
||||||
@@ -2123,7 +2127,7 @@ export class CoverGrid
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -2156,7 +2160,7 @@ export class CoverGrid
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -52,6 +52,12 @@ import { dictByName } from '@utils/binding';
|
|||||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||||
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
|
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
|
||||||
import '@components/playlist-picker/playlist-picker.js';
|
import '@components/playlist-picker/playlist-picker.js';
|
||||||
|
import {
|
||||||
|
ICON_CAN_REQUEST,
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
ICON_REQUESTED,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The region the album header's own failures are rendered in.
|
* The region the album header's own failures are rendered in.
|
||||||
@@ -193,6 +199,25 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
@state() private selectedVersionKey: string = '';
|
@state() private selectedVersionKey: string = '';
|
||||||
@state() private coverArtURL = '';
|
@state() private coverArtURL = '';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether to draw the whole release rather than only the files on
|
||||||
|
* disk — `null` while nobody has said, which is the automatic rule
|
||||||
|
* (`buildLibraryEntry`: show the release once the tags say the album
|
||||||
|
* is incomplete).
|
||||||
|
*
|
||||||
|
* It is a *tri-state* on purpose. The automatic rule is right when
|
||||||
|
* it fires and the switch has to be able to agree with it, or the
|
||||||
|
* control would start out contradicting the page it is sitting on;
|
||||||
|
* a plain boolean would need its default recomputed every time the
|
||||||
|
* completeness answer changed underneath it.
|
||||||
|
*
|
||||||
|
* The rule alone was not enough, which is the report: it depends on
|
||||||
|
* the files declaring a per-disc total, so a library whose tags
|
||||||
|
* never said sat permanently on "only my tracks" with no way to ask
|
||||||
|
* for the rest — and no way to tell that there was a rest.
|
||||||
|
*/
|
||||||
|
@state() private showFullTracklist: boolean | null = null;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The local album's own tracks — the authoritative answer to "what
|
* The local album's own tracks — the authoritative answer to "what
|
||||||
* is actually on disk," independent of `this.releases`, which
|
* is actually on disk," independent of `this.releases`, which
|
||||||
@@ -559,6 +584,19 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* ── Tracklist ── */
|
/* ── Tracklist ── */
|
||||||
|
.tracklist-scope {
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
flex-wrap: wrap;
|
||||||
|
gap: 6px 12px;
|
||||||
|
margin-bottom: 8px;
|
||||||
|
}
|
||||||
|
|
||||||
|
.tracklist-scope-hint {
|
||||||
|
font-size: var(--yj-text-xs);
|
||||||
|
color: var(--yj-text-tertiary, #888);
|
||||||
|
}
|
||||||
|
|
||||||
.tracklist {
|
.tracklist {
|
||||||
display: flex;
|
display: flex;
|
||||||
flex-direction: column;
|
flex-direction: column;
|
||||||
@@ -914,6 +952,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
this.releases = [];
|
this.releases = [];
|
||||||
this.versionEntries = [];
|
this.versionEntries = [];
|
||||||
this.selectedVersionKey = '';
|
this.selectedVersionKey = '';
|
||||||
|
this.showFullTracklist = null;
|
||||||
this.localTracks = [];
|
this.localTracks = [];
|
||||||
this.filePaths = new Map();
|
this.filePaths = new Map();
|
||||||
this.askedFor = new Set();
|
this.askedFor = new Set();
|
||||||
@@ -1817,17 +1856,23 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
// Guarded on `known` rather than on "fewer tracks than the
|
// Guarded on `known` rather than on "fewer tracks than the
|
||||||
// cluster", which would swap in a catalog tracklist for
|
// cluster", which would swap in a catalog tracklist for
|
||||||
// every album whose tags simply never declared a total.
|
// every album whose tags simply never declared a total.
|
||||||
|
//
|
||||||
|
// And guarded on the *user's* answer first, because the
|
||||||
|
// automatic rule can only fire where the tags declared a
|
||||||
|
// total: an album that says nothing is not an album that is
|
||||||
|
// complete, and it used to be shown as one.
|
||||||
const answer = this.completenessAnswer();
|
const answer = this.completenessAnswer();
|
||||||
const incomplete = answer?.known && !answer.complete;
|
|
||||||
|
|
||||||
if (incomplete) {
|
if (this.showFullTracklist ?? (answer?.known && !answer.complete)) {
|
||||||
const fullRelease = this.findLibraryCluster(clusters);
|
const fullRelease = this.fullReleaseCluster(clusters);
|
||||||
|
|
||||||
if (fullRelease) {
|
if (fullRelease) {
|
||||||
return {
|
return {
|
||||||
key: 'synthetic:library',
|
key: 'synthetic:library',
|
||||||
label: 'Your Library',
|
label: 'Your Library',
|
||||||
sublabel: `${answer?.owned ?? 0} of ${answer?.expected ?? 0} tracks · ${this.clusterLabel(fullRelease)}`,
|
sublabel: answer?.known
|
||||||
|
? `${answer.owned} of ${answer.expected} tracks · ${this.clusterLabel(fullRelease)}`
|
||||||
|
: `${this.clusterLabel(fullRelease)} · full tracklist`,
|
||||||
group: 'aggregate',
|
group: 'aggregate',
|
||||||
syntheticKind: 'library',
|
syntheticKind: 'library',
|
||||||
tracks: fullRelease.representative.tracks ?? [],
|
tracks: fullRelease.representative.tracks ?? [],
|
||||||
@@ -1861,6 +1906,25 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The release to draw when the whole album is wanted rather than
|
||||||
|
* the files on disk.
|
||||||
|
*
|
||||||
|
* `findLibraryCluster` is the right answer where it has one — the
|
||||||
|
* release the user's tracks overlap most — but it is a guess over
|
||||||
|
* the `inLibrary` flags and returns nothing at all when none of
|
||||||
|
* them are set, which is every untagged library. Falling back to
|
||||||
|
* the highest-scoring cluster is what makes the switch work there;
|
||||||
|
* that is the same release the page would call "Standard", and the
|
||||||
|
* sublabel names it either way rather than leaving the user to
|
||||||
|
* wonder whose tracklist they are reading.
|
||||||
|
*/
|
||||||
|
private fullReleaseCluster(
|
||||||
|
clusters: ReleaseCluster[],
|
||||||
|
): ReleaseCluster | undefined {
|
||||||
|
return this.findLibraryCluster(clusters) ?? clusters[0];
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fallback only: used when there's no local album to anchor on
|
* Fallback only: used when there's no local album to anchor on
|
||||||
* (see `buildLibraryEntry`). Finds the cluster with the highest
|
* (see `buildLibraryEntry`). Finds the cluster with the highest
|
||||||
@@ -2238,6 +2302,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
@catalog-retry=${this.retryCatalog}
|
@catalog-retry=${this.retryCatalog}
|
||||||
></catalog-scope-notice>
|
></catalog-scope-notice>
|
||||||
${this.renderVersionSelector()}
|
${this.renderVersionSelector()}
|
||||||
|
${this.renderTracklistScope()}
|
||||||
${this.renderTracklist()}
|
${this.renderTracklist()}
|
||||||
</div>
|
</div>
|
||||||
<track-details></track-details>
|
<track-details></track-details>
|
||||||
@@ -2378,7 +2443,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
data-testid="album-queue"
|
data-testid="album-queue"
|
||||||
@click=${() => this.queueOwned()}
|
@click=${() => this.queueOwned()}
|
||||||
>
|
>
|
||||||
<wa-icon slot="start" name="list"></wa-icon>
|
<wa-icon slot="start" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to queue
|
Add to queue
|
||||||
</wa-button>
|
</wa-button>
|
||||||
${partial
|
${partial
|
||||||
@@ -2681,7 +2746,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
of the same Free glyph carry the toggle instead. -->
|
of the same Free glyph carry the toggle instead. -->
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="start"
|
slot="start"
|
||||||
name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
|
name=${this.isRequested ? ICON_REQUESTED : ICON_CAN_REQUEST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
${this.isRequested ? 'Requested' : 'Request this'}
|
${this.isRequested ? 'Requested' : 'Request this'}
|
||||||
</wa-button>
|
</wa-button>
|
||||||
@@ -3036,6 +3101,86 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
|
|
||||||
/* ── Tracklist ── */
|
/* ── Tracklist ── */
|
||||||
|
|
||||||
|
/**
|
||||||
|
* "Show the whole album" — the switch between the files on disk and
|
||||||
|
* the release they are part of.
|
||||||
|
*
|
||||||
|
* The page could already draw the full release with the missing
|
||||||
|
* rows dimmed, and did so automatically once the tags said the
|
||||||
|
* album was incomplete. What it could not do was be *asked*: where
|
||||||
|
* the files declare no per-disc total and the catalog has none
|
||||||
|
* either, the rule never fires, so a partly-owned album showed only
|
||||||
|
* the tracks the user had and nothing said the rest existed.
|
||||||
|
*
|
||||||
|
* Three things about when it appears, all of them the same rule —
|
||||||
|
* a control that cannot change what is on screen is worse than no
|
||||||
|
* control, which is what the version dropdown's own guard is for:
|
||||||
|
*
|
||||||
|
* - Only against the synthetic "Your Library" entry. Every other
|
||||||
|
* entry *is* a catalog tracklist already.
|
||||||
|
* - Only when a catalog release exists to switch to.
|
||||||
|
* - Only when the two differ. A complete album's release has the
|
||||||
|
* same rows as its files, so the switch would redraw the same
|
||||||
|
* list and read as broken.
|
||||||
|
*/
|
||||||
|
private renderTracklistScope() {
|
||||||
|
const current = this.currentVersion();
|
||||||
|
|
||||||
|
if (current?.syntheticKind !== 'library') return nothing;
|
||||||
|
if (this.localTracks.length === 0) return nothing;
|
||||||
|
|
||||||
|
const full = this.fullReleaseCluster(this.clustersOf(this.versionEntries));
|
||||||
|
const fullCount = full?.representative.tracks?.length ?? 0;
|
||||||
|
|
||||||
|
if (fullCount === 0 || fullCount <= this.localTracks.length) {
|
||||||
|
return nothing;
|
||||||
|
}
|
||||||
|
|
||||||
|
const showing = current.tracks.length > this.localTracks.length;
|
||||||
|
|
||||||
|
return html`
|
||||||
|
<div class="tracklist-scope">
|
||||||
|
<wa-switch
|
||||||
|
size="small"
|
||||||
|
?checked=${showing}
|
||||||
|
@change=${this.handleTracklistScopeChange}
|
||||||
|
>
|
||||||
|
Show the whole album
|
||||||
|
</wa-switch>
|
||||||
|
<span class="tracklist-scope-hint">
|
||||||
|
${showing
|
||||||
|
? `${this.localTracks.length} of ${fullCount} tracks are in your library`
|
||||||
|
: `${fullCount - this.localTracks.length} more tracks are on this release`}
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
`;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The clusters behind the current entries.
|
||||||
|
*
|
||||||
|
* `buildClusters` computes them and keeps only the entries, so this
|
||||||
|
* recovers them rather than storing the array twice — two copies of
|
||||||
|
* a list rebuilt on four different events is how they come to
|
||||||
|
* disagree.
|
||||||
|
*/
|
||||||
|
private clustersOf(entries: VersionEntry[]): ReleaseCluster[] {
|
||||||
|
return entries
|
||||||
|
.filter((e) => e.group === 'cluster')
|
||||||
|
.map((e) => e.cluster)
|
||||||
|
.filter((c): c is ReleaseCluster => !!c);
|
||||||
|
}
|
||||||
|
|
||||||
|
private handleTracklistScopeChange = (e: Event) => {
|
||||||
|
this.showFullTracklist = (e.target as HTMLInputElement).checked;
|
||||||
|
|
||||||
|
// The entries are derived, so the switch rebuilds them rather
|
||||||
|
// than patching the one it changed. `buildClusters` re-defaults
|
||||||
|
// the selection, which lands back on "Your Library" — the only
|
||||||
|
// entry this control is ever shown against.
|
||||||
|
this.buildClusters();
|
||||||
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The heading is there and is not drawn.
|
* The heading is there and is not drawn.
|
||||||
*
|
*
|
||||||
@@ -3199,7 +3344,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
@click=${() => this.onContextMenuAction('add-to-queue')}
|
@click=${() => this.onContextMenuAction('add-to-queue')}
|
||||||
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item
|
<wa-dropdown-item
|
||||||
@@ -3218,7 +3363,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
this.openPlaylistSubmenu();
|
this.openPlaylistSubmenu();
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span class="submenu-arrow">▶</span>
|
<span class="submenu-arrow">▶</span>
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
|
|||||||
@@ -59,6 +59,12 @@ import { dict, dictByName } from '@utils/binding';
|
|||||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||||
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
|
import { showTrackDetailsForPath } from '@utils/track-details-opener.js';
|
||||||
import '@components/playlist-picker/playlist-picker.js';
|
import '@components/playlist-picker/playlist-picker.js';
|
||||||
|
import {
|
||||||
|
ICON_CAN_REQUEST,
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
ICON_REQUESTED,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/* ── Constants ── */
|
/* ── Constants ── */
|
||||||
|
|
||||||
@@ -2674,7 +2680,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
@click=${() => this.onContextMenuAction('add-to-queue')}
|
@click=${() => this.onContextMenuAction('add-to-queue')}
|
||||||
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item
|
<wa-dropdown-item
|
||||||
@@ -2693,7 +2699,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
void this.openPlaylistSubmenu(true);
|
void this.openPlaylistSubmenu(true);
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span class="submenu-arrow">▶</span>
|
<span class="submenu-arrow">▶</span>
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -2737,7 +2743,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
Play
|
Play
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item @click=${() => void this.onReleaseAction('add-to-queue')}>
|
<wa-dropdown-item @click=${() => void this.onReleaseAction('add-to-queue')}>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item @click=${() => void this.onReleaseAction('play-next')}>
|
<wa-dropdown-item @click=${() => void this.onReleaseAction('play-next')}>
|
||||||
@@ -2751,7 +2757,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
<wa-dropdown-item @click=${() => void this.onReleaseRequestToggle()}>
|
<wa-dropdown-item @click=${() => void this.onReleaseRequestToggle()}>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name=${requested ? 'xmark' : 'bookmark'}
|
name=${requested ? ICON_REQUESTED : ICON_CAN_REQUEST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
${requested ? 'Cancel Request' : 'Request This'}
|
${requested ? 'Cancel Request' : 'Request This'}
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -2789,9 +2795,15 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
appearance=${request ? 'filled' : 'outlined'}
|
appearance=${request ? 'filled' : 'outlined'}
|
||||||
@click=${() => void this.toggleFollow(request?.id)}
|
@click=${() => void this.toggleFollow(request?.id)}
|
||||||
>
|
>
|
||||||
|
<!-- This was bookmark-check, which is not in
|
||||||
|
names.txt and so has rendered the missing-icon
|
||||||
|
fallback — a circled question mark — on every
|
||||||
|
followed artist since it was written. A
|
||||||
|
backtick around that name would end this
|
||||||
|
template literal, which is why there is none. -->
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="start"
|
slot="start"
|
||||||
name=${request ? 'bookmark-check' : 'bookmark'}
|
name=${request ? ICON_REQUESTED : ICON_CAN_REQUEST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
${request ? 'Following' : 'Follow for new releases'}
|
${request ? 'Following' : 'Follow for new releases'}
|
||||||
</wa-button>
|
</wa-button>
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
|||||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||||
import { dict, dictByName } from '@utils/binding';
|
import { dict, dictByName } from '@utils/binding';
|
||||||
|
import { ICON_QUEUE } from '@utils/icon-language';
|
||||||
|
|
||||||
/** The region explore's own action failures (play/queue) are rendered in. */
|
/** The region explore's own action failures (play/queue) are rendered in. */
|
||||||
export const ExploreRegion = 'explore';
|
export const ExploreRegion = 'explore';
|
||||||
@@ -1342,7 +1343,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
|||||||
Play
|
Play
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item @click=${() => this.onContextMenuAction('add-to-queue')}>
|
<wa-dropdown-item @click=${() => this.onContextMenuAction('add-to-queue')}>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item @click=${() => this.onContextMenuAction('play-next')}>
|
<wa-dropdown-item @click=${() => this.onContextMenuAction('play-next')}>
|
||||||
|
|||||||
@@ -35,6 +35,10 @@ import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js'
|
|||||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||||
import '@components/playlist-picker/playlist-picker.js';
|
import '@components/playlist-picker/playlist-picker.js';
|
||||||
import { dictByName } from '@utils/binding';
|
import { dictByName } from '@utils/binding';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/** Pixels to change card width per scroll tick. */
|
/** Pixels to change card width per scroll tick. */
|
||||||
const ZOOM_STEP = 16;
|
const ZOOM_STEP = 16;
|
||||||
@@ -1211,7 +1215,7 @@ export class GenresView
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -1257,7 +1261,7 @@ export class GenresView
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -4,6 +4,11 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
|||||||
import { toggleRequest } from '@utils/library-status';
|
import { toggleRequest } from '@utils/library-status';
|
||||||
import { notificationStore } from '@store/notification-store';
|
import { notificationStore } from '@store/notification-store';
|
||||||
import { describeError } from '@utils/describe-error';
|
import { describeError } from '@utils/describe-error';
|
||||||
|
import {
|
||||||
|
ICON_CAN_REQUEST,
|
||||||
|
ICON_IN_LIBRARY,
|
||||||
|
ICON_REQUESTED,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Library status for an entity (artist, album, or track).
|
* Library status for an entity (artist, album, or track).
|
||||||
@@ -248,18 +253,25 @@ export class LibraryStatusIndicator extends LitElement {
|
|||||||
* hourglass says "wait, this is under way", which overstates what a
|
* hourglass says "wait, this is under way", which overstates what a
|
||||||
* request is: nothing may be downloading, nothing may ever be found,
|
* request is: nothing may be downloading, nothing may ever be found,
|
||||||
* and the user can leave one sitting on the list indefinitely. A
|
* and the user can leave one sitting on the list indefinitely. A
|
||||||
* bookmark says the honest thing — it is on your list — and reads as
|
* bookmark says the honest thing — it is on your list.
|
||||||
* the opposite of the plus that put it there, which is what a
|
*
|
||||||
* toggle's two states have to do.
|
* The *other* state is the outline of that same bookmark, not a
|
||||||
|
* plus. Two states of one toggle have to read as each other's
|
||||||
|
* opposite, and a plus and a bookmark do not — this badge showed a
|
||||||
|
* plus on the same page as a "Request this" button already using
|
||||||
|
* the outline/solid pair, forty pixels away. That is the fault
|
||||||
|
* `utils/library-status.ts` was written for, one layer down: it
|
||||||
|
* made the two agree on what wanting *means* and left them
|
||||||
|
* disagreeing on what it looks like.
|
||||||
*/
|
*/
|
||||||
private iconName(): string {
|
private iconName(): string {
|
||||||
switch (this.status) {
|
switch (this.status) {
|
||||||
case 'in-library':
|
case 'in-library':
|
||||||
return 'check';
|
return ICON_IN_LIBRARY;
|
||||||
case 'queued':
|
case 'queued':
|
||||||
return 'bookmark';
|
return ICON_REQUESTED;
|
||||||
default:
|
default:
|
||||||
return 'plus';
|
return ICON_CAN_REQUEST;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import { creditStore } from '@store/credit-store';
|
|||||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import { srOnly } from '../../styles/sr-only.css';
|
import { srOnly } from '../../styles/sr-only.css';
|
||||||
|
import { ICON_QUEUE } from '@utils/icon-language';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* What is playing, at the size a phone has room for (plan 016 B2,
|
* What is playing, at the size a phone has room for (plan 016 B2,
|
||||||
@@ -339,7 +340,7 @@ export class NowPlayingView extends LitElement {
|
|||||||
aria-label="Show the queue"
|
aria-label="Show the queue"
|
||||||
@click=${this.openQueue}
|
@click=${this.openQueue}
|
||||||
>
|
>
|
||||||
<wa-icon name="list"></wa-icon>
|
<wa-icon name=${ICON_QUEUE}></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
</header>
|
</header>
|
||||||
`;
|
`;
|
||||||
|
|||||||
@@ -70,6 +70,10 @@ import {
|
|||||||
} from '@utils/explore-link';
|
} from '@utils/explore-link';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import { list } from '@utils/binding';
|
import { list } from '@utils/binding';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
/** 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. */
|
||||||
@@ -1358,7 +1362,7 @@ export class PlaylistDetails
|
|||||||
</button>
|
</button>
|
||||||
<div class="playlist-avatar">
|
<div class="playlist-avatar">
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="list"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</div>
|
</div>
|
||||||
<div class="playlist-info">
|
<div class="playlist-info">
|
||||||
@@ -1665,7 +1669,7 @@ export class PlaylistDetails
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -1720,7 +1724,7 @@ export class PlaylistDetails
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import { notificationStore } from '@store/notification-store';
|
|||||||
import { describeError } from '@utils/describe-error';
|
import { describeError } from '@utils/describe-error';
|
||||||
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
||||||
import { list } from '@utils/binding';
|
import { list } from '@utils/binding';
|
||||||
|
import { ICON_NEW } from '@utils/icon-language';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A reusable playlist picker that displays existing playlists
|
* A reusable playlist picker that displays existing playlists
|
||||||
@@ -307,7 +308,7 @@ export class PlaylistPicker extends LitElement {
|
|||||||
`
|
`
|
||||||
: nothing}
|
: nothing}
|
||||||
<wa-dropdown-item @click=${this.handleShowCreate}>
|
<wa-dropdown-item @click=${this.handleShowCreate}>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_NEW}></wa-icon>
|
||||||
New Playlist
|
New Playlist
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -37,6 +37,10 @@ import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
|||||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||||
import '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
import '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
||||||
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js';
|
||||||
|
import {
|
||||||
|
ICON_NEW,
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
const SCROLL_DEBOUNCE_MS = 100;
|
const SCROLL_DEBOUNCE_MS = 100;
|
||||||
|
|
||||||
@@ -1496,7 +1500,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
@dragleave=${this.onNewButtonDragLeave}
|
@dragleave=${this.onNewButtonDragLeave}
|
||||||
@drop=${this.onNewButtonDrop}
|
@drop=${this.onNewButtonDrop}
|
||||||
>
|
>
|
||||||
<wa-icon name="plus"></wa-icon>
|
<wa-icon name=${ICON_NEW}></wa-icon>
|
||||||
New Playlist
|
New Playlist
|
||||||
</button>
|
</button>
|
||||||
<button
|
<button
|
||||||
@@ -1641,10 +1645,10 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
>
|
>
|
||||||
<div class="drop-zone-icon">
|
<div class="drop-zone-icon">
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="plus"
|
name=${ICON_NEW}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</div>
|
</div>
|
||||||
<wa-icon name="list"></wa-icon>
|
<wa-icon name=${ICON_PLAYLIST}></wa-icon>
|
||||||
<p>No playlists yet</p>
|
<p>No playlists yet</p>
|
||||||
<p style="font-size: 12px;">
|
<p style="font-size: 12px;">
|
||||||
Create a playlist or drop
|
Create a playlist or drop
|
||||||
@@ -1666,7 +1670,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
>
|
>
|
||||||
<div class="drop-zone-icon">
|
<div class="drop-zone-icon">
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="plus"
|
name=${ICON_NEW}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</div>
|
</div>
|
||||||
<p>
|
<p>
|
||||||
@@ -1701,7 +1705,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
>
|
>
|
||||||
<div class="drop-zone-icon">
|
<div class="drop-zone-icon">
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="plus"
|
name=${ICON_NEW}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</div>
|
</div>
|
||||||
</li>
|
</li>
|
||||||
|
|||||||
@@ -60,6 +60,11 @@ import {
|
|||||||
trackLink,
|
trackLink,
|
||||||
exploreLinkStyles,
|
exploreLinkStyles,
|
||||||
} from '@utils/explore-link';
|
} from '@utils/explore-link';
|
||||||
|
import {
|
||||||
|
ICON_NEW,
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
/** Above this many tracks, clearing the queue asks first. */
|
/** Above this many tracks, clearing the queue asks first. */
|
||||||
const CLEAR_CONFIRM_THRESHOLD = 20;
|
const CLEAR_CONFIRM_THRESHOLD = 20;
|
||||||
|
|
||||||
@@ -1755,7 +1760,7 @@ export class QueuePanel
|
|||||||
title="Add queue to playlist"
|
title="Add queue to playlist"
|
||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
@@ -1794,11 +1799,11 @@ export class QueuePanel
|
|||||||
? html`<div class="empty-state">
|
? html`<div class="empty-state">
|
||||||
<div class="drop-zone-icon">
|
<div class="drop-zone-icon">
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="plus"
|
name=${ICON_NEW}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</div>
|
</div>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name="list"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
<p>Queue is empty</p>
|
<p>Queue is empty</p>
|
||||||
<p style="font-size: 12px;">
|
<p style="font-size: 12px;">
|
||||||
@@ -1875,7 +1880,7 @@ export class QueuePanel
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -4,6 +4,10 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
|||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
|
|
||||||
import type { DragActiveDetail } from '@utils/drag-controller';
|
import type { DragActiveDetail } from '@utils/drag-controller';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_REQUESTED,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
type View = 'home' | 'playlists' | 'artists' | 'genres' | 'albums' | 'tracks' | 'explore' | 'downloads' | 'autotag' | 'jobs' | 'settings';
|
type View = 'home' | 'playlists' | 'artists' | 'genres' | 'albums' | 'tracks' | 'explore' | 'downloads' | 'autotag' | 'jobs' | 'settings';
|
||||||
|
|
||||||
@@ -197,13 +201,13 @@ export class AppSidebar extends LitElement {
|
|||||||
|
|
||||||
private navItems: NavItem[] = [
|
private navItems: NavItem[] = [
|
||||||
{ id: 'home', label: 'Home', icon: 'house' },
|
{ id: 'home', label: 'Home', icon: 'house' },
|
||||||
{ id: 'playlists', label: 'Playlists', icon: 'list' },
|
{ id: 'playlists', label: 'Playlists', icon: ICON_PLAYLIST },
|
||||||
{ id: 'artists', label: 'Artists', icon: 'user-group' },
|
{ id: 'artists', label: 'Artists', icon: 'user-group' },
|
||||||
{ id: 'genres', label: 'Genres', icon: 'masks-theater' },
|
{ id: 'genres', label: 'Genres', icon: 'masks-theater' },
|
||||||
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
|
{ id: 'albums', label: 'Albums', icon: 'compact-disc' },
|
||||||
{ id: 'tracks', label: 'Tracks', icon: 'music' },
|
{ id: 'tracks', label: 'Tracks', icon: 'music' },
|
||||||
{ id: 'explore', label: 'Explore', icon: 'globe' },
|
{ id: 'explore', label: 'Explore', icon: 'globe' },
|
||||||
{ id: 'downloads', label: 'Downloads', icon: 'bookmark' },
|
{ id: 'downloads', label: 'Downloads', icon: ICON_REQUESTED },
|
||||||
{ id: 'autotag', label: 'Autotag', icon: 'tag' },
|
{ id: 'autotag', label: 'Autotag', icon: 'tag' },
|
||||||
{ id: 'jobs', label: 'Jobs', icon: 'list-check' },
|
{ id: 'jobs', label: 'Jobs', icon: 'list-check' },
|
||||||
{ id: 'settings', label: 'Settings', icon: 'gear' },
|
{ id: 'settings', label: 'Settings', icon: 'gear' },
|
||||||
|
|||||||
@@ -61,6 +61,10 @@ import {
|
|||||||
import '@components/smart-playlist-editor/smart-playlist-editor.js';
|
import '@components/smart-playlist-editor/smart-playlist-editor.js';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import { list } from '@utils/binding';
|
import { list } from '@utils/binding';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -1481,7 +1485,7 @@ export class SmartPlaylistDetails
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_QUEUE}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
@@ -1521,7 +1525,7 @@ export class SmartPlaylistDetails
|
|||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
slot="icon"
|
slot="icon"
|
||||||
name="plus"
|
name=${ICON_PLAYLIST}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span
|
<span
|
||||||
|
|||||||
@@ -74,6 +74,10 @@ import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js';
|
|||||||
import '@components/playlist-picker/playlist-picker.js';
|
import '@components/playlist-picker/playlist-picker.js';
|
||||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||||
import type { CoverArtUrls } from '@components/track-details/track-details.js';
|
import type { CoverArtUrls } from '@components/track-details/track-details.js';
|
||||||
|
import {
|
||||||
|
ICON_PLAYLIST,
|
||||||
|
ICON_QUEUE,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
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';
|
||||||
@@ -2342,7 +2346,7 @@ export class TrackList
|
|||||||
@click=${() => this.onContextMenuAction('add-to-queue')}
|
@click=${() => this.onContextMenuAction('add-to-queue')}
|
||||||
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
@mouseenter=${() => this.ctxMenu.closePlaylistSubmenu()}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_QUEUE}></wa-icon>
|
||||||
Add to Queue
|
Add to Queue
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
<wa-dropdown-item
|
<wa-dropdown-item
|
||||||
@@ -2364,7 +2368,7 @@ export class TrackList
|
|||||||
void this.ctxMenu.showPlaylistSubmenu(this.selection.getSelectedKeysOrdered());
|
void this.ctxMenu.showPlaylistSubmenu(this.selection.getSelectedKeysOrdered());
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<wa-icon slot="icon" name="plus"></wa-icon>
|
<wa-icon slot="icon" name=${ICON_PLAYLIST}></wa-icon>
|
||||||
Add to Playlist
|
Add to Playlist
|
||||||
<span class="submenu-arrow">▶</span>
|
<span class="submenu-arrow">▶</span>
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ solid/arrows-rotate
|
|||||||
solid/arrow-up-short-wide
|
solid/arrow-up-short-wide
|
||||||
solid/backward-step
|
solid/backward-step
|
||||||
solid/bars
|
solid/bars
|
||||||
|
solid/bars-staggered
|
||||||
regular/bookmark
|
regular/bookmark
|
||||||
solid/bookmark
|
solid/bookmark
|
||||||
solid/box-open
|
solid/box-open
|
||||||
|
|||||||
@@ -0,0 +1,100 @@
|
|||||||
|
/**
|
||||||
|
* What each icon in this app means, once.
|
||||||
|
*
|
||||||
|
* The set was a mix: `plus` meant "add to the queue", "add to a
|
||||||
|
* playlist", "make a new playlist" and "you do not own this" — the
|
||||||
|
* first two *adjacent in the same context menu* — while `list` meant
|
||||||
|
* the queue, the Playlists destination, and (in `queue-panel` alone)
|
||||||
|
* adding to the queue. Two icons carrying seven meanings between them
|
||||||
|
* is not a vocabulary, and a user cannot learn one that says four
|
||||||
|
* things.
|
||||||
|
*
|
||||||
|
* The rule these are chosen by: **an icon names the noun it acts on,
|
||||||
|
* not the verb.** "Add to queue" and "add to playlist" are the same
|
||||||
|
* verb on different nouns, so the noun is what has to differ — which is
|
||||||
|
* also why adding to a playlist wears the Playlists destination's own
|
||||||
|
* icon rather than a generic plus. `plus` survives for exactly the one
|
||||||
|
* thing it is unambiguous about, making something that did not exist.
|
||||||
|
*
|
||||||
|
* Import these rather than writing a name inline. A literal string is
|
||||||
|
* how the last set drifted, and nothing catches it: a wrong-but-real
|
||||||
|
* icon renders perfectly.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** Start playing this now. */
|
||||||
|
export const ICON_PLAY = 'play';
|
||||||
|
|
||||||
|
/** Start playing this now, in a shuffled order. */
|
||||||
|
export const ICON_SHUFFLE = 'shuffle';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The queue, and putting something into it.
|
||||||
|
*
|
||||||
|
* One glyph for the noun and the action, so the button that opens the
|
||||||
|
* queue and the menu item that adds to it are visibly the same subject.
|
||||||
|
* The queue used to wear `list`, which is the Playlists destination.
|
||||||
|
*/
|
||||||
|
export const ICON_QUEUE = 'bars-staggered';
|
||||||
|
|
||||||
|
/** Put this next in the queue rather than at the end. */
|
||||||
|
export const ICON_PLAY_NEXT = 'forward-step';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A playlist, and adding something to one.
|
||||||
|
*
|
||||||
|
* The same icon as the Playlists destination in the sidebar, which is
|
||||||
|
* the point: the menu item says where the thing is going.
|
||||||
|
*/
|
||||||
|
export const ICON_PLAYLIST = 'list';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Make a new thing that did not exist — a playlist, a rule, a library.
|
||||||
|
*
|
||||||
|
* This is the only meaning `plus` keeps. It used to carry four.
|
||||||
|
*/
|
||||||
|
export const ICON_NEW = 'plus';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The request ("want") toggle, as an outline/solid pair.
|
||||||
|
*
|
||||||
|
* Two states of one control have to read as each other's opposite,
|
||||||
|
* which a plus and a bookmark do not. The pair was already in the app
|
||||||
|
* and already correct — `explore-album-details`'s "Want this" button
|
||||||
|
* has used it since it was written, and `favorites-controller` uses the
|
||||||
|
* same shape for `regular/heart` → `heart` — while the badge forty
|
||||||
|
* pixels away showed a plus for the same state.
|
||||||
|
*
|
||||||
|
* That is `utils/library-status.ts`'s fault one layer down: it made the
|
||||||
|
* two surfaces agree on *what wanting means* and left them disagreeing
|
||||||
|
* on what it looks like.
|
||||||
|
*/
|
||||||
|
export const ICON_CAN_REQUEST = 'regular/bookmark';
|
||||||
|
export const ICON_REQUESTED = 'solid/bookmark';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* You have this.
|
||||||
|
*
|
||||||
|
* Deliberately not drawn on the common case — see the tracklist, where
|
||||||
|
* absence is what gets marked. This is for the places that answer the
|
||||||
|
* question directly, like the badge on a catalog card.
|
||||||
|
*/
|
||||||
|
export const ICON_IN_LIBRARY = 'check';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Something is being fetched right now.
|
||||||
|
*
|
||||||
|
* Distinct from `ICON_REQUESTED`: a request may sit on the list
|
||||||
|
* forever without anything happening, which is exactly why the badge's
|
||||||
|
* "queued" state stopped being an hourglass.
|
||||||
|
*/
|
||||||
|
export const ICON_DOWNLOADING = 'download';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Take this away.
|
||||||
|
*
|
||||||
|
* One icon for removing from a playlist, from the queue and from the
|
||||||
|
* library, because the difference that matters is stated in the words
|
||||||
|
* beside it and in the confirmation — "Remove from Library" says in its
|
||||||
|
* impact line that the files are not deleted.
|
||||||
|
*/
|
||||||
|
export const ICON_REMOVE = 'trash';
|
||||||
@@ -0,0 +1,238 @@
|
|||||||
|
/**
|
||||||
|
* Asking to see the whole album.
|
||||||
|
*
|
||||||
|
* The page could already draw the full release with the missing rows
|
||||||
|
* dimmed, and did so automatically once the tags said the album was
|
||||||
|
* incomplete. What it could not do was be *asked*: the rule depends on
|
||||||
|
* the files declaring a per-disc total, so where they declare none —
|
||||||
|
* which is a great deal of any library — a partly-owned album showed
|
||||||
|
* only the tracks on disk and nothing said the rest existed.
|
||||||
|
*
|
||||||
|
* The switch is the explicit route. Its rules are all one rule: a
|
||||||
|
* control that cannot change what is on screen is worse than no
|
||||||
|
* control, which is the same test the version dropdown answers.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import { page } from 'vitest/browser';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/explore-album-details/explore-album-details';
|
||||||
|
import { stub, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
const MBID = 'rg-0001';
|
||||||
|
|
||||||
|
function track(n: number, owned = false) {
|
||||||
|
return {
|
||||||
|
position: n,
|
||||||
|
discNumber: 1,
|
||||||
|
title: `Track ${n}`,
|
||||||
|
length: 200000,
|
||||||
|
mbid: `rec-${n}`,
|
||||||
|
inLibrary: owned,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function release(mbid: string, date: string, trackCount: number, owned = 0) {
|
||||||
|
return {
|
||||||
|
mbid,
|
||||||
|
title: 'Glass Harbour',
|
||||||
|
date,
|
||||||
|
status: 'Official',
|
||||||
|
tracks: Array.from({ length: trackCount }, (_, i) =>
|
||||||
|
track(i + 1, i < owned),
|
||||||
|
),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Local files with no recording MBIDs — an untagged rip, which is the
|
||||||
|
* case the automatic rule cannot see. */
|
||||||
|
function localTracks(count: number) {
|
||||||
|
return Array.from({ length: count }, (_, i) => ({
|
||||||
|
TrackName: `Track ${i + 1}`,
|
||||||
|
TrackNumber: i + 1,
|
||||||
|
DiscNumber: 1,
|
||||||
|
TrackLength: '210000',
|
||||||
|
RecordingMBID: '',
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
const UNKNOWN = { owned: 0, expected: 0, known: false, complete: false };
|
||||||
|
|
||||||
|
async function albumWith(
|
||||||
|
releases: unknown[],
|
||||||
|
completeness: Record<string, unknown>,
|
||||||
|
local: unknown[] = [],
|
||||||
|
): Promise<LitElement> {
|
||||||
|
stub('explore.Service.BrowseReleases', releases);
|
||||||
|
stub('library.Library.GetAlbumCompleteness', completeness);
|
||||||
|
stub('library.Library.GetAlbumTracks', local);
|
||||||
|
|
||||||
|
const el = await fixture<LitElement>('explore-album-details', {
|
||||||
|
releaseGroupMBID: MBID,
|
||||||
|
localAlbumId: 7,
|
||||||
|
albumName: 'Glass Harbour',
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const scopeSwitch = (el: LitElement) => shadow(el, '.tracklist-scope wa-switch');
|
||||||
|
|
||||||
|
async function toggle(el: LitElement) {
|
||||||
|
const sw = scopeSwitch(el) as HTMLInputElement | null;
|
||||||
|
if (!sw) throw new Error('no tracklist scope switch on the page');
|
||||||
|
|
||||||
|
sw.checked = !sw.checked;
|
||||||
|
sw.dispatchEvent(new Event('change'));
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the "show the whole album" switch', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('explore.Service.LookupReleaseGroup', {
|
||||||
|
mbid: MBID,
|
||||||
|
title: 'Glass Harbour',
|
||||||
|
artistCredit: 'Tideline',
|
||||||
|
});
|
||||||
|
stub('explore.Service.GetThumbnail', '');
|
||||||
|
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||||
|
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The report, exactly: two tracks on disk, twelve on the release,
|
||||||
|
* and nothing to say so because the tags declared no total.
|
||||||
|
*/
|
||||||
|
it('reveals the rest of the release when the total is unknown', async () => {
|
||||||
|
const el = await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 12, 2)],
|
||||||
|
UNKNOWN,
|
||||||
|
localTracks(2),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(shadowAll(el, '.track-row')).toHaveLength(2);
|
||||||
|
|
||||||
|
await toggle(el);
|
||||||
|
|
||||||
|
const rows = shadowAll(el, '.track-row');
|
||||||
|
expect(rows).toHaveLength(12);
|
||||||
|
// Nothing resolves to a file, so every row is marked unowned —
|
||||||
|
// the dimming is the signal, and it is not this switch's job to
|
||||||
|
// invent ownership it cannot prove.
|
||||||
|
expect(rows.filter((r) => r.classList.contains('unowned'))).toHaveLength(12);
|
||||||
|
});
|
||||||
|
|
||||||
|
/** And back again — a switch that only goes one way is a button. */
|
||||||
|
it('goes back to the files on disk', async () => {
|
||||||
|
const el = await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 12, 2)],
|
||||||
|
UNKNOWN,
|
||||||
|
localTracks(2),
|
||||||
|
);
|
||||||
|
|
||||||
|
await toggle(el);
|
||||||
|
expect(shadowAll(el, '.track-row')).toHaveLength(12);
|
||||||
|
|
||||||
|
await toggle(el);
|
||||||
|
expect(shadowAll(el, '.track-row')).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The automatic rule still fires, and the control has to agree with
|
||||||
|
* the page it is sitting on rather than starting out contradicting
|
||||||
|
* it. This is what the tri-state is for.
|
||||||
|
*/
|
||||||
|
it('starts checked when the tags already said the album is short', async () => {
|
||||||
|
stub(
|
||||||
|
'library.Library.GetFilePathsByRecordingMBIDs',
|
||||||
|
Object.fromEntries(
|
||||||
|
Array.from({ length: 9 }, (_, i) => [
|
||||||
|
`rec-${i + 1}`,
|
||||||
|
[`/music/0${i + 1}.mp3`],
|
||||||
|
]),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
const el = await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 12, 9)],
|
||||||
|
{ owned: 9, expected: 12, known: true, complete: false },
|
||||||
|
localTracks(9),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(shadowAll(el, '.track-row')).toHaveLength(12);
|
||||||
|
expect((scopeSwitch(el) as HTMLInputElement).checked).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
/** And the user outranks it: turning it off asks for the files. */
|
||||||
|
it('lets the automatic answer be overridden', async () => {
|
||||||
|
const el = await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 12, 9)],
|
||||||
|
{ owned: 9, expected: 12, known: true, complete: false },
|
||||||
|
localTracks(9),
|
||||||
|
);
|
||||||
|
|
||||||
|
await toggle(el);
|
||||||
|
|
||||||
|
expect(shadowAll(el, '.track-row')).toHaveLength(9);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A control the accessibility tree cannot name is not a control, and
|
||||||
|
* this app has shipped that fault twice — `wa-slider` pointed
|
||||||
|
* `aria-labelledby` at an empty internal label, and `config-field`
|
||||||
|
* rendered a `<label>` as a sibling with no `for`.
|
||||||
|
*
|
||||||
|
* `wa-switch` gets it right for a *different* reason than either:
|
||||||
|
* its `<input role="switch">` sits inside a native `<label>` that also
|
||||||
|
* holds the `<slot>`, so the name is computed across the flattened
|
||||||
|
* tree from light-DOM text. That is worth an assertion rather than an
|
||||||
|
* assumption — and it has to be the browser's own answer, since
|
||||||
|
* querying shadow roots cannot compute a name.
|
||||||
|
*/
|
||||||
|
it('is named for anyone not looking at it', async () => {
|
||||||
|
await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 12, 2)],
|
||||||
|
UNKNOWN,
|
||||||
|
localTracks(2),
|
||||||
|
);
|
||||||
|
|
||||||
|
await expect
|
||||||
|
.element(page.getByRole('switch', { name: 'Show the whole album' }))
|
||||||
|
.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('is absent where it could not change anything', () => {
|
||||||
|
it('when the album is entirely owned', async () => {
|
||||||
|
// Ten files, a ten-track release: the switch would redraw the
|
||||||
|
// same list, which reads as broken.
|
||||||
|
const el = await albumWith(
|
||||||
|
[release('rel-1', '2019-04-01', 10, 10)],
|
||||||
|
{ owned: 10, expected: 10, known: true, complete: true },
|
||||||
|
localTracks(10),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(scopeSwitch(el)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('when there is no catalog release to switch to', async () => {
|
||||||
|
const el = await albumWith([], UNKNOWN, localTracks(4));
|
||||||
|
|
||||||
|
expect(scopeSwitch(el)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('when the album is not in the library at all', async () => {
|
||||||
|
// Every entry here is already a catalog tracklist; there is no
|
||||||
|
// "only my tracks" to go back to.
|
||||||
|
const el = await albumWith([release('rel-1', '2019-04-01', 12)], UNKNOWN);
|
||||||
|
|
||||||
|
expect(scopeSwitch(el)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -19,6 +19,11 @@ import {
|
|||||||
update,
|
update,
|
||||||
visual,
|
visual,
|
||||||
} from '@test/support/render';
|
} from '@test/support/render';
|
||||||
|
import {
|
||||||
|
ICON_CAN_REQUEST,
|
||||||
|
ICON_IN_LIBRARY,
|
||||||
|
ICON_REQUESTED,
|
||||||
|
} from '@utils/icon-language';
|
||||||
|
|
||||||
describe('<app-sidebar>', () => {
|
describe('<app-sidebar>', () => {
|
||||||
it('renders a testid per destination, which is how e2e navigates', async () => {
|
it('renders a testid per destination, which is how e2e navigates', async () => {
|
||||||
@@ -154,9 +159,20 @@ describe('<library-status-indicator>', () => {
|
|||||||
it('defaults to "not in library"', async () => {
|
it('defaults to "not in library"', async () => {
|
||||||
const el = await fixture('library-status-indicator');
|
const el = await fixture('library-status-indicator');
|
||||||
|
|
||||||
expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe('plus');
|
expect(shadow(el, 'wa-icon')?.getAttribute('name')).toBe(ICON_CAN_REQUEST);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Named from the vocabulary rather than written out, or this test
|
||||||
|
* pins the glyphs *against* the table it is supposed to follow —
|
||||||
|
* which is what it did: it asserted `plus` for the un-owned state,
|
||||||
|
* the same glyph two adjacent menu items were using for two other
|
||||||
|
* meanings, and passing was the reason nobody looked.
|
||||||
|
*
|
||||||
|
* What is still worth asserting is that the three differ, which is
|
||||||
|
* the property the states need and the one the table cannot state
|
||||||
|
* about itself here.
|
||||||
|
*/
|
||||||
it('uses a distinct glyph per state', async () => {
|
it('uses a distinct glyph per state', async () => {
|
||||||
const glyphs: (string | null | undefined)[] = [];
|
const glyphs: (string | null | undefined)[] = [];
|
||||||
|
|
||||||
@@ -166,7 +182,8 @@ describe('<library-status-indicator>', () => {
|
|||||||
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
|
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
|
||||||
}
|
}
|
||||||
|
|
||||||
expect(glyphs).toEqual(['check', 'bookmark', 'plus']);
|
expect(glyphs).toEqual([ICON_IN_LIBRARY, ICON_REQUESTED, ICON_CAN_REQUEST]);
|
||||||
|
expect(new Set(glyphs).size).toBe(3);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('phrases its label around the entity it describes', async () => {
|
it('phrases its label around the entity it describes', async () => {
|
||||||
|
|||||||
@@ -0,0 +1,140 @@
|
|||||||
|
/**
|
||||||
|
* The icon vocabulary is one table, and nothing writes around it.
|
||||||
|
*
|
||||||
|
* A wrong-but-real icon name renders perfectly: no error, no fallback,
|
||||||
|
* no failing assertion anywhere. That is how `plus` came to mean "add
|
||||||
|
* to the queue", "add to a playlist", "make a new playlist" and "you do
|
||||||
|
* not own this" — the first two adjacent in the same context menu —
|
||||||
|
* while `list` meant the queue, the Playlists destination *and* adding
|
||||||
|
* to the queue.
|
||||||
|
*
|
||||||
|
* `src/icons/index.ts` catches a name that is not *bundled*. Nothing
|
||||||
|
* catches a name that is bundled and means something else, so this
|
||||||
|
* sweeps the source for the governed ones. It is the same shape as
|
||||||
|
* `TestNoDirectRuntimeEmits` and `TestNoWritesOnTheReadPool` in the
|
||||||
|
* backend, and exists for the same reason: the rule is about every call
|
||||||
|
* site, so checking one is checking nothing.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import { bundledIconNames } from '../../src/icons';
|
||||||
|
import * as icons from '@utils/icon-language';
|
||||||
|
|
||||||
|
/** Every component source, as text. */
|
||||||
|
const SOURCES = import.meta.glob<string>('../../src/**/*.ts', {
|
||||||
|
eager: true,
|
||||||
|
query: '?raw',
|
||||||
|
import: 'default',
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The names that carry a meaning the table owns.
|
||||||
|
*
|
||||||
|
* Deliberately not every bundled name. `check` is `ICON_IN_LIBRARY`
|
||||||
|
* here and also the "Copied" confirmation in `job-log-view`, which is
|
||||||
|
* a different, perfectly good meaning — governing it would force a
|
||||||
|
* false rename. What belongs on this list is a name that was actually
|
||||||
|
* overloaded.
|
||||||
|
*/
|
||||||
|
const GOVERNED = [
|
||||||
|
'plus',
|
||||||
|
'list',
|
||||||
|
'bookmark',
|
||||||
|
'solid/bookmark',
|
||||||
|
'regular/bookmark',
|
||||||
|
'bars-staggered',
|
||||||
|
];
|
||||||
|
|
||||||
|
/** The one file allowed to say them, plus its own test. */
|
||||||
|
const DEFINITION = /icon-language\.(ts|test\.ts)$/;
|
||||||
|
|
||||||
|
describe('the icon vocabulary', () => {
|
||||||
|
/**
|
||||||
|
* A sweep over nothing passes. This is the assertion that makes the
|
||||||
|
* rest of the file mean something, and it is the first thing that
|
||||||
|
* breaks if the glob pattern stops matching after a move.
|
||||||
|
*/
|
||||||
|
it('actually reads the source', () => {
|
||||||
|
const paths = Object.keys(SOURCES);
|
||||||
|
|
||||||
|
expect(paths.length).toBeGreaterThan(100);
|
||||||
|
expect(paths.some((p) => p.endsWith('/track-list.ts'))).toBe(true);
|
||||||
|
expect(SOURCES[paths[0]!]).toContain('import');
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each(GOVERNED)('is not written around for %s', (name) => {
|
||||||
|
const offenders: string[] = [];
|
||||||
|
|
||||||
|
for (const [path, source] of Object.entries(SOURCES)) {
|
||||||
|
if (DEFINITION.test(path)) continue;
|
||||||
|
|
||||||
|
// Both spellings: an icon in a template, and an icon name in a
|
||||||
|
// data table (which is how the sidebar and bottom-nav carry
|
||||||
|
// theirs).
|
||||||
|
const literal = new RegExp(
|
||||||
|
`(name="${name}"|icon: '${name}'|name=\\$\\{[^}]*'${name}')`,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (literal.test(source)) offenders.push(path);
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(offenders).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A meaning with no icon behind it is the state the badge's `queued`
|
||||||
|
* spent a year in — declared, styled, and produced by nothing.
|
||||||
|
*/
|
||||||
|
it('gives every meaning a name', () => {
|
||||||
|
const values = Object.entries(icons).filter(([k]) => k.startsWith('ICON_'));
|
||||||
|
|
||||||
|
expect(values.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
for (const [key, value] of values) {
|
||||||
|
expect(`${key}=${value}`).toMatch(/^ICON_[A-Z_]+=[a-z]+[a-z/-]*$/);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Every name in the table is a name the app actually ships.
|
||||||
|
*
|
||||||
|
* This is the loop the vocabulary closes. A name that is not bundled
|
||||||
|
* renders a circled question mark and reports itself to
|
||||||
|
* `__yjIconMisses` — at *runtime*, from a state something has to
|
||||||
|
* reach first. `bookmark-check` is Font Awesome **Pro**, and it was
|
||||||
|
* on `explore-artist-details`'s Follow button, drawn for every
|
||||||
|
* followed artist, invisible to `offline-icons.spec.ts` because no
|
||||||
|
* spec had ever followed one. Reaching the state is no longer how
|
||||||
|
* this is found.
|
||||||
|
*/
|
||||||
|
it('names only icons that are bundled', () => {
|
||||||
|
const bundled = new Set(bundledIconNames());
|
||||||
|
const missing = Object.entries(icons)
|
||||||
|
.filter(([k]) => k.startsWith('ICON_'))
|
||||||
|
.filter(([, v]) => !bundled.has(v as string))
|
||||||
|
.map(([k, v]) => `${k} (${v})`);
|
||||||
|
|
||||||
|
expect(missing).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The two states of the request toggle have to be the same glyph in
|
||||||
|
* two weights, or they do not read as each other's opposite — which
|
||||||
|
* is what a plus against a bookmark was.
|
||||||
|
*/
|
||||||
|
it('makes the request toggle an outline/solid pair', () => {
|
||||||
|
expect(icons.ICON_CAN_REQUEST).toBe(`regular/${icons.ICON_REQUESTED.replace('solid/', '')}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The queue and the Playlists destination wore the same icon, and
|
||||||
|
* "add to queue" and "add to playlist" sat next to each other wearing
|
||||||
|
* a third same one. Whatever the table says, these three have to
|
||||||
|
* differ from each other.
|
||||||
|
*/
|
||||||
|
it('keeps the queue, playlists and creating something apart', () => {
|
||||||
|
const three = [icons.ICON_QUEUE, icons.ICON_PLAYLIST, icons.ICON_NEW];
|
||||||
|
|
||||||
|
expect(new Set(three).size).toBe(3);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user