Compare commits

...
Author SHA1 Message Date
logan 998ce75fb6 docs(android): the identity is read back, not declared twice
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m32s
CI / e2e (pull_request) Successful in 8m22s
android-tier.md carried a warning block telling the reader not to use
run:device or deploy-device, and offered a manual sequence instead.
Both are wrong now: the tasks are the way in, and the warning would
read as a live hazard. It becomes a note about what changed, and the
manual sequence stays as the smallest thing that works when you want no
script between you and adb.

"The identity is declared twice" was the section this file had carried
for five phases saying nothing enforced that the two ids agree. It
describes the enforcement now, plus what APP_ID means since it stopped
being a setting it never was.

NOTES.md takes the four measurements: that the uninstall existed only
to cover a missing -r (which is what makes deleting it a fix rather
than a trade), that the emulator tasks installed on a phone, what
reading the id back costs, and the boot-wait race filed as #162.
2026-08-20 14:35:16 -04:00
logan 4b392cb4c4 fix(android): point the emulator script at the built APK's id
The third declaration of the app's identity, and the one #159 did not
cash out in: PKG defaulted to "app.yellowjacket" while
`make android-install` installs whatever is in bin/, which after
`wails3 task android:assemble:apk` is app.yellowjacket.dev. So
android-launch, android-logs and android-smoke addressed a package the
build had not produced, and the certificate-change message named the
wrong id to uninstall -- the release one.

It is derived from bin/yellowjacket.apk the same way the tasks are, so
it follows whichever variant was built last. YJ_ANDROID_PKG still
overrides, and the literal survives only for a tree with no APK yet,
where these commands are asking about whatever is already installed and
there is nothing to read.

cmd_inspect's probe order goes with it: "$PKG.dev" would append a
second suffix to an id that already carries one, so the candidates are
derived from the resolved id in either direction -- debug sibling
first, release second, as before.
2026-08-20 14:35:08 -04:00
logan 8d2109b87e fix(android): install and launch the package the APK declares
The four adb-driven tasks in build/android/Taskfile.yml began with
`adb uninstall {{.APP_ID}}`, where APP_ID defaulted to
"app.yellowjacket" -- the release id. `run` and `run:device` build the
*debug* variant, whose applicationIdSuffix makes it
"app.yellowjacket.dev", so both uninstalled the user's released app,
took the library with it, installed a different package, and then
failed to launch the one they had just removed.

The id is read back from the built APK now (scripts/android-pkgid.sh,
`aapt2 dump packagename`) rather than written down a second time, so
the thing installed and the thing launched agree by construction --
whatever Gradle resolved the applicationId to, suffixes included, is in
the file. An APK it cannot read is a hard failure and never a fallback
to a default; guessing is the bug. APP_ID survives with no default as
an *assertion*: it is checked against the artifact and refused, naming
both, before anything is installed or a target is even chosen.

The uninstall is gone rather than corrected. It was there to make the
bare `install` on the next line work at all -- Android refuses an
install over an existing package without -r -- so `install -r` removes
the reason for it. What is left is the one case an uninstall is really
the remedy, a changed signing certificate, and that is exactly the case
where doing it silently costs the user their library. So it is reported
with the command to run, which is the answer scripts/android-emulator.sh
had already reached for `make android-install`.

And the emulator tasks now say "emulator" to adb. A bare `adb install`
with 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 -- the same data loss,
from the task whose name gives no warning. Several matching targets is
an error naming them rather than a silent pick of the first.

