Compare commits
13
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4b2eec5703 | ||
|
|
3871d37fdb | ||
|
|
09b005557c | ||
|
|
ef5574d18b | ||
|
|
31dafb0ce0 | ||
|
|
9e7e7ce5a1 | ||
|
|
9aaa8beb99 | ||
|
|
2e29e67664 | ||
|
|
f26b44db08 | ||
|
|
b43172a60c | ||
|
|
2be6fb3066 | ||
|
|
867ced8c81 | ||
|
|
fd71ef53c5 |
@@ -0,0 +1,142 @@
|
||||
---
|
||||
description: Take on the next actionable backlog issue end to end, and stop
|
||||
---
|
||||
Take on exactly one issue from the YellowJacket backlog, end to end, and stop.
|
||||
|
||||
Repo: yonlu/yellowjacket at https://git.ljones.me — API base
|
||||
https://git.ljones.me/api/v1/repos/yonlu/yellowjacket, auth with
|
||||
`-H "Authorization: token $GITEA_TOKEN"`. Default branch is `main`.
|
||||
|
||||
## 1. Orient before you pick
|
||||
|
||||
Read, in this order: `CLAUDE.md` (the architecture and the reasons behind
|
||||
it), `.pi/journal.md` (what happened last), `.planning/NOTES.md` (what was
|
||||
already considered and rejected), and `.planning/plans/active/`. Do not skip
|
||||
this because the issue looks small — most of this codebase's traps are
|
||||
written down in exactly one of those four places, and the ones that bite are
|
||||
the ones you didn't read.
|
||||
|
||||
## 2. Pick the issue
|
||||
|
||||
List open issues. Choose the single highest-value one that is *actionable
|
||||
right now*:
|
||||
|
||||
- Order by `Priority/Critical` → `High` → `Medium` → `Low`. Within a tier,
|
||||
prefer `Reviewed/Confirmed`, then `Kind/Bug` over `Kind/Enhancement` over
|
||||
`Kind/Feature`.
|
||||
- Consult issue #73 (the roadmap) — if it sequences the candidates, that
|
||||
ordering wins over the label ordering.
|
||||
- **Skip** anything labelled `Status/Blocked`, `Status/In Progress`,
|
||||
`Status/Abandoned`, `Reviewed/Won't Fix`, `Reviewed/Duplicate`,
|
||||
`Reviewed/Invalid`, or already carrying an open PR.
|
||||
- **Skip anything someone else is already on.** The label is not the only
|
||||
claim, because a concurrent session may not have applied it — several pi
|
||||
sessions run against this repo from separate worktrees under
|
||||
`~/.paseo/worktrees/`. Run `git ls-remote --heads origin` and skip any
|
||||
issue whose number or slug matches an existing branch (`60-…`,
|
||||
`fix/<slug>`). A duplicated fix costs more than a skipped issue.
|
||||
- **Skip** anything that cannot be verified without hardware you do not
|
||||
have: physical-device Android behaviour (audio output, on-device file
|
||||
writes, real gesture input). A browser at 424px is not a phone — see the
|
||||
Chrome 113 section of `CLAUDE.md`.
|
||||
- **Skip** intermittent-failure issues unless you can reproduce the failure
|
||||
on demand within a few minutes. Chasing a 1-in-3 flake is an unbounded
|
||||
task and does not belong in a scheduled run.
|
||||
- If nothing qualifies, say so, do nothing, and stop. An empty run is a
|
||||
correct outcome.
|
||||
|
||||
## 3. Claim it
|
||||
|
||||
Add `Status/In Progress` to the issue and comment that you are picking it
|
||||
up. Then branch:
|
||||
|
||||
```
|
||||
git fetch origin && git checkout -b <type>/<short-slug> origin/main
|
||||
```
|
||||
|
||||
`<type>` matches the issue's `Kind` (`fix/`, `feat/`, `refactor/`, `test/`,
|
||||
`docs/`, `ci/`). Branch from `origin/main`, never by checking out `main`
|
||||
itself — this repo is worked from several git worktrees at once and `main`
|
||||
is checked out in one of them, so `git checkout main` fails outright.
|
||||
|
||||
## 4. Do the work
|
||||
|
||||
Fix the issue that was reported and nothing else. Match the surrounding
|
||||
code's style. Follow the constraints in `CLAUDE.md` rather than reasoning
|
||||
from first principles — where it explains why something is shaped the way it
|
||||
is, that shape is load-bearing and there is usually a test pinning it.
|
||||
|
||||
**Anything else you discover becomes a new issue, not a bigger diff.** File
|
||||
it with the right `Area/`, `Kind/`, `Priority/` labels, describe the
|
||||
symptom before the theory, and link it from your PR. Scope creep is the
|
||||
failure mode this instruction exists to prevent.
|
||||
|
||||
If the work turns out to be materially larger than the issue implied, stop:
|
||||
comment on the issue with what you found and what it would actually take,
|
||||
remove `Status/In Progress`, push nothing, and end the run.
|
||||
|
||||
## 5. Verify — the right tier, not the cheapest one
|
||||
|
||||
Run `make generate` if you touched `.sql` or `.templ`, and `make bindings`
|
||||
if you changed a bound Go signature. Then run what the change actually
|
||||
demands:
|
||||
|
||||
- Go change → `make lint` and `make test` (both cover all three build
|
||||
configurations).
|
||||
- Frontend component or store → `make ui-test`.
|
||||
- User-visible flow → `make e2e` against `make dev-headless`. **Check the
|
||||
port first**: `ss -ltn | grep 34115`. If it is occupied, another worktree
|
||||
is already running the app — do not start a second one and do not run
|
||||
`make e2e`. Attaching to someone else's build produces a green result
|
||||
about code that is not yours, which is worse than no result. Either
|
||||
choose an issue that does not need this tier, or stop and say why.
|
||||
- Anything cosmetic or layout-related → look at a screenshot. Several bugs
|
||||
in this repo's history were invisible to every assertion and obvious in an
|
||||
image.
|
||||
|
||||
A tier you skipped is a claim you did not check. If a tier fails for reasons
|
||||
unrelated to your change, say so explicitly rather than quietly moving on.
|
||||
|
||||
## 6. Keep the documentation true
|
||||
|
||||
If you changed structure, behaviour, or a constraint, update `CLAUDE.md` in
|
||||
the same commit. That file is this project's memory; a change that leaves it
|
||||
describing the old shape is worse than no change. Append a short entry to
|
||||
`.pi/journal.md` covering what you did, what you verified, and what you left
|
||||
open.
|
||||
|
||||
## 7. Commit and open the PR
|
||||
|
||||
Conventional Commits, imperative subject, ≤72 chars, scope optional. The
|
||||
body explains *why*. Push the branch — never push to `main`, never
|
||||
force-push.
|
||||
|
||||
Open the PR:
|
||||
|
||||
```
|
||||
curl -sS -X POST \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
https://git.ljones.me/api/v1/repos/yonlu/yellowjacket/pulls \
|
||||
-d '{"head":"<branch>","base":"main","title":"<subject>","body":"<body>"}'
|
||||
```
|
||||
|
||||
The body states: what the issue was, what you changed and why, **which
|
||||
verification tiers you ran and their results**, anything you deliberately
|
||||
did not do, and `Closes #<n>`.
|
||||
|
||||
Then wait for CI (`ci.yml`, jobs `check` and `e2e`) and report the result on
|
||||
the PR. If it fails, read the log — `gitea_ci`'s `job_logs` 404s on this
|
||||
Gitea build, so use
|
||||
`GET /api/v1/repos/yonlu/yellowjacket/actions/runs/<run>/jobs` for per-step
|
||||
status and `GET /api/v1/repos/yonlu/yellowjacket/actions/jobs/<id>/logs` for
|
||||
the log — and fix it. Two consecutive failed CI runs on the same cause: stop,
|
||||
comment what you know on the PR, and leave it for a human.
|
||||
|
||||
**Do not merge.** Comment on the issue linking the PR, leave
|
||||
`Status/In Progress` on, and end the run.
|
||||
|
||||
## Finally
|
||||
|
||||
Report in three lines: which issue you took, what state it is in
|
||||
(PR open / CI green / stopped and why), and any issues you filed.
|
||||
@@ -4573,3 +4573,215 @@ fault in it, and it is filed as #172 with the per-element budget. #64
|
||||
umbrella; folding shuffle and repeat back onto the primary row was
|
||||
considered and rejected — it buys 52px, leaves the art at 91px, and
|
||||
costs a third arrangement of the same five buttons.
|
||||
|
||||
## The volume is not ours on Android, and the predicate could not be a width (measured 2026-08-21)
|
||||
|
||||
#64 asked for the in-app volume control to be absent on Android. Its
|
||||
first Finding said `volume-control` "already stands down at narrow
|
||||
widths", which was true of one of its two copies and is why the issue
|
||||
had been read as nearly done. The bar's copy goes by width; the
|
||||
full-screen view's copy was deliberately kept, with a comment saying a
|
||||
slider does belong there.
|
||||
|
||||
**The crux was platform versus width, and three options were on the
|
||||
issue.** What settled it is that the *backend* half of the same issue —
|
||||
pin the level at 1.0 — makes a width rule wrong on the platform the
|
||||
issue is about: an Android tablet at >=600px gets the bottom bar, and
|
||||
the bar's slider would then move a level that is pinned. That is a
|
||||
control that cannot act, which `library-status-indicator` already
|
||||
settled is worse than none. The same rule is wrong the other way below
|
||||
600px, where a narrow desktop window has no hardware keys.
|
||||
|
||||
So the frontend asks the player — `SystemOwnsVolume` — and the answer
|
||||
is right at every width in both mount points. **The predicate is named
|
||||
after the capability rather than the platform**, which is what makes it
|
||||
testable: only `platformOwnsVolume` is behind a build tag, in two files
|
||||
that declare nothing else, and everything else is decided against a
|
||||
field a Go test sets either way. `frontend/test/components/
|
||||
volume-ownership.test.ts` stubs the binding and so exercises the
|
||||
*Android* rendering on an ordinary Linux runner; both of its tests were
|
||||
confirmed to fail on the build before the change.
|
||||
|
||||
**Measured at 424x439, by flipping `platformOwnsVolume` to true in the
|
||||
`!android` file and rebuilding** — the real binding, the real store, the
|
||||
real component, everything except the tag:
|
||||
|
||||
| element | before | after |
|
||||
|---|---|---|
|
||||
| header | 48 | 48 |
|
||||
| **album art** | **39** | **68** |
|
||||
| title / artist / album | 63 | 63 |
|
||||
| transport (seek + controls + volume) | **172** | **143** |
|
||||
| — seek bar | 19 | 19 |
|
||||
| — player-controls | 116 | 116 |
|
||||
| — volume-control | 21 | **0** |
|
||||
|
||||
29px, which is the 21px control plus the 8px flex gap it stops drawing:
|
||||
a gap is only painted between boxes, so `:host([hidden])` costs the
|
||||
transport nothing rather than leaving a hole. That is #172's "~30px of
|
||||
pure gain" confirmed, and the art is 74% larger. It is still the
|
||||
second-smallest thing on the screen, which is #51's evidence.
|
||||
|
||||
Three smaller things worth keeping.
|
||||
|
||||
**`:host([hidden])` has to be written down.** The UA's `[hidden]`
|
||||
rule is `display: none`, but `volume-control`'s own `:host` sets
|
||||
`display: inline-flex` and outranks it — so setting `hidden` alone
|
||||
hides nothing. Same family as the nested-`#queue-button` specificity
|
||||
trap from the session before.
|
||||
|
||||
**Rendering `nothing` and hiding the host are two different
|
||||
assertions**, and the component test makes both: an empty shadow root
|
||||
is what stops a by-role or positional query finding a button that
|
||||
cannot act, and `hidden` is what stops the host occupying space. Either
|
||||
alone passes on a build that gets the other wrong.
|
||||
|
||||
**The bar's centring survives the control going away.** #23's outer
|
||||
columns are the same `min()` expression rather than content-sized, so
|
||||
at 900px with the volume gone the bar's centre, `audio-player`'s centre
|
||||
and `player-controls`' centre are all 450 — checked, because "the
|
||||
transport is centred with a slider bolted to one side" is the fault
|
||||
that rule exists for and removing the slider is the obvious way to
|
||||
re-break it.
|
||||
|
||||
**What no tier here can check**: the constant itself, and ducking
|
||||
against a real audio-focus change. The first is a source sweep
|
||||
(`TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform`), the second is
|
||||
`TestSystemVolumeStillDucks` against the arithmetic. Neither is a
|
||||
device, and no device was attached.
|
||||
|
||||
### The device answered three of the four (measured 2026-08-21, TLP301 / Android 14 / SDK 34 / arm64, Chrome 113 at 424x439)
|
||||
|
||||
A Light Phone III was attached after the PR was opened, so what that PR
|
||||
listed as unverifiable was re-checked rather than left as a caveat.
|
||||
|
||||
**The whole chain resolves on the device.** `__yj.call("player.Player.
|
||||
SystemOwnsVolume", [])` answers `true` — build tag, `platformOwnsVolume`,
|
||||
`Player.systemVolume` and the generated binding, end to end. That is the
|
||||
one thing the source sweep only approximates, and it took a real arm64
|
||||
device because nothing else here compiles the `android` file at all.
|
||||
(`GOOS=android GOARCH=arm64 CGO_ENABLED=1 go build ./backend/...` with
|
||||
the NDK's clang compiles it in ~40 s and is worth running first; it
|
||||
catches a type error but not a wrong constant.)
|
||||
|
||||
**The control is absent in both mount points**, on the real engine:
|
||||
`.bottom-bar volume-control` is `hidden` with an empty shadow root, and
|
||||
so is `now-playing-view`'s. Measured on the device, transport **143px**,
|
||||
which is the figure the desktop-headless "after" predicted exactly. The
|
||||
art is 75px there rather than 68 because the fixture's `.names` block is
|
||||
one line shorter, not because anything differs.
|
||||
|
||||
**Nothing persists a level nobody chose, and this is the measurement
|
||||
that took some care.** The default (50) surviving proves nothing, since
|
||||
50 is also what a fresh row holds. So: force-stop, pull `yj.db`, set
|
||||
`player_state.volume = 37`, push it back through
|
||||
`run-as … dd` (a `cp` from `/sdcard` is refused — the app sandbox
|
||||
cannot read it), relaunch, and drive a queue change to make the row be
|
||||
rewritten. Reading it back **the WAL has to be pulled with it** — the
|
||||
main file still showed the old `last_track_path` and reads as a write
|
||||
that never happened. With `yj.db-wal` beside it: `last_track_path` is
|
||||
the new track, so `saveState` ran, and `volume` is still **37**.
|
||||
|
||||
**The duck cannot be verified on this device, and now for a stated
|
||||
reason rather than for want of hardware.** `WailsForegroundService`
|
||||
builds its `AudioFocusRequest` without `setWillPauseWhenDucked` on
|
||||
API >= 26, so the framework attenuates the stream itself and never
|
||||
delivers `AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK`. The device's own log
|
||||
says so: `MediaFocusControl: requestAudioFocus() … AA=USAGE_MEDIA/
|
||||
CONTENT_TYPE_MUSIC … req=1 flags=0x0` — no
|
||||
`AUDIOFOCUS_FLAG_PAUSES_ON_DUCKABLE_LOSS`. **`minSdk` is 21**, so the
|
||||
Go-side duck is not dead code; it is reachable on Android 5.0 to 7.1
|
||||
and on nothing newer. Any future "verify ducking on a device" needs one
|
||||
of those, and asking for a modern phone will not do it.
|
||||
|
||||
Two smaller things from the same session.
|
||||
|
||||
**A fresh install downloads the real catalog, and it is 209px of the
|
||||
screen while it does.** `YJ_CORE_INDEX_URL` is stubbed in
|
||||
`dev-headless.sh` and in CI but is real on a device, so the first
|
||||
measurement taken was of a screen with `job-band` on it and the art at
|
||||
**0px**. That is not a defect and not #172 — it is the environment.
|
||||
`explore.Service.StopIndexBuild` and a relaunch is the clean state.
|
||||
|
||||
**The first-run wizard does not dismiss when a library appears by a
|
||||
route other than its own** (#175) — it was still up, full-screen and
|
||||
intercepting pointer events, after `AddLibrary` succeeded through the
|
||||
binding, and was gone after a relaunch. Filed.
|
||||
|
||||
## The context menu was clipped on the device, and the fix needed four measurements nothing here could make (measured 2026-08-21, TLP301 / Chrome 113 / 424x439)
|
||||
|
||||
#60 had been diagnosed from the Web Awesome source and was right. What
|
||||
the device added was the numbers, and three things the reading had not
|
||||
reached.
|
||||
|
||||
**The clip, reproduced before any code was written.** Long-press on the
|
||||
lowest visible track row at 424x439:
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| viewport | 424x439 |
|
||||
| `.main-panel` | 0 to **318**, computed `contain: content` |
|
||||
| menu panel | 191 to **401**, 210px tall |
|
||||
| clipped away | **83px, three of seven items** |
|
||||
| `wa-popup` computed position | `fixed` |
|
||||
| `HTMLElement.prototype.hasOwnProperty('popover')` | **false** |
|
||||
| row height | **29px** (against a 44px floor and a 48px ask) |
|
||||
|
||||
A screenshot shows the menu sliced off flush with the mini player's top
|
||||
edge. Both halves of the diagnosis are therefore measured, not inferred.
|
||||
|
||||
**"A dialog escapes containment" was the premise, and it was untested.**
|
||||
Every dialog in this app is mounted in `index.html`, *outside*
|
||||
`.main-panel` — so nothing here was evidence about a dialog opened from
|
||||
inside a view, which is what this change needed. A probe `<dialog>`
|
||||
appended to `track-list`'s shadow root and `showModal()`n paints to
|
||||
y=439, over the mini player and the tab bar. A top-layer element's
|
||||
containing block is the viewport, paint-contained ancestor or not.
|
||||
Checking that first cost ten minutes and would have cost a rebuild.
|
||||
|
||||
**The UA stylesheet is the thing that makes a naive sheet look wrong.**
|
||||
That same probe came out **354px wide on a 424px screen**, centred,
|
||||
because a native `<dialog>` carries `max-width: calc(100% - 6px - 2em)`
|
||||
and `margin: auto`. `max-width: none` and explicit margins are four
|
||||
declarations that are pure undoing.
|
||||
|
||||
**A retry loop cannot win against a steal that happens later.**
|
||||
`MenuKeyboard` focuses the first item and returns as soon as it lands;
|
||||
`wa-dialog` then focuses `[autofocus]` or *itself* on the frame after
|
||||
`showModal()`, and it cannot see our first item to prefer it — the
|
||||
panel is slotted through `menu-surface`, so the dialog's own
|
||||
`querySelector` stops at the `<slot>`. Measured: the sheet opened with
|
||||
`document.activeElement` on the `<dialog>` and every arrow key went
|
||||
nowhere. Lengthening the retry budget does not help, because the first
|
||||
attempt *succeeds*. The surface announcing `menu-shown` after
|
||||
`wa-after-show`, and the keyboard re-asserting, is the fix.
|
||||
|
||||
**The submenu was made worse before it was made better, and only a
|
||||
measurement caught it.** `#playlist-submenu` is a
|
||||
`placement="right-start"` flyout anchored to its row. Making the menu a
|
||||
full-width sheet moved that anchor to x=0, so the flip put the playlist
|
||||
picker at **x −182 to 0 — entirely off-screen**, and "Add to Playlist"
|
||||
led nowhere at all. Before the change the anchor row started at x≈245
|
||||
and the same flip landed it on screen. It is a `menu-surface` too now
|
||||
and stacks as a second sheet. Two lessons: a change that moves an
|
||||
anchor changes every flip decision downstream of it, and *the scope I
|
||||
declared on the issue was wrong* — I had said I would measure the
|
||||
submenu and file it, and the measurement said fix it.
|
||||
|
||||
**And the sweep found two call sites the conversion missed.** Twelve
|
||||
were converted by hand; `menu-surface.test.ts` reads every source file
|
||||
and fails on a `<wa-popup>` outside a three-file allowlist, which
|
||||
immediately named `queue-panel`'s add-to-playlist popup (a real menu,
|
||||
converted) and `now-playing`'s cover preview (a hover affordance in the
|
||||
bottom bar — allowlisted, since a touch device never opens it and
|
||||
nothing clips it). A thirteenth menu written as a bare popup would pass
|
||||
every tier here and be clipped on the device, which is precisely why
|
||||
the guard is a source sweep rather than a rendered assertion.
|
||||
|
||||
**What no tier here can see remains the clip itself.** This runner's
|
||||
Chromium and CI's WebKit both have the Popover API, so the popup is
|
||||
top-layered and correct and a "not clipped" assertion passes on the
|
||||
broken build. The specs assert the *mechanism* — that the surface is a
|
||||
native `<dialog>` at phone width — which is the same move
|
||||
`queue-as-a-screen.spec.ts` makes about containment and for the same
|
||||
reason.
|
||||
|
||||
@@ -646,7 +646,11 @@ rather than renaming them.
|
||||
level rather than writing through to the volume, so it cannot
|
||||
accumulate and nothing persists or emits a level the user did not
|
||||
choose — and it only ever fires below API 26, where the framework
|
||||
does not already duck the app itself.
|
||||
does not already duck the app itself. On that platform "the user's
|
||||
level" is a constant, since #64 pins it at maximum and refuses every
|
||||
way to move it; the duck is the one thing that still may, and it
|
||||
works unchanged because it was always an offset applied *to* that
|
||||
level rather than a write of it.
|
||||
- `system` — OS-specific paths (XDG on Linux, `%LOCALAPPDATA%` on Windows).
|
||||
- `explore` — Catalog search and browse over `explore_index`. See below.
|
||||
Its **shelves** (`shelves.go`) are the page Explore shows before
|
||||
@@ -1363,8 +1367,17 @@ against the real components:
|
||||
`wa-dropdown-item` sets its `role` in its *own* first update, so a
|
||||
`[role^="menuitem"]` query at `updateComplete` finds nothing — which
|
||||
reads exactly like a menu that opened and refused to take focus.
|
||||
- **`focus()` on a popup that has not positioned itself is a silent
|
||||
no-op**, so the first focus is retried across a few frames.
|
||||
- **`focus()` on a surface that has not shown itself is a silent
|
||||
no-op**, so the first focus is retried on a *time* budget rather
|
||||
than a frame count — the thing being waited for is another
|
||||
component's animation. And a retry is not enough on its own for the
|
||||
sheet below: `wa-dialog` focuses `[autofocus]` or *itself* on the
|
||||
frame after `showModal()`, and it cannot see the first menu item to
|
||||
prefer it, because the panel is slotted through `menu-surface` and
|
||||
the dialog's own `querySelector` stops at the `<slot>`. The first
|
||||
attempt therefore *succeeds* and is then overwritten, which no
|
||||
amount of waiting fixes — so the surface announces `menu-shown` when
|
||||
it has settled and `MenuKeyboard.refocus()` re-asserts.
|
||||
- **Focus is only taken back if the menu had it.** A click elsewhere
|
||||
closes the menu too, and pulling focus to the row the user
|
||||
right-clicked a moment ago is worse than leaving it.
|
||||
@@ -1372,6 +1385,69 @@ against the real components:
|
||||
moving focus without setting it leaves the highlight on whichever
|
||||
item the mouse last touched.
|
||||
|
||||
**And a menu is drawn where it fits: a popup on a desktop, a bottom
|
||||
sheet on a phone** (#60). `components/menu-surface/` is that one
|
||||
decision. The host renders the panel it always rendered and slots it
|
||||
into whichever surface is up, so `ContextMenuController` still drives
|
||||
`.active` and `.anchor` as though it were talking to a `wa-popup`, and
|
||||
fourteen call sites changed one tag name each and nothing else.
|
||||
|
||||
**It is a correctness fix, not a taste one, and the failure was
|
||||
measured on the device rather than inferred.** Chrome 113 has no
|
||||
Popover API, so `wa-popup` takes its own documented fallback and
|
||||
positions with `strategy: "fixed"`; `.main-panel` carries
|
||||
`contain: layout style paint`, and paint containment *clips* fixed
|
||||
descendants. On the reference device the main panel spans 0-318 of a
|
||||
439px viewport while the open menu spanned 191-401 — three of its seven
|
||||
items cut off, with no way to reach them. `showModal()` is Chrome 37
|
||||
and uses the real top layer, so a dialog is immune by construction.
|
||||
|
||||
Six things about it are load-bearing.
|
||||
|
||||
**"Dialogs are fine" needed checking, because every other dialog in
|
||||
this app is mounted in `index.html`** — outside `.main-panel` — so it
|
||||
was not evidence about one opened from inside a view. A probe dialog
|
||||
appended to `track-list`'s shadow root paints to y=439, over the mini
|
||||
player and the tab bar, with the contained ancestor still in place. A
|
||||
top-layer element's containing block is the viewport, contained
|
||||
ancestor or not.
|
||||
|
||||
**The sheet has to un-do the UA stylesheet.** A native `<dialog>`
|
||||
carries `max-width: calc(100% - 6px - 2em)` and `margin: auto`, which
|
||||
drew a 354px panel floating in the middle of a 424px screen.
|
||||
`max-width: none` plus explicit margins is what makes it a sheet.
|
||||
|
||||
**The row sizing lives in `contextMenuStyles`, not in the component.**
|
||||
The panel is the *host's* light DOM — it stays in the host's shadow
|
||||
root, so only the host's stylesheet can reach it. `menu-surface` puts
|
||||
`data-sheet` on the panel and that shared stylesheet does the rest,
|
||||
which is how fourteen menus went from 29px rows to 48px ones in one
|
||||
edit.
|
||||
|
||||
**A dismissal has to travel back.** `wa-dialog` closes itself on
|
||||
Escape, which would leave the controller believing the menu is open —
|
||||
and the failure mode is not a stuck sheet but the *next* long-press
|
||||
doing nothing, which reads as the gesture breaking. `menu-dismiss` is
|
||||
that signal; the three surfaces that do not use `ContextMenuController`
|
||||
bind it themselves.
|
||||
|
||||
**The playlist submenu is a sheet too, and it had to be.** It is a
|
||||
`placement="right-start"` flyout, and making the menu full-width moved
|
||||
its anchor — measured at x −182 to 0, entirely off-screen, so "Add to
|
||||
Playlist" led nowhere. It stacks as a second sheet over the first,
|
||||
which is also why `menu-shown` does not re-assert focus while the
|
||||
submenu is open.
|
||||
|
||||
**And which call sites exist is swept, not remembered.** A thirteenth
|
||||
menu written as a bare `<wa-popup>` works perfectly in every tier here
|
||||
and is clipped on the device, so `menu-surface.test.ts` reads the
|
||||
source and fails on one outside a three-file allowlist —
|
||||
`menu-surface` itself, `job-indicator` (in `.top-bar`, which no
|
||||
ancestor contains — the contrast that proved the diagnosis on #62) and
|
||||
`now-playing`'s cover preview (a hover affordance, which a touch device
|
||||
never opens). **The sweep found two of the fourteen**; twelve were
|
||||
converted by hand.
|
||||
|
||||
**And a menu opens from a finger, through the event it already has.**
|
||||
`utils/long-press.ts` is one document-capture listener installed once
|
||||
from `index.ts`: a touch that holds still for 500 ms dispatches a
|
||||
@@ -1725,12 +1801,67 @@ where the zero value has to be the intended answer, so an existing
|
||||
`config.toml` with no key gets the new default without a migration.
|
||||
Inline, the icon becomes the mute toggle and is named after that action
|
||||
rather than after the state, because with the slider beside it there is
|
||||
nothing left to disclose. It stands down below 600px whatever the
|
||||
setting says — that is about the platform rather than preference, and
|
||||
is why `mediacontrols`' Android handler implements no volume callback.
|
||||
(Only the *bar's* copy: `now-playing-view` renders one and it is
|
||||
visible on a phone. #64 asks for it to be gone on Android outright,
|
||||
which is a platform question the frontend cannot currently ask.)
|
||||
nothing left to disclose. The bar's copy stands down below 600px, which
|
||||
is about *room*: five controls and a slider do not fit a 360px bar, and
|
||||
`now-playing-view` is where seeking and volume go on a phone.
|
||||
|
||||
**Whether there is a volume to control at all is a different question,
|
||||
and it is asked of the player** (#64). On Android the hardware keys are
|
||||
the volume control and the framework mixes our stream against the
|
||||
device level, so `player`'s own level is pinned at maximum, `SetVolume`
|
||||
/ `ChangeVolume` / `MuteToggle` are refused, and `volume-control`
|
||||
renders `nothing` — in both of its mount points, at every width.
|
||||
`mediacontrols`' Android handler implementing no volume callback is the
|
||||
same fact one layer down.
|
||||
|
||||
Five things about it are load-bearing.
|
||||
|
||||
**It could not be a width, and that is not a preference.** Every other
|
||||
stand-down rule in this app is keyed on a viewport, because a width is
|
||||
what a browser can answer and what every tier can test. This one is a
|
||||
property of the build: keyed on width, an Android *tablet* at 600px or
|
||||
more draws the bottom bar's slider over a pinned level — a control that
|
||||
cannot act, on exactly the platform the rule exists for, which
|
||||
`library-status-indicator` already settled is worse than none. The
|
||||
same rule is wrong in the other direction below 600px, where a narrow
|
||||
desktop window has no hardware keys to fall back on.
|
||||
|
||||
**The predicate is named after the capability, not the platform.**
|
||||
`SystemOwnsVolume` is what the frontend asks; `platformOwnsVolume` is
|
||||
the one build-tagged constant behind it, in two files that declare
|
||||
nothing else. That is `mediacontrols`' split with
|
||||
`androidpayload.go`'s reasoning: a tagged file is compiled by nothing
|
||||
`make lint` or `make test` runs, so everything decidable off a phone is
|
||||
decided against `Player.systemVolume`, a field a test sets either way.
|
||||
The frontend's absent branch is therefore testable in the component
|
||||
tier with a stubbed binding, and the constant itself is covered by a
|
||||
source sweep plus, once, a real arm64 device answering `true` — which
|
||||
is the only tier that compiles the `android` file at all.
|
||||
|
||||
**Mute goes with it, because it is a level of zero by another name** —
|
||||
and because with no control rendered it is the one state on such a
|
||||
platform the user could not get out of.
|
||||
|
||||
**Nothing persists a level nobody chose.** The maximum the player runs
|
||||
at is synthetic, so `restoreStateLocked` *remembers* the stored volume
|
||||
instead of applying it and `saveState` writes that same value back.
|
||||
The alternative — a second query that omits the column — buys nothing
|
||||
and is a second write path to keep in step.
|
||||
|
||||
**And ducking is untouched, which is what makes the pin safe.**
|
||||
`SetDuck` applies its attenuation by re-applying the *user's* level
|
||||
through `setVolumeLocked`, so pinning that level to maximum leaves the
|
||||
offset arithmetic exactly as it was. It is the only thing that may move
|
||||
the output on such a platform, and it is the one volume-shaped path
|
||||
that is not refused.
|
||||
|
||||
One thing to know before anyone offers to test it on a phone: **the
|
||||
duck is unreachable above API 25.** `WailsForegroundService` builds its
|
||||
`AudioFocusRequest` without `setWillPauseWhenDucked` from Oreo, so the
|
||||
framework attenuates us itself and never sends
|
||||
`AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK` — a device confirms it by logging
|
||||
`requestAudioFocus() … flags=0x0`. `minSdk` is 21, so this is live code
|
||||
rather than dead, on Android 5.0 to 7.1 and nowhere else.
|
||||
|
||||
**And below 600px that bar carries three controls, not five** (#59).
|
||||
Shuffle, repeat and the queue button leave it; what is left is art,
|
||||
|
||||
@@ -71,6 +71,19 @@ type Player struct {
|
||||
// not something the user chose.
|
||||
duckAmount float64
|
||||
|
||||
// systemVolume is what SystemOwnsVolume answers: the platform's own
|
||||
// control is the only one, so ours neither acts nor persists. It is
|
||||
// a field rather than the build constant read directly so that a
|
||||
// test can exercise both sides on any machine. See systemvolume.go.
|
||||
systemVolume bool
|
||||
|
||||
// storedVolume and storedMuted hold the persisted level as it was
|
||||
// found at restore, for a platform whose volume we do not own: the
|
||||
// maximum we then run at is not a level the user chose, so saveState
|
||||
// writes back what it read rather than overwriting it.
|
||||
storedVolume UserVolume
|
||||
storedMuted bool
|
||||
|
||||
// trackLengthMs holds the authoritative track duration in
|
||||
// milliseconds, sourced from the database (which uses the
|
||||
// custom header parser). The go-mp3 decoder's Len() can be
|
||||
@@ -156,6 +169,8 @@ func NewPlayer(logger *slog.Logger, db *database.DB) *Player {
|
||||
logger: logger,
|
||||
db: db,
|
||||
state: Stopped,
|
||||
systemVolume: platformOwnsVolume,
|
||||
storedVolume: DefaultUserVol,
|
||||
baseStreamer: generators.Silence(-1),
|
||||
format: beep.Format{
|
||||
SampleRate: speakerSampleRate,
|
||||
@@ -875,6 +890,10 @@ func (p *Player) SetVolume(desiredVolume UserVolume) {
|
||||
p.mu.Lock()
|
||||
defer p.mu.Unlock()
|
||||
|
||||
if p.systemVolume {
|
||||
return
|
||||
}
|
||||
|
||||
p.setVolumeLocked(desiredVolume)
|
||||
p.emitVolumeChanged()
|
||||
p.saveState()
|
||||
@@ -923,6 +942,10 @@ func (p *Player) ChangeVolume(deltaVolume int) error {
|
||||
p.mu.Lock()
|
||||
defer p.mu.Unlock()
|
||||
|
||||
if p.systemVolume {
|
||||
return nil
|
||||
}
|
||||
|
||||
p.setVolumeLocked(p.getUserVolume() + UserVolume(deltaVolume))
|
||||
p.emitVolumeChanged()
|
||||
p.saveState()
|
||||
@@ -953,6 +976,14 @@ func (p *Player) MuteToggle() error {
|
||||
return errNoAudioFileLoaded
|
||||
}
|
||||
|
||||
// Mute is a level of zero by another name, so it goes with the rest
|
||||
// of the volume where the system owns it -- and it would be the one
|
||||
// state on such a platform the user could not get out of, since with
|
||||
// no control rendered there is nothing left to un-mute with.
|
||||
if p.systemVolume {
|
||||
return nil
|
||||
}
|
||||
|
||||
speaker.Lock()
|
||||
p.volume.Silent = !p.volume.Silent
|
||||
speaker.Unlock()
|
||||
@@ -1403,7 +1434,15 @@ func (p *Player) saveState() {
|
||||
volume := int64(DefaultUserVol)
|
||||
muted := false
|
||||
|
||||
if p.volume != nil {
|
||||
switch {
|
||||
case p.systemVolume:
|
||||
// The maximum this platform runs at is not a level anybody
|
||||
// chose, so it is not one to remember. Writing back what
|
||||
// restore found keeps the row a description of the user's
|
||||
// setting without needing a second query that omits the column.
|
||||
volume = int64(p.storedVolume)
|
||||
muted = p.storedMuted
|
||||
case p.volume != nil:
|
||||
volume = int64(p.getUserVolume())
|
||||
muted = p.volume.Silent
|
||||
}
|
||||
@@ -1487,11 +1526,20 @@ func (p *Player) restoreStateLocked() {
|
||||
}
|
||||
}
|
||||
|
||||
vol := clampVolume(UserVolume(state.Volume))
|
||||
p.setVolumeLocked(vol)
|
||||
if p.systemVolume {
|
||||
// Remembered, not applied: the device's keys are the volume
|
||||
// control here, so the player runs wide open and hands the
|
||||
// stored level back untouched at the next save.
|
||||
p.storedVolume = clampVolume(UserVolume(state.Volume))
|
||||
p.storedMuted = state.Muted
|
||||
p.setVolumeLocked(MaxUserVol)
|
||||
} else {
|
||||
vol := clampVolume(UserVolume(state.Volume))
|
||||
p.setVolumeLocked(vol)
|
||||
|
||||
if state.Muted {
|
||||
p.volume.Silent = true
|
||||
if state.Muted {
|
||||
p.volume.Silent = true
|
||||
}
|
||||
}
|
||||
|
||||
// Restore last track if the file still exists.
|
||||
@@ -1531,8 +1579,9 @@ func (p *Player) restoreStateLocked() {
|
||||
}
|
||||
|
||||
p.logger.Info("Player state restored",
|
||||
"volume", vol,
|
||||
"muted", state.Muted,
|
||||
"volume", p.getUserVolume(),
|
||||
"muted", p.volume.Silent,
|
||||
"systemVolume", p.systemVolume,
|
||||
"trackPath", state.LastTrackPath,
|
||||
"positionSeconds", state.LastPositionSeconds,
|
||||
)
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
package player
|
||||
|
||||
// Who owns the volume, and what follows when it is not us.
|
||||
//
|
||||
// On Android the hardware keys *are* the volume control and the
|
||||
// framework mixes our stream against the device level, so a second
|
||||
// control inside the app is a slider that moves something the user
|
||||
// already moved (#64). Where that is true the player's own level sits
|
||||
// at maximum, nothing changes it, and nothing persists it.
|
||||
//
|
||||
// **The predicate is named after the capability, not the platform.**
|
||||
// The frontend asks "is there a volume for me to control", which is a
|
||||
// question about this build; asking "is this a phone" instead would
|
||||
// key the answer to a viewport, and an Android tablet at 600px or more
|
||||
// would then draw the bottom bar's slider over a level pinned at
|
||||
// maximum -- a control that cannot act, which is the thing
|
||||
// `library-status-indicator` already settled is worse than none.
|
||||
//
|
||||
// **Only `platformOwnsVolume` is behind a build tag**, in two files
|
||||
// that declare nothing else. A tagged file is compiled by nothing
|
||||
// `make lint` or `make test` runs and is untestable off a phone, which
|
||||
// is the reasoning `mediacontrols/androidpayload.go` states for
|
||||
// keeping its contract out of one -- so everything decidable here is
|
||||
// decided against `Player.systemVolume`, a field a test sets either
|
||||
// way, and the tag decides only what that field starts as.
|
||||
//
|
||||
// The one thing this must not disturb is ducking. `SetDuck` applies
|
||||
// its attenuation by re-applying the *user's* level through
|
||||
// `setVolumeLocked`, so pinning that level to maximum leaves the
|
||||
// offset arithmetic exactly as it was: an OS asking us to get out of
|
||||
// the way of a navigation prompt is not the user setting a volume, and
|
||||
// it is the only thing that may move the output on such a platform.
|
||||
|
||||
// SystemOwnsVolume reports whether the platform's own control is the
|
||||
// only volume control there is, so this app neither offers one nor
|
||||
// remembers a level.
|
||||
//
|
||||
// It is bound: the frontend renders no `<volume-control>` when it is
|
||||
// true, at any width.
|
||||
func (p *Player) SystemOwnsVolume() bool {
|
||||
return p.systemVolume
|
||||
}
|
||||
@@ -0,0 +1,11 @@
|
||||
//go:build android
|
||||
|
||||
package player
|
||||
|
||||
// platformOwnsVolume is true on Android: volume is the device's, set
|
||||
// with the hardware keys, and `mediacontrols`' Android handler
|
||||
// implements no volume callback for the same reason.
|
||||
//
|
||||
// See systemvolume.go for why this constant is the whole of what a
|
||||
// build tag decides here.
|
||||
const platformOwnsVolume = true
|
||||
@@ -0,0 +1,10 @@
|
||||
//go:build !android
|
||||
|
||||
package player
|
||||
|
||||
// platformOwnsVolume is false everywhere but Android: a desktop mixer
|
||||
// is per-application, so our level is the one the user reaches for.
|
||||
//
|
||||
// See systemvolume.go for why this constant is the whole of what a
|
||||
// build tag decides here.
|
||||
const platformOwnsVolume = false
|
||||
@@ -0,0 +1,215 @@
|
||||
package player
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gopxl/beep/v2/effects"
|
||||
|
||||
"yellowjacket/backend/database"
|
||||
)
|
||||
|
||||
// pinnedPlayer is a player on a platform whose volume belongs to the
|
||||
// device. The field is set rather than the build constant read,
|
||||
// because the constant is true on exactly one platform and no tier
|
||||
// here runs on it -- see systemvolume.go.
|
||||
func pinnedPlayer(t *testing.T, db *database.DB) *Player {
|
||||
t.Helper()
|
||||
|
||||
p := NewPlayer(slog.Default(), db)
|
||||
p.systemVolume = true
|
||||
p.volume = &effects.Volume{Base: 2}
|
||||
p.setVolumeLocked(MaxUserVol)
|
||||
|
||||
return p
|
||||
}
|
||||
|
||||
// TestSystemVolumeRefusesEveryWayToChangeTheLevel is the first half of
|
||||
// #64: where the device owns the volume, ours sits at maximum and none
|
||||
// of the three routes to a level moves it. Mute is in that list
|
||||
// because it is a level of zero by another name, and because with no
|
||||
// control rendered it is the one state on such a platform there would
|
||||
// be nothing to get out of.
|
||||
func TestSystemVolumeRefusesEveryWayToChangeTheLevel(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
p := pinnedPlayer(t, nil)
|
||||
|
||||
if !p.SystemOwnsVolume() {
|
||||
t.Fatal("SystemOwnsVolume() = false on a pinned player")
|
||||
}
|
||||
|
||||
if got := p.getUserVolume(); got != MaxUserVol {
|
||||
t.Errorf("starting volume = %d, want %d", got, MaxUserVol)
|
||||
}
|
||||
|
||||
p.SetVolume(20)
|
||||
|
||||
if got := p.getUserVolume(); got != MaxUserVol {
|
||||
t.Errorf("volume after SetVolume(20) = %d, want %d", got, MaxUserVol)
|
||||
}
|
||||
|
||||
if err := p.ChangeVolume(-30); err != nil {
|
||||
t.Fatalf("ChangeVolume: %v", err)
|
||||
}
|
||||
|
||||
if got := p.getUserVolume(); got != MaxUserVol {
|
||||
t.Errorf("volume after ChangeVolume(-30) = %d, want %d", got, MaxUserVol)
|
||||
}
|
||||
|
||||
if err := p.MuteToggle(); err != nil {
|
||||
t.Fatalf("MuteToggle: %v", err)
|
||||
}
|
||||
|
||||
if p.volume.Silent {
|
||||
t.Error("MuteToggle silenced a player whose volume the system owns")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAnUnpinnedPlayerStillChangesItsVolume is the other side of the
|
||||
// same switch. Without it the test above passes on a player that
|
||||
// refuses everything, which is what a mis-wired field would produce.
|
||||
func TestAnUnpinnedPlayerStillChangesItsVolume(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
p := NewPlayer(slog.Default(), nil)
|
||||
p.volume = &effects.Volume{Base: 2}
|
||||
p.setVolumeLocked(MaxUserVol)
|
||||
|
||||
if p.SystemOwnsVolume() {
|
||||
t.Fatal("SystemOwnsVolume() = true off Android")
|
||||
}
|
||||
|
||||
p.SetVolume(20)
|
||||
|
||||
if got := p.getUserVolume(); got != 20 {
|
||||
t.Errorf("volume after SetVolume(20) = %d, want 20", got)
|
||||
}
|
||||
|
||||
if err := p.MuteToggle(); err != nil {
|
||||
t.Fatalf("MuteToggle: %v", err)
|
||||
}
|
||||
|
||||
if !p.volume.Silent {
|
||||
t.Error("MuteToggle did not silence an ordinary player")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSystemVolumeStillDucks is the issue's second Finding, made a
|
||||
// test: pinning the user's level must leave the OS's attenuation
|
||||
// working, because a duck is not a volume the user chose and is the
|
||||
// only thing that may move the output on such a platform.
|
||||
func TestSystemVolumeStillDucks(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
p := pinnedPlayer(t, nil)
|
||||
open := p.volume.Volume
|
||||
|
||||
p.SetDuck(true)
|
||||
|
||||
if p.volume.Volume >= open {
|
||||
t.Errorf(
|
||||
"ducked output = %v, want less than %v", p.volume.Volume, open,
|
||||
)
|
||||
}
|
||||
|
||||
if got := p.getUserVolume(); got != MaxUserVol {
|
||||
t.Errorf("user volume while ducked = %d, want %d", got, MaxUserVol)
|
||||
}
|
||||
|
||||
// A refused SetVolume must not disturb the offset either: it
|
||||
// returns before setVolumeLocked, which is what re-applies it.
|
||||
ducked := p.volume.Volume
|
||||
|
||||
p.SetVolume(10)
|
||||
|
||||
if p.volume.Volume != ducked {
|
||||
t.Errorf(
|
||||
"output after a refused SetVolume = %v, want %v",
|
||||
p.volume.Volume, ducked,
|
||||
)
|
||||
}
|
||||
|
||||
p.SetDuck(false)
|
||||
|
||||
if p.volume.Volume != open {
|
||||
t.Errorf("output after unduck = %v, want %v", p.volume.Volume, open)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSystemVolumeWritesBackTheLevelItFound is the rest of the
|
||||
// Direction: "make sure nothing writes a persisted volume from that
|
||||
// platform". The maximum the player runs at is synthetic, so saving
|
||||
// must not record it over whatever the row already said.
|
||||
func TestSystemVolumeWritesBackTheLevelItFound(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
|
||||
// A level set by some earlier, unpinned session.
|
||||
writer := NewPlayer(slog.Default(), db)
|
||||
writer.volume = &effects.Volume{Base: 2}
|
||||
writer.setVolumeLocked(30)
|
||||
writer.SaveState()
|
||||
|
||||
p := pinnedPlayer(t, db)
|
||||
p.RestoreState()
|
||||
|
||||
if got := p.getUserVolume(); got != MaxUserVol {
|
||||
t.Errorf("restored volume = %d, want %d (the level is pinned)", got, MaxUserVol)
|
||||
}
|
||||
|
||||
if p.volume.Silent {
|
||||
t.Error("restore muted a player whose volume the system owns")
|
||||
}
|
||||
|
||||
p.SaveState()
|
||||
|
||||
state, err := db.Queries.GetPlayerState(db.Ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPlayerState: %v", err)
|
||||
}
|
||||
|
||||
if state.Volume != 30 {
|
||||
t.Errorf("persisted volume = %d, want 30 (untouched)", state.Volume)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform sweeps the
|
||||
// source, because the pair of tagged files is the one thing here no
|
||||
// tier compiles both halves of: `make lint` and `make test` build the
|
||||
// `!android` side only, so a deleted or edited android file fails
|
||||
// nothing until somebody has a phone in their hand.
|
||||
func TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
want := map[string]string{
|
||||
"systemvolume_other.go": "const platformOwnsVolume = false",
|
||||
"systemvolume_android.go": "const platformOwnsVolume = true",
|
||||
}
|
||||
|
||||
tags := map[string]string{
|
||||
"systemvolume_other.go": "//go:build !android",
|
||||
"systemvolume_android.go": "//go:build android",
|
||||
}
|
||||
|
||||
for name, decl := range want {
|
||||
src, err := os.ReadFile(filepath.Join(".", name))
|
||||
if err != nil {
|
||||
t.Errorf("%s: %v", name, err)
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
if !strings.Contains(string(src), decl) {
|
||||
t.Errorf("%s does not declare %q", name, decl)
|
||||
}
|
||||
|
||||
if !strings.Contains(string(src), tags[name]) {
|
||||
t.Errorf("%s does not carry %q", name, tags[name])
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -130,6 +130,25 @@ test.describe('the shell on a phone', () => {
|
||||
// are here, and they are the *same* components -- this view
|
||||
// composes the transport rather than reimplementing it.
|
||||
await expect(app.locator('now-playing-view seek-bar')).toBeVisible();
|
||||
|
||||
// Volume is here **because the player says there is one** (#64),
|
||||
// not because this is a phone. This tier is the platform that owns
|
||||
// its own volume, so what it can assert is that the control's
|
||||
// presence follows that answer -- an inverted polarity in
|
||||
// `volume-style-store` fails here and in `bottom-bar.spec.ts`, and
|
||||
// the *absent* branch is checked in the component tier, where the
|
||||
// binding can be stubbed. Nothing here can reach the Android side.
|
||||
const systemOwns = await app.evaluate(
|
||||
async () =>
|
||||
(await window.__yjEvents.call(
|
||||
'player.Player.SystemOwnsVolume',
|
||||
[],
|
||||
5_000,
|
||||
)) as boolean,
|
||||
);
|
||||
|
||||
expect(systemOwns, 'this platform should own its own volume').toBe(false);
|
||||
|
||||
await expect(app.locator('now-playing-view volume-control')).toBeVisible();
|
||||
|
||||
// Back goes where the user came from, through the nav stack.
|
||||
|
||||
@@ -142,6 +142,18 @@ export function SetVolume(desiredVolume: $models.UserVolume): $CancellablePromis
|
||||
return $Call.ByID(1375836663, desiredVolume);
|
||||
}
|
||||
|
||||
/**
|
||||
* SystemOwnsVolume reports whether the platform's own control is the
|
||||
* only volume control there is, so this app neither offers one nor
|
||||
* remembers a level.
|
||||
*
|
||||
* It is bound: the frontend renders no `<volume-control>` when it is
|
||||
* true, at any width.
|
||||
*/
|
||||
export function SystemOwnsVolume(): $CancellablePromise<boolean> {
|
||||
return $Call.ByID(1027623185);
|
||||
}
|
||||
|
||||
/**
|
||||
* TrackLengthInSeconds returns the duration of the current track.
|
||||
*/
|
||||
|
||||
+13
-9
@@ -522,16 +522,20 @@ body div.sidebar {
|
||||
}
|
||||
|
||||
/* Volume stands down here whatever the setting says, because this
|
||||
is about room and about the platform rather than about
|
||||
preference: the hardware keys own volume on a phone, which is
|
||||
also why mediacontrols' Android handler implements no volume
|
||||
callback. It moved from `audio-player`'s own media query when
|
||||
#42 moved the control into the bar — same rule, and now stated
|
||||
where the element actually is.
|
||||
is about room: five controls and a slider do not fit a 360px
|
||||
bar, and the full-screen now-playing view is where seeking and
|
||||
volume go on a phone. It moved from `audio-player`'s own media
|
||||
query when #42 moved the control into the bar — same rule, and
|
||||
now stated where the element actually is.
|
||||
|
||||
`.bottom-bar volume-control`, not the one in
|
||||
`now-playing-view`: that view is the phone's transport and is
|
||||
where a slider does belong. */
|
||||
**This rule used to carry the platform argument too, and no
|
||||
longer does** (#64). "The hardware keys own the volume" is not a
|
||||
width: it is false of a narrow desktop window and true of an
|
||||
Android tablet, which this selector gets backwards both ways.
|
||||
The player answers it now — `SystemOwnsVolume` — and
|
||||
`volume-control` renders nothing when it is true, at every
|
||||
width and in both of its mount points. What is left here is the
|
||||
question a stylesheet can actually answer. */
|
||||
.bottom-bar volume-control {
|
||||
display: none;
|
||||
}
|
||||
|
||||
@@ -26,14 +26,15 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||
import { RovingGridController } from '@utils/roving-grid';
|
||||
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@components/playlist-picker/playlist-picker.js';
|
||||
import { dict, list } from '@utils/binding';
|
||||
@@ -131,17 +132,17 @@ export class ArtistsView
|
||||
private contextMenuArtistId: number | null = null;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup():
|
||||
| WaPopup
|
||||
| MenuTarget
|
||||
| undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
@@ -1336,11 +1337,8 @@ export class ArtistsView
|
||||
|
||||
private renderContextMenu() {
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.contextMenuOpen}
|
||||
>
|
||||
@@ -1434,13 +1432,12 @@ export class ArtistsView
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.playlistSubmenuOpen}
|
||||
>
|
||||
@@ -1468,7 +1465,7 @@ export class ArtistsView
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { LitElement, html, css } from 'lit';
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||
@@ -27,6 +27,26 @@ export class VolumeControl extends LitElement {
|
||||
@state()
|
||||
private popup = volumeStyleStore.popup;
|
||||
|
||||
/**
|
||||
* Whether there is a volume of ours to control at all (#64).
|
||||
*
|
||||
* The decision is made here rather than at either mount point,
|
||||
* because there are two -- the bottom bar's copy lives in
|
||||
* `index.html`, which has no module scope to make it conditional --
|
||||
* and one of them is a control the shell cannot un-render. So the
|
||||
* control answers for itself, and the bar and the phone's
|
||||
* full-screen transport get the same answer without either knowing
|
||||
* the question exists.
|
||||
*
|
||||
* It renders `nothing` *and* hides the host: an empty shadow root is
|
||||
* what stops a positional or role query finding a button that cannot
|
||||
* act, and `:host([hidden])` is what stops the element occupying a
|
||||
* flex item's worth of the transport -- the `:host` display above
|
||||
* outranks the UA's `[hidden]` rule, so it has to be said.
|
||||
*/
|
||||
@state()
|
||||
private available = volumeStyleStore.available;
|
||||
|
||||
private unsubscribeStyle?: () => void;
|
||||
|
||||
// Locally-tracked volume while the user is actively dragging or scrolling.
|
||||
@@ -43,6 +63,13 @@ export class VolumeControl extends LitElement {
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
/* See the available field. A gap is only drawn between boxes,
|
||||
so a hidden host costs its parent nothing -- which is where the
|
||||
29px this gives back to Now Playing comes from (#172). */
|
||||
:host([hidden]) {
|
||||
display: none;
|
||||
}
|
||||
|
||||
button {
|
||||
background: none;
|
||||
border: none;
|
||||
@@ -154,12 +181,15 @@ export class VolumeControl extends LitElement {
|
||||
|
||||
this.unsubscribeStyle = volumeStyleStore.subscribe(() => {
|
||||
this.popup = volumeStyleStore.popup;
|
||||
this.setAvailable(volumeStyleStore.available);
|
||||
|
||||
// Switching to the slider while the popup is open would leave the
|
||||
// document listener installed for a popup that no longer renders.
|
||||
if (!this.popup) this.closeSlider();
|
||||
});
|
||||
|
||||
this.setAvailable(volumeStyleStore.available);
|
||||
|
||||
void volumeStyleStore.init();
|
||||
}
|
||||
|
||||
@@ -233,7 +263,25 @@ export class VolumeControl extends LitElement {
|
||||
// RENDER
|
||||
// ===================================================================
|
||||
|
||||
/**
|
||||
* `hidden` is set imperatively rather than reflected from the state,
|
||||
* because it has to be on the *host* and a `@state` does not reflect.
|
||||
* It is the right attribute besides: it takes the element out of the
|
||||
* accessibility tree as well as out of the layout.
|
||||
*/
|
||||
private setAvailable(available: boolean) {
|
||||
this.available = available;
|
||||
this.hidden = !available;
|
||||
|
||||
// A popup left open when the control goes away would keep its
|
||||
// document click listener installed for markup that no longer
|
||||
// renders.
|
||||
if (!available) this.closeSlider();
|
||||
}
|
||||
|
||||
override render() {
|
||||
if (!this.available) return nothing;
|
||||
|
||||
const muted = this.player.muted;
|
||||
|
||||
// Inline, the icon is the mute toggle rather than a disclosure:
|
||||
|
||||
@@ -23,7 +23,8 @@ import { gridColumnsFor, gridSpacingFor } from '@utils/grid-spacing';
|
||||
import { queueStore } from '@store/queue-store';
|
||||
import type { QueueSource } from '@store/queue-store';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@components/playlist-picker/playlist-picker.js';
|
||||
@@ -51,7 +52,7 @@ import {
|
||||
ContextMenuController,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { creditLink, exploreLinkStyles } from '../../utils/explore-link';
|
||||
import { creditStore } from '@store/credit-store';
|
||||
@@ -415,17 +416,17 @@ export class CoverGrid
|
||||
splitIndex = 0;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
// ContextMenuHost interface.
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -2087,11 +2088,8 @@ export class CoverGrid
|
||||
const { ctxMenu } = this;
|
||||
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${ctxMenu.contextMenuOpen
|
||||
@@ -2204,13 +2202,12 @@ export class CoverGrid
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${ctxMenu.playlistSubmenuOpen}
|
||||
>
|
||||
${ctxMenu.playlistSubmenuOpen
|
||||
@@ -2232,7 +2229,7 @@ export class CoverGrid
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<track-details></track-details>
|
||||
`;
|
||||
|
||||
@@ -49,9 +49,11 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import { dictByName } from '@utils/binding';
|
||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||
@@ -327,7 +329,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
@state() private ctxMenuTrack: MBTrack | null = null;
|
||||
|
||||
@query('#track-context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup?: WaPopup;
|
||||
@@ -337,11 +339,11 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
|
||||
// -- ContextMenuHost interface --
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -3781,11 +3783,8 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
const track = this.ctxMenuTrack;
|
||||
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="track-context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen && track
|
||||
@@ -3846,13 +3845,12 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.playlistSubmenuOpen}
|
||||
>
|
||||
${this.ctxMenu.playlistSubmenuOpen
|
||||
@@ -3869,7 +3867,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -61,9 +61,11 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import { dict, dictByName } from '@utils/binding';
|
||||
import type { TrackDetails } from '@components/track-details/track-details.js';
|
||||
@@ -215,7 +217,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
@state() private ctxMenuTarget: ContextMenuTarget | null = null;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup?: WaPopup;
|
||||
@@ -233,11 +235,11 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
|
||||
// -- ContextMenuHost interface --
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -2621,11 +2623,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
const target = this.ctxMenuTarget;
|
||||
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen && target
|
||||
@@ -2643,13 +2642,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.playlistSubmenuOpen}
|
||||
>
|
||||
${this.ctxMenu.playlistSubmenuOpen
|
||||
@@ -2666,7 +2664,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
|
||||
|
||||
@@ -37,9 +37,11 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import { dict, dictByName } from '@utils/binding';
|
||||
import { ICON_QUEUE } from '@utils/icon-language';
|
||||
@@ -206,13 +208,13 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
@state() private ctxMenuTarget: ExploreMenuTarget | null = null;
|
||||
|
||||
@litQuery('#explore-context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
// -- ContextMenuHost interface --
|
||||
// No playlist submenu — same reason as the album/artist detail
|
||||
// pages: every action here resolves its one file lazily.
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
@@ -1341,11 +1343,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
const owned = Boolean(target?.localId);
|
||||
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="explore-context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen && target
|
||||
@@ -1374,7 +1373,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
|
||||
|
||||
@@ -24,14 +24,15 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||
import { RovingGridController } from '@utils/roving-grid';
|
||||
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@components/playlist-picker/playlist-picker.js';
|
||||
import { dictByName } from '@utils/binding';
|
||||
@@ -137,19 +138,19 @@ export class GenresView
|
||||
private contextMenuGenreName: string | null = null;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
// ----- ContextMenuHost interface -----
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup():
|
||||
| WaPopup
|
||||
| MenuTarget
|
||||
| undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
@@ -1176,11 +1177,8 @@ export class GenresView
|
||||
|
||||
private renderContextMenu() {
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.contextMenuOpen}
|
||||
>
|
||||
@@ -1284,13 +1282,12 @@ export class GenresView
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.playlistSubmenuOpen}
|
||||
>
|
||||
@@ -1318,7 +1315,7 @@ export class GenresView
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,318 @@
|
||||
/**
|
||||
* Where a context menu is drawn: a popup on a desktop, a bottom sheet
|
||||
* on a phone (#60).
|
||||
*
|
||||
* Every context menu in this app is a `.context-menu-panel` inside a
|
||||
* `<wa-popup>` anchored to the touch point, driven by
|
||||
* `ContextMenuController`. On the reference device that is structurally
|
||||
* broken, and the failure was measured on the hardware rather than
|
||||
* inferred:
|
||||
*
|
||||
* - Chrome 113 has **no Popover API** (`popover` is Chrome 114), so
|
||||
* `wa-popup` takes its own documented fallback and positions with
|
||||
* `strategy: "fixed"` instead of the top layer. Measured on the
|
||||
* device: `HTMLElement.prototype.hasOwnProperty('popover')` is false
|
||||
* and the popup's computed `position` is `fixed`.
|
||||
* - `index.css` puts `contain: layout style paint` on `.main-panel`,
|
||||
* the ancestor of every view. Paint containment **clips** fixed
|
||||
* descendants. Measured: `.main-panel` computes `contain: content`
|
||||
* and spans 0-318 of a 439px viewport, while the open menu spans
|
||||
* 191-401 — so 83px of it, three of its seven items, is cut off.
|
||||
*
|
||||
* A `<dialog>` fixes it by construction rather than by styling, because
|
||||
* `showModal()` is Chrome 37 and uses the real top layer. **That was
|
||||
* measured too, and it needed to be**: every other dialog in this app
|
||||
* is mounted in `index.html`, *outside* `.main-panel`, so "dialogs are
|
||||
* fine" was not evidence about a dialog opened from inside a view. A
|
||||
* probe dialog appended to `track-list`'s shadow root paints to y=439,
|
||||
* over the mini player and the tab bar, with the contained ancestor
|
||||
* still there.
|
||||
*
|
||||
* Four things about this component are load-bearing.
|
||||
*
|
||||
* **It is one element with two presentations, not two components.**
|
||||
* The host keeps rendering exactly the panel it rendered before and
|
||||
* slots it into whichever surface is up, so the twelve call sites
|
||||
* changed one tag name each and nothing else — no second item model, no
|
||||
* second keyboard model, and `ContextMenuController` still drives
|
||||
* `.active` and `.anchor` as if it were talking to a `wa-popup`.
|
||||
*
|
||||
* **Which surface exists is `matchMedia`, not a media query.** The
|
||||
* decision is whether a `<dialog>` is in the tree at all, which is
|
||||
* `job-band` and `player-controls`' rule: a `display: none` surface is
|
||||
* still in the shadow root and still something a positional or by-role
|
||||
* query finds.
|
||||
*
|
||||
* **The sheet has to un-do the UA stylesheet to be full-bleed.**
|
||||
* A native `<dialog>` carries `max-width: calc(100% - 6px - 2em)` and
|
||||
* `margin: auto`, which on the device produced a 354px panel floating
|
||||
* in the middle of a 424px screen. `max-width: none` and explicit
|
||||
* margins are what make it a sheet rather than a small centred box.
|
||||
* The *positioning* needs no such care: a top-layer dialog's containing
|
||||
* block is the viewport even with a paint-contained ancestor, which is
|
||||
* why `bottom: 0` reaches y=439 and not the main panel's 318.
|
||||
*
|
||||
* **Dismissal has to travel back.** `wa-dialog` closes itself on
|
||||
* Escape, which would otherwise leave the controller's
|
||||
* `contextMenuOpen` true and the menu unopenable until something else
|
||||
* cleared it. `menu-dismiss` is that signal, and the controller listens
|
||||
* for it on the document beside the click and contextmenu listeners it
|
||||
* already has.
|
||||
*/
|
||||
import { LitElement, css, html } from 'lit';
|
||||
import { customElement, property, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
|
||||
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
|
||||
/** The event a surface dispatches when it closed itself. */
|
||||
export const MENU_DISMISS_EVENT = 'menu-dismiss';
|
||||
|
||||
/**
|
||||
* The event a surface dispatches once it has finished showing.
|
||||
*
|
||||
* Only the sheet sends it, and only because `wa-dialog` moves focus to
|
||||
* itself on the frame after `showModal()` -- see `MenuKeyboard.refocus`
|
||||
* for why waiting longer is not the fix.
|
||||
*/
|
||||
export const MENU_SHOWN_EVENT = 'menu-shown';
|
||||
|
||||
/**
|
||||
* A `wa-popup` anchor: a real element or a virtual one.
|
||||
*
|
||||
* `undefined` rather than `null` for "not set yet", because that is
|
||||
* what `wa-popup`'s own property accepts — this surface hands the value
|
||||
* straight through and must not widen it.
|
||||
*/
|
||||
type MenuAnchor = WaPopup['anchor'] | undefined;
|
||||
|
||||
/** A `wa-dialog`, as much of it as this file needs. */
|
||||
type DialogEl = HTMLElement & { open: boolean };
|
||||
|
||||
@customElement('menu-surface')
|
||||
export class MenuSurface extends LitElement {
|
||||
/** Whether the menu is showing. Set by `ContextMenuController`. */
|
||||
@property({ type: Boolean }) active = false;
|
||||
|
||||
/**
|
||||
* Where the popup hangs from. Ignored in sheet mode, which is
|
||||
* anchored to the bottom of the screen rather than to the touch
|
||||
* point — that is the whole point of a sheet.
|
||||
*/
|
||||
@property({ attribute: false }) anchor: MenuAnchor = undefined;
|
||||
|
||||
/**
|
||||
* `wa-popup`'s placement, defaulted because all twelve call sites
|
||||
* passed the same one. Kept as a property so a future menu that
|
||||
* wants another does not have to reach past this component.
|
||||
*/
|
||||
@property() placement = 'bottom-start';
|
||||
|
||||
/**
|
||||
* What to call the sheet, for a surface whose content is not a
|
||||
* `.context-menu-panel` with an `aria-label` of its own -- the
|
||||
* playlist submenu, whose content is a `playlist-picker`.
|
||||
*/
|
||||
@property() label = '';
|
||||
|
||||
@state() private sheet = false;
|
||||
|
||||
@query('wa-popup') private popup?: WaPopup;
|
||||
|
||||
@query('wa-dialog') private dialog?: DialogEl;
|
||||
|
||||
private phoneQuery?: MediaQueryList;
|
||||
|
||||
static override styles = css`
|
||||
:host {
|
||||
display: contents;
|
||||
}
|
||||
|
||||
wa-popup {
|
||||
z-index: 200;
|
||||
}
|
||||
|
||||
/* The sheet. A native dialog's UA stylesheet centres it and
|
||||
caps its width, which on the device drew a 354px box in the
|
||||
middle of a 424px screen — so all four of these are undoing
|
||||
that rather than decorating. */
|
||||
wa-dialog::part(dialog) {
|
||||
margin: auto auto 0 auto;
|
||||
max-width: none;
|
||||
max-height: 85vh;
|
||||
width: 100%;
|
||||
border-radius: 12px 12px 0 0;
|
||||
background: var(--yj-bg-elevated, #343a40);
|
||||
padding: 0;
|
||||
}
|
||||
|
||||
/* **A long menu scrolls; it does not hang off the bottom.**
|
||||
Measured on the device at 80vh: seven 48px rows plus the grip
|
||||
came to 364px against a 351px dialog, so the last row's
|
||||
bottom was at y=452 on a 439px screen -- the one row a
|
||||
destructive action is most likely to be. The cap has to stay
|
||||
(a sheet covering the whole screen is a page, not a sheet),
|
||||
so the body is what gives. */
|
||||
wa-dialog::part(body) {
|
||||
padding: 0;
|
||||
overflow-y: auto;
|
||||
}
|
||||
|
||||
/* A sheet is dragged at with a thumb, so it says where its top
|
||||
edge is. Decorative: the panel below it carries the actions. */
|
||||
.grip {
|
||||
width: 36px;
|
||||
height: 4px;
|
||||
margin: 8px auto 4px;
|
||||
border-radius: 2px;
|
||||
background: var(--yj-text-tertiary, #888);
|
||||
}
|
||||
`;
|
||||
|
||||
override connectedCallback(): void {
|
||||
super.connectedCallback();
|
||||
|
||||
// Looked up here rather than at module load, so a test can
|
||||
// install its own matchMedia before the element is created.
|
||||
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
|
||||
this.sheet = this.phoneQuery?.matches ?? false;
|
||||
this.phoneQuery?.addEventListener('change', this.onPhoneChange);
|
||||
}
|
||||
|
||||
override disconnectedCallback(): void {
|
||||
super.disconnectedCallback();
|
||||
this.phoneQuery?.removeEventListener('change', this.onPhoneChange);
|
||||
}
|
||||
|
||||
private onPhoneChange = (e: MediaQueryListEvent): void => {
|
||||
this.sheet = e.matches;
|
||||
};
|
||||
|
||||
/**
|
||||
* Re-run the popup's positioning.
|
||||
*
|
||||
* Forwarded rather than dropped because `page-header` calls it when
|
||||
* it opens the overflow menu: the popup is rendered before the
|
||||
* button it anchors to has settled. A sheet has nothing to
|
||||
* reposition -- it is anchored to the bottom of the screen -- so
|
||||
* there it is deliberately a no-op rather than an error.
|
||||
*/
|
||||
reposition(): void {
|
||||
this.popup?.reposition();
|
||||
}
|
||||
|
||||
/**
|
||||
* The panel the host slotted in. It is light DOM here and stays in
|
||||
* the host's shadow root, which is what keeps the host's own
|
||||
* `contextMenuStyles` applying to it in both presentations.
|
||||
*/
|
||||
private get panel(): HTMLElement | null {
|
||||
return this.querySelector('.context-menu-panel');
|
||||
}
|
||||
|
||||
override updated(): void {
|
||||
const panel = this.panel;
|
||||
|
||||
// The sheet's rows are bigger, and that rule lives in the one
|
||||
// stylesheet every call site already includes rather than in
|
||||
// twelve places. The attribute is how it knows.
|
||||
if (panel) panel.toggleAttribute('data-sheet', this.sheet);
|
||||
|
||||
if (this.sheet) {
|
||||
this.syncSheet(panel);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.popup) {
|
||||
if (this.anchor) this.popup.anchor = this.anchor;
|
||||
|
||||
this.popup.active = this.active;
|
||||
}
|
||||
}
|
||||
|
||||
private syncSheet(panel: HTMLElement | null): void {
|
||||
const dialog = this.dialog;
|
||||
|
||||
if (!dialog) return;
|
||||
|
||||
// The dialog is named after the menu it contains, so no call
|
||||
// site has to say the same thing twice: the panel already
|
||||
// carries `role="menu"` and an `aria-label` naming what it acts
|
||||
// on. `without-header` renders no heading, which is
|
||||
// `name-dialog`'s documented `aria-label` path.
|
||||
const label = panel?.getAttribute('aria-label') || this.label;
|
||||
|
||||
if (label) dialog.setAttribute('label', label);
|
||||
|
||||
nameDialogsIn(this.shadowRoot);
|
||||
|
||||
if (dialog.open !== this.active) dialog.open = this.active;
|
||||
}
|
||||
|
||||
/**
|
||||
* `wa-dialog` closed itself — Escape, or its own close button.
|
||||
* The controller owns `contextMenuOpen`, so it has to hear about
|
||||
* it or the menu is left open in state and shut on screen.
|
||||
*/
|
||||
private onDialogShown = (): void => {
|
||||
if (!this.active) return;
|
||||
|
||||
this.dispatchEvent(
|
||||
new CustomEvent(MENU_SHOWN_EVENT, {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
private onDialogHide = (): void => {
|
||||
if (!this.active) return;
|
||||
|
||||
this.dispatchEvent(
|
||||
new CustomEvent(MENU_DISMISS_EVENT, {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
override render() {
|
||||
if (this.sheet) {
|
||||
// **The anchor stays out of the sheet.** One call site --
|
||||
// `page-header`'s overflow menu -- slots its own trigger
|
||||
// button as the thing the popup hangs from, and a sheet
|
||||
// hangs from the bottom of the screen instead. Rendering
|
||||
// that slot outside the dialog is what keeps the button on
|
||||
// the page rather than inside the surface it opens.
|
||||
return html`
|
||||
<slot name="anchor"></slot>
|
||||
<wa-dialog
|
||||
without-header
|
||||
data-testid="menu-sheet"
|
||||
@wa-after-show=${this.onDialogShown}
|
||||
@wa-hide=${this.onDialogHide}
|
||||
>
|
||||
<div class="grip"></div>
|
||||
<slot></slot>
|
||||
</wa-dialog>
|
||||
`;
|
||||
}
|
||||
|
||||
return html`
|
||||
<wa-popup placement=${this.placement} flip shift>
|
||||
<slot name="anchor" slot="anchor"></slot>
|
||||
<slot></slot>
|
||||
</wa-popup>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
declare global {
|
||||
interface HTMLElementTagNameMap {
|
||||
'menu-surface': MenuSurface;
|
||||
}
|
||||
}
|
||||
@@ -343,6 +343,12 @@ export class NowPlayingView extends LitElement {
|
||||
a media query because the bottom bar wants a
|
||||
different answer at this same viewport. -->
|
||||
<player-controls context="full"></player-controls>
|
||||
<!-- Rendered unconditionally and absent on its own
|
||||
terms where the device owns the volume (#64): the
|
||||
control asks the player, not this view and not the
|
||||
viewport. A hidden host draws no gap, so that is
|
||||
29px of a 439px screen back to the album art
|
||||
(#172). -->
|
||||
<volume-control></volume-control>
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, property, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import {
|
||||
@@ -183,8 +183,8 @@ export class PageHeader extends LitElement {
|
||||
@query('#page-header-overflow')
|
||||
private menuPanel?: HTMLElement;
|
||||
|
||||
@query('wa-popup')
|
||||
private popup?: WaPopup;
|
||||
@query('menu-surface')
|
||||
private popup?: MenuSurface;
|
||||
|
||||
private menuKeyboard = new MenuKeyboard(() => this.closeMenu());
|
||||
|
||||
@@ -418,7 +418,7 @@ export class PageHeader extends LitElement {
|
||||
outline-offset: -1px;
|
||||
}
|
||||
|
||||
wa-popup {
|
||||
menu-surface {
|
||||
z-index: 200;
|
||||
}
|
||||
|
||||
@@ -670,10 +670,17 @@ export class PageHeader extends LitElement {
|
||||
return html`
|
||||
<div class="actions">
|
||||
${this.actions.map((a) => this.renderActionButton(a))}
|
||||
<wa-popup
|
||||
<!-- A sheet below 600px, like every other menu in the
|
||||
app (#60). The clipping that issue is about does
|
||||
not bite here — this one opens downward from the
|
||||
top of a full-height view, so it has somewhere to
|
||||
go even without top-layer promotion — but the touch
|
||||
targets do: on a phone *every* action of a page
|
||||
that overflows lives in here, at wa-dropdown-item
|
||||
defaults. One surface, so there is no second
|
||||
answer to what a menu looks like. -->
|
||||
<menu-surface
|
||||
placement="bottom-end"
|
||||
flip
|
||||
shift
|
||||
.active=${this.menuOpen}
|
||||
>
|
||||
<button
|
||||
@@ -711,7 +718,7 @@ export class PageHeader extends LitElement {
|
||||
`,
|
||||
)}
|
||||
</div>
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
|
||||
@@ -7,7 +7,8 @@ import {
|
||||
} from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@lit-labs/virtualizer';
|
||||
import type { LitVirtualizer } from '@lit-labs/virtualizer';
|
||||
@@ -35,7 +36,7 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { notificationStore } from '@store/notification-store';
|
||||
@@ -145,10 +146,10 @@ export class PlaylistDetails
|
||||
private dragImageEl: HTMLElement | null = null;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
@query('track-details')
|
||||
private trackDetailsDialog!: TrackDetails;
|
||||
@@ -163,11 +164,11 @@ export class PlaylistDetails
|
||||
// ContextMenuHost interface
|
||||
// =================================================================
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -1617,11 +1618,8 @@ export class PlaylistDetails
|
||||
|
||||
private renderContextMenu() {
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.contextMenuOpen}
|
||||
>
|
||||
@@ -1783,13 +1781,12 @@ export class PlaylistDetails
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.playlistSubmenuOpen}
|
||||
>
|
||||
@@ -1816,7 +1813,7 @@ export class PlaylistDetails
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,9 @@ import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, state, query } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
|
||||
import {
|
||||
@@ -140,7 +142,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
private pendingDropPaths: string[] = [];
|
||||
|
||||
@query('#playlist-context-menu')
|
||||
private playlistContextMenuPopup!: WaPopup;
|
||||
private playlistContextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('duplicate-tracks-dialog')
|
||||
private duplicateDialog!: DuplicateTracksDialog;
|
||||
@@ -1059,7 +1061,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
);
|
||||
}
|
||||
|
||||
private closePlaylistContextMenu() {
|
||||
private closePlaylistContextMenu = () => {
|
||||
if (!this.playlistContextMenuOpen) return;
|
||||
|
||||
this.menuKeyboard.close();
|
||||
@@ -1072,7 +1074,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
if (popup) {
|
||||
popup.active = false;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
private async onPlaylistContextAction(
|
||||
action: string,
|
||||
@@ -1500,13 +1502,11 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
</div>`
|
||||
: this.renderPlaylistList()}
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this
|
||||
.playlistContextMenuOpen}
|
||||
@menu-dismiss=${this.closePlaylistContextMenu}
|
||||
>
|
||||
${this.playlistContextMenuOpen
|
||||
? html`
|
||||
@@ -1564,7 +1564,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<duplicate-tracks-dialog
|
||||
@playlist-action-complete=${() =>
|
||||
|
||||
@@ -9,7 +9,8 @@ import {
|
||||
} from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import { QueueController } from '@store/controllers/queue-controller';
|
||||
import { creditStore } from '@store/credit-store';
|
||||
@@ -34,7 +35,7 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import {
|
||||
@@ -140,13 +141,13 @@ export class QueuePanel
|
||||
private delegationAttached = false;
|
||||
|
||||
@query('#add-to-playlist-popup')
|
||||
private addToPlaylistPopup!: WaPopup;
|
||||
private addToPlaylistPopup!: MenuSurface;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
/** Unsubscribes the credit-arrival repaint. */
|
||||
private creditsUnsub?: () => void;
|
||||
@@ -305,11 +306,11 @@ export class QueuePanel
|
||||
// ContextMenuHost interface
|
||||
// =================================================================
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -1092,7 +1093,7 @@ export class QueuePanel
|
||||
}
|
||||
}
|
||||
|
||||
private closePlaylistPicker() {
|
||||
private closePlaylistPicker = () => {
|
||||
if (!this.playlistPickerOpen) return;
|
||||
|
||||
this.playlistPickerOpen = false;
|
||||
@@ -1102,7 +1103,7 @@ export class QueuePanel
|
||||
if (popup) {
|
||||
popup.active = false;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
private onPlaylistActionComplete = () => {
|
||||
this.closePlaylistPicker();
|
||||
@@ -2042,9 +2043,11 @@ export class QueuePanel
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="add-to-playlist-popup"
|
||||
label="Add to playlist"
|
||||
placement="bottom-end"
|
||||
@menu-dismiss=${this.closePlaylistPicker}
|
||||
.active=${this.playlistPickerOpen}
|
||||
>
|
||||
${this.playlistPickerOpen
|
||||
@@ -2060,7 +2063,7 @@ export class QueuePanel
|
||||
></playlist-picker>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<div
|
||||
class="list-area"
|
||||
@@ -2103,11 +2106,8 @@ export class QueuePanel
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen
|
||||
@@ -2195,13 +2195,12 @@ export class QueuePanel
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.playlistSubmenuOpen}
|
||||
>
|
||||
${this.ctxMenu.playlistSubmenuOpen &&
|
||||
@@ -2224,7 +2223,7 @@ export class QueuePanel
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<track-details></track-details>
|
||||
`;
|
||||
|
||||
@@ -26,7 +26,7 @@ import {
|
||||
contextMenuStyles,
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { focusRovingRow, nextRovingIndex } from '@utils/roving-rows';
|
||||
import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import {
|
||||
@@ -40,7 +40,8 @@ import {
|
||||
} from '@utils/drag-image';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@lit-labs/virtualizer';
|
||||
import type { LitVirtualizer } from '@lit-labs/virtualizer';
|
||||
@@ -176,10 +177,10 @@ export class SmartPlaylistDetails
|
||||
private dragImageEl: HTMLElement | null = null;
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
@query('track-details')
|
||||
private trackDetailsDialog!: TrackDetails;
|
||||
@@ -188,11 +189,11 @@ export class SmartPlaylistDetails
|
||||
// ContextMenuHost interface
|
||||
// =================================================================
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -1465,11 +1466,8 @@ export class SmartPlaylistDetails
|
||||
|
||||
private renderContextMenu() {
|
||||
return html`
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen
|
||||
@@ -1583,13 +1581,12 @@ export class SmartPlaylistDetails
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu
|
||||
.playlistSubmenuOpen}
|
||||
>
|
||||
@@ -1616,7 +1613,7 @@ export class SmartPlaylistDetails
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ import {
|
||||
isContextMenuKey,
|
||||
} from '@utils/context-menu-controller.js';
|
||||
|
||||
import type { ContextMenuHost } from '@utils/context-menu-controller.js';
|
||||
import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js';
|
||||
import { PlayerController } from '@store/controllers/player-controller';
|
||||
import { SearchController } from '@store/controllers/search-controller';
|
||||
import '@components/page-header/page-header';
|
||||
@@ -63,7 +63,8 @@ import type {
|
||||
} from '@lit-labs/virtualizer';
|
||||
import { flow } from '@lit-labs/virtualizer/layouts/flow.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import type { MenuSurface } from '../menu-surface/menu-surface';
|
||||
import '../menu-surface/menu-surface';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import { describeError } from '@utils/describe-error';
|
||||
@@ -231,18 +232,18 @@ export class TrackList
|
||||
private tracks: library.Track[] = [];
|
||||
|
||||
@query('#context-menu')
|
||||
private contextMenuPopup!: WaPopup;
|
||||
private contextMenuPopup!: MenuSurface;
|
||||
|
||||
@query('#playlist-submenu')
|
||||
private playlistSubmenuPopup!: WaPopup;
|
||||
private playlistSubmenuPopup!: MenuSurface;
|
||||
|
||||
// -- ContextMenuHost interface --
|
||||
|
||||
getContextMenuPopup(): WaPopup | undefined {
|
||||
getContextMenuPopup(): MenuTarget | undefined {
|
||||
return this.contextMenuPopup;
|
||||
}
|
||||
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined {
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined {
|
||||
return this.playlistSubmenuPopup;
|
||||
}
|
||||
|
||||
@@ -2323,11 +2324,8 @@ export class TrackList
|
||||
</div>
|
||||
`}
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="context-menu"
|
||||
placement="bottom-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.contextMenuOpen}
|
||||
>
|
||||
${this.ctxMenu.contextMenuOpen
|
||||
@@ -2413,13 +2411,12 @@ export class TrackList
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<wa-popup
|
||||
<menu-surface
|
||||
id="playlist-submenu"
|
||||
label="Add to playlist"
|
||||
placement="right-start"
|
||||
flip
|
||||
shift
|
||||
.active=${this.ctxMenu.playlistSubmenuOpen}
|
||||
>
|
||||
${this.ctxMenu.playlistSubmenuOpen && this.selection.hasSelection
|
||||
@@ -2437,7 +2434,7 @@ export class TrackList
|
||||
</div>
|
||||
`
|
||||
: nothing}
|
||||
</wa-popup>
|
||||
</menu-surface>
|
||||
|
||||
<track-details></track-details>
|
||||
`;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { EventsOn } from '@runtime/runtime';
|
||||
import { GetPopupVolume } from '@go/config/config.js';
|
||||
import { SystemOwnsVolume } from '@go/player/player.js';
|
||||
import { Events } from '../events';
|
||||
|
||||
type Subscriber = () => void;
|
||||
@@ -28,10 +29,37 @@ type Subscriber = () => void;
|
||||
* becomes one. An install that has chosen the popup sees it swap once
|
||||
* on load, which is the cheaper of the two wrong first frames: the
|
||||
* inline slider occupies the space the popup's button would have.
|
||||
*
|
||||
* **`available` is the question one step earlier — whether there is a
|
||||
* volume of ours to draw at all (#64).** On Android the hardware keys
|
||||
* are the volume control and the backend pins its own level at
|
||||
* maximum, so a slider here would move nothing.
|
||||
*
|
||||
* It is asked of the *player* rather than of the viewport, and that is
|
||||
* the whole design decision. Every other stand-down rule in this app
|
||||
* is a width, because a width is what a browser can answer and what
|
||||
* every tier can test — but this one is a property of the build. Keyed
|
||||
* on width instead, an Android tablet at 600px or more would draw the
|
||||
* bottom bar's slider over a pinned level: a control that cannot act,
|
||||
* which `library-status-indicator` settled is worse than none.
|
||||
*
|
||||
* It lives beside `popup` because both answer "what presentation does
|
||||
* the volume control get", both readers are the same two components,
|
||||
* and "none" is a presentation. A second store would be a second
|
||||
* subscription in the same `connectedCallback` saying the same thing.
|
||||
*
|
||||
* The initial value is `true` on the same first-frame rule: there is a
|
||||
* volume on every platform but one, and the platform that pins it sees
|
||||
* the control once at boot and never again in the session — the answer
|
||||
* cannot change while the app runs, so by the time the lazily-mounted
|
||||
* now-playing view exists it has long been settled by the bar's own
|
||||
* copy.
|
||||
*/
|
||||
class VolumeStyleStore {
|
||||
private value = false;
|
||||
|
||||
private hasVolume = true;
|
||||
|
||||
private loaded = false;
|
||||
|
||||
private subscribers = new Set<Subscriber>();
|
||||
@@ -47,13 +75,21 @@ class VolumeStyleStore {
|
||||
return this.value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this app has a volume of its own to control. False where
|
||||
* the device owns it; see the class comment.
|
||||
*/
|
||||
get available(): boolean {
|
||||
return this.hasVolume;
|
||||
}
|
||||
|
||||
/** Reads the setting once. Safe to call from every mount. */
|
||||
async init(): Promise<void> {
|
||||
if (this.loaded) return;
|
||||
|
||||
this.loaded = true;
|
||||
|
||||
await this.refresh();
|
||||
await Promise.all([this.refreshAvailability(), this.refresh()]);
|
||||
}
|
||||
|
||||
subscribe(fn: Subscriber): () => void {
|
||||
@@ -77,6 +113,28 @@ class VolumeStyleStore {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Asked once, not on `GeneralConfigChanged`: this is a property of
|
||||
* the platform the binary was built for and cannot change while
|
||||
* the app is running.
|
||||
*/
|
||||
private async refreshAvailability(): Promise<void> {
|
||||
try {
|
||||
const owned = await SystemOwnsVolume();
|
||||
|
||||
if (owned === !this.hasVolume) return;
|
||||
|
||||
this.hasVolume = !owned;
|
||||
this.notify();
|
||||
} catch (err) {
|
||||
// The control renders, which is the answer on every
|
||||
// platform but one and is the recoverable way to be wrong:
|
||||
// a working control nobody needs, rather than a missing one
|
||||
// somebody does.
|
||||
console.error('failed to ask who owns the volume', err);
|
||||
}
|
||||
}
|
||||
|
||||
private notify(): void {
|
||||
for (const fn of this.subscribers) fn();
|
||||
}
|
||||
|
||||
@@ -5,7 +5,20 @@ import type {
|
||||
} from 'lit';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
|
||||
/**
|
||||
* What this controller needs of a surface: something it can switch on
|
||||
* and point at. Both `wa-popup` and `menu-surface` satisfy it.
|
||||
*/
|
||||
export type MenuTarget = HTMLElement & {
|
||||
active: boolean;
|
||||
anchor?: WaPopup['anchor'];
|
||||
};
|
||||
|
||||
import { registerViewAware } from './view-lifecycle';
|
||||
import {
|
||||
MENU_DISMISS_EVENT,
|
||||
MENU_SHOWN_EVENT,
|
||||
} from '../components/menu-surface/menu-surface';
|
||||
|
||||
/**
|
||||
* Host interface for components using the ContextMenuController.
|
||||
@@ -17,10 +30,18 @@ export interface ContextMenuHost
|
||||
extends ReactiveControllerHost {
|
||||
updateComplete: Promise<boolean>;
|
||||
shadowRoot: ShadowRoot | null;
|
||||
/** Return the main context-menu popup element. */
|
||||
getContextMenuPopup(): WaPopup | undefined;
|
||||
/** Return the playlist submenu popup element. */
|
||||
getPlaylistSubmenuPopup(): WaPopup | undefined;
|
||||
/**
|
||||
* Return the main context-menu surface.
|
||||
*
|
||||
* `MenuSurface` since #60, which is a `wa-popup` above 600px and a
|
||||
* bottom sheet below it. The type is the narrow shape this
|
||||
* controller drives rather than either element, so a host that
|
||||
* still renders a bare `wa-popup` — the playlist submenu does —
|
||||
* satisfies it unchanged.
|
||||
*/
|
||||
getContextMenuPopup(): MenuTarget | undefined;
|
||||
/** Return the playlist submenu surface. */
|
||||
getPlaylistSubmenuPopup(): MenuTarget | undefined;
|
||||
/**
|
||||
* Called when the context menu is closed by an
|
||||
* outside click/contextmenu/mousedown. Components
|
||||
@@ -33,6 +54,15 @@ export interface ContextMenuHost
|
||||
/** Submenu close delay in milliseconds. */
|
||||
const SUBMENU_CLOSE_DELAY = 150;
|
||||
|
||||
/**
|
||||
* How long to keep trying to put focus on a menu's first item.
|
||||
*
|
||||
* Long enough to outlast `wa-dialog`'s show animation, which ends by
|
||||
* focusing the dialog; short enough that a menu which genuinely has no
|
||||
* items stops rather than spinning for the life of the page.
|
||||
*/
|
||||
const FOCUS_RETRY_BUDGET_MS = 500;
|
||||
|
||||
/** A menu item, focusable and clickable. Web Awesome sets `role` itself. */
|
||||
type MenuItem = HTMLElement & { active?: boolean; disabled?: boolean };
|
||||
|
||||
@@ -73,6 +103,30 @@ export class MenuKeyboard {
|
||||
void this.focusFirstItem(panel);
|
||||
}
|
||||
|
||||
/**
|
||||
* Take focus back, for a surface that finished showing after we
|
||||
* had already placed it.
|
||||
*
|
||||
* `wa-dialog` focuses `[autofocus]` or *itself* on the animation
|
||||
* frame after `showModal()`, and it cannot see our first menu item
|
||||
* to prefer it: the panel is slotted through `menu-surface`, so the
|
||||
* dialog's own `querySelector` stops at the `<slot>`. Retrying on a
|
||||
* longer budget does not fix this either -- the first attempt
|
||||
* *succeeds*, and the steal happens afterwards. Measured on the
|
||||
* device: the sheet opened with focus on the `<dialog>` and every
|
||||
* arrow key went nowhere.
|
||||
*
|
||||
* So the surface says when it has settled and this re-asserts. It
|
||||
* is a no-op for a menu that is closed or that already has focus.
|
||||
*/
|
||||
refocus(): void {
|
||||
const panel = this.panel;
|
||||
|
||||
if (!panel || panel.contains(deepActiveElement())) return;
|
||||
|
||||
void this.focusFirstItem(panel);
|
||||
}
|
||||
|
||||
/**
|
||||
* Focus the first item, once the items are items.
|
||||
*
|
||||
@@ -91,11 +145,24 @@ export class MenuKeyboard {
|
||||
|
||||
await Promise.all(candidates.map((el) => el.updateComplete ?? null));
|
||||
|
||||
// …and once the popup has positioned itself. `wa-popup` places the
|
||||
// …and once the surface has shown itself. `wa-popup` places the
|
||||
// panel on an animation frame, and `focus()` on a not-yet-shown
|
||||
// element is a silent no-op — which looks identical to a menu
|
||||
// that opened and refused to take focus.
|
||||
for (let attempt = 0; attempt < 3; attempt++) {
|
||||
//
|
||||
// **The budget is time, not frames, because #60 gave this a
|
||||
// second kind of surface.** Three frames was enough for a
|
||||
// popup; a `wa-dialog` runs a show *animation* and moves focus
|
||||
// to the dialog itself when it finishes, which lands after
|
||||
// those frames and takes the focus back. Measured on the
|
||||
// device: the sheet opened with `document.activeElement` on the
|
||||
// `<dialog>`, so every arrow key went nowhere. Retrying to a
|
||||
// deadline is `roving-grid`'s rule for the same reason — the
|
||||
// thing being waited for is another component's animation, not
|
||||
// a fixed number of paints.
|
||||
const deadline = Date.now() + FOCUS_RETRY_BUDGET_MS;
|
||||
|
||||
while (Date.now() < deadline) {
|
||||
// Bail if the menu closed while we waited.
|
||||
if (this.panel !== panel) return;
|
||||
|
||||
@@ -259,6 +326,20 @@ export class ContextMenuController
|
||||
/** Bound close handler for document events. */
|
||||
private closeHandler = () => this.close();
|
||||
|
||||
/**
|
||||
* A surface finished showing; see `MenuKeyboard.refocus`.
|
||||
*
|
||||
* **Not while the submenu is up.** Both surfaces send this, and the
|
||||
* submenu's sheet opens *over* the main one -- so re-asserting
|
||||
* focus on the main panel's first item would snatch it straight
|
||||
* back out of the playlist picker the user just opened.
|
||||
*/
|
||||
private shownHandler = () => {
|
||||
if (this.contextMenuOpen && !this.playlistSubmenuOpen) {
|
||||
this.keyboard.refocus();
|
||||
}
|
||||
};
|
||||
|
||||
/** Bound mousedown handler for outside-click detection. */
|
||||
private mousedownCloseHandler = (
|
||||
e: MouseEvent,
|
||||
@@ -325,12 +406,28 @@ export class ContextMenuController
|
||||
'mousedown',
|
||||
this.mousedownCloseHandler,
|
||||
);
|
||||
document.addEventListener(
|
||||
MENU_DISMISS_EVENT,
|
||||
this.closeHandler,
|
||||
);
|
||||
document.addEventListener(
|
||||
MENU_SHOWN_EVENT,
|
||||
this.shownHandler,
|
||||
);
|
||||
}
|
||||
|
||||
private detach(): void {
|
||||
if (!this.listening) return;
|
||||
|
||||
this.listening = false;
|
||||
document.removeEventListener(
|
||||
MENU_DISMISS_EVENT,
|
||||
this.closeHandler,
|
||||
);
|
||||
document.removeEventListener(
|
||||
MENU_SHOWN_EVENT,
|
||||
this.shownHandler,
|
||||
);
|
||||
document.removeEventListener(
|
||||
'click',
|
||||
this.closeHandler,
|
||||
@@ -550,6 +647,48 @@ export const contextMenuStyles = css`
|
||||
z-index: 200;
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------
|
||||
The sheet (#60).
|
||||
|
||||
menu-surface puts data-sheet on the panel when it is drawn
|
||||
as a bottom sheet, and these rules are here rather than in that
|
||||
component because the panel is the *host's* light DOM: it lives
|
||||
in the host's shadow root, so only the host's stylesheet can
|
||||
reach it. This file is the one every call site already includes,
|
||||
which is what makes twelve menus grow thumb-sized rows from one
|
||||
edit.
|
||||
|
||||
Measured on the device before the change: rows were 29px, against
|
||||
the 44px floor plan 018 promises and the 48px this issue asks
|
||||
for. --------------------------------------------------------- */
|
||||
.context-menu-panel[data-sheet] {
|
||||
border: none;
|
||||
border-radius: 0;
|
||||
box-shadow: none;
|
||||
min-width: 0;
|
||||
padding: 4px 0 8px;
|
||||
background-color: transparent;
|
||||
}
|
||||
|
||||
.context-menu-panel[data-sheet] wa-dropdown-item {
|
||||
font-size: var(--yj-text-md, 0.9375rem);
|
||||
min-height: 48px;
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
.context-menu-panel[data-sheet] wa-dropdown-item::part(base) {
|
||||
min-height: 48px;
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
/* A submenu arrow means "a flyout opens to the right", which is not
|
||||
what happens on a phone and is not a thing a thumb can aim at.
|
||||
The row still works — it is the tap handler that opens the
|
||||
playlist picker — so what goes is the arrow, not the item. */
|
||||
.context-menu-panel[data-sheet] .submenu-arrow {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.context-menu-panel {
|
||||
background-color: var(
|
||||
--yj-bg-elevated,
|
||||
|
||||
@@ -0,0 +1,222 @@
|
||||
/**
|
||||
* Where a context menu is drawn (#60).
|
||||
*
|
||||
* **This file asserts the mechanism, not the symptom, and that is the
|
||||
* whole point of it.** The defect is that on the reference device's
|
||||
* Chrome 113 a `wa-popup` has no Popover API to promote it to the top
|
||||
* layer, so it falls back to `position: fixed` and is then *clipped* by
|
||||
* `.main-panel`'s `contain: paint`. No tier here can reproduce that:
|
||||
* this runner's Chromium and CI's WebKit both have the Popover API, so
|
||||
* the popup is top-layered and looks perfectly correct. A test that
|
||||
* asserted "the menu is not clipped" would pass on the broken build.
|
||||
*
|
||||
* What is checkable everywhere is *which surface exists*. A native
|
||||
* `<dialog>` uses the real top layer, which Chrome 37 has, so "it is a
|
||||
* dialog at phone width" is the property that makes the device
|
||||
* behaviour follow. The measurements that needed the hardware are on
|
||||
* the PR and in `.planning/NOTES.md`.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
import '@components/menu-surface/menu-surface';
|
||||
import { MENU_DISMISS_EVENT } from '@components/menu-surface/menu-surface';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
/** Every source file, as text. */
|
||||
const SOURCES = import.meta.glob<string>('../../src/**/*.ts', {
|
||||
eager: true,
|
||||
query: '?raw',
|
||||
import: 'default',
|
||||
});
|
||||
|
||||
/**
|
||||
* The two files allowed to render a raw `wa-popup`.
|
||||
*
|
||||
* `menu-surface` *is* the popup, in its desktop presentation.
|
||||
* `job-indicator` is the documented exception and the contrast that
|
||||
* proved the diagnosis: it lives in `.top-bar`, no ancestor of which
|
||||
* has containment, so even the fixed fallback lands correctly on the
|
||||
* device -- measured on #62, unclipped at every width.
|
||||
*
|
||||
* `now-playing`'s cover preview is the third, and it is a different
|
||||
* reason: it is not a menu. It opens on `mouseenter` over the album
|
||||
* art, so a touch device never sees it at all, and a bottom sheet for
|
||||
* a hover preview would be absurd. It is also in the bottom bar rather
|
||||
* than the main panel, so nothing clips it either.
|
||||
*
|
||||
* **Both were found by this sweep, not by the conversion**, which is
|
||||
* the argument for having it: twelve call sites were converted by hand
|
||||
* and two more existed.
|
||||
*/
|
||||
const MAY_USE_POPUP = [
|
||||
'menu-surface/menu-surface.ts',
|
||||
'jobs/job-indicator.ts',
|
||||
'now-playing/now-playing.ts',
|
||||
];
|
||||
|
||||
/**
|
||||
* Answer `matchMedia` for the phone query, on `transport-context`'s
|
||||
* pattern: what is under test is the component's reaction to the
|
||||
* answer, not whether this runner's window can get below 600px.
|
||||
*/
|
||||
const realMatchMedia = window.matchMedia;
|
||||
|
||||
function pretendPhone(phone: boolean): void {
|
||||
window.matchMedia = ((query: string) => ({
|
||||
matches: phone && query.includes('599'),
|
||||
media: query,
|
||||
addEventListener: () => {},
|
||||
removeEventListener: () => {},
|
||||
})) as unknown as typeof window.matchMedia;
|
||||
}
|
||||
|
||||
/** A surface with the panel a real call site slots into it. */
|
||||
async function surfaceWithPanel(): Promise<HTMLElement> {
|
||||
const el = await fixture('menu-surface');
|
||||
|
||||
el.innerHTML =
|
||||
'<div class="context-menu-panel" role="menu" aria-label="Track actions">' +
|
||||
'<wa-dropdown-item>Play</wa-dropdown-item>' +
|
||||
'</div>';
|
||||
|
||||
const surface = el as unknown as HTMLElement & {
|
||||
active: boolean;
|
||||
updateComplete: Promise<unknown>;
|
||||
};
|
||||
|
||||
surface.active = true;
|
||||
await surface.updateComplete;
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
/**
|
||||
* The sweep, in the spirit of `icon-language.test.ts` and
|
||||
* `TestNoDirectRuntimeEmits`: the rule is about *every* call site, and
|
||||
* checking one checks nothing.
|
||||
*
|
||||
* Twelve menus were converted by hand. A thirteenth written as a bare
|
||||
* `<wa-popup>` would work perfectly in every tier here and be clipped
|
||||
* on the device, which is exactly the failure this whole change is
|
||||
* about and exactly the one no runtime assertion can see.
|
||||
*/
|
||||
describe('every menu goes through the one surface', () => {
|
||||
it('reads the sources at all', () => {
|
||||
// A sweep over an empty glob passes, so this is asserted first.
|
||||
expect(Object.keys(SOURCES).length).toBeGreaterThan(100);
|
||||
});
|
||||
|
||||
it('leaves no raw wa-popup outside the two files allowed one', () => {
|
||||
const offenders = Object.entries(SOURCES)
|
||||
.filter(([path]) => !MAY_USE_POPUP.some((ok) => path.endsWith(ok)))
|
||||
.filter(([, src]) => src.includes('<wa-popup'))
|
||||
.map(([path]) => path.replace(/^.*\/src\//, 'src/'));
|
||||
|
||||
expect(
|
||||
offenders,
|
||||
'these render a popup directly; use <menu-surface> so the phone gets a sheet',
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('menu-surface', () => {
|
||||
afterEach(() => {
|
||||
window.matchMedia = realMatchMedia;
|
||||
});
|
||||
|
||||
describe('above the phone breakpoint', () => {
|
||||
beforeEach(() => pretendPhone(false));
|
||||
|
||||
it('draws a popup, which is what the desktop has always had', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
expect(el.shadowRoot?.querySelector('wa-popup')).not.toBeNull();
|
||||
expect(el.shadowRoot?.querySelector('wa-dialog')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not mark the panel as a sheet', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
expect(
|
||||
el.querySelector('.context-menu-panel')?.hasAttribute('data-sheet'),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('at phone width', () => {
|
||||
beforeEach(() => pretendPhone(true));
|
||||
|
||||
/**
|
||||
* The load-bearing one. `wa-dialog` renders a *native* `<dialog>`,
|
||||
* and it is the native element -- not the wrapper -- that gets the
|
||||
* top layer and therefore escapes the paint containment that clips
|
||||
* the popup on the device.
|
||||
*/
|
||||
it('draws a native dialog, which is what escapes the clip', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
|
||||
|
||||
expect(wrapper, 'no wa-dialog at phone width').not.toBeNull();
|
||||
expect(el.shadowRoot?.querySelector('wa-popup')).toBeNull();
|
||||
|
||||
await (wrapper as HTMLElement & { updateComplete: Promise<unknown> })
|
||||
.updateComplete;
|
||||
|
||||
expect(
|
||||
wrapper?.shadowRoot?.querySelector('dialog'),
|
||||
'the wrapper is not backed by a native dialog',
|
||||
).not.toBeNull();
|
||||
});
|
||||
|
||||
/**
|
||||
* The rows are sized by `contextMenuStyles`, which lives in the
|
||||
* *host's* shadow root — so the only thing this component can do is
|
||||
* say which mode it is in. That attribute is the contract between
|
||||
* the two, and it is what twelve call sites get their thumb-sized
|
||||
* rows from.
|
||||
*/
|
||||
it('marks the panel as a sheet, which is what sizes the rows', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
expect(
|
||||
el.querySelector('.context-menu-panel')?.hasAttribute('data-sheet'),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
/**
|
||||
* `wa-dialog` closes itself on Escape. Without this the controller
|
||||
* would still believe the menu was open, and the *next* long-press
|
||||
* would do nothing — which is the failure mode that looks like the
|
||||
* gesture breaking rather than the dialog.
|
||||
*/
|
||||
it('reports a dismissal it did not initiate', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
let dismissed = 0;
|
||||
|
||||
document.addEventListener(MENU_DISMISS_EVENT, () => {
|
||||
dismissed += 1;
|
||||
});
|
||||
|
||||
el.shadowRoot
|
||||
?.querySelector('wa-dialog')
|
||||
?.dispatchEvent(new CustomEvent('wa-hide', { bubbles: false }));
|
||||
|
||||
expect(dismissed, 'no menu-dismiss reached the document').toBe(1);
|
||||
});
|
||||
|
||||
/**
|
||||
* A dialog with no accessible name is what `utils/name-dialog.ts`
|
||||
* exists for; here the name is already written on the panel, so no
|
||||
* call site says it twice.
|
||||
*/
|
||||
it('names the sheet after the menu it contains', async () => {
|
||||
const el = await surfaceWithPanel();
|
||||
|
||||
const wrapper = el.shadowRoot?.querySelector('wa-dialog');
|
||||
|
||||
expect(wrapper?.getAttribute('label')).toBe('Track actions');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Who owns the volume, and what the control does when it is not us
|
||||
* (#64).
|
||||
*
|
||||
* **This is the tier that can exercise the Android branch**, and it is
|
||||
* the reason the predicate is a backend answer rather than a build tag
|
||||
* the frontend cannot see: `SystemOwnsVolume` is a stub here, so the
|
||||
* "no volume" rendering is checked on an ordinary Linux CI runner with
|
||||
* no device anywhere. What no tier here can check is the *constant*
|
||||
* behind it, which `TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform`
|
||||
* sweeps the Go source for instead.
|
||||
*
|
||||
* It is a file of its own because `volumeStyleStore` asks once and
|
||||
* latches — the answer is a property of the binary and cannot change
|
||||
* while the app runs, so there is deliberately no event that refreshes
|
||||
* it. Vitest gives each file its own module registry, which is what
|
||||
* lets the stub be in place before the singleton is first touched.
|
||||
* The *available* case is the rest of `transport.test.ts`, which mounts
|
||||
* the same element under the default stub.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
|
||||
import '@components/audio-player/volume-control/volume-control';
|
||||
import '@components/now-playing-view/now-playing-view';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, resetHarness, stub } from '@test/support/harness';
|
||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||
import type { TrackInfo } from '@store/player-store';
|
||||
|
||||
const TRACK: TrackInfo = {
|
||||
fileName: 'tideline.mp3',
|
||||
filePath: '/music/tideline.mp3',
|
||||
trackLength: 245,
|
||||
seekPosition: 0,
|
||||
state: 'playing',
|
||||
title: 'Tideline',
|
||||
artist: 'Sea Change',
|
||||
album: 'Ebb',
|
||||
coverArt: '',
|
||||
coverArtSmall: '',
|
||||
coverArtMedium: '',
|
||||
coverArtLarge: '',
|
||||
trackChangeId: 1,
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
|
||||
describe('a platform whose volume we do not own', () => {
|
||||
beforeEach(async () => {
|
||||
resetHarness();
|
||||
stub('player.Player.SystemOwnsVolume', true);
|
||||
stub('config.Config.GetPopupVolume', false);
|
||||
|
||||
// The store latches on the first mount; do it here so every test
|
||||
// below sees a settled answer rather than the first frame.
|
||||
const warm = await fixture('volume-control');
|
||||
|
||||
await warm.updateComplete;
|
||||
});
|
||||
|
||||
it('renders no control at all, and no empty shadow root to find', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await el.updateComplete;
|
||||
|
||||
// Both halves matter. An empty shadow root is what stops a
|
||||
// positional or by-role query finding a button that cannot act;
|
||||
// `hidden` is what stops the host taking a flex item's worth of
|
||||
// space in the transport it sits in.
|
||||
expect(shadowAll(el, 'button')).toHaveLength(0);
|
||||
expect(shadowAll(el, 'wa-slider')).toHaveLength(0);
|
||||
expect(el.hidden, 'the host is not hidden').toBe(true);
|
||||
});
|
||||
|
||||
it('leaves the rest of the phone transport alone', async () => {
|
||||
emit(Events.TrackChanged, TRACK);
|
||||
|
||||
const view = await fixture('now-playing-view');
|
||||
|
||||
await view.updateComplete;
|
||||
|
||||
// Seeking and the transport buttons are not volume, and #64 is
|
||||
// allowed to remove one control, not to thin the screen out.
|
||||
expect(shadow(view, 'seek-bar')).not.toBeNull();
|
||||
expect(shadow(view, 'player-controls')).not.toBeNull();
|
||||
|
||||
const volume = shadow(view, 'volume-control') as HTMLElement | null;
|
||||
|
||||
expect(volume, 'the element is still mounted').not.toBeNull();
|
||||
expect(volume!.hidden, 'a mounted volume-control is not hidden').toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user