Compare commits
22
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
67eeb75e7b | ||
|
|
fe1fbefee7 | ||
|
|
de04339494 | ||
|
|
998ce75fb6 | ||
|
|
4b392cb4c4 | ||
|
|
8d2109b87e | ||
|
|
b741b01cdf | ||
|
|
8a757c9bb4 | ||
|
|
d714bd7090 | ||
|
|
d64b069053 | ||
|
|
5490b2423e | ||
|
|
ffc9490a32 | ||
|
|
dc6625d33a | ||
|
|
dc8db159f9 | ||
|
|
86e7444603 | ||
|
|
2365806d18 | ||
|
|
7d348f243a | ||
|
|
ddd04623f7 | ||
|
|
9ad1477b1e | ||
|
|
70ab3ddf94 | ||
|
|
4f7529c315 | ||
|
|
bb21072386 |
@@ -194,9 +194,13 @@ like the app's fault and none is:
|
||||
|---|---|---|
|
||||
| x86_64 | modernc's raw `lstat` vs seccomp | SIGSYS, syscall 6 |
|
||||
| arm64, translated | Go reads `ID_AA64ISAR0_EL1` | SIGILL |
|
||||
| arm64, real device | — | unverified, still |
|
||||
| arm64, real device | **runs** (2026-08-20) | — |
|
||||
|
||||
**A physical arm64 device remains the only verification path.**
|
||||
**A physical arm64 device remains the only verification path**, and it
|
||||
has now been walked: a Light Phone III (TLP301, Android 14 / SDK 34,
|
||||
arm64-v8a, WebView Chrome 113 at 424x439). The app builds, installs,
|
||||
launches and stays up; `make android-smoke SECONDS=60` passes on it.
|
||||
What that run *found* is the lifecycle fault below.
|
||||
|
||||
### What was fixed to get here
|
||||
|
||||
@@ -210,6 +214,11 @@ no-op. `backend/system` gained no import of the Wails application
|
||||
package, which matters for the same reason `backend/events` is split by
|
||||
the `indexbuild` tag.
|
||||
|
||||
**And `main()` is now latched to one run per process** (#52). That is
|
||||
the second `os.Exit(1)` in this file's history and it had the same
|
||||
signature as the first, which is the argument for #160: both were named
|
||||
exactly by an `slog` line that went to `/dev/null`.
|
||||
|
||||
### What is still not done
|
||||
|
||||
The shell is still a desktop shell, and the x86_64 half of the APK is
|
||||
@@ -249,10 +258,25 @@ one.
|
||||
`build/android/Taskfile.yml` ships more than the Makefile wraps, and
|
||||
they are the right thing to reach for when you want something one-off:
|
||||
|
||||
> **These four were unsafe until #159 and are now the way in.** All of
|
||||
> them began with `adb uninstall {{.APP_ID}}`, where `APP_ID` defaulted
|
||||
> to `app.yellowjacket` — the **release** id — while `run` and
|
||||
> `run:device` build the **debug** variant, whose id is
|
||||
> `app.yellowjacket.dev`. So they uninstalled the user's app, taking
|
||||
> the library with it, installed a different package, and then failed
|
||||
> to launch the one they had removed.
|
||||
>
|
||||
> They share `scripts/android-deploy.sh` now, which **never**
|
||||
> uninstalls (`install -r`, and a changed signing certificate is
|
||||
> reported with the command rather than acted on), reads the package id
|
||||
> back out of the built APK, and refuses a target that is not the kind
|
||||
> the task names. There is nothing left to avoid; the manual sequence
|
||||
> below is kept because it is still the smallest thing that works.
|
||||
|
||||
```
|
||||
wails3 task android:run # debug build + emulator install + launch
|
||||
wails3 task android:run:device # same, first connected physical device
|
||||
wails3 task android:deploy-device # production APK to a device
|
||||
wails3 task android:run:device # debug build + install + launch on a phone
|
||||
wails3 task android:deploy-device # release build, same
|
||||
wails3 task android:bundle:fat # AAB, for a Play Store upload
|
||||
wails3 task android:studio # open build/android/ in Android Studio
|
||||
wails3 task android:device:list
|
||||
@@ -260,6 +284,16 @@ wails3 task android:logs:all
|
||||
wails3 task android:clean
|
||||
```
|
||||
|
||||
**`run` and `deploy-emulator` mean the emulator, and now say so to
|
||||
adb.** They used a bare `adb install`, which with exactly one device
|
||||
attached picks that device whatever it is — so with a phone plugged in
|
||||
and no emulator running, the task whose summary reads "in the Android
|
||||
Emulator" installed on the phone. They pass `--target emulator` and
|
||||
refuse with `make android-emulator` as the remedy.
|
||||
|
||||
**`DEVICE_ID=<serial>` still names a device, and several attached
|
||||
devices is now an error rather than a silent pick of the first.**
|
||||
|
||||
Two are deliberately **not** wrapped. `android:logs` greps logcat for
|
||||
`(Wails|yellowjacket)`, which catches the `WailsBridge` tag but misses
|
||||
the app's own process tag (`app.yellowjacket` — lowercase, so `Wails`
|
||||
@@ -269,16 +303,51 @@ instead. And `ensure-emulator` boots whatever `-list-avds | tail -1`
|
||||
returns, with no pidfile and no boot wait, so it cannot be stopped or
|
||||
sequenced.
|
||||
|
||||
## The identity is declared twice
|
||||
## The identity is read back from the APK
|
||||
|
||||
It used to be **declared twice**, and that is what #159 was.
|
||||
`applicationId` in `build/android/app/build.gradle` is what Gradle
|
||||
installs. `APP_ID` in `build/android/Taskfile.yml` is what every
|
||||
adb-driven task uninstalls, launches and filters. **Nothing enforces
|
||||
that they agree**, and `ANDROID.md`'s advice to set `APP_ID` in
|
||||
`build/config.yml` does not work in beta.8 — `wails3 task` never reads
|
||||
that file (verified with `--dry`), and even when set it feeds only the
|
||||
adb commands, never Gradle. Change both or the official `run`/`deploy`
|
||||
tasks address a package that is not installed.
|
||||
installs; `APP_ID` in `build/android/Taskfile.yml` was what every
|
||||
adb-driven task uninstalled, launched and filtered, and nothing
|
||||
enforced that they agree. They did not: the debug buildType carries
|
||||
`applicationIdSuffix ".dev"`, so every task that assembles a debug APK
|
||||
addressed the release id. This file flagged the hazard for five phases
|
||||
and it cashed out twice — once as a wrong `am start`, once as an
|
||||
uninstall of the user's library.
|
||||
|
||||
**`scripts/android-pkgid.sh` is the one answer now.** It prints the
|
||||
package id an APK declares (`aapt2 dump packagename`, falling back to
|
||||
`aapt dump badging`), and the deploy path installs and launches *that*.
|
||||
The APK is the authority because the task that installs it has just
|
||||
built it: whatever Gradle resolved the applicationId to, suffixes and
|
||||
flavours included, is in the file, and no default can disagree with it.
|
||||
An APK it cannot read is a hard failure, never a fallback to a written
|
||||
down default — guessing is the bug.
|
||||
|
||||
**`APP_ID` survives as an assertion, not a setting**, and has no
|
||||
default. `wails3 task android:run APP_ID=app.yellowjacket` says "this
|
||||
build had better declare that id" and is refused, naming both, *before*
|
||||
anything is installed or a device is even chosen. It could never have
|
||||
been a setting: `ANDROID.md`'s advice to put it in `build/config.yml`
|
||||
does not work in beta.8 — `wails3 task` never reads that file (verified
|
||||
with `--dry`) — and even when set it fed only the adb commands, never
|
||||
Gradle.
|
||||
|
||||
`scripts/android-emulator.sh` derives `PKG` the same way, from
|
||||
`bin/yellowjacket.apk` when one is built, so `make android-install`,
|
||||
`android-launch`, `android-logs` and `android-smoke` follow whichever
|
||||
variant is actually in `bin/`. `YJ_ANDROID_PKG` still overrides, and
|
||||
the old literal survives only for a tree with no APK built yet.
|
||||
|
||||
**The uninstall is gone and is not coming back.** It existed to make
|
||||
the bare `install` on the next line work at all — without `-r` Android
|
||||
refuses an install over an existing package — so `install -r` removes
|
||||
the *reason* for it rather than merely removing it. What is left is the
|
||||
one case an uninstall really is the remedy, a changed signing
|
||||
certificate, and that is exactly the case where performing it silently
|
||||
costs the user their library. So it is named and not done, which is the
|
||||
answer `scripts/android-emulator.sh` had already reached for
|
||||
`make android-install`.
|
||||
|
||||
Related, and it will bite once: the launcher activity is
|
||||
`com.wails.app.MainActivity` and the applicationId is
|
||||
@@ -287,6 +356,27 @@ resolves the leading dot against the *applicationId* and fails with a
|
||||
class-not-found that reads like a broken build. Always the
|
||||
fully-qualified form.
|
||||
|
||||
**`wails3 task android:run:device` is the way to put a debug build on a
|
||||
real device**, since #159. What #52 used, before it was safe, was the
|
||||
longer form, and it is still the smallest thing that works if you want
|
||||
no script between you and adb:
|
||||
|
||||
```bash
|
||||
wails3 task android:build ARCH=arm64 && wails3 task android:assemble:apk
|
||||
adb install -r bin/yellowjacket.apk # -r, never uninstall
|
||||
adb shell am start -n app.yellowjacket.dev/com.wails.app.MainActivity
|
||||
```
|
||||
|
||||
The id in that last line is the one thing to keep an eye on by hand —
|
||||
`./scripts/android-pkgid.sh bin/yellowjacket.apk` is what the tasks ask,
|
||||
and it is a good habit before any `am start` written out in full.
|
||||
|
||||
`YJ_ANDROID_PKG=app.yellowjacket.dev` still overrides what
|
||||
`scripts/android-emulator.sh` — and therefore `make android-smoke`,
|
||||
`android-logs`, `android-launch` — addresses, but it is rarely needed
|
||||
now: that default is read from `bin/yellowjacket.apk`, so it already
|
||||
follows whichever variant was built last.
|
||||
|
||||
## What only a device can answer
|
||||
|
||||
The emulator cannot run this app (three separate reasons, none of them
|
||||
@@ -311,6 +401,91 @@ system bars, the back gesture, focus and audio interruptions,
|
||||
permission dialogs, the keyboard — not about what the app draws. The
|
||||
drawing is what the other five tiers already cover.
|
||||
|
||||
**The third such fault was the activity lifecycle** (#52), and it is
|
||||
the one to re-check after touching `main()`, `WailsBridge` or
|
||||
`MainActivity`. Android destroys and recreates an activity **without
|
||||
restarting the process**, and Wails' `nativeInit` — which
|
||||
`MainActivity.onCreate` calls — runs `go mainFunc()` every time. So
|
||||
Go's `main()` ran again on a live app, `app.Run()` refused (`a.starting`
|
||||
is still true behind Android's `select{}`), and the `os.Exit(1)` under
|
||||
it took the healthy first app down with it.
|
||||
|
||||
### The lifecycle check, and how to trigger it on demand
|
||||
|
||||
This is the regression guard for #52 on this tier, because no other
|
||||
tier runs `main()` on Android at all. The Go-side guard
|
||||
(`TestMainClaimsBeforeItDoesAnything`) catches work creeping above the
|
||||
latch; only the device catches the latch not working.
|
||||
|
||||
**Trigger a relaunch with a configuration change the manifest does not
|
||||
declare.** `AndroidManifest.xml` lists
|
||||
`orientation|screenSize|keyboardHidden|uiMode`, so those are handled
|
||||
in-place and are *not* triggers. `fontScale` is not listed, and it is a
|
||||
one-liner:
|
||||
|
||||
```bash
|
||||
adb shell settings put system font_scale 1.15 # restore the old value after
|
||||
```
|
||||
|
||||
That is the same in-process destroy/recreate that "Don't keep
|
||||
activities", a locale change and a memory trim produce, but on demand.
|
||||
|
||||
**"Don't keep activities" is the report's own lever and did not work on
|
||||
this device**: `settings put global always_finish_activities 1` reads
|
||||
back as `1`, `am set-always-finish-activities` does not exist on this
|
||||
build, and the activity was never finished on backgrounding. Do not
|
||||
spend an afternoon on it; use the config change.
|
||||
|
||||
**The assertion is the pid, and the tell is two bridge inits in one.**
|
||||
|
||||
```bash
|
||||
adb logcat -d | grep -E "Wails bridge initialized|has died|finishDrawing of relaunch"
|
||||
```
|
||||
|
||||
Healthy is one pid appearing twice — the process surviving the
|
||||
recreation:
|
||||
|
||||
```
|
||||
I/WailsBridge(28420): Wails bridge initialized
|
||||
I/WailsBridge(28420): Wails bridge initialized <- same pid, recreated
|
||||
```
|
||||
|
||||
Broken is that pair followed within a second by:
|
||||
|
||||
```
|
||||
I/WindowManager: finishDrawing of relaunch: Window{...MainActivity} 603ms
|
||||
I/ActivityManager: Process app.yellowjacket.dev (pid 22956) has died: fg TOP
|
||||
W/ActivityTaskManager: Force removing ActivityRecord{...}: app died, no saved state
|
||||
```
|
||||
|
||||
Two things about reading that. **`has died: fg TOP` is not a memory
|
||||
kill** — the system does not reclaim the foreground process, so this is
|
||||
the app leaving of its own accord. And there is **no crash record
|
||||
anywhere**: `logcat -b crash` is empty, no `AndroidRuntime`, no
|
||||
`libc: Fatal signal`, no tombstone. That is the `os.Exit` signature,
|
||||
and it is why "the system killed it" is the wrong first hypothesis.
|
||||
|
||||
**Surviving is only half of it — check the recreated WebView is still
|
||||
wired to the running app.** A plausible-looking fix (making
|
||||
`WailsBridge.initialized` static, so the second `nativeInit` is skipped)
|
||||
keeps the process alive and silently breaks this, because `nativeInit`
|
||||
is also what re-points the JNI reference at the new bridge. Go would go
|
||||
on executing JavaScript against the destroyed activity's WebView: the
|
||||
app opens, renders, and never receives another backend event.
|
||||
|
||||
Ask the page, after a relaunch and a resume:
|
||||
|
||||
```bash
|
||||
make android-inspect
|
||||
make android-eval EXPR='(()=>{window.__probe=[];const o=window._wails.dispatchWailsEvent.bind(window._wails);window._wails.dispatchWailsEvent=(e)=>{window.__probe.push(e&&e.name);return o(e)};return "ok"})()'
|
||||
# background and foreground the app, then:
|
||||
make android-eval EXPR='JSON.stringify(window.__probe)'
|
||||
```
|
||||
|
||||
A healthy build answers with events from the live services —
|
||||
`["IndexStatusChanged","JobsChanged","JobsChanged","android:storageAccess"]`.
|
||||
`[]` means the bridge reference is stale.
|
||||
|
||||
## Asking the device, not just looking at it
|
||||
|
||||
A real phone can be inspected, and that turns this tier from "reported
|
||||
@@ -340,6 +515,75 @@ Four things about it, each of which costs an hour if met cold:
|
||||
script. Plug in over USB for anything longer than a couple of probes.
|
||||
- **The socket name carries the pid**, which changes on every launch, so
|
||||
it is resolved rather than remembered.
|
||||
- **A reinstall resets the runtime permissions**, and the grant dialog
|
||||
is a separate activity that takes focus — so the app is up, `am start`
|
||||
reports "delivered to currently running top-most instance", and
|
||||
`pidof` is empty because it never got to the foreground.
|
||||
`dumpsys window | grep mCurrentFocus` naming
|
||||
`GrantPermissionsActivity` is the tell. `adb shell pm grant
|
||||
app.yellowjacket.dev android.permission.READ_MEDIA_AUDIO` (and
|
||||
`POST_NOTIFICATIONS`) ahead of the launch skips it.
|
||||
|
||||
### Calling a binding on the device
|
||||
|
||||
**The runtime call does not go over HTTP on Android**, and this is worth
|
||||
knowing before an hour is spent on it. The WebView cannot deliver a
|
||||
`fetch()` POST body to `shouldInterceptRequest`, so v3 routes runtime
|
||||
calls through the `addJavascriptInterface` bridge instead: the
|
||||
@wailsio/runtime installs a `customTransport` that calls
|
||||
`window.wails.invokeAsync(id, payload)` and receives the answer on
|
||||
`window._wailsAndroidCallback`. Two consequences:
|
||||
|
||||
- **`.playwright/init-events.js` does not transfer to the device.** Its
|
||||
outbound half hooks `fetch`, which sees nothing here, and its
|
||||
`call()` posts to `/wails/runtime`, which answers
|
||||
`Invalid runtime call: missing object value` — the interceptor got the
|
||||
URL with no body. Its *inbound* half is still right, because
|
||||
`dispatchWailsEvent` is the entry point in every mode.
|
||||
- **Hooking `fetch` from an eval is too late anyway**, on any platform:
|
||||
the bundle captured its reference at module scope, so a wrapper
|
||||
installed afterwards records nothing. That is why the harness is an
|
||||
`initScript` and not a step in a spec.
|
||||
|
||||
What works is to borrow the bridge, chaining the runtime's own callback
|
||||
so its pending calls still resolve:
|
||||
|
||||
```js
|
||||
const pending = new Map();
|
||||
const prev = window._wailsAndroidCallback;
|
||||
window._wailsAndroidCallback = (id, response, error) => {
|
||||
if (!pending.has(id)) return prev && prev(id, response, error);
|
||||
const p = pending.get(id); pending.delete(id);
|
||||
const env = JSON.parse(response || "{}");
|
||||
return env.ok ? p.resolve(env.data ?? env.text) : p.reject(new Error(env.error));
|
||||
};
|
||||
window.__yj = { call(name, args) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const id = "yj" + Math.random().toString(36).slice(2);
|
||||
pending.set(id, { resolve, reject });
|
||||
window.wails.invokeAsync(id, JSON.stringify({
|
||||
object: 0, method: 0, windowName: "",
|
||||
args: { "call-id": id, methodName: "yellowjacket/backend/" + name, args: args || [] },
|
||||
clientId: window._wails.clientId,
|
||||
}));
|
||||
});
|
||||
} };
|
||||
```
|
||||
|
||||
That turns the device into a tier that can be *driven* rather than only
|
||||
looked at — `__yj.call("player.Player.LoadFile", [path])` and
|
||||
`__yj.call("library.Library.AddLibrary", ["/sdcard/Music/..."])` are how
|
||||
#53 was measured. Names are the Go ones (`GetTracks`, not
|
||||
`GetAllTracks`); an unknown one comes back as a plain
|
||||
`unknown bound method name`, so a wrong guess is loud.
|
||||
|
||||
**Getting audio onto the phone**: `adb push` into
|
||||
`/sdcard/Android/data/<pkg>/files/` looks like it works and then the
|
||||
files are not there — scoped storage. `/sdcard/Music/...` plus
|
||||
`pm grant … READ_MEDIA_AUDIO` does work, and `AddLibrary` takes the
|
||||
plain path. The generated fixtures are **~2 seconds** each, which is
|
||||
fine for a scan and useless for watching a seek bar, so synthesise a
|
||||
long one: `ffmpeg -f lavfi -i sine=frequency=440:duration=240`.
|
||||
|
||||
**And the reason to bother: the phone is an engine, not a screen.** The
|
||||
first device here renders in **Chrome 113** at 424x439 CSS px. Every
|
||||
|
||||
@@ -3824,3 +3824,481 @@ Staging it is `/__test/emit` with a `JobsChanged` snapshot; a job with
|
||||
`state: "running"` never completes, so it stays up until an empty
|
||||
snapshot is emitted, which is what makes an idle re-measurement look
|
||||
like the fix not working.
|
||||
|
||||
## Two repaint mechanisms, and neither is pinned alone (measured 2026-08-20)
|
||||
|
||||
`CLAUDE.md` already states the rule — *a virtualized list repaints only
|
||||
when you tell it to, and the accidental way you were telling it may be
|
||||
the thing you are about to delete* — found in `artists-view` and
|
||||
`genres-view`. `queue-panel` is a second instance with numbers, and the
|
||||
numbers are the part worth keeping.
|
||||
|
||||
It repaints its rows **two** ways:
|
||||
|
||||
- `onSelectionChanged()` calls `virtualizer.requestUpdate()`, which is
|
||||
the intended one and the one `track-list` has always had;
|
||||
- `.keyFunction=${(track) => track.id}` is a **per-render arrow**, so it
|
||||
is a changed property on every host update and repaints the rows by
|
||||
itself.
|
||||
|
||||
Removing *either* alone changes nothing observable. That is why #43
|
||||
could not be settled by reading the code: the hypothesis in its Findings
|
||||
(the repaint is missing) was checkable, false, and would have looked
|
||||
identical either way.
|
||||
|
||||
Removing **both** does not break selection either — it delays it. Time
|
||||
from click to `aria-selected`, three clicks each:
|
||||
|
||||
| build | ms to highlight |
|
||||
|---|---|
|
||||
| healthy | 5, 16, 17 |
|
||||
| both mechanisms removed | 134, 3,866, 5,816 |
|
||||
|
||||
The highlight arrives on whatever unrelated render happens next (the
|
||||
player's 1 Hz position report is the usual candidate). **Four seconds is
|
||||
indistinguishable from broken to a user, and invisible to a spec** —
|
||||
`expect.poll`'s default 5 s timeout passes the degraded build on every
|
||||
assertion. `queue-selection.spec.ts` bounds its selection assertions at
|
||||
500 ms for that reason, which is ~30x the healthy case and an order of
|
||||
magnitude under the degraded one.
|
||||
|
||||
The general form, for the next spec about anything push-driven: **a poll
|
||||
generous enough to be stable is generous enough to miss a latency
|
||||
regression entirely.** If "late" is a failure mode worth having, the
|
||||
timeout has to say so.
|
||||
|
||||
## A hit-scan says how much of a row is not selectable (measured 2026-08-20)
|
||||
|
||||
`explore-link` stops the click's propagation on purpose — "the row must
|
||||
not also treat it as a selection" — so a click on a track, album or
|
||||
artist *name* navigates and selects nothing. That is app-wide and
|
||||
deliberate, and the useful question about any given list is how much of
|
||||
its row it costs.
|
||||
|
||||
Asking `elementFromPoint` what is under each x across a row, at three
|
||||
heights:
|
||||
|
||||
| list | link coverage |
|
||||
|---|---|
|
||||
| queue panel | 12% |
|
||||
| track list | 21% |
|
||||
|
||||
This killed a fix in progress. #43 reads as "selection is broken in the
|
||||
queue panel, and fine in the track list", the obvious mechanism is that
|
||||
the queue's narrow rows are mostly name, and it is **wrong**: the panel
|
||||
is *less* link-covered than the list it is being compared against. The
|
||||
scan takes a minute and is worth running before demoting anybody's links
|
||||
— `explore-album-details`'s tracklist (number / title / artist /
|
||||
duration) is the one that plausibly *is* mostly link, and is the one
|
||||
#5 is about to add selection to.
|
||||
|
||||
## A layout is still moving when a guard says it has arrived (measured 2026-08-20)
|
||||
|
||||
`album-dropdown.spec.ts` failed with `Expected 80, Received 10` twice
|
||||
over two sessions, and #133 already strengthened its guard from
|
||||
"scrollable at all" to "has at least the range the assertion needs".
|
||||
That was necessary and could not be sufficient, and the reason is
|
||||
structural rather than a matter of thresholds: **a guard and the write
|
||||
it guards are separate CDP round trips**, so the page is free to
|
||||
re-lay-out between them. Polling harder cannot close a window between
|
||||
two moments; only removing the window can.
|
||||
|
||||
Measured directly, sampling `scrollHeight - clientHeight` on
|
||||
`.grid-scroll-container` every frame across a 1440x900 → 900x600 resize,
|
||||
three runs:
|
||||
|
||||
| t (ms) | range |
|
||||
|---|---|
|
||||
| 0 | 0 |
|
||||
| 1 | **88** |
|
||||
| 8–14 | 330 (settled) |
|
||||
|
||||
88 satisfies a guard asking for 80 and is not the settled value, so the
|
||||
guard can pass while the grid is one layout pass from done. Under
|
||||
full-suite load the transient is worse — the observed failure had 10 —
|
||||
which is why it shows up on the second run of a suite and not in ten
|
||||
consecutive runs of the file alone (0/10 both before and after the fix).
|
||||
|
||||
The shape to write instead: **one page-side call that performs the
|
||||
action and returns what it observes**, with `expect.poll` retrying
|
||||
*that*. `scrollTo()` sets `scrollTop` and returns `scrollTop`, so the
|
||||
assertion is about what the grid did rather than about what it was
|
||||
ready to do. `layout-overflow.spec.ts`'s sidebar probe already had the
|
||||
fused half and was missing the retry; it has both now.
|
||||
|
||||
Worth generalising: a spec that resizes and then measures is asserting
|
||||
about a moving target for the next dozen frames. Fuse, then poll.
|
||||
|
||||
## "The first N tracks" is not a way to ask for an ordinary one (2026-08-20)
|
||||
|
||||
`queue-selection.spec.ts` staged its queue from the first few rows of
|
||||
`library.Library.GetTracks(0)` and clicked a track *name*, which
|
||||
`explore-link` routes to that track's **album** page. Four tracks in the
|
||||
fixture library have no album at all — `01 Tone A`, `02 Tone B`,
|
||||
`Title Only`, `no-tags-at-all` — and a name with nothing to route to
|
||||
renders as **plain text**, not as a link.
|
||||
|
||||
Two things follow, and the second is the sharper one.
|
||||
|
||||
**The order is the scan's.** `GetTracks` returns `audio_files.id` order,
|
||||
i.e. the order the scan inserted rows, which depends on concurrency and
|
||||
directory traversal. Locally the first eight are all from two proper
|
||||
albums, so the spec passed twice over; CI rebuilds its seed with a real
|
||||
scan, got a different eight, and failed on both engines. This is the
|
||||
same family as "a seed freezes every default it has already persisted" —
|
||||
the fixture library is not a list, it is a *set* with an incidental
|
||||
order, and no spec should depend on that order.
|
||||
|
||||
**A loose locator hid it.** The row was located with
|
||||
`.locator('.explore-link').first()`, and a row has two — the title and
|
||||
the artist. When the title is plain text, `first()` silently resolves to
|
||||
the **artist** link, so the click went somewhere real and the assertion
|
||||
was about a destination the test had not exercised. `.track-title
|
||||
.explore-link` is the locator that says which one it means; the loose
|
||||
one turned a fixture problem into a mystery.
|
||||
|
||||
The general rule for this repo's fixture library: it is deliberately
|
||||
full of edge cases (untagged, unicode, duplicates, extremes), so a spec
|
||||
that wants an *ordinary* track has to **say so** — filter on the
|
||||
property it depends on rather than slicing.
|
||||
## A nested rule starting with an element name is dropped on the phone (2026-08-20)
|
||||
|
||||
`CLAUDE.md` records that the device renders in **Chrome 113**, which
|
||||
does not have relaxed CSS nesting (Chrome 120). The consequence is
|
||||
sharper than "some syntax is unavailable": a nested rule whose selector
|
||||
begins with a bare identifier is not a parse error you would notice, it
|
||||
is **silently dropped**.
|
||||
|
||||
Three such rules were live in `frontend/index.css`, all inside
|
||||
`.bottom-bar`, and all therefore dead on the phone and only on the
|
||||
phone:
|
||||
|
||||
```css
|
||||
.bottom-bar {
|
||||
#track-info { p { … } } /* the metadata's ellipsis */
|
||||
now-playing { overflow: hidden; }
|
||||
audio-player { margin: 0.5em 1em; }
|
||||
}
|
||||
```
|
||||
|
||||
The first is the interesting one: it is the *ellipsis* on the bottom
|
||||
bar's track title and artist, so on the device that text has never
|
||||
truncated — the same class of fault as `now-playing`'s marquee, whose
|
||||
`text-overflow` sat on the wrong box and had never produced an ellipsis
|
||||
in any mode. Both are invisible to every assertion and visible in a
|
||||
screenshot.
|
||||
|
||||
`& p`, `& now-playing`, `& audio-player` are valid in both, so the fix
|
||||
is one character per rule. What is worth keeping is the rule of thumb:
|
||||
**inside a nested block, always write `&`** — and note that a rule
|
||||
inside `@media` is *not* nested, so `@media … { bottom-nav { … } }`
|
||||
elsewhere in that file is fine and needs nothing.
|
||||
|
||||
`make css-check` does not catch this (it looks for backticks that end a
|
||||
tagged template early). Filed as an issue: the check is the natural
|
||||
place for it, being the same shape of trap — a silent, phone-only,
|
||||
screenshot-only failure.
|
||||
|
||||
## Centring a bar costs the control in the middle of it (measured 2026-08-20)
|
||||
|
||||
#23 asks for the transport centred in the bottom bar. The obvious
|
||||
implementation — make the outer two grid tracks the same width, so the
|
||||
middle is centred by construction — is right, and the first cut of it
|
||||
was a regression, because "the same width" was taken to mean *the
|
||||
metadata's* width on both sides.
|
||||
|
||||
Measured at 800px, with the seek bar's own track:
|
||||
|
||||
| layout | seek track | transport column |
|
||||
|---|---|---|
|
||||
| `320px 1fr auto` (before) | 257 | 407 |
|
||||
| both sides `--now-playing-width` | **61** | 179 |
|
||||
| both sides `min(--now-playing-width, 25%)` | 246 | 364 |
|
||||
|
||||
At 200% text the middle row is worse still: 130 before, **0** with the
|
||||
uncapped sides. Centring is free at 1440 and expensive at 800, so a
|
||||
change checked only at a comfortable width looks perfect.
|
||||
|
||||
The general form: **a symmetric layout reserves space on the side that
|
||||
does not need it.** The right-hand group here is ~141px (volume plus
|
||||
the queue button) and was being given 320 to keep the arithmetic
|
||||
symmetric. Cap the side tracks against the *bar*, not against their
|
||||
content, and the middle gets the difference.
|
||||
|
||||
The spec that pins this is two assertions, not one, and that split is
|
||||
deliberate: an uncapped build is *perfectly centred* and fails only the
|
||||
seek-bar width, so a spec asserting centring alone would have passed
|
||||
the regression.
|
||||
|
||||
## The phone's way into Now Playing was under the artwork (measured 2026-08-20)
|
||||
|
||||
`phone-shell.spec.ts`'s "opens the full-screen now playing" failed in CI
|
||||
on both engines, three times across two branches that could not have
|
||||
caused it, and passed on re-run each time. It was filed as a flake
|
||||
(#150). It is not one: **it depends on which track is playing.**
|
||||
|
||||
`.expand` — the phone's only route into `<now-playing-view>` — is
|
||||
`position: absolute; inset: 0` inside `.cover-art-wrapper`, and
|
||||
`.cover-art` is a **later sibling**. Both have `z-index: auto`, so they
|
||||
tie on paint order and the later one wins. With an `<img>` that costs
|
||||
nothing; with no artwork the placeholder `wa-icon` renders and takes
|
||||
every click aimed at the button underneath it.
|
||||
|
||||
Measured at 390px with `elementFromPoint` at the button's centre:
|
||||
|
||||
| playing track | hit test |
|
||||
|---|---|
|
||||
| has artwork | `button.expand` |
|
||||
| no artwork | **`wa-icon`** |
|
||||
| no artwork, with `z-index: 1` | `button.expand` |
|
||||
|
||||
So on a phone, the only way into the full-screen player stopped working
|
||||
whenever the current song had no cover — and this has nothing to do with
|
||||
the fixture: any library has untagged files.
|
||||
|
||||
Three things worth keeping.
|
||||
|
||||
**"Flaky in CI" was the wrong diagnosis and it cost three cycles.** The
|
||||
spec starts the *first* row of the track list, so which track it plays
|
||||
is the order the scan inserted rows in — the same root cause as #156,
|
||||
one spec over. A test whose subject is a hit test has to *choose* the
|
||||
case that breaks it.
|
||||
|
||||
**The first two hypotheses were both wrong, and both were plausible.**
|
||||
A custom element's upgrade replacing its own contents, and the cover
|
||||
preview's `mouseenter` opening a popup under the pointer. Neither
|
||||
survived contact with `elementFromPoint`, which took a minute and would
|
||||
have saved the other two cycles.
|
||||
|
||||
**And the spec that pins it needs the 90-second track**, because a
|
||||
2-second one finishes before the assertions run — the trap
|
||||
`fixtures.ts` already documents. Note the filter that does *not* work:
|
||||
`library.Track.CoverArt` is empty for all 31 fixture rows, so "the
|
||||
first track with no cover art" selects nothing in particular. The
|
||||
placeholder's presence is asserted instead, which is the property the
|
||||
test actually depends on.
|
||||
|
||||
## An activity recreation kills the process, deterministically (measured 2026-08-20)
|
||||
|
||||
#52's report was "sometimes crashes or restarts when reopened after
|
||||
running in the background". Measured on a real device, the *fault* is
|
||||
not intermittent at all — only its trigger is.
|
||||
|
||||
Device: Light Phone III (TLP301), Android 14 / SDK 34, arm64-v8a,
|
||||
WebView **Chrome 113** at 424x439 CSS px. Debug build
|
||||
(`app.yellowjacket.dev`), installed beside the released `v0.3.1` with
|
||||
`install -r`.
|
||||
|
||||
**Conditional on the activity actually being recreated in a live
|
||||
process, the process died 8 times out of 8** — 3 by hand, then 5/5 in a
|
||||
scripted loop. The runs where it survived were runs where no recreation
|
||||
happened (one `Wails bridge initialized` in the log rather than two), so
|
||||
they are inconclusive rather than passes; a harness that does not check
|
||||
for the second init reports those as green and reads as flakiness.
|
||||
After the fix: 5/5 recreations survived, plus 6 background/foreground
|
||||
cycles and 3 interleaved recreations on one pid.
|
||||
|
||||
The mechanism is three log lines:
|
||||
|
||||
```
|
||||
12:47:56.159 I/WailsBridge(22956): Wails bridge initialized
|
||||
12:48:38.898 I/WailsBridge(22956): Wails bridge initialized <- same pid
|
||||
12:48:39.357 I/ActivityManager: Process app.yellowjacket.dev (pid 22956) has died: fg TOP
|
||||
```
|
||||
|
||||
`nativeInit` runs `go mainFunc()` on every activity creation; the second
|
||||
`main()` reaches `app.Run()`, which refuses because `a.starting` is
|
||||
still true behind Android's `select{}`, and `os.Exit(1)` takes the whole
|
||||
process — including the healthy first app — with it.
|
||||
|
||||
Four things worth keeping:
|
||||
|
||||
- **`has died: fg TOP` is not a memory kill.** The system does not
|
||||
reclaim the foreground process. This reads as "the OS killed us",
|
||||
which is the wrong hypothesis and the reason the issue sat unverified.
|
||||
- **There is no crash record of any kind**: `logcat -b crash` empty, no
|
||||
`AndroidRuntime`, no `libc: Fatal signal`, no tombstone. `os.Exit` is
|
||||
not a crash. The one line that named the fault —
|
||||
`slog.Error("application error", "err", ...)`, carrying
|
||||
`"application is running or a previous run has failed"` — went to
|
||||
`/dev/null`. That is #160.
|
||||
- **"Don't keep activities" does not work on this device.**
|
||||
`settings put global always_finish_activities 1` reads back as `1`,
|
||||
`am set-always-finish-activities` does not exist on this build, and
|
||||
the activity was never finished on backgrounding. The report's own
|
||||
suggested lever is a dead end here. What *does* work, deterministically
|
||||
and in one line, is a configuration change the manifest does not
|
||||
declare: `adb shell settings put system font_scale 1.15`
|
||||
(`AndroidManifest.xml` declares `orientation|screenSize|
|
||||
keyboardHidden|uiMode`, so none of those are triggers).
|
||||
- **Surviving is only half the property.** The recreated WebView has to
|
||||
still be wired to the running app, which was verified by hooking
|
||||
`window._wails.dispatchWailsEvent` and backgrounding/foregrounding:
|
||||
`["IndexStatusChanged","JobsChanged","JobsChanged",
|
||||
"android:storageAccess"]`. The tempting Java-side fix — making
|
||||
`WailsBridge.initialized` static — passes the pid check and fails
|
||||
this one, because `nativeInit` is also what re-points the JNI
|
||||
reference at the new bridge.
|
||||
|
||||
## The Taskfile's device tasks uninstall the released app (2026-08-20)
|
||||
|
||||
`android:run:device` builds the **debug** variant
|
||||
(`applicationIdSuffix ".dev"`) and then runs
|
||||
`adb uninstall {{.APP_ID}}`, where `APP_ID` defaults to
|
||||
`app.yellowjacket` — the **release** id. So it deletes the user's
|
||||
installed app and its library, installs a different package, and then
|
||||
fails to launch the one it removed. `deploy-device` carries the same
|
||||
uninstall. Filed as #159; `android-tier.md` had been recommending
|
||||
`run:device` as the way onto a device.
|
||||
|
||||
This is the hazard that file already names — "The identity is declared
|
||||
twice ... **Nothing enforces that they agree**" — reached by a second
|
||||
route: the two ids differ not because someone edited one, but because
|
||||
the debug buildType suffixes it.
|
||||
|
||||
## The uninstall was there to make a bare `install` work (measured 2026-08-20)
|
||||
|
||||
Fixing #159 turned up *why* the `adb uninstall` was in all four tasks,
|
||||
which the issue does not say and which decides whether it can simply be
|
||||
deleted. The line under it was `adb install`, with **no `-r`** — and
|
||||
Android refuses an install over an existing package without it. So the
|
||||
uninstall was not a deliberate clean-slate step; it was the price of
|
||||
the missing flag, paid on every run, and `install -r` removes the
|
||||
reason for it rather than merely removing it.
|
||||
|
||||
That matters because "should the uninstall go at all" looked like a
|
||||
trade — drop it and a signing-certificate change fails with
|
||||
`INSTALL_FAILED_UPDATE_INCOMPATIBLE` instead of being handled. It is
|
||||
not a trade: nothing else was relying on it. The certificate case is
|
||||
reported with the command to run, which is what
|
||||
`scripts/android-emulator.sh` already did for `make android-install`,
|
||||
so this is one existing judgement applied consistently rather than a
|
||||
new one.
|
||||
|
||||
## `wails3 task android:run` installs on a phone (measured 2026-08-20)
|
||||
|
||||
The emulator tasks (`run`, `deploy-emulator`) used a bare `adb install`
|
||||
with no `-s`. adb with exactly one device attached uses that device
|
||||
whatever kind it is, so with a phone plugged in and no emulator
|
||||
running, the task whose summary reads "in the Android Emulator"
|
||||
installed on the phone — and, before #159 was fixed, ran
|
||||
`adb uninstall app.yellowjacket` against it first. The reported data
|
||||
loss was reachable from the *emulator* task, which is not what the
|
||||
issue describes and is worse, because nothing in the name warns you.
|
||||
|
||||
Measured after the fix, phone attached and emulator stopped:
|
||||
|
||||
```
|
||||
$ ./scripts/android-deploy.sh --apk bin/yellowjacket.apk --target emulator
|
||||
android-deploy: no emulator target is online.
|
||||
LP3LHMA531900746 device
|
||||
Start one with: make android-emulator
|
||||
```
|
||||
|
||||
The general form: **a task that names a target has to say so to adb.**
|
||||
The device tasks always filtered on `$1 !~ /^emulator-/`; the emulator
|
||||
tasks filtered on nothing at all.
|
||||
|
||||
## The package id can be read back, and costs nothing (2026-08-20)
|
||||
|
||||
`aapt2 dump packagename <apk>` answers in one word and ~40 ms, from
|
||||
`$ANDROID_HOME/build-tools/*/aapt2` (versioned, so resolved not
|
||||
pinned); `aapt dump badging` is the fallback for older build-tools and
|
||||
is what #159's own measurement used. That is cheap enough to do on
|
||||
every deploy, which is what makes "the two ids agree by construction"
|
||||
affordable rather than aspirational — the alternative considered was
|
||||
giving the debug-flavoured tasks `APP_ID` + `.dev`, which is one line
|
||||
and leaves the class of bug alive for the next flavour or suffix.
|
||||
|
||||
The guard runs **before** a target is chosen, deliberately: it is a
|
||||
question about the artifact, so it can be exercised with nothing
|
||||
plugged in, and a build whose id is wrong should be refused whether or
|
||||
not there is anything to install it onto. That is what let the negative
|
||||
test run safely with the user's phone attached:
|
||||
|
||||
```
|
||||
$ ./scripts/android-deploy.sh --apk bin/yellowjacket.apk \
|
||||
--target device --expect app.yellowjacket
|
||||
android-pkgid: refusing to act on a package this APK does not declare.
|
||||
the APK declares: app.yellowjacket.dev
|
||||
the task expects: app.yellowjacket
|
||||
rc=2
|
||||
```
|
||||
|
||||
That is exactly #159's configuration — debug APK, release id, real
|
||||
device — refused with no adb call made.
|
||||
|
||||
## `make android-emulator`'s boot wait can be satisfied by a phone (2026-08-20)
|
||||
|
||||
Noticed while booting the emulator for #159's verification, with a
|
||||
phone also attached. `scripts/android-emulator.sh start` reported
|
||||
`waiting for boot ok / android 14` about **eight seconds** after
|
||||
launching the emulator, which had not appeared in `adb devices` yet —
|
||||
`pick_device`'s last resort is "exactly one device online", and at that
|
||||
moment the one online device was the phone. So it waited for the
|
||||
phone's boot, found it long since booted, and returned. The emulator
|
||||
took another ~10 s to come up.
|
||||
|
||||
Harmless here (the emulator was up before anything used it) and a
|
||||
straightforward race otherwise: `start` should wait for a device that
|
||||
is an emulator, not for whatever `pick_device` returns. Filed as #162.
|
||||
|
||||
## The Android runtime transport is not HTTP (measured 2026-08-20)
|
||||
|
||||
Found while trying to drive the phone for #53. `wails3` routes runtime
|
||||
calls through `addJavascriptInterface` on Android, not through
|
||||
`/wails/runtime` — the WebView cannot deliver a `fetch()` POST body to
|
||||
`shouldInterceptRequest`, which the v3 source says in as many words
|
||||
(`application_android.go`, "The Android transport"). The runtime
|
||||
installs a `customTransport` over `window.wails.invokeAsync(id,
|
||||
payload)` and takes the answer on `window._wailsAndroidCallback`.
|
||||
|
||||
Two things follow, and both cost time before the source was read:
|
||||
|
||||
- **`.playwright/init-events.js` does not transfer to the device.** Its
|
||||
outbound half hooks `fetch`; a POST to `/wails/runtime` answers
|
||||
`Invalid runtime call: missing object value`, which reads like a
|
||||
wrong payload shape and is actually the interceptor receiving a URL
|
||||
with no body at all. The payload shape was right the whole time. Its
|
||||
*inbound* half is still correct, because `dispatchWailsEvent` is the
|
||||
entry point in every mode.
|
||||
- **Hooking `fetch` from an `eval` is too late on any platform.** The
|
||||
bundle captured its reference at module scope, so a wrapper installed
|
||||
afterwards records nothing — which is exactly why the harness is an
|
||||
`initScript`. Measured: zero calls captured while the app was
|
||||
demonstrably making them.
|
||||
|
||||
The working recipe is in `android-tier.md`; it chains the runtime's own
|
||||
callback rather than replacing it, so its pending calls still resolve.
|
||||
This is what makes the device a tier that can be *driven*.
|
||||
|
||||
## #53's frontend is byte-identical to the build it was reported against (2026-08-20)
|
||||
|
||||
`git diff v0.3.1 HEAD -- frontend/src/components/audio-player/seekbar/
|
||||
frontend/src/store/player-store.ts` is **empty**; the whole diff in that
|
||||
area is `backend/player/`. The phone carries the released `v0.3.1`, so
|
||||
whatever #53 saw, the component was not what changed — and v0.4.0 is
|
||||
where the player audit (#122–#127) landed.
|
||||
|
||||
Measured on that phone, current `main`, with a synthesised 4-minute
|
||||
track: the Now Playing seek bar tracks correctly when mounted
|
||||
mid-playback (`seekValue` 28 of 240), when the view is opened before
|
||||
playback starts, after a tap on the track, and across an activity
|
||||
recreation (same pid, bar resumes at 30 → 35). The issue's stated
|
||||
symptom did not reproduce in any of them.
|
||||
|
||||
Reverting **only** `backend/player/` to v0.3.1 — the frontend and
|
||||
everything else at HEAD — does reproduce a real position defect on the
|
||||
same device: six seconds into a 20-second file with no database row,
|
||||
played after a 240-second one, the bar read **01:27 of 240**. That is
|
||||
#125's stale `trackLengthMs` ("cleared only by UnloadTrack, so a file
|
||||
with no row inherited the previous track's duration"), and it is fixed
|
||||
at HEAD. Note the *shape* of it: the fraction is roughly right and the
|
||||
absolute numbers are wrong, so it presents as a clock that lies rather
|
||||
than as a handle that will not move.
|
||||
|
||||
The one-line experiment is worth remembering: v0.3.1's `backend/player`
|
||||
compiles against HEAD with a single shim
|
||||
(`SetPlaybackFinishedHandler` gained a `srcErr error` parameter), which
|
||||
makes "did the backend fix cause this" a ten-minute question instead of
|
||||
a full checkout.
|
||||
|
||||
@@ -317,6 +317,60 @@ stacking dialogs. Window state moved off that path entirely, onto a
|
||||
window still exists and `OnShutdown` has neither a context nor a
|
||||
window.
|
||||
|
||||
**An activity is a view onto the process, and `main()` runs once per
|
||||
process.** On Android the Wails entry point is `nativeInit`, which
|
||||
`MainActivity.onCreate` calls — and it does two things: it re-points the
|
||||
native library's global JNI reference at the calling `WailsBridge`, and
|
||||
it runs `go mainFunc()`. Android destroys and recreates an activity
|
||||
**without restarting the process** (a configuration change the manifest
|
||||
does not declare, memory pressure, or every background under "Don't keep
|
||||
activities"), so `main()` ran again on a live app. `application.New`
|
||||
returns the *existing* app rather than building a second one,
|
||||
`app.Run()` then refuses — `a.starting` is still true, because Android's
|
||||
`platformRun` is `select{}` and never returns — and the `os.Exit(1)`
|
||||
under that error took the **first**, healthy app down with it: its
|
||||
database, its queue, and the audio a `mediaPlayback` foreground service
|
||||
was holding the process alive to play. `mainStarted` latches it, first
|
||||
statement in `main()`.
|
||||
|
||||
Four things about it are load-bearing.
|
||||
|
||||
**The answer to "restore the session or cold-start" is settled by
|
||||
playback, not by preference.** The audio lives in the Go process, so a
|
||||
cold start on every activity recreation would stop the music mid-song —
|
||||
which is the exact thing the foreground service exists to prevent. The
|
||||
activity is a view; the app is the process. The frontend already
|
||||
cooperates, because a recreated WebView loads the page fresh and fetches
|
||||
its state from a backend that never went away.
|
||||
|
||||
**Returning early is not a degraded mode, and that is why the latch is
|
||||
in Go rather than in Java.** The obvious fix — making
|
||||
`WailsBridge.initialized` `static`, so the second `nativeInit` is
|
||||
skipped — keeps the process alive and silently breaks the app, because
|
||||
skipping `nativeInit` skips the reference re-point too: Go would keep
|
||||
executing JavaScript against the *destroyed* activity's WebView, and the
|
||||
app would open, render, and never receive another backend event. The
|
||||
latch lets `nativeInit` do its first job and declines only its second.
|
||||
|
||||
**`ServiceShutdown` has never run on Android**, and nothing should be
|
||||
built on the assumption that it will. `App.Quit()` reaches an
|
||||
`androidApp.destroy()` that is an empty method, and `Run()`'s deferred
|
||||
`shutdownServices()` cannot fire behind `select{}`. Durability on this
|
||||
platform is the persist writers, which submit on every mutation rather
|
||||
than at exit — which is also why `MainActivity.onDestroy` no longer
|
||||
calls `bridge.shutdown()`: the activity going away is not the app
|
||||
shutting down, and there is no callback for the process going away
|
||||
because Android simply kills it.
|
||||
|
||||
**No tier here can see any of this**, so the guard is split. A source
|
||||
sweep (`TestMainClaimsBeforeItDoesAnything`) asserts the latch is the
|
||||
*first* statement of `main()` — the failure it exists for is not
|
||||
deletion, which is loud, but a line creeping in above it, since a second
|
||||
`NewYellowJacketApp` opens the SQLite database again on every
|
||||
recreation. The rest is a documented device check in
|
||||
`.pi/skills/yellowjacket-dev/references/android-tier.md`, with the
|
||||
logcat signature and a one-line way to force a recreation.
|
||||
|
||||
`internalServiceMethods` auto-excludes `ServiceStartup`,
|
||||
`ServiceShutdown`, `ServiceName` and `ServeHTTP` from bindings, so this
|
||||
shape **removed** 12 spurious bindings and the bogus `context` model
|
||||
@@ -1572,6 +1626,50 @@ And **the bar does not resize when a job starts**, which is the case the
|
||||
whole thing is for — a ResizeObserver on the header alone never fires,
|
||||
so every element child is observed too.
|
||||
|
||||
**The bottom bar is three columns whose outer two are the same width,
|
||||
and that is what "centred" means.** It was `320px 1fr auto`, so the
|
||||
transport sat in the middle of the space the metadata and the queue
|
||||
button did not use — its centre was ~140px right of the window's at
|
||||
every size (#23). The outer tracks are now the same expression, so the
|
||||
middle one is centred by construction rather than by arithmetic that
|
||||
has to be redone whenever a control joins the bar.
|
||||
|
||||
Four things about it are load-bearing.
|
||||
|
||||
**The side width is the metadata's, capped at a quarter of the bar**,
|
||||
and the cap is not tidiness — it was measured as a regression first.
|
||||
Reserving the full `--now-playing-width` on *both* sides costs the
|
||||
transport twice: at 800px the outer pair wanted 640 of 800 and the seek
|
||||
bar's track fell from **257px to 61px**, and to 0 at 200% text. The
|
||||
control you drag was being squeezed to centre the buttons above it.
|
||||
With the cap it is 246px at 800, which is parity with the uncentred
|
||||
layout.
|
||||
|
||||
**The cap is a `min()` rather than a breakpoint** because
|
||||
`--now-playing-width` is *user state* — the metadata panel has a drag
|
||||
handle — and the same reasoning the queue panel's overlay mode uses
|
||||
applies: a rule that assumed the default 320 would be wrong by whatever
|
||||
the user dragged. Tying both sides to that variable is also what keeps
|
||||
the handle meaningful; a plain `1fr … 1fr` would centre the transport
|
||||
just as well and silently make dragging a no-op.
|
||||
|
||||
**The volume moved out of `audio-player` and into the bar** (#42),
|
||||
because the transport column has to hold the transport and nothing
|
||||
else or "centred" means centred with a slider bolted to one side. It
|
||||
lives in `.bar-end` with the queue button — one cell, not two columns,
|
||||
since the centring compares *columns* and a separate volume track would
|
||||
make the outer pair unequal by whatever the slider measures.
|
||||
|
||||
And **the slider is inline by default, with the popup as a setting**
|
||||
whose stored flag names the *popup*: `backend/config`'s polarity rule,
|
||||
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.
|
||||
|
||||
**900 is the worst desktop width, not the 800×600 minimum.** The
|
||||
sidebar collapses to icons *below* 900, so the main panel is 843px at
|
||||
899 and 700px at 900 — the narrowest content area any desktop width
|
||||
|
||||
@@ -666,6 +666,49 @@ func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetPopupVolume reports whether the bottom bar's volume control is a
|
||||
// click-to-open popup rather than an inline slider (#42).
|
||||
func (c *Config) GetPopupVolume() bool {
|
||||
if c.General == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
return c.General.PopupVolume
|
||||
}
|
||||
|
||||
// SetPopupVolume saves the volume control's presentation.
|
||||
//
|
||||
// Nothing to validate: both values are legal at every width, and the
|
||||
// frontend additionally stands the inline slider down below the phone
|
||||
// breakpoint whatever this says, because that is about room rather than
|
||||
// about preference.
|
||||
func (c *Config) SetPopupVolume(popup bool) error {
|
||||
if c.General == nil {
|
||||
c.General = &GeneralConfig{}
|
||||
c.General.ApplyDefaults()
|
||||
}
|
||||
|
||||
c.General.PopupVolume = popup
|
||||
|
||||
if err := c.Save(); err != nil {
|
||||
return fmt.Errorf(
|
||||
"could not save config: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
events.Emit(
|
||||
c.ctx,
|
||||
events.GeneralConfigChanged,
|
||||
map[string]any{
|
||||
"PopupVolume": popup,
|
||||
},
|
||||
)
|
||||
|
||||
c.logger.Info("volume control presentation updated", "popup", popup)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetViewVisibility reports which primary views the sidebar should
|
||||
// show, answered for every known view rather than only the ones the
|
||||
// config mentions -- so the frontend filters on a value and never has
|
||||
|
||||
@@ -188,3 +188,43 @@ func TestEmit_FavoritesChangeCarriesFullConfig(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestEmit_PopupVolumeRoundTripsAndDefaultsToInline pins both halves of
|
||||
// #42's storage decision.
|
||||
//
|
||||
// The **default** is the load-bearing one: inline is what a fresh
|
||||
// install and an existing `config.toml` with no such key must both
|
||||
// produce, which is why the field names the popup rather than the
|
||||
// inline slider. A flag spelled the other way round would default to
|
||||
// false, hand every existing install the popup this issue exists to
|
||||
// stop being the only option, and need a migration to say otherwise.
|
||||
func TestEmit_PopupVolumeRoundTripsAndDefaultsToInline(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
conf, rec := setupRecordedConfig(t)
|
||||
|
||||
if conf.GetPopupVolume() {
|
||||
t.Error("a config with no PopupVolume key wants the popup, want inline")
|
||||
}
|
||||
|
||||
if err := conf.SetPopupVolume(true); err != nil {
|
||||
t.Fatalf("SetPopupVolume: %v", err)
|
||||
}
|
||||
|
||||
if !conf.GetPopupVolume() {
|
||||
t.Error("GetPopupVolume = false after setting it true")
|
||||
}
|
||||
|
||||
data := payloadMap(t, rec, events.GeneralConfigChanged)
|
||||
if data["PopupVolume"] != true {
|
||||
t.Errorf("PopupVolume = %v, want true", data["PopupVolume"])
|
||||
}
|
||||
|
||||
if err := conf.SetPopupVolume(false); err != nil {
|
||||
t.Fatalf("SetPopupVolume(false): %v", err)
|
||||
}
|
||||
|
||||
if conf.GetPopupVolume() {
|
||||
t.Error("GetPopupVolume = true after setting it false")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,6 +54,16 @@ type GeneralConfig struct {
|
||||
// so an existing config with no such key refuses by default rather
|
||||
// than needing a migration to become careful.
|
||||
AllowMeteredCatalogDownload bool `toml:"AllowMeteredCatalogDownload"`
|
||||
// PopupVolume draws the bottom bar's volume as a click-to-open popup
|
||||
// instead of a slider that is always there (#42).
|
||||
//
|
||||
// The polarity is the rule this file already states twice: **the
|
||||
// zero value is the intended answer**. Inline is the new default, so
|
||||
// the flag has to name the *other* choice — an `InlineVolume bool`
|
||||
// would default to false and give every existing install the popup
|
||||
// this issue exists to stop being the only option, and would need a
|
||||
// migration to say otherwise.
|
||||
PopupVolume bool `toml:"PopupVolume"`
|
||||
}
|
||||
|
||||
// ApplyDefaults fills zero-value fields with sensible defaults.
|
||||
|
||||
+25
-55
@@ -4,18 +4,28 @@ includes:
|
||||
common: ../Taskfile.yml
|
||||
|
||||
vars:
|
||||
# The *installed* package name, which every adb-driven task below uses
|
||||
# to uninstall, launch and filter. It must agree with `applicationId`
|
||||
# in app/build.gradle, and nothing enforces that.
|
||||
# APP_ID is an *assertion*, not a setting, and it has no default.
|
||||
#
|
||||
# ANDROID.md says to set this in build/config.yml. That does not work
|
||||
# in beta.8, checked both ways: `wails3 task` builds its var set from
|
||||
# CLI KEY=VALUE arguments and the Taskfile tree only -- nothing reads
|
||||
# config.yml -- and even when set it feeds only these adb commands,
|
||||
# never Gradle. So the identity is declared twice, here and in
|
||||
# build.gradle, and a change to one alone means the official run and
|
||||
# deploy tasks address a package that is not installed.
|
||||
APP_ID: '{{.APP_ID | default "app.yellowjacket"}}'
|
||||
# It used to be the id every adb-driven task below uninstalled,
|
||||
# launched and filtered, defaulting to "app.yellowjacket". It could
|
||||
# never have been a setting: `wails3 task` builds its var set from CLI
|
||||
# KEY=VALUE arguments and the Taskfile tree only -- nothing reads
|
||||
# build/config.yml, contrary to ANDROID.md, checked with --dry -- and
|
||||
# even when set it fed only the adb commands, never Gradle. So the
|
||||
# identity was declared twice, here and as `applicationId` in
|
||||
# app/build.gradle, with nothing enforcing that they agree.
|
||||
#
|
||||
# They did not agree. The debug buildType carries
|
||||
# `applicationIdSuffix ".dev"`, so the tasks that assemble a debug APK
|
||||
# addressed the *release* id -- on a device, the user's installed app
|
||||
# and their library (#159).
|
||||
#
|
||||
# The id is now read back from the built APK by scripts/android-
|
||||
# pkgid.sh, so the thing installed and the thing launched agree by
|
||||
# construction. Passing APP_ID= says "this build had better declare
|
||||
# that id", and the deploy refuses before touching anything if it does
|
||||
# not -- which is the check that would have caught #159 statically.
|
||||
APP_ID: '{{.APP_ID | default ""}}'
|
||||
MIN_SDK: '21'
|
||||
TARGET_SDK: '35'
|
||||
# The emulator runs the host architecture; physical devices are arm64
|
||||
@@ -372,9 +382,7 @@ tasks:
|
||||
ARCH: '{{.ARCH | default .HOST_ARCH}}'
|
||||
cmds:
|
||||
- task: ensure-emulator
|
||||
- '"{{.ADB}}" uninstall {{.APP_ID}} 2>/dev/null || true'
|
||||
- '"{{.ADB}}" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"'
|
||||
- '"{{.ADB}}" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity'
|
||||
- './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target emulator{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
|
||||
|
||||
run:
|
||||
summary: Build, install and launch a debug build in the Android Emulator
|
||||
@@ -383,9 +391,7 @@ tasks:
|
||||
- task: build
|
||||
cmds:
|
||||
- task: assemble:apk
|
||||
- '"{{.ADB}}" uninstall {{.APP_ID}} 2>/dev/null || true'
|
||||
- '"{{.ADB}}" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"'
|
||||
- '"{{.ADB}}" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity'
|
||||
- './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target emulator{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
|
||||
|
||||
device:list:
|
||||
summary: Lists connected Android devices and emulators (serials)
|
||||
@@ -400,25 +406,7 @@ tasks:
|
||||
ARCH: arm64
|
||||
cmds:
|
||||
- task: assemble:apk
|
||||
- |
|
||||
DEVICE='{{.DEVICE_ID | default ""}}'
|
||||
if [ -z "$DEVICE" ]; then
|
||||
DEVICE="${DEVICE_ID:-}"
|
||||
fi
|
||||
if [ -z "$DEVICE" ]; then
|
||||
DEVICE=$("{{.ADB}}" devices | awk 'NR > 1 && $2 == "device" && $1 !~ /^emulator-/ { print $1; exit }')
|
||||
fi
|
||||
if [ -z "$DEVICE" ]; then
|
||||
echo "Error: no connected physical Android device found."
|
||||
echo "Pass DEVICE_ID=<serial> to target a device explicitly."
|
||||
echo "Find connected device serials with: {{.ADB}} devices"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "Deploying {{.BIN_DIR}}/{{.APP_NAME}}.apk to device $DEVICE..."
|
||||
"{{.ADB}}" -s "$DEVICE" uninstall {{.APP_ID}} 2>/dev/null || true
|
||||
"{{.ADB}}" -s "$DEVICE" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"
|
||||
"{{.ADB}}" -s "$DEVICE" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity
|
||||
- './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target device{{if .DEVICE_ID}} --serial "{{.DEVICE_ID}}"{{end}}{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
|
||||
preconditions:
|
||||
- sh: '[ -x "{{.ADB}}" ] || command -v adb'
|
||||
msg: "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)"
|
||||
@@ -430,25 +418,7 @@ tasks:
|
||||
vars:
|
||||
ARCH: arm64
|
||||
cmds:
|
||||
- |
|
||||
DEVICE='{{.DEVICE_ID | default ""}}'
|
||||
if [ -z "$DEVICE" ]; then
|
||||
DEVICE="${DEVICE_ID:-}"
|
||||
fi
|
||||
if [ -z "$DEVICE" ]; then
|
||||
DEVICE=$("{{.ADB}}" devices | awk 'NR > 1 && $2 == "device" && $1 !~ /^emulator-/ { print $1; exit }')
|
||||
fi
|
||||
if [ -z "$DEVICE" ]; then
|
||||
echo "Error: no connected physical Android device found."
|
||||
echo "Pass DEVICE_ID=<serial> to target a device explicitly."
|
||||
echo "Find connected device serials with: {{.ADB}} devices"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "Deploying {{.BIN_DIR}}/{{.APP_NAME}}.apk to device $DEVICE..."
|
||||
"{{.ADB}}" -s "$DEVICE" uninstall {{.APP_ID}} 2>/dev/null || true
|
||||
"{{.ADB}}" -s "$DEVICE" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"
|
||||
"{{.ADB}}" -s "$DEVICE" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity
|
||||
- './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target device{{if .DEVICE_ID}} --serial "{{.DEVICE_ID}}"{{end}}{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
|
||||
preconditions:
|
||||
- sh: '[ -x "{{.ADB}}" ] || command -v adb'
|
||||
msg: "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)"
|
||||
|
||||
@@ -891,13 +891,41 @@ public class MainActivity extends AppCompatActivity {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The activity going away is not the app shutting down.
|
||||
*
|
||||
* <p>The scaffold called {@code bridge.shutdown()} here, which is
|
||||
* the natural reading of onDestroy and is wrong for this app twice
|
||||
* over. Android destroys and recreates an activity for a
|
||||
* configuration change the manifest does not declare, under memory
|
||||
* pressure, and on every background if the user has "Don't keep
|
||||
* activities" on -- all **without restarting the process**. And
|
||||
* when the user really does leave, this app's reason for existing
|
||||
* in the background is that a song is playing, which is what the
|
||||
* {@code mediaPlayback} foreground service is holding the process
|
||||
* alive for. Either way, tearing the Go side down here would stop
|
||||
* the music.
|
||||
*
|
||||
* <p>It was harmless only by accident: {@code nativeShutdown} calls
|
||||
* {@code App.Quit()}, whose Android {@code destroy()} is an empty
|
||||
* method, and {@code Run()}'s deferred {@code shutdownServices()}
|
||||
* can never fire because Android's {@code platformRun} is
|
||||
* {@code select{}} and does not return. So no {@code
|
||||
* ServiceShutdown} has ever run on Android, and removing this call
|
||||
* changes nothing today -- it stops the day someone implements
|
||||
* {@code destroy()} from silently killing playback on a rotation.
|
||||
*
|
||||
* <p>There is no callback for "the process is going away"; Android
|
||||
* simply kills it. Durability on this platform is the persist
|
||||
* writers, which submit on every mutation rather than at exit.
|
||||
*
|
||||
* <p>See #52, and CLAUDE.md, "An activity is a view onto the
|
||||
* process".
|
||||
*/
|
||||
@Override
|
||||
protected void onDestroy() {
|
||||
super.onDestroy();
|
||||
unregisterSystemEventReceivers();
|
||||
if (bridge != null) {
|
||||
bridge.shutdown();
|
||||
}
|
||||
if (webView != null) {
|
||||
webView.destroy();
|
||||
}
|
||||
|
||||
@@ -129,7 +129,24 @@ public class WailsBridge {
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the native Go library
|
||||
* Initialize the native Go library.
|
||||
*
|
||||
* <p><b>{@code initialized} is deliberately per-instance, and making
|
||||
* it {@code static} is the trap this comment exists for.</b> A
|
||||
* recreated activity builds a new bridge and calls this again, in a
|
||||
* process where the native library is already loaded and Go's
|
||||
* {@code main()} is already running -- so "initialise once per
|
||||
* process" looks like exactly the right rule. It is not, because
|
||||
* {@code nativeInit} does <i>two</i> things: it runs
|
||||
* {@code go mainFunc()}, and it stores the global JNI reference to
|
||||
* <i>this</i> bridge. Skip it and Go keeps executing JavaScript
|
||||
* against the destroyed activity's WebView: the app opens, renders,
|
||||
* and never receives another backend event.
|
||||
*
|
||||
* <p>So this is called every time, and the half that must not repeat
|
||||
* is latched on the Go side instead, at the top of {@code main()} --
|
||||
* which is also where the damage was ({@code os.Exit(1)}), and the
|
||||
* only place that can see it. See #52.
|
||||
*/
|
||||
public void initialize() {
|
||||
if (initialized) {
|
||||
|
||||
@@ -110,27 +110,30 @@ test.describe('the album dropdown', () => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
|
||||
try {
|
||||
// Wait for the range the assertion below actually needs, not for
|
||||
// "scrollable at all" (#133). The guard used to be
|
||||
// `scrollHeight > clientHeight + 40` while the next line asks to
|
||||
// reach 80, so any range in 41-79 satisfied it and could not
|
||||
// satisfy the assertion — and the grid passes through exactly
|
||||
// that while it settles, because it recomputes its columns after
|
||||
// the resize rather than during it. The settled range here is
|
||||
// 330, so this waits rather than weakening anything.
|
||||
// The container has to be a scroller at all, which is the thing
|
||||
// the defect behind this spec broke and is a property rather
|
||||
// than a moment.
|
||||
await expect
|
||||
.poll(() => scrollRange(app))
|
||||
.toMatchObject({ room: true, overflowY: 'auto' });
|
||||
.toMatchObject({ overflowY: 'auto' });
|
||||
|
||||
await app.evaluate((target) => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
if (sc) sc.scrollTop = target;
|
||||
}, SCROLL_TARGET);
|
||||
|
||||
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
|
||||
// **Scrolling it and reading it back are one round trip** (#151).
|
||||
//
|
||||
// #133 made the guard ask for the range this needs rather than
|
||||
// for "scrollable at all", which was necessary and is not
|
||||
// sufficient: a guard and the write it guards are separate
|
||||
// `evaluate` calls, so the grid can satisfy the range and settle
|
||||
// out of it before the write lands. It still does — observed as
|
||||
// `Expected 80, Received 10` in the second of three consecutive
|
||||
// full-suite runs, with the spec green alone on the same app
|
||||
// straight afterwards.
|
||||
//
|
||||
// Polling harder cannot close a window between two moments; only
|
||||
// removing the window can. So the probe sets `scrollTop` and
|
||||
// returns what it reads back, in one page-side call, and the
|
||||
// poll retries *that* — which also means the assertion is about
|
||||
// what the grid did rather than about what it was ready to do.
|
||||
await expect.poll(() => scrollTo(app, SCROLL_TARGET)).toBe(SCROLL_TARGET);
|
||||
|
||||
// And the dropdown it opens is on screen, wherever the manager
|
||||
// decides that leaves the scroll. It is *not* "the position is
|
||||
@@ -261,7 +264,7 @@ async function closeDropdown(app: Page): Promise<void> {
|
||||
});
|
||||
}
|
||||
|
||||
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
||||
/** Whether the grid is a scroller at all, which is what the bug broke. */
|
||||
async function scrollRange(app: Page) {
|
||||
return app.evaluate((target) => {
|
||||
const sc = document
|
||||
@@ -269,22 +272,37 @@ async function scrollRange(app: Page) {
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
return {
|
||||
// `room` is the precondition of the assertion that follows it:
|
||||
// enough range to actually reach the target. A threshold below
|
||||
// what the caller depends on is not a guard.
|
||||
// Reported for the failure message rather than waited on: `room`
|
||||
// was the guard #133 strengthened, and #151 is that a guard in
|
||||
// its own round trip cannot speak for the write in the next one.
|
||||
// `scrollTo` below is the assertion now; this says *why* it did
|
||||
// not reach the target when it does not.
|
||||
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
||||
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
||||
};
|
||||
}, SCROLL_TARGET);
|
||||
}
|
||||
|
||||
async function scrollTop(app: Page): Promise<number> {
|
||||
return app.evaluate(
|
||||
() =>
|
||||
document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container')?.scrollTop ?? -1,
|
||||
);
|
||||
/**
|
||||
* Scroll the grid and report where it actually landed, in one call.
|
||||
*
|
||||
* The whole point is that the set and the read share a moment: a
|
||||
* `scrollTop` write is clamped to the range *at the instant it lands*,
|
||||
* so reading it back in a second round trip asks a container that may
|
||||
* have re-laid out in between.
|
||||
*/
|
||||
async function scrollTo(app: Page, target: number): Promise<number> {
|
||||
return app.evaluate((to) => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
if (!sc) return -1;
|
||||
|
||||
sc.scrollTop = to;
|
||||
|
||||
return sc.scrollTop;
|
||||
}, target);
|
||||
}
|
||||
|
||||
/** Whether the open dropdown is inside the scroll container's viewport. */
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
import { test, expect, callBinding, NO_QUEUE_SOURCE } from '../support/fixtures.js';
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* The bottom bar's two promises (#23, #42): the transport is centred in
|
||||
* the window, and the volume is a slider rather than a popup.
|
||||
*
|
||||
* **"Centred" is measured against the window, not against the space
|
||||
* left over**, which is the whole of #23. The bar was
|
||||
* `320px 1fr auto`, so the transport sat in the middle of what the
|
||||
* metadata and the queue button did not use — its centre was ~140px
|
||||
* right of the window's at every size, which reads as an alignment
|
||||
* mistake rather than as a layout choice.
|
||||
*
|
||||
* The mechanism is that the outer two columns are the same width, so
|
||||
* this asserts the *outcome* (centre lines up) rather than the CSS. A
|
||||
* spec that checked `grid-template-columns` would pass on any build
|
||||
* that kept the declaration and broke the result.
|
||||
*/
|
||||
|
||||
/** Where the transport sits, against where the window's centre is. */
|
||||
const geometry = (app: Page) =>
|
||||
app.evaluate(() => {
|
||||
const bar = document.querySelector<HTMLElement>('.bottom-bar')!;
|
||||
const player = document.querySelector<HTMLElement>('audio-player')!;
|
||||
const b = bar.getBoundingClientRect();
|
||||
const p = player.getBoundingClientRect();
|
||||
|
||||
const seek = player.shadowRoot
|
||||
?.querySelector('seek-bar')
|
||||
?.shadowRoot?.querySelector('wa-slider');
|
||||
|
||||
return {
|
||||
offset: Math.round(p.left + p.width / 2 - (b.left + b.width / 2)),
|
||||
barHeight: Math.round(b.height),
|
||||
seekWidth: seek ? Math.round(seek.getBoundingClientRect().width) : -1,
|
||||
};
|
||||
});
|
||||
|
||||
/** Something has to be playing before the transport draws a seek bar. */
|
||||
async function play(app: Page): Promise<void> {
|
||||
const paths = await app.evaluate(async () => {
|
||||
const tracks = (await window.__yjEvents.call(
|
||||
'library.Library.GetTracks',
|
||||
[0],
|
||||
10_000,
|
||||
)) as { FilePath: string }[];
|
||||
|
||||
return tracks.slice(0, 3).map((t) => t.FilePath);
|
||||
});
|
||||
|
||||
await callBinding(app, 'queue.Queue.SetQueue', [
|
||||
paths,
|
||||
0,
|
||||
false,
|
||||
NO_QUEUE_SOURCE,
|
||||
]);
|
||||
await callBinding(app, 'queue.Queue.Play');
|
||||
await expect(app.getByTestId('now-playing-title')).not.toBeEmpty();
|
||||
}
|
||||
|
||||
test.describe('the bottom bar', () => {
|
||||
test.afterEach(async ({ app }) => {
|
||||
await callBinding(app, 'queue.Queue.Clear').catch(() => {
|
||||
/* already empty */
|
||||
});
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
});
|
||||
|
||||
/**
|
||||
* Four widths, because a centring bug is a function of width: the old
|
||||
* layout was off by half the difference between the two outer
|
||||
* columns, so it was wrong by a different amount at each one and
|
||||
* exactly right at none.
|
||||
*/
|
||||
for (const width of [800, 900, 1100, 1440]) {
|
||||
test(`centres the transport in the window at ${width}px`, async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.setViewportSize({ width, height: 700 });
|
||||
await play(app);
|
||||
|
||||
await expect.poll(() => geometry(app).then((g) => g.offset)).toBe(0);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The seek bar is what the centring is *paid for* with, so it is
|
||||
* asserted rather than assumed.
|
||||
*
|
||||
* Reserving the metadata's full width on both sides centres the
|
||||
* transport perfectly and squeezes the control you drag: measured
|
||||
* during this work at **61px of track at 800px**, against 257 before
|
||||
* the change. The side columns are capped at a quarter of the bar for
|
||||
* that reason, and this is the number that says so — 246 at 800px,
|
||||
* which is parity with the uncentred layout.
|
||||
*/
|
||||
test('does not pay for the centring with the seek bar', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 800, height: 700 });
|
||||
await play(app);
|
||||
|
||||
await expect
|
||||
.poll(() => geometry(app).then((g) => g.seekWidth))
|
||||
.toBeGreaterThan(200);
|
||||
});
|
||||
|
||||
/**
|
||||
* #42: the slider is simply there. Three gestures — click open, drag,
|
||||
* click closed — is what a bottom bar has room not to ask for.
|
||||
*/
|
||||
test('shows the volume slider without a click', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
|
||||
const volume = app.locator('.bottom-bar volume-control');
|
||||
|
||||
await expect(volume).toBeVisible();
|
||||
await expect(volume.locator('wa-slider')).toBeVisible();
|
||||
});
|
||||
|
||||
/**
|
||||
* And the inline icon is the mute toggle, because with the slider
|
||||
* beside it there is nothing left to disclose. The name follows the
|
||||
* action rather than the state for the same reason.
|
||||
*/
|
||||
test('names the inline icon after what it does', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
|
||||
await expect(
|
||||
app.locator('.bottom-bar volume-control').getByRole('button', {
|
||||
name: 'Mute',
|
||||
}),
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
/**
|
||||
* The bar is a fixed 4em row and the transport sits in it. A slider
|
||||
* with a label grows `#slider` by 8px unless `wa-slider-label.css`
|
||||
* suppresses it, which moved the whole bar the last time — so the
|
||||
* height is pinned here rather than left to a screenshot.
|
||||
*/
|
||||
test('stays 4em tall', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
await play(app);
|
||||
|
||||
await expect.poll(() => geometry(app).then((g) => g.barHeight)).toBe(64);
|
||||
});
|
||||
});
|
||||
@@ -29,18 +29,13 @@ test.describe('a control says what it controls', () => {
|
||||
});
|
||||
|
||||
test('the volume slider is announced as Volume', async ({ app }) => {
|
||||
// The popup renders no slider at all while closed, the same way the
|
||||
// queue panel renders no list — so this has to open it first.
|
||||
await app.getByRole('button', { name: /volume/i }).click();
|
||||
|
||||
// No disclosure to open first, and no state to put back afterwards:
|
||||
// #42 made the slider inline, so it is simply there. The assertion
|
||||
// is unchanged — the *name* is the subject here, and the route to
|
||||
// the control got shorter rather than different.
|
||||
await expect(
|
||||
app.getByRole('slider', { name: 'Volume' }),
|
||||
).toBeVisible();
|
||||
|
||||
// Leave the transport as it was found: the specs share one page in
|
||||
// file order, and an open popup covers the buttons beneath it.
|
||||
await app.keyboard.press('Escape');
|
||||
await app.locator('body').click({ position: { x: 5, y: 5 } });
|
||||
});
|
||||
|
||||
test('naming the slider did not move the transport', async ({ app }) => {
|
||||
|
||||
@@ -132,22 +132,34 @@ test.describe('the app fits in its own window', () => {
|
||||
// be dragged here, but a scaled display or a large system font can
|
||||
// still land the layout in it, and clipping the nav with no scroll
|
||||
// is the failure that made Settings unreachable.
|
||||
const reachable = await app.locator('app-sidebar').evaluate((el) => {
|
||||
const settings = el.shadowRoot?.querySelector<HTMLElement>(
|
||||
'[data-testid="nav-settings"]',
|
||||
);
|
||||
//
|
||||
// The scroll and the measurement share one `evaluate` — #151's
|
||||
// rule, which this already had — and the whole probe is polled,
|
||||
// which it did not: a viewport change settles asynchronously, so a
|
||||
// single attempt reads whatever the sidebar happened to be doing.
|
||||
// The probe is safe to repeat because scrolling to the bottom twice
|
||||
// is scrolling to the bottom.
|
||||
await expect
|
||||
.poll(() =>
|
||||
app.locator('app-sidebar').evaluate((el) => {
|
||||
const settings = el.shadowRoot?.querySelector<HTMLElement>(
|
||||
'[data-testid="nav-settings"]',
|
||||
);
|
||||
|
||||
if (!settings) return null;
|
||||
if (!settings) return null;
|
||||
|
||||
el.scrollTop = el.scrollHeight;
|
||||
el.scrollTop = el.scrollHeight;
|
||||
|
||||
const item = settings.getBoundingClientRect();
|
||||
const pane = el.getBoundingClientRect();
|
||||
const item = settings.getBoundingClientRect();
|
||||
const pane = el.getBoundingClientRect();
|
||||
|
||||
return item.bottom <= Math.ceil(pane.bottom) && item.top >= Math.floor(pane.top);
|
||||
});
|
||||
|
||||
expect(reachable).toBe(true);
|
||||
return (
|
||||
item.bottom <= Math.ceil(pane.bottom) &&
|
||||
item.top >= Math.floor(pane.top)
|
||||
);
|
||||
}),
|
||||
)
|
||||
.toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
import { test, expect, LONG_TRACK } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* The phone shell (plan 016 B2, phase 1).
|
||||
@@ -138,6 +138,78 @@ test.describe('the shell on a phone', () => {
|
||||
.toHaveAttribute('data-active-view', 'tracks');
|
||||
});
|
||||
|
||||
/**
|
||||
* The same journey with a track that has **no cover art** (#150).
|
||||
*
|
||||
* The test above starts the *first* row of the track list, so which
|
||||
* track it plays is the order the scan inserted them in — and the
|
||||
* answer decided whether it passed. A track with artwork renders an
|
||||
* `<img>`, which is no obstacle; one without renders a placeholder
|
||||
* `wa-icon`, which took every click aimed at the button beneath it,
|
||||
* because that button is absolutely positioned with `z-index: auto`
|
||||
* and the art is a *later* sibling. They tied, and the later one won.
|
||||
*
|
||||
* So this picks a track *for* the property that broke it, which is
|
||||
* the only way the assertion means anything: the version above passes
|
||||
* on a broken build roughly two runs in three, which is exactly how
|
||||
* it came to cost three CI cycles across two branches that could not
|
||||
* have caused it.
|
||||
*/
|
||||
test('opens the full-screen now playing for a track with no art', async ({
|
||||
app,
|
||||
}) => {
|
||||
// `LONG_TRACK` by name, and not "the first track with no
|
||||
// CoverArt": the *library* model reports that field empty for
|
||||
// every row in this fixture (31 of 31), so filtering on it selects
|
||||
// nothing in particular and picked a 2-second track, which had
|
||||
// finished before the assertions ran. The placeholder check below
|
||||
// is what actually holds the property this test needs.
|
||||
const started = await app.evaluate(async (longTitle) => {
|
||||
const tracks = (await window.__yjEvents.call(
|
||||
'library.Library.GetTracks',
|
||||
[0],
|
||||
10_000,
|
||||
)) as { FilePath: string; TrackName: string }[];
|
||||
|
||||
const bare = tracks.find((t) => t.TrackName === longTitle);
|
||||
|
||||
if (!bare) return null;
|
||||
|
||||
await window.__yjEvents.call(
|
||||
'queue.Queue.SetQueue',
|
||||
[[bare.FilePath], 0, false, { type: '', id: 0, label: '' }],
|
||||
10_000,
|
||||
);
|
||||
await window.__yjEvents.call('queue.Queue.Play', [], 5_000);
|
||||
|
||||
return bare.TrackName;
|
||||
}, LONG_TRACK);
|
||||
|
||||
expect(started).toBe(LONG_TRACK);
|
||||
|
||||
await expect(app.getByTestId('now-playing-title')).not.toBeEmpty();
|
||||
|
||||
// **The placeholder is the whole point**, so it is asserted rather
|
||||
// than assumed: this test is about the thing that renders when
|
||||
// there is no artwork. If the fixture ever gives this album a
|
||||
// cover, this fails and says so instead of passing while measuring
|
||||
// the easy case.
|
||||
//
|
||||
// One selector rather than a chain from the host: Playwright's CSS
|
||||
// engine pierces an open shadow root, and chaining from the host
|
||||
// element does not reach into it.
|
||||
await expect(
|
||||
app.locator('now-playing .cover-placeholder'),
|
||||
).toBeAttached();
|
||||
|
||||
await app.getByTestId('open-now-playing').click();
|
||||
|
||||
await expect(app.getByTestId('main-content'))
|
||||
.toHaveAttribute('data-active-view', 'now-playing');
|
||||
|
||||
await app.getByTestId('npv-back').click();
|
||||
});
|
||||
|
||||
test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
|
||||
@@ -154,9 +226,26 @@ test.describe('the shell on a phone', () => {
|
||||
await expect(app.locator('now-playing')).toBeVisible();
|
||||
|
||||
// Volume is the hardware keys' job on a phone, and a 4px seek bar
|
||||
// is not a thumb target -- both belong to a later phase's
|
||||
// full-screen now-playing view.
|
||||
await expect(app.locator('audio-player volume-control')).toBeHidden();
|
||||
// is not a thumb target -- both belong to the full-screen
|
||||
// now-playing view.
|
||||
//
|
||||
// `.bottom-bar volume-control`, not `audio-player volume-control`:
|
||||
// #42 moved the control out of that component and into the bar, and
|
||||
// **the old locator would have kept passing** — `toBeHidden()` is
|
||||
// satisfied by an element that does not exist, so this assertion
|
||||
// would have gone on reporting success about nothing. Its partner
|
||||
// below is what makes this one mean something.
|
||||
await expect(app.locator('.bottom-bar volume-control')).toBeHidden();
|
||||
|
||||
// The element is there and hidden, rather than absent: the check
|
||||
// above cannot tell those apart on its own.
|
||||
await expect(app.locator('.bottom-bar volume-control')).toHaveCount(1);
|
||||
|
||||
// And the seek bar is still inside the transport, where it stands
|
||||
// down by its own media query.
|
||||
await expect(
|
||||
app.locator('audio-player').locator('seek-bar'),
|
||||
).toBeHidden();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,299 @@
|
||||
import {
|
||||
test,
|
||||
expect,
|
||||
callBinding,
|
||||
navigateTo,
|
||||
LONG_TRACK,
|
||||
NO_QUEUE_SOURCE,
|
||||
} from '../support/fixtures.js';
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* The queue panel's mouse model (#43): single click selects, ctrl and
|
||||
* shift extend, double click plays from that row.
|
||||
*
|
||||
* **All four already worked, and nothing pinned any of them** — which is
|
||||
* the whole reason the report could be made and could not be settled.
|
||||
* `queue-reorder.spec.ts` covers the keyboard, `queue-overlay.spec.ts`
|
||||
* covers the panel's mode, and the component tier has the reorder
|
||||
* arithmetic; the pointer path had no coverage in either tier, so
|
||||
* "selection is broken here" and "selection is fine here" were equally
|
||||
* consistent with a green suite.
|
||||
*
|
||||
* Two things this spec is deliberately shaped around.
|
||||
*
|
||||
* **The clicks are real.** A `dispatchEvent(new MouseEvent('click'))`
|
||||
* on a row exercises the delegated handler and *not* the question being
|
||||
* asked, which is what the pointer lands on: the rows carry
|
||||
* `explore-link` names that take their own clicks, and a synthetic
|
||||
* event aimed at the row reports a selection the mouse would never have
|
||||
* produced. Every click here goes through Playwright.
|
||||
*
|
||||
* **The playing assertions use the 90-second fixture.** Every other
|
||||
* track is 2–6 seconds, so "double click plays row 3" read against a
|
||||
* 2-second track reports whatever auto-advance moved on to — measured
|
||||
* during this work as row 3 double-clicked and row 4 playing, which
|
||||
* reads exactly like an off-by-one in `PlayIndex` and is not one.
|
||||
*/
|
||||
|
||||
/**
|
||||
* How long a click may take to show up as a highlight.
|
||||
*
|
||||
* **A poll with the default 5s timeout cannot see this defect**, and
|
||||
* that is the point of naming it. `queue-panel` repaints its rows two
|
||||
* ways — `onSelectionChanged()` calls `virtualizer.requestUpdate()`,
|
||||
* and `.keyFunction` is a per-render arrow, which is itself a changed
|
||||
* property the virtualizer reacts to. With **both** removed the
|
||||
* highlight still arrives, on whatever unrelated render happens next:
|
||||
* measured at 134ms, 3,866ms and 5,816ms for three clicks, against
|
||||
* 5ms, 16ms and 17ms on a healthy build.
|
||||
*
|
||||
* A user cannot tell "four seconds late" from "broken", which is very
|
||||
* close to what this issue reports. So the assertion is that the
|
||||
* highlight is *prompt*, with a bound ~30x the measured healthy case
|
||||
* and an order of magnitude under the degraded one.
|
||||
*/
|
||||
const HIGHLIGHT_MS = 500;
|
||||
|
||||
/** The queue's own answer, never the DOM's. */
|
||||
async function playing(app: Page): Promise<{ index: number; title: string }> {
|
||||
const state = await callBinding<{
|
||||
currentIndex: number;
|
||||
tracks: { title: string }[];
|
||||
}>(app, 'queue.Queue.GetState');
|
||||
|
||||
return {
|
||||
index: state.currentIndex,
|
||||
title: state.tracks[state.currentIndex]?.title ?? '',
|
||||
};
|
||||
}
|
||||
|
||||
/** Which rows are selected, as the accessibility tree sees it. */
|
||||
const selected = (app: Page) =>
|
||||
app.evaluate(() =>
|
||||
[
|
||||
...document
|
||||
.querySelector('queue-panel')!
|
||||
.shadowRoot!.querySelectorAll('[data-index]'),
|
||||
]
|
||||
.filter((row) => row.getAttribute('aria-selected') === 'true')
|
||||
.map((row) => Number((row as HTMLElement).dataset['index'])),
|
||||
);
|
||||
|
||||
/**
|
||||
* Six tracks with the long one in the middle, so a "play from here"
|
||||
* assertion has something to land on that will still be playing when it
|
||||
* is read back.
|
||||
*/
|
||||
async function queueSixAndOpen(app: Page): Promise<void> {
|
||||
const paths = await app.evaluate(async (longTitle) => {
|
||||
// `TrackName`, not `Title`: the library model names it after the
|
||||
// tag, and the *queue* is what calls it `title`.
|
||||
const tracks = (await window.__yjEvents.call(
|
||||
'library.Library.GetTracks',
|
||||
[0],
|
||||
10_000,
|
||||
)) as { FilePath: string; TrackName: string; Album: string }[];
|
||||
|
||||
const long = tracks.find((t) => t.TrackName === longTitle);
|
||||
|
||||
/**
|
||||
* **Tracks that have an album**, which is a requirement of one of
|
||||
* the tests and was previously left to luck (#156).
|
||||
*
|
||||
* `explore-link` routes a track name to its *album's* page, so a
|
||||
* track with no album renders a name that navigates nowhere — and
|
||||
* the fixture library deliberately contains two (`01 Tone A`,
|
||||
* `02 Tone B`). Which tracks arrive first is `audio_files.id`
|
||||
* order, i.e. the order the **scan** inserted them, which depends
|
||||
* on concurrency and directory traversal: locally the first eight
|
||||
* all had albums and the spec passed twice over, and CI rebuilds
|
||||
* its seed with a real scan and got a different eight.
|
||||
*
|
||||
* Asking for what the test needs is the fix. It is not a
|
||||
* narrowing: every assertion here wants an ordinary track, and
|
||||
* "the first five rows" was never a way to ask for one in a
|
||||
* library whose whole purpose is edge cases.
|
||||
*/
|
||||
const rest = tracks
|
||||
.filter((t) => t.TrackName !== longTitle && t.Album !== '')
|
||||
.slice(0, 5);
|
||||
|
||||
// Index 3 is the long one: far enough down that a shift-extend has
|
||||
// room either side of it.
|
||||
return [
|
||||
...rest.slice(0, 3).map((t) => t.FilePath),
|
||||
long!.FilePath,
|
||||
...rest.slice(3).map((t) => t.FilePath),
|
||||
];
|
||||
}, LONG_TRACK);
|
||||
|
||||
await callBinding(app, 'queue.Queue.SetQueue', [
|
||||
paths,
|
||||
0,
|
||||
false,
|
||||
NO_QUEUE_SOURCE,
|
||||
]);
|
||||
|
||||
// A closed panel renders no list at all.
|
||||
await app.locator('#queue-button').click();
|
||||
await expect(app.locator('queue-panel .track-item').first()).toBeVisible();
|
||||
await expect(app.locator('queue-panel .track-item')).toHaveCount(6);
|
||||
}
|
||||
|
||||
/** The row at a data-index, not the nth child: see the note in the file. */
|
||||
const row = (app: Page, index: number) =>
|
||||
app.locator(`queue-panel .track-item[data-index="${index}"]`);
|
||||
|
||||
test.describe('selecting in the queue with a mouse', () => {
|
||||
// The suite shares one backend in file order, and a queue and an open
|
||||
// panel both outlive the page. `queue-reorder.spec.ts` sets the
|
||||
// precedent and the reason: a spec that spends state fails the next
|
||||
// one, in a list that reads like a regression in whatever you hold.
|
||||
test.afterEach(async ({ app }) => {
|
||||
await callBinding(app, 'queue.Queue.Clear').catch(() => {
|
||||
/* an empty queue is the state we were asking for */
|
||||
});
|
||||
|
||||
const open = await app.locator('queue-panel[open]').count();
|
||||
|
||||
if (open > 0) await app.locator('#queue-button').click();
|
||||
});
|
||||
|
||||
test('a single click selects that row and only that row', async ({ app }) => {
|
||||
await queueSixAndOpen(app);
|
||||
|
||||
await row(app, 1).click();
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([1]);
|
||||
|
||||
// And it *replaces* rather than accumulating, which is the half a
|
||||
// test of one click cannot see.
|
||||
await row(app, 4).click();
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([4]);
|
||||
});
|
||||
|
||||
test('ctrl adds a row and shift extends a range', async ({ app }) => {
|
||||
await queueSixAndOpen(app);
|
||||
|
||||
await row(app, 1).click();
|
||||
await row(app, 3).click({ modifiers: ['Control'] });
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([1, 3]);
|
||||
|
||||
// From the last row touched, so 3→5, keeping the ctrl-picked 1.
|
||||
await row(app, 5).click({ modifiers: ['Shift'] });
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([1, 3, 4, 5]);
|
||||
|
||||
// A plain click collapses the whole thing back to one.
|
||||
await row(app, 2).click();
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([2]);
|
||||
});
|
||||
|
||||
test('a double click plays from that row', async ({ app }) => {
|
||||
await queueSixAndOpen(app);
|
||||
|
||||
// Row 3 is the 90-second track. Asked of the backend, because the
|
||||
// panel's own highlight is a different claim.
|
||||
await row(app, 3).dblclick();
|
||||
|
||||
await expect.poll(() => playing(app)).toEqual({
|
||||
index: 3,
|
||||
title: LONG_TRACK,
|
||||
});
|
||||
|
||||
// Playing is not selecting: the double click clears the selection
|
||||
// it made on the way through, or every play leaves a row looking
|
||||
// picked out for an action the user did not ask for.
|
||||
await expect.poll(() => selected(app)).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The one collision the report is actually about.
|
||||
*
|
||||
* Every track, album and artist name in the app navigates
|
||||
* (`utils/explore-link.ts`), and it does that by **stopping the
|
||||
* click's propagation** — in its own words, "the row must not also
|
||||
* treat it as a selection". So a click that lands on the name text
|
||||
* navigates and selects nothing, in the queue panel and in the track
|
||||
* list alike.
|
||||
*
|
||||
* That is deliberate and it is pinned here rather than argued with,
|
||||
* because the measurement says the queue is not the surface where it
|
||||
* hurts: a horizontal hit-scan of a row at three heights makes the
|
||||
* queue row **12%** link and the track list's row **21%** — the panel
|
||||
* the report calls broken is *less* covered by links than the list it
|
||||
* calls correct. What is left is one deliberate exception, and a
|
||||
* change to it should have to fail a test.
|
||||
*/
|
||||
test('a click on a name navigates instead, and that is the exception', async ({
|
||||
app,
|
||||
}) => {
|
||||
await queueSixAndOpen(app);
|
||||
|
||||
await row(app, 1).click();
|
||||
await expect
|
||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||
.toEqual([1]);
|
||||
|
||||
// `.track-title .explore-link`, not `.explore-link` first(): a row
|
||||
// has two, and which one `first()` finds depends on whether the
|
||||
// *title* is a link at all. It is not, for a track with no album —
|
||||
// `explore-link` renders plain text where it cannot route — so the
|
||||
// loose locator silently clicked the **artist** instead and the
|
||||
// assertion below was about a different destination than the one
|
||||
// being exercised (#156).
|
||||
await row(app, 2).locator('.track-title .explore-link').click();
|
||||
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'explore-album-details',
|
||||
);
|
||||
|
||||
// Row 2 did not join the selection — the link took the click.
|
||||
await expect.poll(() => selected(app)).toEqual([1]);
|
||||
|
||||
await navigateTo(app, 'tracks');
|
||||
});
|
||||
|
||||
/**
|
||||
* And the other half of that bargain: the link holds its navigation
|
||||
* for one double-click interval and drops it if a second click
|
||||
* arrives, so double-clicking a *name* still plays the row rather
|
||||
* than navigating away from it. That is what makes the exception
|
||||
* above survivable, and it is the part most likely to break silently
|
||||
* if the grace interval is ever removed.
|
||||
*/
|
||||
test('a double click on a name plays rather than navigating', async ({
|
||||
app,
|
||||
}) => {
|
||||
await queueSixAndOpen(app);
|
||||
|
||||
// Read rather than assumed: which view the app lands on is the
|
||||
// user's `DefaultPage`, so naming one here would be asserting on a
|
||||
// config value in a test about a double click.
|
||||
const before = await app
|
||||
.getByTestId('main-content')
|
||||
.getAttribute('data-active-view');
|
||||
|
||||
await row(app, 3).locator('.explore-link').first().dblclick();
|
||||
|
||||
await expect.poll(() => playing(app)).toEqual({
|
||||
index: 3,
|
||||
title: LONG_TRACK,
|
||||
});
|
||||
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
before!,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -69,6 +69,14 @@ export function GetPinDefaultPlaylist(): $CancellablePromise<boolean> {
|
||||
return $Call.ByID(3818283301);
|
||||
}
|
||||
|
||||
/**
|
||||
* GetPopupVolume reports whether the bottom bar's volume control is a
|
||||
* click-to-open popup rather than an inline slider (#42).
|
||||
*/
|
||||
export function GetPopupVolume(): $CancellablePromise<boolean> {
|
||||
return $Call.ByID(2885777);
|
||||
}
|
||||
|
||||
/**
|
||||
* GetQueueFallback returns what plays, if anything, once the queue
|
||||
* runs out.
|
||||
@@ -207,6 +215,18 @@ export function SetPinDefaultPlaylist(pin: boolean): $CancellablePromise<void> {
|
||||
return $Call.ByID(372446849, pin);
|
||||
}
|
||||
|
||||
/**
|
||||
* SetPopupVolume saves the volume control's presentation.
|
||||
*
|
||||
* Nothing to validate: both values are legal at every width, and the
|
||||
* frontend additionally stands the inline slider down below the phone
|
||||
* breakpoint whatever this says, because that is about room rather than
|
||||
* about preference.
|
||||
*/
|
||||
export function SetPopupVolume(popup: boolean): $CancellablePromise<void> {
|
||||
return $Call.ByID(1430308453, popup);
|
||||
}
|
||||
|
||||
/**
|
||||
* SetQueueFallback validates and saves a new queue-fallback mode.
|
||||
*/
|
||||
|
||||
+78
-5
@@ -211,7 +211,39 @@ body div.sidebar {
|
||||
padding: 0.25em;
|
||||
background-color: var(--yj-bg-elevated, #343a40);
|
||||
display: grid;
|
||||
grid-template-columns: var(--now-playing-width, 320px) 1fr auto;
|
||||
/* Three columns whose outer two are the *same* width, which is what
|
||||
centres the middle one (#23). It was `var(--now-playing-width) 1fr
|
||||
auto`, so the transport's centre sat at `W/2 + 140px` — in the
|
||||
middle of the space left over, which is not the same thing and
|
||||
reads as an alignment mistake at every window size.
|
||||
|
||||
The outer width is still `--now-playing-width`, so **the metadata
|
||||
panel's drag handle keeps meaning something**: widening it takes
|
||||
room from the transport on both sides at once, symmetrically. An
|
||||
`1fr … 1fr` pair would have centred the transport just as well and
|
||||
silently made that handle a no-op.
|
||||
|
||||
**The cap is what stops that being a regression**, and it was
|
||||
measured as one first. Reserving the full metadata width on both
|
||||
sides costs the transport twice: at 800px the outer pair wanted
|
||||
640 of 800, and the seek bar's track went from 257px to 61px
|
||||
(and to 0 at 200% text) — the control you drag, squeezed out to
|
||||
centre the buttons above it. So the side tracks are the metadata
|
||||
width *or a quarter of the bar*, whichever is smaller, which
|
||||
leaves the drag handle meaningful everywhere it has room to be
|
||||
and hands the difference to the transport where it does not.
|
||||
|
||||
`minmax(0, …)` on the outer tracks and `min-content` on the middle
|
||||
decide who yields when even that is not enough: the metadata and
|
||||
the end group shrink (both truncate; neither loses an action), and
|
||||
the transport keeps at least its buttons. Without the `min-content`
|
||||
floor the middle collapses first, because a `1fr` track's minimum
|
||||
is `auto` only until something else insists. */
|
||||
--bar-side: min(var(--now-playing-width, 320px), 25%);
|
||||
grid-template-columns:
|
||||
minmax(0, var(--bar-side))
|
||||
minmax(min-content, 1fr)
|
||||
minmax(0, var(--bar-side));
|
||||
align-items: center;
|
||||
contain: layout style;
|
||||
|
||||
@@ -239,23 +271,44 @@ body div.sidebar {
|
||||
text-wrap-mode: nowrap;
|
||||
overflow: hidden;
|
||||
|
||||
p {
|
||||
/* `& p`, not `p`. **A nested rule that begins with a bare
|
||||
element selector is silently dropped before Chrome 120**
|
||||
(relaxed nesting), and the phone this app runs on renders
|
||||
in Chrome 113 -- so this ellipsis, and the two rules
|
||||
below, have never applied on the device. Nothing fails;
|
||||
the text simply overflows there. The `&` form is valid in
|
||||
both, which is why it is used for every element selector
|
||||
in this file's nested blocks. */
|
||||
& p {
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
now-playing {
|
||||
& now-playing {
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
audio-player {
|
||||
& audio-player {
|
||||
margin: 0.5em 1em;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
/* The right-hand group, and the thing the left column is matched
|
||||
against. It is one grid cell rather than two columns because the
|
||||
centring rule above compares *columns*: volume and the queue
|
||||
button in separate tracks would make the outer pair unequal by
|
||||
whatever the volume happens to measure. */
|
||||
.bar-end {
|
||||
justify-self: end;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 0.25em;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
#queue-button {
|
||||
justify-self: end;
|
||||
background: none;
|
||||
border: none;
|
||||
color: inherit;
|
||||
@@ -440,6 +493,11 @@ body div.sidebar {
|
||||
}
|
||||
|
||||
@media (max-width: 599px) {
|
||||
/* The phone keeps the two-part bar it had: metadata, then the
|
||||
transport and the queue button. There is no third column to
|
||||
balance because the centring the desktop does is a luxury of
|
||||
having room — at 360px the metadata needs all of the space the
|
||||
controls do not. */
|
||||
.bottom-bar {
|
||||
grid-template-columns: minmax(0, 1fr) auto auto;
|
||||
gap: 0.25em;
|
||||
@@ -448,4 +506,19 @@ body div.sidebar {
|
||||
.bottom-bar audio-player {
|
||||
margin: 0.25em;
|
||||
}
|
||||
|
||||
/* 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.
|
||||
|
||||
`.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. */
|
||||
.bottom-bar volume-control {
|
||||
display: none;
|
||||
}
|
||||
}
|
||||
|
||||
+22
-7
@@ -40,16 +40,31 @@
|
||||
</main>
|
||||
<queue-panel id="queue-panel"></queue-panel>
|
||||
</div>
|
||||
<!-- Three columns, and the outer two are the same width, which is
|
||||
what makes the middle one *centred* rather than merely in the
|
||||
middle of what is left (#23). The transport used to sit in a
|
||||
`320px 1fr auto` grid, so its centre was ~140px right of the
|
||||
window's.
|
||||
|
||||
That is also why the volume moved out of `audio-player` and
|
||||
into the bar (#42): the transport column has to contain the
|
||||
transport and nothing else, or "centred" means centred with a
|
||||
slider bolted to one side. It joins the queue button in
|
||||
`.bar-end`, whose width is what the left column is matched
|
||||
against. -->
|
||||
<footer class="bottom-bar">
|
||||
<now-playing></now-playing>
|
||||
<audio-player></audio-player>
|
||||
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
||||
id="queue-button">
|
||||
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
|
||||
because this file has no module scope. It was `list`,
|
||||
which is the Playlists destination's icon. -->
|
||||
<wa-icon name="bars-staggered"></wa-icon>
|
||||
</button>
|
||||
<div class="bar-end">
|
||||
<volume-control></volume-control>
|
||||
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
||||
id="queue-button">
|
||||
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
|
||||
because this file has no module scope. It was `list`,
|
||||
which is the Playlists destination's icon. -->
|
||||
<wa-icon name="bars-staggered"></wa-icon>
|
||||
</button>
|
||||
</div>
|
||||
</footer>
|
||||
<!-- The phone's primary navigation, hidden above 600px by
|
||||
index.css. Eager rather than a chunk, for the reason
|
||||
|
||||
@@ -18,6 +18,9 @@
|
||||
// track-list — index.html renders one, so it is the first paint.
|
||||
// ---------------------------------------------------------------------------
|
||||
import '@components/audio-player/audio-player.ts';
|
||||
// In the bar rather than inside `audio-player` since #42, so the shell
|
||||
// is what has to register it.
|
||||
import '@components/audio-player/volume-control/volume-control.ts';
|
||||
import '@components/track-list/track-list.ts';
|
||||
import '@components/now-playing/now-playing.ts';
|
||||
import '@components/sidebar/app-sidebar.ts';
|
||||
|
||||
@@ -3,7 +3,6 @@ import { customElement } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import './controls/player-controls';
|
||||
import './seekbar/seek-bar';
|
||||
import './volume-control/volume-control';
|
||||
import '../notifications/inline-notice';
|
||||
import { PlayerRegion } from '@store/player-store';
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
@@ -30,6 +29,7 @@ export class AudioPlayer extends LitElement {
|
||||
|
||||
.player-main {
|
||||
flex: 1;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
/* The phone transport (plan 016 B2): the buttons, and nothing
|
||||
@@ -37,14 +37,17 @@ export class AudioPlayer extends LitElement {
|
||||
viewport, not by the host, so this is the component saying what
|
||||
it drops at phone width rather than the shell reaching in.
|
||||
|
||||
Volume goes because the hardware keys own it on a phone --
|
||||
Android routes them to the media stream, which is also why
|
||||
mediacontrols' Android handler implements no volume callback.
|
||||
The seek bar goes because a 4px-tall target dragged with a thumb
|
||||
is not a seek control; seeking belongs to the full-screen
|
||||
now-playing view, which is the next phase. */
|
||||
now-playing view.
|
||||
|
||||
Volume used to go from here too, and now goes from index.css
|
||||
instead: #42 moved the control out of this component and into
|
||||
the bar, so the shell is what can hide it. The reason is
|
||||
unchanged -- the hardware keys own volume on a phone, which is
|
||||
also why mediacontrols' Android handler implements no volume
|
||||
callback. */
|
||||
@media (max-width: 599px) {
|
||||
volume-control,
|
||||
seek-bar {
|
||||
display: none;
|
||||
}
|
||||
@@ -65,7 +68,6 @@ export class AudioPlayer extends LitElement {
|
||||
<seek-bar></seek-bar>
|
||||
</div>
|
||||
</div>
|
||||
<volume-control></volume-control>
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
|
||||
@@ -22,6 +22,24 @@ export class SeekBar extends LitElement {
|
||||
@state()
|
||||
private seekValue: number = 0;
|
||||
|
||||
/**
|
||||
* Whether the user is dragging the thumb right now.
|
||||
*
|
||||
* It is `@state` rather than a plain field because `updated()` owns
|
||||
* the interval and only reactive state brings `updated()` round. A
|
||||
* bare `stopProgress()` in the input handler mutated nothing, so
|
||||
* nothing re-rendered, so the tail of `updated()` that restarts the
|
||||
* interval never ran — and the only things that could restart it
|
||||
* were a `change` event or the next backend report. Any `input`
|
||||
* without a committed `change` therefore froze the interpolation:
|
||||
* a drag cancelled outside the element, a pointer taken by a scroll,
|
||||
* or a touch on the track treated as a scrub, which on a phone are
|
||||
* ordinary gestures. While playing, the 1 Hz report papered over it
|
||||
* within a second; with reports not arriving it was permanent.
|
||||
*/
|
||||
@state()
|
||||
private dragging: boolean = false;
|
||||
|
||||
/** Whether the right-hand clock shows time remaining or total. */
|
||||
@state()
|
||||
private showRemaining: boolean = true;
|
||||
@@ -133,6 +151,7 @@ export class SeekBar extends LitElement {
|
||||
override disconnectedCallback() {
|
||||
super.disconnectedCallback();
|
||||
this.stopProgress();
|
||||
this.endDrag();
|
||||
}
|
||||
|
||||
override updated() {
|
||||
@@ -154,18 +173,33 @@ export class SeekBar extends LitElement {
|
||||
// A report for a track that is no longer loaded is stale by
|
||||
// definition: the change id is the only thing that distinguishes
|
||||
// it, since the same file can play twice in a row.
|
||||
//
|
||||
// A report arriving mid-drag is deliberately *not* applied: the
|
||||
// thumb belongs to the finger on it, and adopting a report once a
|
||||
// second pulls it back out from under them. The seq is left
|
||||
// unrecorded too, so the first report after the drag still counts
|
||||
// as fresh.
|
||||
const position = this.player.position;
|
||||
const forThisTrack =
|
||||
position !== null && position.trackChangeId === currentChangeId;
|
||||
|
||||
if (position && forThisTrack && position.seq !== this.previousPositionSeq) {
|
||||
if (
|
||||
position &&
|
||||
forThisTrack &&
|
||||
!this.dragging &&
|
||||
position.seq !== this.previousPositionSeq
|
||||
) {
|
||||
this.previousPositionSeq = position.seq;
|
||||
this.seekValue = position.positionSeconds;
|
||||
this.stopProgress();
|
||||
}
|
||||
|
||||
// Start/stop progress interval based on playback state
|
||||
if (this.isPlaying && this.hasTrack) {
|
||||
// One owner for the interval, and this is it. Every other place
|
||||
// that wants it started or stopped says so by changing state that
|
||||
// brings us back here, so the timer cannot be left running by a
|
||||
// path that forgot to stop it or stopped by a path that forgot to
|
||||
// start it again.
|
||||
if (this.isPlaying && this.hasTrack && !this.dragging) {
|
||||
this.startProgress();
|
||||
} else {
|
||||
this.stopProgress();
|
||||
@@ -210,18 +244,48 @@ export class SeekBar extends LitElement {
|
||||
|
||||
private handleChange(e: Event) {
|
||||
const newSeekVal = (e.target as WaSlider).value;
|
||||
this.endDrag();
|
||||
this.setSeekValue(newSeekVal);
|
||||
this.player.seek(newSeekVal);
|
||||
}
|
||||
|
||||
if (this.isPlaying) {
|
||||
this.startProgress();
|
||||
/**
|
||||
* The user is moving the thumb.
|
||||
*
|
||||
* This only records that fact; `updated()` decides what it means for
|
||||
* the interval. `seekValue` follows the slider so the clocks track
|
||||
* the thumb during the drag rather than jumping when it is released.
|
||||
*/
|
||||
private handleInput(e: Event) {
|
||||
this.setSeekValue((e.target as WaSlider).value);
|
||||
|
||||
if (this.dragging) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.dragging = true;
|
||||
|
||||
// A drag that never commits must not strand the flag, or this fix
|
||||
// turns a stall of up to one second into a permanent one -- which
|
||||
// is the failure it exists to remove. `change` is the ordinary
|
||||
// end; these are the ones that are not, and they are on the
|
||||
// document because the pointer is routinely released outside the
|
||||
// element it started in. A drag's listeners belong to the drag,
|
||||
// so they go on with it and come off with it.
|
||||
document.addEventListener('pointerup', this.endDrag);
|
||||
document.addEventListener('pointercancel', this.endDrag);
|
||||
document.addEventListener('touchend', this.endDrag);
|
||||
document.addEventListener('touchcancel', this.endDrag);
|
||||
}
|
||||
|
||||
// Stops progress while user is dragging the thumb
|
||||
private handleInput() {
|
||||
this.stopProgress();
|
||||
}
|
||||
private endDrag = () => {
|
||||
document.removeEventListener('pointerup', this.endDrag);
|
||||
document.removeEventListener('pointercancel', this.endDrag);
|
||||
document.removeEventListener('touchend', this.endDrag);
|
||||
document.removeEventListener('touchcancel', this.endDrag);
|
||||
|
||||
this.dragging = false;
|
||||
};
|
||||
|
||||
private setSeekValue(val: number) {
|
||||
if (val < 0) val = 0;
|
||||
|
||||
@@ -4,6 +4,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||
import type WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||
import { PlayerController } from '@store/controllers/player-controller';
|
||||
import { volumeStyleStore } from '@store/volume-style-store';
|
||||
import { designTokens } from '../../../styles/tokens.css';
|
||||
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
|
||||
|
||||
@@ -22,6 +23,12 @@ export class VolumeControl extends LitElement {
|
||||
@state()
|
||||
private showSlider = false;
|
||||
|
||||
/** Whether this is the click-to-open popup rather than a slider. */
|
||||
@state()
|
||||
private popup = volumeStyleStore.popup;
|
||||
|
||||
private unsubscribeStyle?: () => void;
|
||||
|
||||
// Locally-tracked volume while the user is actively dragging or scrolling.
|
||||
// The store's volume only updates once the backend echoes VolumeChanged
|
||||
// (which we debounce), so we track intent here for responsive UI and to let
|
||||
@@ -86,11 +93,31 @@ export class VolumeControl extends LitElement {
|
||||
--thumb-height: 16px;
|
||||
}
|
||||
|
||||
wa-slider::part(track) {
|
||||
.volume-popup wa-slider::part(track) {
|
||||
background: var(--yj-text-primary, white);
|
||||
height: 120px;
|
||||
}
|
||||
|
||||
/* The inline slider (#42). It is the default now, so the width is
|
||||
a real layout decision rather than a detail: 5em is wide enough
|
||||
to aim at and narrow enough that the bottom bar's *outer*
|
||||
columns stay equal without squeezing the transport — which is
|
||||
the arrangement #23 depends on.
|
||||
|
||||
flex-shrink: 0 for the reason the top bar's children have it
|
||||
(#143): a control that quietly gets narrower under pressure
|
||||
hides the fact that the bar has run out of room. This one stands
|
||||
down at phone width instead, in index.css, where the shell can
|
||||
see the viewport. */
|
||||
.inline-slider {
|
||||
width: 5em;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.inline-slider::part(track) {
|
||||
background: var(--yj-text-primary, white);
|
||||
}
|
||||
|
||||
wa-slider::part(indicator) {
|
||||
background: var(--yj-accent, yellow);
|
||||
}
|
||||
@@ -122,8 +149,23 @@ export class VolumeControl extends LitElement {
|
||||
// LIFECYCLE
|
||||
// ===================================================================
|
||||
|
||||
override connectedCallback() {
|
||||
super.connectedCallback();
|
||||
|
||||
this.unsubscribeStyle = volumeStyleStore.subscribe(() => {
|
||||
this.popup = volumeStyleStore.popup;
|
||||
|
||||
// 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();
|
||||
});
|
||||
|
||||
void volumeStyleStore.init();
|
||||
}
|
||||
|
||||
override disconnectedCallback() {
|
||||
super.disconnectedCallback();
|
||||
this.unsubscribeStyle?.();
|
||||
document.removeEventListener('click', this.boundHandleOutsideClick);
|
||||
clearTimeout(this.volumeDebounceTimer);
|
||||
}
|
||||
@@ -154,10 +196,12 @@ export class VolumeControl extends LitElement {
|
||||
private handleOutsideClick(e: Event) {
|
||||
const path = e.composedPath();
|
||||
|
||||
if (!path.includes(this)) {
|
||||
this.showSlider = false;
|
||||
document.removeEventListener('click', this.boundHandleOutsideClick);
|
||||
}
|
||||
if (!path.includes(this)) this.closeSlider();
|
||||
}
|
||||
|
||||
private closeSlider() {
|
||||
this.showSlider = false;
|
||||
document.removeEventListener('click', this.boundHandleOutsideClick);
|
||||
}
|
||||
|
||||
private handleInput(e: Event) {
|
||||
@@ -192,18 +236,46 @@ export class VolumeControl extends LitElement {
|
||||
override render() {
|
||||
const muted = this.player.muted;
|
||||
|
||||
// Inline, the icon is the mute toggle rather than a disclosure:
|
||||
// there is nothing left to disclose, and a button that opens a
|
||||
// popup containing the slider already beside it would be a control
|
||||
// whose only effect is to duplicate its neighbour.
|
||||
const iconAction = this.popup
|
||||
? this.toggleSlider
|
||||
: () => this.player.toggleMute();
|
||||
const iconLabel = this.popup
|
||||
? muted
|
||||
? 'Muted'
|
||||
: `Volume ${this.currentVolume}%`
|
||||
: muted
|
||||
? 'Unmute'
|
||||
: 'Mute';
|
||||
|
||||
return html`
|
||||
<button
|
||||
class=${muted ? 'muted' : ''}
|
||||
title=${muted ? 'Muted — click for volume' : 'Volume'}
|
||||
aria-label=${muted ? 'Muted' : `Volume ${this.currentVolume}%`}
|
||||
aria-label=${iconLabel}
|
||||
data-muted=${muted ? 'true' : 'false'}
|
||||
@click="${this.toggleSlider}"
|
||||
@click="${iconAction}"
|
||||
@wheel="${this.handleWheel}"
|
||||
>
|
||||
<wa-icon name=${this.volumeIcon}></wa-icon>
|
||||
</button>
|
||||
${this.showSlider
|
||||
${!this.popup
|
||||
? html`
|
||||
<wa-slider
|
||||
class="inline-slider ${muted ? 'muted' : ''}"
|
||||
label="Volume"
|
||||
min="0"
|
||||
max="100"
|
||||
.value="${this.currentVolume}"
|
||||
@input="${this.handleInput}"
|
||||
@wheel="${this.handleWheel}"
|
||||
></wa-slider>
|
||||
`
|
||||
: ''}
|
||||
${this.popup && this.showSlider
|
||||
? html`
|
||||
<div
|
||||
class="volume-popup ${muted ? 'muted' : ''}"
|
||||
|
||||
@@ -25,6 +25,8 @@ import {
|
||||
SetQueueFallback,
|
||||
GetAllowMeteredCatalogDownload,
|
||||
SetAllowMeteredCatalogDownload,
|
||||
GetPopupVolume,
|
||||
SetPopupVolume,
|
||||
} from '@go/config/config.js';
|
||||
import { GetIndexStatus } from '@go/explore/service.js';
|
||||
import { notificationStore } from '@store/notification-store';
|
||||
@@ -125,6 +127,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
@state() private concurrencyMode = 'auto';
|
||||
@state() private defaultPage = 'home';
|
||||
@state() private queueFallback = 'favorites';
|
||||
|
||||
@state() private popupVolume = false;
|
||||
@state() private indexStatus: explore.IndexStatus | null = null;
|
||||
/** Three states, not one: the panel used to say "Loading status…"
|
||||
* for the entire session, because the only thing that ever set
|
||||
@@ -938,20 +942,28 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
|
||||
private async loadLibraries(): Promise<void> {
|
||||
try {
|
||||
const [libs, mode, defaultPage, queueFallback, allowMetered] =
|
||||
await Promise.all([
|
||||
GetAllLibrariesWithTrackCounts(),
|
||||
GetScanConcurrency(),
|
||||
GetDefaultPage(),
|
||||
GetQueueFallback(),
|
||||
GetAllowMeteredCatalogDownload(),
|
||||
]);
|
||||
const [
|
||||
libs,
|
||||
mode,
|
||||
defaultPage,
|
||||
queueFallback,
|
||||
allowMetered,
|
||||
popupVolume,
|
||||
] = await Promise.all([
|
||||
GetAllLibrariesWithTrackCounts(),
|
||||
GetScanConcurrency(),
|
||||
GetDefaultPage(),
|
||||
GetQueueFallback(),
|
||||
GetAllowMeteredCatalogDownload(),
|
||||
GetPopupVolume(),
|
||||
]);
|
||||
|
||||
this.libraries = libs ?? [];
|
||||
this.concurrencyMode = mode;
|
||||
this.defaultPage = defaultPage;
|
||||
this.queueFallback = queueFallback;
|
||||
this.allowMeteredCatalogDownload = allowMetered;
|
||||
this.popupVolume = popupVolume;
|
||||
|
||||
} catch (err) {
|
||||
console.error(
|
||||
@@ -1858,10 +1870,51 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
.value=${this.queueFallback}
|
||||
@config-change=${this.handleQueueFallbackChange}
|
||||
></config-field>
|
||||
<config-field
|
||||
.schema=${{
|
||||
key: 'popupVolume',
|
||||
label: 'Volume opens in a popup',
|
||||
description:
|
||||
'Off, the volume slider is always visible in the '
|
||||
+ 'player bar. On, it hides behind the speaker '
|
||||
+ 'icon. The slider stands down on a phone either '
|
||||
+ 'way, where the hardware keys own volume.',
|
||||
type: 'toggle' as const,
|
||||
}}
|
||||
.value=${this.popupVolume}
|
||||
@config-change=${this.handlePopupVolumeChange}
|
||||
></config-field>
|
||||
</config-section>
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The volume control's presentation (#42).
|
||||
*
|
||||
* In General rather than beside the theme because it is about the
|
||||
* transport's behaviour rather than its colours, and next to "When
|
||||
* the Queue Ends" because both are answers to "how should the
|
||||
* player behave".
|
||||
*/
|
||||
private handlePopupVolumeChange = (
|
||||
e: CustomEvent<ConfigFieldChangeEvent>,
|
||||
): void => {
|
||||
const popup = Boolean(e.detail.value);
|
||||
const previous = this.popupVolume;
|
||||
|
||||
this.popupVolume = popup;
|
||||
|
||||
void SetPopupVolume(popup).catch((err: unknown) => {
|
||||
console.error('failed to save the volume control setting', err);
|
||||
this.popupVolume = previous;
|
||||
notificationStore.transient({
|
||||
key: 'popup-volume-setting',
|
||||
title: 'Setting not saved',
|
||||
text: describeError(err, 'That setting could not be saved.'),
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
// --- Navigation section ---
|
||||
|
||||
/**
|
||||
|
||||
@@ -188,6 +188,27 @@ export class NowPlaying extends LitElement {
|
||||
cursor: pointer;
|
||||
/* The art shows through; this is a target, not a picture. */
|
||||
color: transparent;
|
||||
/* **Above the art, or it is not a target at all** (#150).
|
||||
|
||||
This button is absolutely positioned with z-index auto and
|
||||
the art is a *later* sibling, so the two tie on paint order
|
||||
and the later one wins. With an <img> that costs nothing --
|
||||
an image is not a hit-test obstacle here -- but a track with
|
||||
no artwork renders a placeholder wa-icon, which is, and it
|
||||
takes every click aimed at the button underneath it.
|
||||
|
||||
The failure is therefore per *track*, not per build: on a
|
||||
phone the only way into the full-screen now-playing view
|
||||
stopped working whenever the current song had no cover.
|
||||
Measured with elementFromPoint at the button's centre --
|
||||
wa-icon with a placeholder, button.expand with an image, and
|
||||
button.expand either way once this line exists.
|
||||
|
||||
z-index rather than pointer-events: none on the art, which
|
||||
would take the cover preview's mouseenter with it; and
|
||||
rather than reordering the DOM, which would leave the same
|
||||
tie to be won by the same accident in the other direction. */
|
||||
z-index: 1;
|
||||
}
|
||||
|
||||
.expand:focus-visible {
|
||||
|
||||
@@ -277,6 +277,26 @@ export class QueuePanel
|
||||
return this.queue.tracks.length;
|
||||
}
|
||||
|
||||
/**
|
||||
* Repaint the rows when the selection changes.
|
||||
*
|
||||
* `<lit-virtualizer>` renders through the `virtualize` directive,
|
||||
* which reacts to its *own* properties and not to the host having
|
||||
* re-rendered, so host state like a selection reaches the rows only
|
||||
* if it is pushed. `track-list` has always done this and both
|
||||
* playlist views had to be taught it.
|
||||
*
|
||||
* **There is a second, accidental mechanism here and it must not be
|
||||
* mistaken for this one**: `.keyFunction` below is a per-render
|
||||
* arrow, so it is a changed property on every host update and
|
||||
* repaints the rows by itself. Removing *either* alone changes
|
||||
* nothing observable, which is why #43 could not be settled by
|
||||
* reading the code. With both gone the highlight still arrives —
|
||||
* on whatever unrelated render happens next, measured at 134ms,
|
||||
* 3,866ms and 5,816ms against 5–17ms healthy, which a user cannot
|
||||
* tell from broken. `queue-selection.spec.ts` asserts the
|
||||
* *promptness* rather than the eventual state for that reason.
|
||||
*/
|
||||
onSelectionChanged(): void {
|
||||
this.virtualizer?.requestUpdate();
|
||||
}
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { EventsOn } from '@runtime/runtime';
|
||||
import { GetPopupVolume } from '@go/config/config.js';
|
||||
import { Events } from '../events';
|
||||
|
||||
type Subscriber = () => void;
|
||||
|
||||
/**
|
||||
* Whether the volume control is a click-to-open popup (#42).
|
||||
*
|
||||
* The popup was the only option, and "click open, drag, click closed"
|
||||
* is three gestures for a control a bottom bar has room to just show.
|
||||
* So an inline slider is the default and the popup is a setting.
|
||||
*
|
||||
* **The stored flag names the popup, not the slider**, which is the
|
||||
* polarity rule `backend/config` states for every option it has: the
|
||||
* zero value has to be the intended answer. An `InlineVolume bool`
|
||||
* would default to false, hand the popup to every existing install, and
|
||||
* need a migration to say what the default already says.
|
||||
*
|
||||
* It is a store rather than a field on the component because two
|
||||
* components render `<volume-control>` — the bottom bar and the phone's
|
||||
* full-screen now-playing view — and a setting that only reached
|
||||
* whichever one happened to mount after it changed is the fault
|
||||
* `active-view-store` exists to prevent, one surface over.
|
||||
*
|
||||
* The initial value is the *default* rather than a pending answer, so
|
||||
* the first paint is the inline slider and not an empty gap that
|
||||
* 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.
|
||||
*/
|
||||
class VolumeStyleStore {
|
||||
private value = false;
|
||||
|
||||
private loaded = false;
|
||||
|
||||
private subscribers = new Set<Subscriber>();
|
||||
|
||||
constructor() {
|
||||
EventsOn(Events.GeneralConfigChanged, () => {
|
||||
void this.refresh();
|
||||
});
|
||||
}
|
||||
|
||||
/** Whether to draw the popup. Safe to read before `init()`. */
|
||||
get popup(): boolean {
|
||||
return this.value;
|
||||
}
|
||||
|
||||
/** Reads the setting once. Safe to call from every mount. */
|
||||
async init(): Promise<void> {
|
||||
if (this.loaded) return;
|
||||
|
||||
this.loaded = true;
|
||||
|
||||
await this.refresh();
|
||||
}
|
||||
|
||||
subscribe(fn: Subscriber): () => void {
|
||||
this.subscribers.add(fn);
|
||||
|
||||
return () => this.subscribers.delete(fn);
|
||||
}
|
||||
|
||||
private async refresh(): Promise<void> {
|
||||
try {
|
||||
const popup = await GetPopupVolume();
|
||||
|
||||
if (popup === this.value) return;
|
||||
|
||||
this.value = popup;
|
||||
this.notify();
|
||||
} catch (err) {
|
||||
// Nothing to tell the user: the control renders in its
|
||||
// default presentation, which is a working volume control.
|
||||
console.error('failed to read the volume control setting', err);
|
||||
}
|
||||
}
|
||||
|
||||
private notify(): void {
|
||||
for (const fn of this.subscribers) fn();
|
||||
}
|
||||
}
|
||||
|
||||
export const volumeStyleStore = new VolumeStyleStore();
|
||||
@@ -11,7 +11,7 @@ import '@components/audio-player/controls/player-controls';
|
||||
import '@components/audio-player/seekbar/seek-bar';
|
||||
import '@components/audio-player/volume-control/volume-control';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, calls, lastArgs, flush } from '@test/support/harness';
|
||||
import { emit, calls, lastArgs, flush, stub } from '@test/support/harness';
|
||||
import {
|
||||
fixture,
|
||||
shadow,
|
||||
@@ -397,6 +397,111 @@ describe('<seek-bar>', () => {
|
||||
expect(lastArgs('player.Player.Seek')).toEqual([42]);
|
||||
});
|
||||
|
||||
// #164. `handleInput` used to call `stopProgress()` and mutate no
|
||||
// reactive state, so Lit scheduled no update, `updated()` never ran,
|
||||
// and the tail of `updated()` that restarts the interval never
|
||||
// executed. Only a `change` or the next backend report could bring
|
||||
// it back -- so an `input` that never commits froze the clock, which
|
||||
// on a touch device is an ordinary cancelled gesture. With no
|
||||
// reports arriving, that is permanent.
|
||||
it('keeps ticking after a drag that never commits', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const el = await fixture('seek-bar');
|
||||
|
||||
emit(Events.TrackChanged, TRACK);
|
||||
emit(Events.PlaybackStateChanged, { state: 'playing' });
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
await el.updateComplete;
|
||||
|
||||
// A touch lands on the track and is then cancelled: `input`, and
|
||||
// no `change` ever follows.
|
||||
const slider = shadow<HTMLElement & { value: number }>(el, 'wa-slider');
|
||||
|
||||
if (slider) slider.value = 20;
|
||||
|
||||
slider?.dispatchEvent(new Event('input'));
|
||||
await el.updateComplete;
|
||||
|
||||
document.dispatchEvent(new Event('pointerup'));
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await el.updateComplete;
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3000);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(text(el, '[data-testid="elapsed-time"]')).toBe('00:23');
|
||||
});
|
||||
|
||||
// The other half of the same fix: while the thumb is held, a report
|
||||
// arriving once a second used to overwrite `seekValue` and pull it
|
||||
// back out from under the finger.
|
||||
it('leaves the thumb where the finger is while a drag is live', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const el = await fixture('seek-bar');
|
||||
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
|
||||
emit(Events.PlaybackStateChanged, { state: 'playing' });
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await el.updateComplete;
|
||||
|
||||
const slider = shadow<HTMLElement & { value: number }>(el, 'wa-slider');
|
||||
|
||||
if (slider) slider.value = 60;
|
||||
|
||||
slider?.dispatchEvent(new Event('input'));
|
||||
await el.updateComplete;
|
||||
|
||||
emit(Events.PlaybackPositionChanged, {
|
||||
positionSeconds: 4,
|
||||
trackLength: 90,
|
||||
trackChangeId: 20,
|
||||
seq: 7,
|
||||
playing: true,
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:00');
|
||||
});
|
||||
|
||||
// And the drag must not hold the interval hostage once it ends: the
|
||||
// report that was skipped mid-drag is not recorded as seen, so the
|
||||
// next one is still fresh and is applied.
|
||||
it('takes the backend back as the authority once the drag commits', async () => {
|
||||
vi.useFakeTimers();
|
||||
|
||||
const el = await fixture('seek-bar');
|
||||
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 21 });
|
||||
emit(Events.PlaybackStateChanged, { state: 'playing' });
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await el.updateComplete;
|
||||
|
||||
const slider = shadow<HTMLElement & { value: number }>(el, 'wa-slider');
|
||||
|
||||
if (slider) slider.value = 60;
|
||||
|
||||
slider?.dispatchEvent(new Event('input'));
|
||||
await el.updateComplete;
|
||||
|
||||
slider?.dispatchEvent(new Event('change'));
|
||||
await el.updateComplete;
|
||||
|
||||
emit(Events.PlaybackPositionChanged, {
|
||||
positionSeconds: 61,
|
||||
trackLength: 90,
|
||||
trackChangeId: 21,
|
||||
seq: 9,
|
||||
playing: true,
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:01');
|
||||
});
|
||||
|
||||
it('bounds the slider by the track length', async () => {
|
||||
const el = await fixture('seek-bar');
|
||||
|
||||
@@ -434,13 +539,40 @@ describe('<seek-bar>', () => {
|
||||
* be driven by its own event — watching the volume number, as it used
|
||||
* to, meant pressing M visibly did nothing.
|
||||
*/
|
||||
/**
|
||||
* The volume control has two presentations (#42), and the icon button
|
||||
* means a different thing in each — so both are exercised rather than
|
||||
* whichever one happens to be the default.
|
||||
*
|
||||
* Inline is the default: the slider is simply there, which leaves the
|
||||
* icon with nothing to disclose, so it is the mute toggle and is named
|
||||
* after that action. In the popup it is a disclosure, so it is named
|
||||
* after the *state* it is showing.
|
||||
*/
|
||||
describe('volume control: mute', () => {
|
||||
beforeEach(() => {
|
||||
/**
|
||||
* Put the presentation back to the default between tests.
|
||||
*
|
||||
* `volumeStyleStore` is a singleton whose `init()` reads the setting
|
||||
* once, so stubbing the binding inside a test is too late — a
|
||||
* previous test has already loaded it. `GeneralConfigChanged` is the
|
||||
* store's own refresh trigger and the same one the Settings page
|
||||
* fires, so driving it that way exercises the real path instead of
|
||||
* reaching for a test-only reset.
|
||||
*/
|
||||
const setPresentation = async (popup: boolean) => {
|
||||
stub('config.Config.GetPopupVolume', popup);
|
||||
emit(Events.GeneralConfigChanged, {});
|
||||
await flush();
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
await setPresentation(false);
|
||||
emit(Events.VolumeChanged, 40);
|
||||
emit(Events.MuteChanged, false);
|
||||
});
|
||||
|
||||
it('shows a muted glyph and label once the backend reports mute', async () => {
|
||||
it('shows a muted glyph once the backend reports mute', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||
@@ -453,14 +585,53 @@ describe('volume control: mute', () => {
|
||||
expect(shadow(el, 'button wa-icon')?.getAttribute('name')).toBe(
|
||||
'volume-xmark',
|
||||
);
|
||||
});
|
||||
|
||||
it('names the inline icon after the action it performs', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Mute');
|
||||
|
||||
emit(Events.MuteChanged, true);
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Unmute');
|
||||
});
|
||||
|
||||
it('names the popup icon after the state it discloses', async () => {
|
||||
await setPresentation(true);
|
||||
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await el.updateComplete;
|
||||
|
||||
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe(
|
||||
'Volume 40%',
|
||||
);
|
||||
|
||||
emit(Events.MuteChanged, true);
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Muted');
|
||||
});
|
||||
|
||||
it('shows the slider without a click when it is inline', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
// The whole point of the issue: no disclosure to operate first.
|
||||
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
|
||||
});
|
||||
|
||||
it('keeps showing the volume level while muted, because it is unchanged', async () => {
|
||||
await setPresentation(true);
|
||||
emit(Events.MuteChanged, true);
|
||||
await flush();
|
||||
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await el.updateComplete;
|
||||
await click(el, 'button');
|
||||
|
||||
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
|
||||
@@ -468,11 +639,24 @@ describe('volume control: mute', () => {
|
||||
|
||||
it('toggles mute through the backend rather than locally', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await click(el, 'button');
|
||||
|
||||
expect(calls('player.Player.MuteToggle').length).toBe(1);
|
||||
// Nothing optimistic: the icon follows the backend's event.
|
||||
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||
});
|
||||
|
||||
it('toggles mute from inside the popup, where the icon is a disclosure', async () => {
|
||||
await setPresentation(true);
|
||||
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await el.updateComplete;
|
||||
await click(el, 'button');
|
||||
await click(el, '.mute-toggle');
|
||||
|
||||
expect(calls('player.Player.MuteToggle').length).toBe(1);
|
||||
// Nothing optimistic: the icon follows the backend's event.
|
||||
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"log/slog"
|
||||
"os"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/golang-cz/devslog"
|
||||
"github.com/wailsapp/wails/v3/pkg/application"
|
||||
@@ -28,7 +29,61 @@ var (
|
||||
//go:embed all:frontend/dist
|
||||
var frontendDistAssets embed.FS
|
||||
|
||||
// mainStarted latches the first entry into main().
|
||||
//
|
||||
// **On Android main() is called once per *activity*, and the process
|
||||
// outlives the activity.** Wails' JNI entry point is
|
||||
// `nativeInit`, which does two things: it re-points the native
|
||||
// library's global reference at the calling `WailsBridge`, and it runs
|
||||
// `go mainFunc()`. `MainActivity.onCreate` calls it, and Android
|
||||
// recreates the activity — for a configuration change it does not
|
||||
// declare, under memory pressure, or on every single background when
|
||||
// the user has "Don't keep activities" switched on — **without
|
||||
// restarting the process**.
|
||||
//
|
||||
// So main() ran again, on a live app, and every path out of that is
|
||||
// fatal:
|
||||
//
|
||||
// - `application.New` returns the *existing* `globalApplication` when
|
||||
// there is one, silently discarding the second set of Services.
|
||||
// - `app.Run()` then refuses, by design: `a.starting` is still true,
|
||||
// because Android's `platformRun` is `select{}` and never returns.
|
||||
// It answers "application is running or a previous run has failed".
|
||||
// - which lands on `os.Exit(1)` at the foot of this function, and
|
||||
// that takes down the **first**, perfectly healthy app with it —
|
||||
// its database, its queue, and the audio that a foreground service
|
||||
// is holding the process alive to play.
|
||||
//
|
||||
// ActivityManager then restarts the app, which is the report: "crashes
|
||||
// or restarts when reopened after running in the background". It never
|
||||
// left a tombstone because `os.Exit` is not a crash, and it never left
|
||||
// a log line because an Android app's fd 1 goes to /dev/null.
|
||||
//
|
||||
// The latch is the whole fix, and it has to be **first**: everything
|
||||
// below it — `NewYellowJacketApp` above all, which opens the SQLite
|
||||
// database — is work that must not happen twice in one process.
|
||||
// Returning early is not a degraded mode: `nativeInit` has already
|
||||
// re-attached the bridge, so the recreated activity's WebView talks to
|
||||
// the app that is still running, with its queue and its playback
|
||||
// position intact. See CLAUDE.md, "An activity is a view onto the
|
||||
// process".
|
||||
//
|
||||
// It is inert off Android, where a process has exactly one main().
|
||||
var mainStarted atomic.Bool
|
||||
|
||||
// claimMainOnce reports whether this is the first call to main() in
|
||||
// this process. See mainStarted.
|
||||
func claimMainOnce() bool {
|
||||
return mainStarted.CompareAndSwap(false, true)
|
||||
}
|
||||
|
||||
func main() {
|
||||
// Android calls main() once per activity, and the process outlives
|
||||
// the activity. Nothing below this line may run twice.
|
||||
if !claimMainOnce() {
|
||||
return
|
||||
}
|
||||
|
||||
// **Mobile has no home directory, and this must run before anything
|
||||
// asks for a path.** backend/system resolves config and data from
|
||||
// $HOME or the OS equivalent, and on Android there is neither: its
|
||||
|
||||
+104
@@ -0,0 +1,104 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestMainRunsOncePerProcess pins the latch itself.
|
||||
func TestMainRunsOncePerProcess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mainStarted.Store(false)
|
||||
|
||||
if !claimMainOnce() {
|
||||
t.Fatal("the first call to claimMainOnce must claim it")
|
||||
}
|
||||
|
||||
if claimMainOnce() {
|
||||
t.Fatal("a second call to claimMainOnce must not claim it: " +
|
||||
"on Android that second call is a second main() in a live " +
|
||||
"process, and every path out of it ends in os.Exit(1)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMainClaimsBeforeItDoesAnything is the assertion that actually
|
||||
// guards #52, and it is a source sweep for the reason
|
||||
// TestNoDirectRuntimeEmits is: no tier here runs main() on Android, so
|
||||
// nothing else can see work creeping in above the latch.
|
||||
//
|
||||
// The failure it exists for is not the latch being deleted — that is
|
||||
// loud. It is a line being added above it: a second
|
||||
// NewYellowJacketApp opens the SQLite database a second time in one
|
||||
// process, and it would do so on every activity recreation, silently,
|
||||
// on a build that otherwise looks entirely healthy.
|
||||
func TestMainClaimsBeforeItDoesAnything(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
fset := token.NewFileSet()
|
||||
|
||||
file, err := parser.ParseFile(fset, "main.go", nil, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("parse main.go: %v", err)
|
||||
}
|
||||
|
||||
var fn *ast.FuncDecl
|
||||
|
||||
for _, decl := range file.Decls {
|
||||
d, ok := decl.(*ast.FuncDecl)
|
||||
if ok && d.Name.Name == "main" && d.Recv == nil {
|
||||
fn = d
|
||||
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if fn == nil {
|
||||
t.Fatal("no func main in main.go — this test read the wrong file")
|
||||
}
|
||||
|
||||
if len(fn.Body.List) == 0 {
|
||||
t.Fatal("func main is empty")
|
||||
}
|
||||
|
||||
if !claimsMainOnce(fn.Body.List[0]) {
|
||||
t.Fatalf("the first statement of main() must be the "+
|
||||
"`if !claimMainOnce() { return }` guard, got %T — see #52: "+
|
||||
"Android calls main() once per activity, in a process that "+
|
||||
"outlives the activity, so anything above the guard runs "+
|
||||
"again on every recreation", fn.Body.List[0])
|
||||
}
|
||||
}
|
||||
|
||||
// claimsMainOnce reports whether stmt is `if !claimMainOnce() { return }`.
|
||||
func claimsMainOnce(stmt ast.Stmt) bool {
|
||||
ifStmt, ok := stmt.(*ast.IfStmt)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
unary, ok := ifStmt.Cond.(*ast.UnaryExpr)
|
||||
if !ok || unary.Op != token.NOT {
|
||||
return false
|
||||
}
|
||||
|
||||
call, ok := unary.X.(*ast.CallExpr)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
ident, ok := call.Fun.(*ast.Ident)
|
||||
if !ok || ident.Name != "claimMainOnce" {
|
||||
return false
|
||||
}
|
||||
|
||||
if len(ifStmt.Body.List) != 1 {
|
||||
return false
|
||||
}
|
||||
|
||||
_, ok = ifStmt.Body.List[0].(*ast.ReturnStmt)
|
||||
|
||||
return ok
|
||||
}
|
||||
Executable
+196
@@ -0,0 +1,196 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# Install a built APK onto an Android target and launch it, under the
|
||||
# package id the APK itself declares.
|
||||
#
|
||||
# This is the whole body of build/android/Taskfile.yml's four adb-driven
|
||||
# tasks — deploy-emulator, run, run:device, deploy-device — which were
|
||||
# three lines each, written out four times, and wrong in two ways in all
|
||||
# four (#159):
|
||||
#
|
||||
# adb uninstall app.yellowjacket # the RELEASE id, unconditionally
|
||||
# adb install bin/yellowjacket.apk
|
||||
# adb shell am start -n app.yellowjacket/com.wails.app.MainActivity
|
||||
#
|
||||
# **The uninstall is not here and does not come back.** It was there to
|
||||
# make the bare `install` on the next line work at all — without -r,
|
||||
# Android refuses an install over an existing package — so `install -r`
|
||||
# removes the reason for it rather than merely removing it. What is
|
||||
# left is the one case an uninstall really is the remedy, a changed
|
||||
# signing certificate, and that is exactly the case where performing it
|
||||
# silently costs the user their library. So it is *named* and not done:
|
||||
# an error message carrying the command is a decision the person at the
|
||||
# keyboard gets to make, which is the same answer scripts/android-
|
||||
# emulator.sh already reached for `make android-install`.
|
||||
#
|
||||
# **The id is read back from the artifact**, never defaulted, so the
|
||||
# thing installed and the thing launched cannot disagree — see
|
||||
# scripts/android-pkgid.sh for why that is by construction rather than
|
||||
# by discipline.
|
||||
#
|
||||
# **The target is checked against the task's own name.** The emulator
|
||||
# tasks used a bare `adb`, which with one device attached picks that
|
||||
# device whatever it is — so `wails3 task android:run`, whose summary
|
||||
# says "in the Android Emulator", installed on the phone when a phone
|
||||
# was the only thing plugged in. A task addressing something other than
|
||||
# what it says is the same fault as the package id, one level up.
|
||||
#
|
||||
# Usage:
|
||||
# android-deploy.sh --apk <path> --target emulator|device|any \
|
||||
# [--expect <id>] [--serial <s>] [--no-launch]
|
||||
set -euo pipefail
|
||||
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
SDK="${ANDROID_HOME:-${ANDROID_SDK_ROOT:-$HOME/Android/Sdk}}"
|
||||
ADB="$(command -v adb || echo "$SDK/platform-tools/adb")"
|
||||
|
||||
# **Not "$PKG/.MainActivity".** A leading-dot activity is resolved
|
||||
# against the applicationId, and the scaffold's activity lives in the
|
||||
# Java package com.wails.app, which is deliberately not it. The short
|
||||
# form fails with a class-not-found that reads like a broken build.
|
||||
ACTIVITY="${YJ_ANDROID_ACTIVITY:-com.wails.app.MainActivity}"
|
||||
|
||||
APK=""
|
||||
TARGET="any"
|
||||
EXPECT=""
|
||||
SERIAL="${ANDROID_SERIAL:-${DEVICE_ID:-}}"
|
||||
LAUNCH=1
|
||||
|
||||
die() { echo "android-deploy: $*" >&2; exit 1; }
|
||||
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
--apk) APK="${2:-}"; shift 2 ;;
|
||||
--target) TARGET="${2:-}"; shift 2 ;;
|
||||
--expect) EXPECT="${2:-}"; shift 2 ;;
|
||||
--serial) SERIAL="${2:-}"; shift 2 ;;
|
||||
--no-launch) LAUNCH=0; shift ;;
|
||||
*) die "unknown option $1" ;;
|
||||
esac
|
||||
done
|
||||
|
||||
[ -n "$APK" ] || die "--apk is required"
|
||||
[ -f "$APK" ] || die "no such APK: $APK
|
||||
Build one first: wails3 task android:assemble:apk (debug)
|
||||
wails3 task android:package (release)"
|
||||
[ -x "$ADB" ] || command -v adb >/dev/null ||
|
||||
die "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)"
|
||||
|
||||
case "$TARGET" in
|
||||
emulator | device | any) ;;
|
||||
*) die "--target must be emulator, device or any (got '$TARGET')" ;;
|
||||
esac
|
||||
|
||||
# ---------------------------------------------------------------- #
|
||||
# Which package
|
||||
# ---------------------------------------------------------------- #
|
||||
|
||||
# This runs *before* a target is chosen, deliberately: the guard is a
|
||||
# question about the artifact, so it can be answered — and exercised —
|
||||
# with nothing plugged in, and a build whose id is wrong should be
|
||||
# refused whether or not there is anything to install it onto.
|
||||
#
|
||||
# An unreadable APK, or an id that is not the one the caller named, is a
|
||||
# hard stop before anything is installed or launched. Spelled as two
|
||||
# calls rather than one with a conditional argument: an empty array under
|
||||
# `set -u` is an unbound variable in bash 3.2, which is what macOS ships.
|
||||
if [ -n "$EXPECT" ]; then
|
||||
PKG="$(./scripts/android-pkgid.sh "$APK" --expect "$EXPECT")"
|
||||
else
|
||||
PKG="$(./scripts/android-pkgid.sh "$APK")"
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------- #
|
||||
# Which target
|
||||
# ---------------------------------------------------------------- #
|
||||
|
||||
# An emulator serial is "emulator-<port>"; anything else online is a
|
||||
# physical device. That is the same test the device tasks already made,
|
||||
# and the emulator tasks did not make at all.
|
||||
online_matching() {
|
||||
case "$TARGET" in
|
||||
emulator) "$ADB" devices | awk 'NR > 1 && $2 == "device" && $1 ~ /^emulator-/ { print $1 }' ;;
|
||||
device) "$ADB" devices | awk 'NR > 1 && $2 == "device" && $1 !~ /^emulator-/ { print $1 }' ;;
|
||||
any) "$ADB" devices | awk 'NR > 1 && $2 == "device" { print $1 }' ;;
|
||||
esac
|
||||
}
|
||||
|
||||
if [ -z "$SERIAL" ]; then
|
||||
matches="$(online_matching)"
|
||||
count="$(printf '%s' "$matches" | grep -c . || true)"
|
||||
|
||||
if [ "$count" -eq 0 ]; then
|
||||
echo "android-deploy: no ${TARGET/any/attached} target is online." >&2
|
||||
"$ADB" devices | sed '1d;/^$/d;s/^/ /' >&2 || true
|
||||
if [ "$TARGET" = "emulator" ]; then
|
||||
echo " Start one with: make android-emulator" >&2
|
||||
elif [ "$TARGET" = "device" ]; then
|
||||
echo " Plug a phone in and authorise the adb key." >&2
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Several is ambiguous, and picking the first silently is how a
|
||||
# build lands on a target nobody named. The old run:device did
|
||||
# exactly that.
|
||||
if [ "$count" -gt 1 ]; then
|
||||
echo "android-deploy: several $TARGET targets are online — name one." >&2
|
||||
printf '%s\n' "$matches" | sed 's/^/ /' >&2
|
||||
echo " Pass DEVICE_ID=<serial>, or set ANDROID_SERIAL." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
SERIAL="$matches"
|
||||
fi
|
||||
|
||||
# ---------------------------------------------------------------- #
|
||||
# Install
|
||||
# ---------------------------------------------------------------- #
|
||||
|
||||
echo "android-deploy: $APK ($PKG) -> $SERIAL"
|
||||
|
||||
if ! out="$("$ADB" -s "$SERIAL" install -r "$APK" 2>&1)"; then
|
||||
printf '%s\n' "$out"
|
||||
case "$out" in
|
||||
*INSTALL_FAILED_UPDATE_INCOMPATIBLE* | *"signatures do not match"*)
|
||||
cat >&2 <<EOF
|
||||
|
||||
The copy of $PKG already installed was signed with a different key, and
|
||||
Android never allows that as an update.
|
||||
|
||||
The only way forward is an uninstall — **which deletes that app's data**,
|
||||
and for this app that is the user's library, irreversibly. So it is not
|
||||
done for you. If the installed copy is disposable:
|
||||
|
||||
$ADB -s $SERIAL uninstall $PKG
|
||||
|
||||
If it is not — if this is a released build with a real library on it —
|
||||
install the debug variant instead, which carries applicationIdSuffix
|
||||
".dev" and so sits beside it rather than replacing it:
|
||||
|
||||
wails3 task android:assemble:apk
|
||||
EOF
|
||||
;;
|
||||
*INSTALL_FAILED_VERSION_DOWNGRADE*)
|
||||
cat >&2 <<EOF
|
||||
|
||||
The installed copy of $PKG has a higher versionCode than this build.
|
||||
A bare 'make android' builds versionCode 1; a versioned one builds e.g.
|
||||
10301. Either build with a version:
|
||||
|
||||
YJ_VERSION=1.3.1 YJ_VERSION_CODE=10301 make android
|
||||
|
||||
or, if the installed copy is disposable, remove it:
|
||||
|
||||
$ADB -s $SERIAL uninstall $PKG
|
||||
EOF
|
||||
;;
|
||||
esac
|
||||
exit 1
|
||||
fi
|
||||
printf '%s\n' "$out"
|
||||
|
||||
[ "$LAUNCH" -eq 1 ] || exit 0
|
||||
|
||||
"$ADB" -s "$SERIAL" shell am start -n "$PKG/$ACTIVITY"
|
||||
@@ -34,7 +34,21 @@ cd "$(dirname "$0")/.."
|
||||
|
||||
AVD="${YJ_AVD:-yj-test}"
|
||||
SDK="${ANDROID_SDK_ROOT:-${ANDROID_HOME:-$HOME/Android/Sdk}}"
|
||||
PKG="${YJ_ANDROID_PKG:-app.yellowjacket}"
|
||||
# The third declaration of the app's identity, and the one #159 did not
|
||||
# cash out in -- but the same hazard, so it is derived rather than
|
||||
# written down too. The APK in bin/ is what `make android-install` is
|
||||
# about to install and what `android-launch`, `logs` and `smoke` are
|
||||
# about to address, so it is the authority; whatever Gradle resolved the
|
||||
# applicationId to, suffix included, is in the file.
|
||||
#
|
||||
# The literal survives only as the answer for a tree with no APK built
|
||||
# yet, where these commands are asking about whatever is already on the
|
||||
# device and there is nothing to read. YJ_ANDROID_PKG still overrides.
|
||||
PKG="${YJ_ANDROID_PKG:-}"
|
||||
if [ -z "$PKG" ] && [ -f bin/yellowjacket.apk ]; then
|
||||
PKG="$(./scripts/android-pkgid.sh bin/yellowjacket.apk 2>/dev/null || true)"
|
||||
fi
|
||||
PKG="${PKG:-app.yellowjacket}"
|
||||
# Where `make android-inspect` forwards the WebView's devtools socket.
|
||||
CDP_PORT="${YJ_ANDROID_CDP_PORT:-9222}"
|
||||
# **Not "$PKG/.MainActivity".** A leading-dot activity is resolved
|
||||
@@ -281,15 +295,24 @@ cmd_inspect() {
|
||||
need_sdk
|
||||
pick_device || die "no device -- plug a phone in (USB debugging on) or run 'make android-emulator'"
|
||||
|
||||
local pkg pid
|
||||
local pkg pid candidates
|
||||
pid=""
|
||||
|
||||
for pkg in "$PKG.dev" "$PKG"; do
|
||||
# Debug sibling first, release second, whichever way round $PKG was
|
||||
# resolved -- it is read from the built APK now, so it is already the
|
||||
# .dev id whenever a debug build is what is in bin/, and appending a
|
||||
# second ".dev" to it would probe a package that cannot exist.
|
||||
case "$PKG" in
|
||||
*.dev) candidates="$PKG ${PKG%.dev}" ;;
|
||||
*) candidates="$PKG.dev $PKG" ;;
|
||||
esac
|
||||
|
||||
for pkg in $candidates; do
|
||||
pid=$("$ADB" shell pidof "$pkg" 2>/dev/null | tr -d '\r' | awk '{print $1}')
|
||||
[ -n "$pid" ] && break
|
||||
done
|
||||
|
||||
[ -n "$pid" ] || die "neither $PKG.dev nor $PKG is running; launch it first"
|
||||
[ -n "$pid" ] || die "none of: $candidates is running; launch it first"
|
||||
|
||||
"$ADB" forward --remove-all >/dev/null 2>&1 || true
|
||||
"$ADB" forward "tcp:$CDP_PORT" "localabstract:webview_devtools_remote_$pid" >/dev/null \
|
||||
|
||||
Executable
+97
@@ -0,0 +1,97 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# Print the package id an APK actually declares — and, given --expect,
|
||||
# refuse when that is not the id the caller was about to act on.
|
||||
#
|
||||
# This exists because the identity is declared twice and nothing made
|
||||
# the two agree. `applicationId` in build/android/app/build.gradle is
|
||||
# what Gradle installs; `APP_ID` in build/android/Taskfile.yml was what
|
||||
# every adb-driven task uninstalled, launched and filtered. They differ
|
||||
# for a reason nobody has to get wrong: the debug buildType carries
|
||||
# `applicationIdSuffix ".dev"`, so a debug build is app.yellowjacket.dev
|
||||
# while the default was app.yellowjacket — the *release* id, and on a
|
||||
# real phone the released app with the user's library on it (#159).
|
||||
#
|
||||
# So the id is read back from the artifact rather than written down a
|
||||
# third time. The APK is the authority because the task that installs
|
||||
# it has just built it: whatever Gradle resolved the applicationId to,
|
||||
# suffixes and flavours included, is in the file, and no default can
|
||||
# disagree with it.
|
||||
#
|
||||
# Usage:
|
||||
# android-pkgid.sh <apk> [--expect <id>]
|
||||
#
|
||||
# Exit codes: 0 printed the id; 1 could not read it; 2 --expect failed.
|
||||
set -euo pipefail
|
||||
|
||||
die() { echo "android-pkgid: $*" >&2; exit 1; }
|
||||
|
||||
APK=""
|
||||
EXPECT=""
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
--expect) EXPECT="${2:-}"; shift 2 ;;
|
||||
-*) die "unknown option $1" ;;
|
||||
*) APK="$1"; shift ;;
|
||||
esac
|
||||
done
|
||||
|
||||
[ -n "$APK" ] || die "usage: android-pkgid.sh <apk> [--expect <id>]"
|
||||
[ -f "$APK" ] || die "no such APK: $APK"
|
||||
|
||||
# aapt2 lives under build-tools/<version>/, which is versioned, so it is
|
||||
# resolved rather than pinned. PATH first, so a system aapt2 (Arch ships
|
||||
# one) works without an SDK layout at all.
|
||||
find_aapt() {
|
||||
local sdk name
|
||||
for name in "$@"; do
|
||||
command -v "$name" 2>/dev/null && return 0
|
||||
done
|
||||
sdk="${ANDROID_HOME:-${ANDROID_SDK_ROOT:-$HOME/Android/Sdk}}"
|
||||
for name in "$@"; do
|
||||
ls "$sdk"/build-tools/*/"$name" 2>/dev/null | sort -V | tail -1 | grep . && return 0
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
pkg=""
|
||||
|
||||
# `aapt2 dump packagename` answers in one word and is the cheapest of
|
||||
# the three. aapt1 is the fallback because it is what older build-tools
|
||||
# carry and what the issue's own measurement used.
|
||||
if AAPT2="$(find_aapt aapt2)"; then
|
||||
pkg="$("$AAPT2" dump packagename "$APK" 2>/dev/null | head -1 | tr -d '\r')" || true
|
||||
fi
|
||||
|
||||
if [ -z "$pkg" ] && AAPT="$(find_aapt aapt)"; then
|
||||
pkg="$("$AAPT" dump badging "$APK" 2>/dev/null |
|
||||
sed -n "s/^package: name='\([^']*\)'.*/\1/p" | head -1)" || true
|
||||
fi
|
||||
|
||||
# Guessing here is the bug this file exists to prevent, so an unreadable
|
||||
# APK is a hard failure and never a fallback to a written-down default.
|
||||
if [ -z "$pkg" ]; then
|
||||
die "could not read a package name from $APK.
|
||||
Install the SDK build-tools (aapt2), or set ANDROID_HOME to an SDK
|
||||
that carries them: sdkmanager 'build-tools;34.0.0'"
|
||||
fi
|
||||
|
||||
if [ -n "$EXPECT" ] && [ "$EXPECT" != "$pkg" ]; then
|
||||
cat >&2 <<EOF
|
||||
android-pkgid: refusing to act on a package this APK does not declare.
|
||||
|
||||
the APK declares: $pkg
|
||||
the task expects: $EXPECT
|
||||
APK: $APK
|
||||
|
||||
These must agree, and when they do not it is the *expectation* that is
|
||||
wrong: the APK is what Gradle built. A debug build carries
|
||||
applicationIdSuffix ".dev" (app/build.gradle), so a task that assembles
|
||||
a debug APK and then addresses the unsuffixed id is addressing the
|
||||
released app — which on a real device is the user's install, with their
|
||||
library in it (#159).
|
||||
EOF
|
||||
exit 2
|
||||
fi
|
||||
|
||||
printf '%s\n' "$pkg"
|
||||
Reference in New Issue
Block a user