Compare commits

..
Author SHA1 Message Date
logan 4b2eec5703 Merge remote-tracking branch 'origin/main' into 60-context-menu-action-sheet
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 8m51s
2026-08-21 02:39:29 -04:00
logan 3871d37fdb Merge pull request 'chore(agent): add the scheduled backlog-issue prompt' (#177) from pi-agent-backlog-automation into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 8m54s
Reviewed-on: #177
2026-08-21 06:29:30 +00:00
logan 09b005557c chore(agent): add the scheduled backlog-issue prompt
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m25s
CI / e2e (pull_request) Successful in 9m2s
A scheduled pi session reads this file and works one open issue end to
end: pick, claim, branch, implement, verify on the tier the change
demands, open a PR, stop. It declines rather than improvises where it
cannot verify itself — a busy :34115 means another worktree is running
the app, and a green e2e run against someone else's build is worse than
no run at all.
2026-08-21 02:28:55 -04:00
logan ef5574d18b docs(shell): record the clip, and the four things only a device showed
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m32s
CI / e2e (pull_request) Successful in 8m57s
CLAUDE.md gains the surface beside the keyboard model it shares, and
NOTES.md the measurements: the 83px clip with its screenshot, the probe
that established a top-layer dialog escapes paint containment from
inside a view, the UA stylesheet's 354px, the focus steal a longer
retry cannot beat, and the submenu this change pushed off-screen before
it pulled it back.

The last of those is also a note about scope: the issue was claimed
saying the submenu would be measured and filed, and the measurement
said fix it.
2026-08-21 02:24:04 -04:00
logan 31dafb0ce0 test(shell): assert the surface, and sweep for a menu that skipped it
No tier here can reproduce the defect: this runner's Chromium and CI's
WebKit both have the Popover API, so the popup is top-layered and looks
perfectly correct, and a spec asserting "the menu is not clipped" would
pass on the broken build.  So these 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, for the same reason.

The sweep is the more valuable half.  A thirteenth menu written as a
bare <wa-popup> would work in every tier here and be clipped on the
device, so this reads every source file and fails on one outside a
three-file allowlist, each entry carrying why.  It found two call sites
the by-hand conversion had missed.

Four of the six behavioural tests fail on the build before this change;
the two asserting the desktop popup cannot, because that behaviour was
already there.

Closes #60
2026-08-21 02:24:02 -04:00
logan 9e7e7ce5a1 feat(shell): put every menu in the app through the one surface
Fourteen call sites, one tag name each and nothing else -- which is what
menu-surface's shape buys: the host's panel is slotted into whichever
presentation is up, so no item model, no keyboard model and no styling
moved.  The 48px rows come from contextMenuStyles, the one stylesheet
every one of these hosts already includes, because the panel is the
host's own light DOM and only the host's stylesheet can reach it.

Two of the fourteen were found by the source sweep rather than by the
conversion: queue-panel's add-to-playlist popup, which is a real menu.
now-playing's cover preview is allowlisted instead -- it is a hover
affordance in the bottom bar, so a touch device never opens it and
nothing clips it.

The playlist submenu had to come too, and that is the one place this
change made something worse before it made it better.  It is a
placement="right-start" flyout anchored to its row, and making the menu
full-width moved that anchor to x=0 -- so the flip put the picker at
x -182 to 0, entirely off-screen, and "Add to Playlist" led nowhere at
all.  Before the change the row started at x~245 and the same flip
landed on screen.  It is a sheet now and stacks over the first, which
is also why menu-shown does not re-assert focus while it is open.

The three hosts that do not use ContextMenuController -- page-header's
overflow menu, playlist-view's hand-rolled menu, queue-panel's picker
-- bind menu-dismiss themselves, or Escape would close the sheet and
leave their own open flag set.

page-header is included deliberately: the clipping does not bite there,
since it opens downward from the top of a full-height view, but on a
phone every action of an overflowing page lives in that menu at
wa-dropdown-item defaults.  One surface, so there is no second answer
to what a menu looks like.
2026-08-21 02:23:48 -04:00
logan 9aaa8beb99 feat(shell): draw a context menu where it fits, not where it is anchored
On the reference device every context menu in the app is clipped, and
the two halves of that are structural rather than incidental.  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.  Measured at 424x439 before any of this: the main panel
spans 0-318, the open menu spanned 191-401, and three of its seven
items were cut off with no way to reach them.  Rows were 29px against
a 44px floor.

menu-surface is one element with two presentations -- a wa-popup above
600px, a wa-dialog bottom sheet below it -- so the host keeps rendering
the panel it always rendered and ContextMenuController keeps driving
.active and .anchor as though it were talking to a popup.  showModal()
is Chrome 37 and uses the real top layer, so the sheet is immune by
construction rather than by styling.

Four things needed measuring on the hardware rather than reading.

"A dialog escapes containment" was the premise and was untested here:
every other dialog in this app is mounted in index.html, outside
.main-panel.  A probe dialog appended to track-list's shadow root
paints to y=439, over the mini player and the tab bar.

A native dialog's UA stylesheet centres it and caps its width, which
drew a 354px panel in the middle of a 424px screen -- so four
declarations in this component are pure undoing.

wa-dialog focuses [autofocus] or itself on the frame after
showModal(), and it cannot see our first menu item to prefer it: the
panel is slotted, so its own querySelector stops at the <slot>.  A
longer retry budget does not fix that, because the first attempt
succeeds and is then overwritten -- hence menu-shown and
MenuKeyboard.refocus().  The budget became time-based anyway, since
what is being waited for is another component's animation.

And a dismissal has to travel back: wa-dialog closes itself on Escape,
which would leave the controller believing the menu is open.  The
failure mode there is not a stuck sheet but the *next* long-press
doing nothing, which reads as the gesture breaking.
2026-08-21 02:23:33 -04:00
logan 2e29e67664 Merge pull request #174: Android: leave the volume to the system
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 8m56s
2026-08-21 05:50:06 +00:00
logan f26b44db08 docs(player): close three of the four gaps with a real device
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m26s
CI / e2e (pull_request) Successful in 9m5s
A Light Phone III (Android 14, SDK 34, arm64, Chrome 113 at 424x439)
was attached after the PR was opened, so what it listed as
unverifiable was checked rather than left as a caveat.

SystemOwnsVolume answers true on the device -- the build tag, the
constant, the field and the generated binding, end to end, which is the
one thing a source sweep only approximates and which nothing else here
compiles at all.  The control is absent in both mount points on the
real engine, and the transport measures 143px, exactly what the
desktop-headless "after" predicted.  A stored volume of 37 survives a
session that demonstrably rewrote the row.

The duck is the one that stays open, and now for a stated reason rather
than for want of hardware: the foreground service omits
setWillPauseWhenDucked from Oreo, so the framework attenuates us itself
and never sends AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK -- the device logs
`requestAudioFocus() ... flags=0x0` saying so.  minSdk is 21, so that
path is live code on Android 5.0 to 7.1 and unreachable above it.
Asking for a modern phone will not test it.
2026-08-21 01:34:44 -04:00
logan b43172a60c docs(player): record who owns the volume, and what it gave back
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Successful in 8m47s
CLAUDE.md's volume paragraph ended "#64 asks for it to be gone on
Android outright, which is a platform question the frontend cannot
currently ask", which is no longer true -- it can, and the paragraph
now says why the answer is a capability rather than a viewport and what
that costs.  The mediacontrols entry gains the corollary: on that
platform "the user's level" is a constant, and the duck is the one
thing that may still move the output.

NOTES.md carries the measurements: the per-element budget at 424x439
before and after, the :host([hidden]) specificity trap, the fact that
the bar's centring survives the control going away, and what no tier
here could check.
2026-08-21 00:53:44 -04:00
logan 2be6fb3066 feat(player): draw no volume control where there is no volume
volume-control asks the player whether there is a volume of ours to
control, and renders nothing when there is not.  The decision is in the
control rather than at either mount point because there are two, and
one of them -- the bottom bar's -- lives in index.html, which has no
module scope to make it conditional.

It could not have been a width, and that is the whole design decision.
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 bar's slider over a level the backend has
pinned -- a control that cannot act, on exactly the platform the rule
exists for, which library-status-indicator settled is worse than none.
The same rule is wrong the other way below 600px, where a narrow
desktop window has no hardware keys to fall back on.  index.css keeps
its phone rule, which is now about room and says so.

Rendering nothing and hiding the host are both needed and are separate
assertions: an empty shadow root is what stops a by-role or positional
query finding a button that cannot act, and :host([hidden]) is what
stops the element taking a flex item's worth of the transport.  The
host rule has to be written down, since :host { display: inline-flex }
outranks the UA's [hidden].

Measured at 424x439 by flipping the constant and rebuilding: the album
art goes 39px to 68px and the transport 172px to 143px -- 29px, being
the 21px control plus the 8px gap a hidden box stops drawing.  The
bar's centring is unaffected, since #23's outer columns are the same
min() expression rather than content-sized.

volume-ownership.test.ts is the tier that can exercise the Android
rendering, on an ordinary Linux runner, because the predicate is a
stubbable backend answer.  Both of its tests were confirmed to fail on
the build before this.

Closes #64
Closes #172
2026-08-21 00:53:36 -04:00
logan 867ced8c81 feat(player): leave the volume to the system where the system owns it
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 moves something the user already moved.  Where that is true the
player's level sits at maximum, SetVolume / ChangeVolume / MuteToggle
are refused, and nothing persists a level nobody chose: restore
remembers the stored value instead of applying it, and saveState writes
that same value back rather than recording the synthetic maximum.

Mute is in that list because it is a level of zero by another name --
and because with no control rendered it would be the one state on such
a platform the user could not get out of.

The predicate is named after the capability rather than the platform,
because that is what makes it testable.  Only platformOwnsVolume is
behind a build tag, in two files that declare nothing else; everything
else is decided against Player.systemVolume, a field a test sets either
way.  That is mediacontrols' split, with androidpayload.go's reasoning
for keeping the contract out of a tagged file, and the tagged pair is
covered by a source sweep since no tier here compiles both halves.

SetDuck is deliberately untouched: it 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 still move the output on such a platform, and
TestSystemVolumeStillDucks is that property rather than a comment.
2026-08-21 00:53:21 -04:00
logan fd71ef53c5 Merge pull request 'The phone transport: three controls, sized for a thumb' (#173) from 59-slim-the-mini-player into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 9m5s
2026-08-21 04:14:39 +00:00
30 changed files with 1909 additions and 195 deletions
+142
View File
@@ -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.
+212
View File
@@ -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.
+140 -9
View File
@@ -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,
+56 -7
View File
@@ -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,
)
+42
View File
@@ -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
}
+11
View File
@@ -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
+10
View File
@@ -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
+215
View File
@@ -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])
}
}
}
+19
View File
@@ -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
View File
@@ -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>
`;
+59 -1
View File
@@ -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();
}
+145 -6
View File
@@ -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);
});
});