Closes #159
2026-08-20 14:34:59 -04:00
logan b741b01cdf Merge pull request 'Android: run main() once per process, not once per activity' (#161) from fix/52-android-activity-recreation-restarts-the-process into main
CI / check (push) Successful in 2m36s
CI / e2e (push) Successful in 8m19s
2026-08-20 17:18:19 +00:00
logan 8a757c9bb4 docs(android): record the lifecycle model and the device check
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Successful in 8m9s
The lifecycle answer is load-bearing, so CLAUDE.md states it: an
activity is a view onto the process, and main() runs once per process.
The "restore the session or cold-start" question the issue asks for a
decision on is settled by playback rather than by preference -- the
audio lives in the Go process, so a cold start on every recreation
stops the music mid-song, which is the thing the foreground service
exists to prevent.

android-tier.md's build table said "arm64, real device -- unverified,
still" for five phases. It is verified now, on a Light Phone III
(Android 14, arm64-v8a, WebView Chrome 113 at 424x439), and what the
run found is a lifecycle section: how to force an activity recreation
on demand, the three-line logcat signature, why `has died: fg TOP` is
not a memory kill, and the second assertion that surviving does not
imply working.

It also carries the correction that "Don't keep activities" -- the
report's own suggested lever -- does not work on this device at all,
so nobody spends an afternoon on it. A configuration change the
manifest does not declare does, in one line.

And it stops recommending `wails3 task android:run:device`, which
uninstalls the released app and the user's library to install a build
with a different id (#159), in favour of the manual sequence.

NOTES.md carries the measurements, dated: 8 of 8 recreations fatal
before, 5 of 5 survived after, and the note that runs where no
recreation happened are inconclusive rather than passes -- a harness
that does not check for the second bridge init reports those as green
and reads as flakiness.

Refs #52, #159, #160
2026-08-20 13:04:18 -04:00
logan d714bd7090 fix(android): keep the Go app alive when the activity is destroyed
onDestroy called bridge.shutdown(), which is the natural reading of the
callback and is wrong for this app twice over. Android destroys and
recreates an activity 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 mediaPlayback foreground
service holds the process alive for. Either way, tearing the Go side
down here stops the music.

It was harmless only by accident, and that is worth writing down:
nativeShutdown calls App.Quit(), whose Android destroy() is an empty
method, and Run()'s deferred shutdownServices() cannot fire because
platformRun is `select{}` and never returns. So **no ServiceShutdown has
ever run on Android**. Removing the call changes nothing today; it stops
the day someone implements destroy() from silently killing playback on a
rotation. There is no callback for the process going away -- Android
just kills it -- so durability here is the persist writers, which submit
on every mutation rather than at exit.

WailsBridge.initialize gains the comment for the trap next to it.
Making `initialized` static is the obvious reading of "initialise once
per process" and is wrong: nativeInit also stores the global JNI
reference to *this* bridge, so skipping it leaves Go executing
JavaScript against the destroyed activity's WebView, and the app opens,
renders, and never receives another backend event. The half that must
not repeat is latched in Go instead -- which is also where the damage
was, and the only place that can see it.

Refs #52
2026-08-20 13:04:04 -04:00
logan d64b069053 fix(android): run main() once per process, not once per activity
Wails' Android entry point is `nativeInit`, which `MainActivity.onCreate`
calls, and it runs `go mainFunc()` every time. 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.

Every path out of that is fatal. `application.New` returns the existing
app rather than building a second one, `app.Run()` then refuses because
`a.starting` is still true behind Android's `select{}`, and the
`os.Exit(1)` under that error takes the **first**, healthy app down with
it: its database, its queue, and the audio a mediaPlayback foreground
service is holding the process alive to play. ActivityManager restarts
the app, which is the report.

Measured on a Light Phone III (Android 14, arm64): conditional on the
activity actually being recreated, the process died 8 times out of 8.
The runs that "passed" were runs where no recreation happened, which is
the whole of the report's "sometimes". After this, 5/5 recreations
survive on one pid, plus six background/foreground cycles.

It never left evidence because os.Exit is not a crash: no tombstone, no
AndroidRuntime stack, nothing in `logcat -b crash`, and the slog line
naming the error went to /dev/null with the rest of fd 1.

The latch is first in main() because everything below it -- above all
NewYellowJacketApp, which opens the SQLite database -- is work that must
not happen twice in one process. It is inert off Android.

Returning early is not a degraded mode: nativeInit has already
re-pointed the JNI reference at the new bridge, so the recreated
WebView talks to the app that is still running, with its queue and
playback position intact. Verified by hooking dispatchWailsEvent on the
recreated page: IndexStatusChanged, JobsChanged, android:storageAccess.

No tier here runs main() on Android, so the guard is a source sweep, in
the spirit of TestNoDirectRuntimeEmits. The failure it exists for is not
the latch being deleted -- that is loud -- but a line creeping in above
it.

Closes #52
2026-08-20 13:03:51 -04:00
logan 5490b2423e Merge pull request 'Put the phone Now Playing button above the artwork' (#158) from fix/150-expand-button-under-the-art into main
CI / check (push) Successful in 2m29s
CI / e2e (push) Successful in 8m3s
The button tied with the cover placeholder on paint order and lost, so
it did not work for any track without artwork.

Closes #150
2026-08-20 05:47:46 +00:00
logan ffc9490a32 fix(player): put the phone's Now Playing button above the artwork
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 8m1s
`.expand` is the phone's only route into the full-screen now-playing
view. It is absolutely positioned with `z-index: auto` over
`.cover-art`, which is a *later* sibling with the same z-index, so the
two tie on paint order and the later one wins. An `<img>` costs nothing
there; a track with no artwork renders a placeholder `wa-icon`, which
takes every click aimed at the button underneath it.

So the control did not work whenever the current song had no cover, on
the one platform that has no other way in. Nothing to do with the
fixture: any library has untagged files.

Measured at 390px with elementFromPoint at the button's centre — the
icon with a placeholder, the button with an image, and the button
either way with the z-index. Chosen over `pointer-events: none` on the
art, which would take the cover preview's mouseenter with it, and over
reordering the DOM, which leaves the same tie to be won by the same
accident in the other direction.

This was filed as an e2e flake, and the diagnosis was wrong: it failed
on both engines three times across two branches that could not have
caused it, and passed on re-run each time, because the spec starts the
*first* row of the track list and which track that is depends on the
order the scan inserted rows — the same root cause as #156. The new
spec picks a track *for* having no artwork, and asserts the placeholder
is rendered rather than assuming it, so it cannot quietly go back to
measuring the easy case.

Two things it has to get right, both already documented traps: the
track must be the 90-second one, since a 2-second one finishes before
the assertions run; and `library.Track.CoverArt` is empty for all 31
fixture rows, so "the first track with no cover art" selects nothing in
particular and picked a short one.

Verified by mutation: without the z-index the new spec fails on the
click in 30s, and the pre-existing one beside it passes, which is
exactly how this survived.

Closes #150
2026-08-20 01:33:35 -04:00
logan dc6625d33a Merge pull request 'Centre the transport, and show the volume inline' (#155) from feat/42-inline-volume-and-centred-transport into main
CI / check (push) Successful in 2m28s
CI / e2e (push) Successful in 8m21s
Three columns whose outer two match, so the middle is centred; the
volume moves into the bar as a slider, with the popup as a setting.

Closes #23
Closes #42
2026-08-20 05:22:03 +00:00
logan dc8db159f9 feat(player): centre the transport and show the volume inline
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m32s
CI / e2e (pull_request) Successful in 8m18s
Two issues over one bar, because they are one relayout. #42's own
findings say so: giving wa-slider a label grows it 6px to 14px and
moves the transport, which is #23's subject, so doing them in sequence
means measuring the bar twice and throwing the first set away.

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 width. The outer two tracks are
the same expression now, so the middle is centred by construction.

The side width is the metadata's, capped at a quarter of the bar, and
the cap was measured as a regression before it was a decision:
reserving the full `--now-playing-width` on both sides is perfectly
centred and takes the seek bar's track from 257px to 61px at 800px, and
to 0 at 200% text. The control you drag was paying for the symmetry.
With the cap it is 246, which is parity. It is a `min()` rather than a
breakpoint because that variable is user state — the metadata has a
drag handle — and tying both sides to it is also what keeps dragging
meaningful; a plain `1fr … 1fr` centres just as well and silently makes
the handle a no-op.

The volume moved out of `audio-player` into the bar because the
transport column has to hold the transport and nothing else, and it
joins the queue button in one cell rather than a second column, since
the centring compares columns.

It is a slider by default and a popup by setting. The stored flag names
the *popup*, which is this config's polarity rule — the zero value has
to be the intended answer, so an existing config.toml gets the new
default with no migration. Inline, the icon is the mute toggle and is
named after that action rather than the state, because with the slider
beside it there is nothing to disclose; the component tier now covers
both presentations rather than whichever is default.

Three nested rules in this block began with a bare element selector,
which Chrome 120 relaxed and the phone's Chrome 113 **silently drops** —
including the ellipsis on the bar's own title and artist, which has
therefore never truncated on the device. They are `&`-prefixed now.
Filed as #154 for the class and for a check.

`bottom-bar.spec.ts` pins both halves separately on purpose: an
uncapped build is perfectly centred and fails only the seek-bar width,
so a spec asserting centring alone would have passed the regression
above. Both were verified by mutation.

Closes #23
Closes #42
2026-08-20 00:40:52 -04:00
logan 86e7444603 Merge pull request (#157) from fix/156-queue-selection-fixture-order into main
CI / check (push) Successful in 2m26s
CI / e2e (push) Successful in 7m53s
The spec asked for the first few tracks and needed one with an album.

Closes #156
2026-08-20 04:35:09 +00:00
logan 2365806d18 fix(e2e): ask the fixture for a track that can navigate
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 8m4s
`queue-selection`'s name-click test failed on main on both engines,
having passed in its own PR and in two consecutive local suite runs. I
added it in #152; this is my defect and it had main red.

It staged a queue from the first few rows of `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 — `01 Tone A`,
`02 Tone B`, `Title Only`, `no-tags-at-all` — and a name with nothing
to route to renders as plain text rather than as a link.

Which tracks arrive first is `audio_files.id` order, which is the order
the *scan* inserted them, which depends on concurrency and directory
traversal. Locally the first eight are all from two proper albums; CI
rebuilds its seed with a real scan and got a different eight. The
fixture had a requirement it did not state, so the queue now asks for
tracks that have an album.

A loose locator is what turned that into a mystery rather than a
message. The row was located with `.explore-link` and `first()`, and a
row has two — title and artist. With the title as plain text, `first()`
silently resolved to the *artist* link, so the click went somewhere
real and the assertion was about a destination the test had never
exercised. It names `.track-title .explore-link` now.

Reproduced before fixing, by staging the CI condition deliberately: a
no-album track at row 2 fails the test in 30s on this machine, and the
filtered fixture passes in 752ms.

The Direction's sweep found one other spec slicing `GetTracks` —
`queue-reorder`, which asserts on order alone and needs no property of
the tracks it gets, so it is left as it is.

Closes #156
2026-08-20 00:23:31 -04:00
logan 7d348f243a Merge pull request (#153) from fix/151-fuse-the-scroll-guard-and-the-write into main
CI / check (push) Successful in 2m35s
CI / e2e (push) Failing after 8m41s
Removes the window between the scroll guard and the write it guards.

Closes #151
2026-08-20 03:58:04 +00:00
logan ddd04623f7 test(e2e): fuse the scroll guard and the write it guards
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 8m3s
`album-dropdown`'s "can be scrolled" failed twice over two sessions with
`Expected 80, Received 10`, both times on a branch that could not have
caused it. #133 strengthened the guard from "scrollable at all" to "has
the range this assertion needs", which was necessary and cannot be
sufficient: the guard and the write are separate round trips, so the
page re-lays-out between them.

Measured every frame across the resize, three runs: the range goes 0 →
**88** at 1ms → 330 settled by 8-14ms. 88 satisfies a guard asking for
80 while the grid is still a pass from done, so the guard is capable of
passing on a layout that is about to move. Under full-suite load the
transient is worse — the observed failures read 10 — which is why this
shows up on the second run of a suite and not in ten consecutive runs
of the file alone (0/10 before the change and after it; isolation is
not where this lives).

So the probe sets `scrollTop` and returns what it reads back, in one
page-side call, and the poll retries that. The assertion is now about
what the grid did rather than about what it was ready to do, and there
is no window between deciding and doing for anything to happen in.

#133's own last line asked for the other viewport-shrinking specs to be
swept for the same shape. One had it: `layout-overflow`'s sidebar probe
already fused its scroll and its measurement into one evaluate but ran
it once, so it read whatever the sidebar happened to be doing after the
resize. It is polled now — safe to repeat, because scrolling to the
bottom twice is scrolling to the bottom.

Closes #151
2026-08-19 23:37:41 -04:00
logan 9ad1477b1e Merge pull request 'Pin the queue panel'''s mouse model' (#152) from fix/43-queue-panel-selection into main
CI / check (push) Successful in 2m30s
CI / e2e (push) Successful in 8m0s
Pins the queue panel's single-click select, ctrl/shift extend and
double-click play, none of which were covered in either tier.

Closes #43
2026-08-20 03:14:30 +00:00
30 changed files with 2090 additions and 171 deletions
@@ -194,9 +194,13 @@ like the app's fault and none is:
|---|---|---| |---|---|---|
| x86_64 | modernc's raw `lstat` vs seccomp | SIGSYS, syscall 6 | | x86_64 | modernc's raw `lstat` vs seccomp | SIGSYS, syscall 6 |
| arm64, translated | Go reads `ID_AA64ISAR0_EL1` | SIGILL | | 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 ### 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 package, which matters for the same reason `backend/events` is split by
the `indexbuild` tag. 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 ### What is still not done
The shell is still a desktop shell, and the x86_64 half of the APK is 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 `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: 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 # debug build + emulator install + launch
wails3 task android:run:device # same, first connected physical device wails3 task android:run:device # debug build + install + launch on a phone
wails3 task android:deploy-device # production APK to a device wails3 task android:deploy-device # release build, same
wails3 task android:bundle:fat # AAB, for a Play Store upload wails3 task android:bundle:fat # AAB, for a Play Store upload
wails3 task android:studio # open build/android/ in Android Studio wails3 task android:studio # open build/android/ in Android Studio
wails3 task android:device:list wails3 task android:device:list
@@ -260,6 +284,16 @@ wails3 task android:logs:all
wails3 task android:clean 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 Two are deliberately **not** wrapped. `android:logs` greps logcat for
`(Wails|yellowjacket)`, which catches the `WailsBridge` tag but misses `(Wails|yellowjacket)`, which catches the `WailsBridge` tag but misses
the app's own process tag (`app.yellowjacket` — lowercase, so `Wails` 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 returns, with no pidfile and no boot wait, so it cannot be stopped or
sequenced. 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 `applicationId` in `build/android/app/build.gradle` is what Gradle
installs. `APP_ID` in `build/android/Taskfile.yml` is what every installs; `APP_ID` in `build/android/Taskfile.yml` was what every
adb-driven task uninstalls, launches and filters. **Nothing enforces adb-driven task uninstalled, launched and filtered, and nothing
that they agree**, and `ANDROID.md`'s advice to set `APP_ID` in enforced that they agree. They did not: the debug buildType carries
`build/config.yml` does not work in beta.8 — `wails3 task` never reads `applicationIdSuffix ".dev"`, so every task that assembles a debug APK
that file (verified with `--dry`), and even when set it feeds only the addressed the release id. This file flagged the hazard for five phases
adb commands, never Gradle. Change both or the official `run`/`deploy` and it cashed out twice — once as a wrong `am start`, once as an
tasks address a package that is not installed. 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 Related, and it will bite once: the launcher activity is
`com.wails.app.MainActivity` and the applicationId 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 class-not-found that reads like a broken build. Always the
fully-qualified form. 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 ## What only a device can answer
The emulator cannot run this app (three separate reasons, none of them 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 permission dialogs, the keyboard — not about what the app draws. The
drawing is what the other five tiers already cover. 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 ## Asking the device, not just looking at it
A real phone can be inspected, and that turns this tier from "reported A real phone can be inspected, and that turns this tier from "reported
+351
View File
@@ -3891,3 +3891,354 @@ scan takes a minute and is worth running before demoting anybody's links
— `explore-album-details`'s tracklist (number / title / artist / — `explore-album-details`'s tracklist (number / title / artist /
duration) is the one that plausibly *is* mostly link, and is the one duration) is the one that plausibly *is* mostly link, and is the one
#5 is about to add selection to. #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** |
| 814 | 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.
+98
View File
@@ -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 still exists and `OnShutdown` has neither a context nor a
window. 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`, `internalServiceMethods` auto-excludes `ServiceStartup`,
`ServiceShutdown`, `ServiceName` and `ServeHTTP` from bindings, so this `ServiceShutdown`, `ServiceName` and `ServeHTTP` from bindings, so this
shape **removed** 12 spurious bindings and the bogus `context` model 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, whole thing is for — a ResizeObserver on the header alone never fires,
so every element child is observed too. 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 **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 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 899 and 700px at 900 — the narrowest content area any desktop width
+43
View File
@@ -666,6 +666,49 @@ func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error {
return nil 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 // GetViewVisibility reports which primary views the sidebar should
// show, answered for every known view rather than only the ones the // show, answered for every known view rather than only the ones the
// config mentions -- so the frontend filters on a value and never has // config mentions -- so the frontend filters on a value and never has
+40
View File
@@ -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")
}
}
+10
View File
@@ -54,6 +54,16 @@ type GeneralConfig struct {
// so an existing config with no such key refuses by default rather // so an existing config with no such key refuses by default rather
// than needing a migration to become careful. // than needing a migration to become careful.
AllowMeteredCatalogDownload bool `toml:"AllowMeteredCatalogDownload"` 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. // ApplyDefaults fills zero-value fields with sensible defaults.
+25 -55
View File
@@ -4,18 +4,28 @@ includes:
common: ../Taskfile.yml common: ../Taskfile.yml
vars: vars:
# The *installed* package name, which every adb-driven task below uses # APP_ID is an *assertion*, not a setting, and it has no default.
# to uninstall, launch and filter. It must agree with `applicationId`
# in app/build.gradle, and nothing enforces that.
# #
# ANDROID.md says to set this in build/config.yml. That does not work # It used to be the id every adb-driven task below uninstalled,
# in beta.8, checked both ways: `wails3 task` builds its var set from # launched and filtered, defaulting to "app.yellowjacket". It could
# CLI KEY=VALUE arguments and the Taskfile tree only -- nothing reads # never have been a setting: `wails3 task` builds its var set from CLI
# config.yml -- and even when set it feeds only these adb commands, # KEY=VALUE arguments and the Taskfile tree only -- nothing reads
# never Gradle. So the identity is declared twice, here and in # build/config.yml, contrary to ANDROID.md, checked with --dry -- and
# build.gradle, and a change to one alone means the official run and # even when set it fed only the adb commands, never Gradle. So the
# deploy tasks address a package that is not installed. # identity was declared twice, here and as `applicationId` in
APP_ID: '{{.APP_ID | default "app.yellowjacket"}}' # 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' MIN_SDK: '21'
TARGET_SDK: '35' TARGET_SDK: '35'
# The emulator runs the host architecture; physical devices are arm64 # The emulator runs the host architecture; physical devices are arm64
@@ -372,9 +382,7 @@ tasks:
ARCH: '{{.ARCH | default .HOST_ARCH}}' ARCH: '{{.ARCH | default .HOST_ARCH}}'
cmds: cmds:
- task: ensure-emulator - task: ensure-emulator
- '"{{.ADB}}" uninstall {{.APP_ID}} 2>/dev/null || true' - './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target emulator{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
- '"{{.ADB}}" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"'
- '"{{.ADB}}" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity'
run: run:
summary: Build, install and launch a debug build in the Android Emulator summary: Build, install and launch a debug build in the Android Emulator
@@ -383,9 +391,7 @@ tasks:
- task: build - task: build
cmds: cmds:
- task: assemble:apk - task: assemble:apk
- '"{{.ADB}}" uninstall {{.APP_ID}} 2>/dev/null || true' - './scripts/android-deploy.sh --apk "{{.BIN_DIR}}/{{.APP_NAME}}.apk" --target emulator{{if .APP_ID}} --expect "{{.APP_ID}}"{{end}}'
- '"{{.ADB}}" install "{{.BIN_DIR}}/{{.APP_NAME}}.apk"'
- '"{{.ADB}}" shell am start -n {{.APP_ID}}/com.wails.app.MainActivity'
device:list: device:list:
summary: Lists connected Android devices and emulators (serials) summary: Lists connected Android devices and emulators (serials)
@@ -400,25 +406,7 @@ tasks:
ARCH: arm64 ARCH: arm64
cmds: cmds:
- task: assemble:apk - task: assemble:apk
- | - './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}}'
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
preconditions: preconditions:
- sh: '[ -x "{{.ADB}}" ] || command -v adb' - sh: '[ -x "{{.ADB}}" ] || command -v adb'
msg: "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)" msg: "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)"
@@ -430,25 +418,7 @@ tasks:
vars: vars:
ARCH: arm64 ARCH: arm64
cmds: cmds:
- | - './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}}'
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
preconditions: preconditions:
- sh: '[ -x "{{.ADB}}" ] || command -v adb' - sh: '[ -x "{{.ADB}}" ] || command -v adb'
msg: "adb not found. Install the Android SDK platform-tools (or set ANDROID_HOME)" 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 @Override
protected void onDestroy() { protected void onDestroy() {
super.onDestroy(); super.onDestroy();
unregisterSystemEventReceivers(); unregisterSystemEventReceivers();
if (bridge != null) {
bridge.shutdown();
}
if (webView != null) { if (webView != null) {
webView.destroy(); 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() { public void initialize() {
if (initialized) { if (initialized) {
+47 -29
View File
@@ -110,27 +110,30 @@ test.describe('the album dropdown', () => {
await app.setViewportSize({ width: 900, height: 600 }); await app.setViewportSize({ width: 900, height: 600 });
try { try {
// Wait for the range the assertion below actually needs, not for // The container has to be a scroller at all, which is the thing
// "scrollable at all" (#133). The guard used to be // the defect behind this spec broke and is a property rather
// `scrollHeight > clientHeight + 40` while the next line asks to // than a moment.
// 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.
await expect await expect
.poll(() => scrollRange(app)) .poll(() => scrollRange(app))
.toMatchObject({ room: true, overflowY: 'auto' }); .toMatchObject({ overflowY: 'auto' });
await app.evaluate((target) => { // **Scrolling it and reading it back are one round trip** (#151).
const sc = document //
.querySelector('cover-grid') // #133 made the guard ask for the range this needs rather than
?.shadowRoot?.querySelector('.grid-scroll-container'); // for "scrollable at all", which was necessary and is not
// sufficient: a guard and the write it guards are separate
if (sc) sc.scrollTop = target; // `evaluate` calls, so the grid can satisfy the range and settle
}, SCROLL_TARGET); // out of it before the write lands. It still does — observed as
// `Expected 80, Received 10` in the second of three consecutive
expect(await scrollTop(app)).toBe(SCROLL_TARGET); // 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 // And the dropdown it opens is on screen, wherever the manager
// decides that leaves the scroll. It is *not* "the position is // 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) { async function scrollRange(app: Page) {
return app.evaluate((target) => { return app.evaluate((target) => {
const sc = document const sc = document
@@ -269,22 +272,37 @@ async function scrollRange(app: Page) {
?.shadowRoot?.querySelector('.grid-scroll-container'); ?.shadowRoot?.querySelector('.grid-scroll-container');
return { return {
// `room` is the precondition of the assertion that follows it: // Reported for the failure message rather than waited on: `room`
// enough range to actually reach the target. A threshold below // was the guard #133 strengthened, and #151 is that a guard in
// what the caller depends on is not a guard. // 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, room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
overflowY: sc ? getComputedStyle(sc).overflowY : '', overflowY: sc ? getComputedStyle(sc).overflowY : '',
}; };
}, SCROLL_TARGET); }, SCROLL_TARGET);
} }
async function scrollTop(app: Page): Promise<number> { /**
return app.evaluate( * Scroll the grid and report where it actually landed, in one call.
() => *
document * The whole point is that the set and the read share a moment: a
.querySelector('cover-grid') * `scrollTop` write is clamped to the range *at the instant it lands*,
?.shadowRoot?.querySelector('.grid-scroll-container')?.scrollTop ?? -1, * 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. */ /** Whether the open dropdown is inside the scroll container's viewport. */
+147
View File
@@ -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);
});
});
+4 -9
View File
@@ -29,18 +29,13 @@ test.describe('a control says what it controls', () => {
}); });
test('the volume slider is announced as Volume', async ({ app }) => { test('the volume slider is announced as Volume', async ({ app }) => {
// The popup renders no slider at all while closed, the same way the // No disclosure to open first, and no state to put back afterwards:
// queue panel renders no list — so this has to open it first. // #42 made the slider inline, so it is simply there. The assertion
await app.getByRole('button', { name: /volume/i }).click(); // is unchanged — the *name* is the subject here, and the route to
// the control got shorter rather than different.
await expect( await expect(
app.getByRole('slider', { name: 'Volume' }), app.getByRole('slider', { name: 'Volume' }),
).toBeVisible(); ).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 }) => { test('naming the slider did not move the transport', async ({ app }) => {
+24 -12
View File
@@ -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 // 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 // still land the layout in it, and clipping the nav with no scroll
// is the failure that made Settings unreachable. // is the failure that made Settings unreachable.
const reachable = await app.locator('app-sidebar').evaluate((el) => { //
const settings = el.shadowRoot?.querySelector<HTMLElement>( // The scroll and the measurement share one `evaluate` — #151's
'[data-testid="nav-settings"]', // 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 item = settings.getBoundingClientRect();
const pane = el.getBoundingClientRect(); const pane = el.getBoundingClientRect();
return item.bottom <= Math.ceil(pane.bottom) && item.top >= Math.floor(pane.top); return (
}); item.bottom <= Math.ceil(pane.bottom) &&
item.top >= Math.floor(pane.top)
expect(reachable).toBe(true); );
}),
)
.toBe(true);
}); });
}); });
+93 -4
View File
@@ -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). * The phone shell (plan 016 B2, phase 1).
@@ -138,6 +138,78 @@ test.describe('the shell on a phone', () => {
.toHaveAttribute('data-active-view', 'tracks'); .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 }) => { test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
await app.setViewportSize({ width: 1440, height: 900 }); 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(); await expect(app.locator('now-playing')).toBeVisible();
// Volume is the hardware keys' job on a phone, and a 4px seek bar // 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 // is not a thumb target -- both belong to the full-screen
// full-screen now-playing view. // now-playing view.
await expect(app.locator('audio-player volume-control')).toBeHidden(); //
// `.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();
}); });
}); });
+31 -3
View File
@@ -93,10 +93,31 @@ async function queueSixAndOpen(app: Page): Promise<void> {
'library.Library.GetTracks', 'library.Library.GetTracks',
[0], [0],
10_000, 10_000,
)) as { FilePath: string; TrackName: string }[]; )) as { FilePath: string; TrackName: string; Album: string }[];
const long = tracks.find((t) => t.TrackName === longTitle); const long = tracks.find((t) => t.TrackName === longTitle);
const rest = tracks.filter((t) => t.TrackName !== longTitle).slice(0, 5);
/**
* **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 // Index 3 is the long one: far enough down that a shift-extend has
// room either side of it. // room either side of it.
@@ -223,7 +244,14 @@ test.describe('selecting in the queue with a mouse', () => {
.poll(() => selected(app), { timeout: HIGHLIGHT_MS }) .poll(() => selected(app), { timeout: HIGHLIGHT_MS })
.toEqual([1]); .toEqual([1]);
await row(app, 2).locator('.explore-link').first().click(); // `.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( await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view', 'data-active-view',
@@ -69,6 +69,14 @@ export function GetPinDefaultPlaylist(): $CancellablePromise<boolean> {
return $Call.ByID(3818283301); 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 * GetQueueFallback returns what plays, if anything, once the queue
* runs out. * runs out.
@@ -207,6 +215,18 @@ export function SetPinDefaultPlaylist(pin: boolean): $CancellablePromise<void> {
return $Call.ByID(372446849, pin); 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. * SetQueueFallback validates and saves a new queue-fallback mode.
*/ */
+78 -5
View File
@@ -211,7 +211,39 @@ body div.sidebar {
padding: 0.25em; padding: 0.25em;
background-color: var(--yj-bg-elevated, #343a40); background-color: var(--yj-bg-elevated, #343a40);
display: grid; 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; align-items: center;
contain: layout style; contain: layout style;
@@ -239,23 +271,44 @@ body div.sidebar {
text-wrap-mode: nowrap; text-wrap-mode: nowrap;
overflow: hidden; 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; overflow: hidden;
text-overflow: ellipsis; text-overflow: ellipsis;
} }
} }
} }
now-playing { & now-playing {
overflow: hidden; overflow: hidden;
} }
audio-player { & audio-player {
margin: 0.5em 1em; 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 { #queue-button {
justify-self: end;
background: none; background: none;
border: none; border: none;
color: inherit; color: inherit;
@@ -440,6 +493,11 @@ body div.sidebar {
} }
@media (max-width: 599px) { @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 { .bottom-bar {
grid-template-columns: minmax(0, 1fr) auto auto; grid-template-columns: minmax(0, 1fr) auto auto;
gap: 0.25em; gap: 0.25em;
@@ -448,4 +506,19 @@ body div.sidebar {
.bottom-bar audio-player { .bottom-bar audio-player {
margin: 0.25em; 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
View File
@@ -40,16 +40,31 @@
</main> </main>
<queue-panel id="queue-panel"></queue-panel> <queue-panel id="queue-panel"></queue-panel>
</div> </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"> <footer class="bottom-bar">
<now-playing></now-playing> <now-playing></now-playing>
<audio-player></audio-player> <audio-player></audio-player>
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false" <div class="bar-end">
id="queue-button"> <volume-control></volume-control>
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out <button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
because this file has no module scope. It was `list`, id="queue-button">
which is the Playlists destination's icon. --> <!-- ICON_QUEUE in src/utils/icon-language.ts, written out
<wa-icon name="bars-staggered"></wa-icon> because this file has no module scope. It was `list`,
</button> which is the Playlists destination's icon. -->
<wa-icon name="bars-staggered"></wa-icon>
</button>
</div>
</footer> </footer>
<!-- The phone's primary navigation, hidden above 600px by <!-- The phone's primary navigation, hidden above 600px by
index.css. Eager rather than a chunk, for the reason index.css. Eager rather than a chunk, for the reason
+3
View File
@@ -18,6 +18,9 @@
// track-list — index.html renders one, so it is the first paint. // track-list — index.html renders one, so it is the first paint.
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
import '@components/audio-player/audio-player.ts'; 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/track-list/track-list.ts';
import '@components/now-playing/now-playing.ts'; import '@components/now-playing/now-playing.ts';
import '@components/sidebar/app-sidebar.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 '@awesome.me/webawesome/dist/components/icon/icon.js';
import './controls/player-controls'; import './controls/player-controls';
import './seekbar/seek-bar'; import './seekbar/seek-bar';
import './volume-control/volume-control';
import '../notifications/inline-notice'; import '../notifications/inline-notice';
import { PlayerRegion } from '@store/player-store'; import { PlayerRegion } from '@store/player-store';
import { designTokens } from '../../styles/tokens.css'; import { designTokens } from '../../styles/tokens.css';
@@ -30,6 +29,7 @@ export class AudioPlayer extends LitElement {
.player-main { .player-main {
flex: 1; flex: 1;
min-width: 0;
} }
/* The phone transport (plan 016 B2): the buttons, and nothing /* 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 viewport, not by the host, so this is the component saying what
it drops at phone width rather than the shell reaching in. 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 The seek bar goes because a 4px-tall target dragged with a thumb
is not a seek control; seeking belongs to the full-screen 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) { @media (max-width: 599px) {
volume-control,
seek-bar { seek-bar {
display: none; display: none;
} }
@@ -65,7 +68,6 @@ export class AudioPlayer extends LitElement {
<seek-bar></seek-bar> <seek-bar></seek-bar>
</div> </div>
</div> </div>
<volume-control></volume-control>
</div> </div>
`; `;
} }
@@ -4,6 +4,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@awesome.me/webawesome/dist/components/slider/slider.js'; import '@awesome.me/webawesome/dist/components/slider/slider.js';
import type WaSlider from '@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 { PlayerController } from '@store/controllers/player-controller';
import { volumeStyleStore } from '@store/volume-style-store';
import { designTokens } from '../../../styles/tokens.css'; import { designTokens } from '../../../styles/tokens.css';
import { waSliderLabel } from '../../../styles/wa-slider-label.css'; import { waSliderLabel } from '../../../styles/wa-slider-label.css';
@@ -22,6 +23,12 @@ export class VolumeControl extends LitElement {
@state() @state()
private showSlider = false; 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. // Locally-tracked volume while the user is actively dragging or scrolling.
// The store's volume only updates once the backend echoes VolumeChanged // 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 // (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; --thumb-height: 16px;
} }
wa-slider::part(track) { .volume-popup wa-slider::part(track) {
background: var(--yj-text-primary, white); background: var(--yj-text-primary, white);
height: 120px; 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) { wa-slider::part(indicator) {
background: var(--yj-accent, yellow); background: var(--yj-accent, yellow);
} }
@@ -122,8 +149,23 @@ export class VolumeControl extends LitElement {
// LIFECYCLE // 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() { override disconnectedCallback() {
super.disconnectedCallback(); super.disconnectedCallback();
this.unsubscribeStyle?.();
document.removeEventListener('click', this.boundHandleOutsideClick); document.removeEventListener('click', this.boundHandleOutsideClick);
clearTimeout(this.volumeDebounceTimer); clearTimeout(this.volumeDebounceTimer);
} }
@@ -154,10 +196,12 @@ export class VolumeControl extends LitElement {
private handleOutsideClick(e: Event) { private handleOutsideClick(e: Event) {
const path = e.composedPath(); const path = e.composedPath();
if (!path.includes(this)) { if (!path.includes(this)) this.closeSlider();
this.showSlider = false; }
document.removeEventListener('click', this.boundHandleOutsideClick);
} private closeSlider() {
this.showSlider = false;
document.removeEventListener('click', this.boundHandleOutsideClick);
} }
private handleInput(e: Event) { private handleInput(e: Event) {
@@ -192,18 +236,46 @@ export class VolumeControl extends LitElement {
override render() { override render() {
const muted = this.player.muted; 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` return html`
<button <button
class=${muted ? 'muted' : ''} class=${muted ? 'muted' : ''}
title=${muted ? 'Muted — click for volume' : 'Volume'} title=${muted ? 'Muted — click for volume' : 'Volume'}
aria-label=${muted ? 'Muted' : `Volume ${this.currentVolume}%`} aria-label=${iconLabel}
data-muted=${muted ? 'true' : 'false'} data-muted=${muted ? 'true' : 'false'}
@click="${this.toggleSlider}" @click="${iconAction}"
@wheel="${this.handleWheel}" @wheel="${this.handleWheel}"
> >
<wa-icon name=${this.volumeIcon}></wa-icon> <wa-icon name=${this.volumeIcon}></wa-icon>
</button> </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` ? html`
<div <div
class="volume-popup ${muted ? 'muted' : ''}" class="volume-popup ${muted ? 'muted' : ''}"
@@ -25,6 +25,8 @@ import {
SetQueueFallback, SetQueueFallback,
GetAllowMeteredCatalogDownload, GetAllowMeteredCatalogDownload,
SetAllowMeteredCatalogDownload, SetAllowMeteredCatalogDownload,
GetPopupVolume,
SetPopupVolume,
} from '@go/config/config.js'; } from '@go/config/config.js';
import { GetIndexStatus } from '@go/explore/service.js'; import { GetIndexStatus } from '@go/explore/service.js';
import { notificationStore } from '@store/notification-store'; import { notificationStore } from '@store/notification-store';
@@ -125,6 +127,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
@state() private concurrencyMode = 'auto'; @state() private concurrencyMode = 'auto';
@state() private defaultPage = 'home'; @state() private defaultPage = 'home';
@state() private queueFallback = 'favorites'; @state() private queueFallback = 'favorites';
@state() private popupVolume = false;
@state() private indexStatus: explore.IndexStatus | null = null; @state() private indexStatus: explore.IndexStatus | null = null;
/** Three states, not one: the panel used to say "Loading status" /** Three states, not one: the panel used to say "Loading status"
* for the entire session, because the only thing that ever set * 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> { private async loadLibraries(): Promise<void> {
try { try {
const [libs, mode, defaultPage, queueFallback, allowMetered] = const [
await Promise.all([ libs,
GetAllLibrariesWithTrackCounts(), mode,
GetScanConcurrency(), defaultPage,
GetDefaultPage(), queueFallback,
GetQueueFallback(), allowMetered,
GetAllowMeteredCatalogDownload(), popupVolume,
]); ] = await Promise.all([
GetAllLibrariesWithTrackCounts(),
GetScanConcurrency(),
GetDefaultPage(),
GetQueueFallback(),
GetAllowMeteredCatalogDownload(),
GetPopupVolume(),
]);
this.libraries = libs ?? []; this.libraries = libs ?? [];
this.concurrencyMode = mode; this.concurrencyMode = mode;
this.defaultPage = defaultPage; this.defaultPage = defaultPage;
this.queueFallback = queueFallback; this.queueFallback = queueFallback;
this.allowMeteredCatalogDownload = allowMetered; this.allowMeteredCatalogDownload = allowMetered;
this.popupVolume = popupVolume;
} catch (err) { } catch (err) {
console.error( console.error(
@@ -1858,10 +1870,51 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
.value=${this.queueFallback} .value=${this.queueFallback}
@config-change=${this.handleQueueFallbackChange} @config-change=${this.handleQueueFallbackChange}
></config-field> ></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> </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 --- // --- Navigation section ---
/** /**
@@ -188,6 +188,27 @@ export class NowPlaying extends LitElement {
cursor: pointer; cursor: pointer;
/* The art shows through; this is a target, not a picture. */ /* The art shows through; this is a target, not a picture. */
color: transparent; 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 { .expand:focus-visible {
+85
View File
@@ -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();
+83 -4
View File
@@ -11,7 +11,7 @@ import '@components/audio-player/controls/player-controls';
import '@components/audio-player/seekbar/seek-bar'; import '@components/audio-player/seekbar/seek-bar';
import '@components/audio-player/volume-control/volume-control'; import '@components/audio-player/volume-control/volume-control';
import { Events } from '../../src/events'; 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 { import {
fixture, fixture,
shadow, shadow,
@@ -434,13 +434,40 @@ describe('<seek-bar>', () => {
* be driven by its own event watching the volume number, as it used * be driven by its own event watching the volume number, as it used
* to, meant pressing M visibly did nothing. * 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', () => { 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.VolumeChanged, 40);
emit(Events.MuteChanged, false); 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'); const el = await fixture('volume-control');
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false'); expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
@@ -453,14 +480,53 @@ describe('volume control: mute', () => {
expect(shadow(el, 'button wa-icon')?.getAttribute('name')).toBe( expect(shadow(el, 'button wa-icon')?.getAttribute('name')).toBe(
'volume-xmark', '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'); 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 () => { it('keeps showing the volume level while muted, because it is unchanged', async () => {
await setPresentation(true);
emit(Events.MuteChanged, true); emit(Events.MuteChanged, true);
await flush(); await flush();
const el = await fixture('volume-control'); const el = await fixture('volume-control');
await el.updateComplete;
await click(el, 'button'); await click(el, 'button');
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40); expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
@@ -468,11 +534,24 @@ describe('volume control: mute', () => {
it('toggles mute through the backend rather than locally', async () => { it('toggles mute through the backend rather than locally', async () => {
const el = await fixture('volume-control'); 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, 'button');
await click(el, '.mute-toggle'); await click(el, '.mute-toggle');
expect(calls('player.Player.MuteToggle').length).toBe(1); 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'); expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
}); });
}); });
+55
View File
@@ -6,6 +6,7 @@ import (
"log/slog" "log/slog"
"os" "os"
"strings" "strings"
"sync/atomic"
"github.com/golang-cz/devslog" "github.com/golang-cz/devslog"
"github.com/wailsapp/wails/v3/pkg/application" "github.com/wailsapp/wails/v3/pkg/application"
@@ -28,7 +29,61 @@ var (
//go:embed all:frontend/dist //go:embed all:frontend/dist
var frontendDistAssets embed.FS 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() { 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 // **Mobile has no home directory, and this must run before anything
// asks for a path.** backend/system resolves config and data from // asks for a path.** backend/system resolves config and data from
// $HOME or the OS equivalent, and on Android there is neither: its // $HOME or the OS equivalent, and on Android there is neither: its
+104
View File
@@ -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
}
+196
View File
@@ -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"
+27 -4
View File
@@ -34,7 +34,21 @@ cd "$(dirname "$0")/.."
AVD="${YJ_AVD:-yj-test}" AVD="${YJ_AVD:-yj-test}"
SDK="${ANDROID_SDK_ROOT:-${ANDROID_HOME:-$HOME/Android/Sdk}}" 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. # Where `make android-inspect` forwards the WebView's devtools socket.
CDP_PORT="${YJ_ANDROID_CDP_PORT:-9222}" CDP_PORT="${YJ_ANDROID_CDP_PORT:-9222}"
# **Not "$PKG/.MainActivity".** A leading-dot activity is resolved # **Not "$PKG/.MainActivity".** A leading-dot activity is resolved
@@ -281,15 +295,24 @@ cmd_inspect() {
need_sdk need_sdk
pick_device || die "no device -- plug a phone in (USB debugging on) or run 'make android-emulator'" 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="" 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}') pid=$("$ADB" shell pidof "$pkg" 2>/dev/null | tr -d '\r' | awk '{print $1}')
[ -n "$pid" ] && break [ -n "$pid" ] && break
done 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 --remove-all >/dev/null 2>&1 || true
"$ADB" forward "tcp:$CDP_PORT" "localabstract:webview_devtools_remote_$pid" >/dev/null \ "$ADB" forward "tcp:$CDP_PORT" "localabstract:webview_devtools_remote_$pid" >/dev/null \
+97
View File
@@ -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"