76e1c444cc893178ca5f5d590991b24e7ef211ae
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 logan
|
76e1c444cc |
feat(android): tap to play, hold to select
Phase 1 of #63, and the design the issue asks for as one piece is .planning/plans/active/019-android-touch-model.md. **A finger has no second button and no modifier keys**, so the primary action has to be the primary gesture: tap plays the row, and the hold that opened a context menu now enters selection mode with that row selected. Three decisions in it, and two diverge from the report. **The predicate is the pointer, not the platform or the viewport.** `pointerType === 'touch'`, per event, which is already how long-press.ts decided and is the only such test in the frontend. This is #64's rule -- named after the capability -- and it carries #64's warning: keyed on a width, an Android *tablet* at 600px gets click-selects/double-click-plays on a touchscreen, which is the inversion this issue exists to fix, on the platform it exists for. A touchscreen laptop cannot be described by a width at all. Per event, a mouse keeps desktop semantics on the very same row, and there is no second declaration of what a phone does. **There is no double-tap, and the number is why.** The report asks for single tap to play *and* double tap for the menu. Those cannot both be honoured: the first tap of a double tap is indistinguishable from a single tap until the interval expires, so "tap plays" becomes "tap waits". Measured on the device, the play command to TrackChanged is 155/123/85/56/91 ms -- median ~100 -- and the app's own DOUBLE_CLICK_GRACE_MS is 250. That is 3.5x the primary interaction, 250ms of it spent deliberately doing nothing, on every track anyone plays, to reach a menu the hold already reaches. So the menu and the selection action bar are the same surface, which is also the platform's convention and removes a concept rather than adding one. **Tap-to-play and selection mode ship together**, because splitting them is a regression dressed as an increment: a touch user selects by tapping today and acts through the long-press menu, so moving tap to play on its own would leave a window with no way to select forty tracks at all. **What lets this reassign the hold without touching one of the fourteen context menus**: the layer announces `yj-tap` / `yj-long-press` (composed, cancelable) and acts on nothing. A component claims one with preventDefault. An **unclaimed long press still becomes a `contextmenu`**, so the card grids, Explore, the playlist rows and every other menu behave exactly as they did, and only lists that opt in get selection mode. An unclaimed *tap* does nothing at all and the click follows normally, which is what leaves every button in the app alone -- only a claimed tap has its click swallowed, or playing a track would also select it. **And the device found the one thing no browser tier can see.** Chrome 113's Android WebView fires its own `contextmenu` on a long press. long-press.ts stood down when a trusted one arrived, which was right while both paths ended in a context menu; they no longer do, so standing down means the gesture silently does the *old* thing. Measured, before the fix, holding a track row: {"log":["contextmenu isTrusted=true"], "state":{"bar":null,"menuActive":true,"selected":1}} `yj-long-press` was never announced, the menu opened, and all 26 tests passed -- dispatched pointer events do not make a browser synthesise one. So the native event is a **trigger, not a competitor**: the gesture is announced from it and only a claim suppresses it. Unclaimed it propagates untouched, which is the same "browser wins" outcome reached by asking instead of assuming. The tier could not find that and can hold it, because this module has always told its own events apart by identity rather than isTrusted, so an untrusted one from a test takes exactly the browser's path. Verified on the device by *performing* the gestures rather than describing the page -- `adb shell input tap` and `input swipe x y x y 700` reach the WebView as real pointer events, which is new here and is written down in the plan with the pixel mapping. Tap plays; a hold raises the bar with one selected and no menu; a tap toggles to two, back to one, and the mode ends with the last row; an album card still opens its context menu. 29 new tests. The e2e spec is rewritten to assert **both** halves -- the row selects, and a card elsewhere still opens the real menu -- because a spec that only checked the row would pass on a build that had silently broken the other thirteen. Phases 2-4 (swipe to queue, the other three surfaces, and what #67 inherits) are in the plan and not in this commit. |
||
|
logan
|
4f32d4e13c |
Merge pull request 'Android: raise every remaining control to the 44px touch floor' (#198) from 186-touch-targets-settings into main
Closes #186 |
||
|
logan
|
4f628b1f52 |
fix(ui): raise the last controls below the touch floor
The rest of #186's second table, and one thing it could not have said. .section-toggle 187x15 autotag .folders-menu-trigger 32x18 autotag .back-button 32x32 artist-details Requests / Downloads tabs 85x34, 96x34 .search-mode-tab 89x26, 79x26 explore explore search input 325x18 in a 36px box **back-button was six controls, not one.** The issue names it in artist-details because that is the view the sweep opened; the same declaration is byte-identical in artist-details, genre-details, playlist-details, smart-playlist-details, explore-artist-details and explore-album-details, 32px in all six. So it is styles/back-button. css.ts now, adopted by each, and a source sweep fails on a seventh copy -- because the failure this invites is not a size changing, it is somebody adding a detail view and writing `.back-button` out again, which no device sweep would catch for the same reason this one did not. That is icon-language.test.ts's shape, and the argument for it here is the inverse of the column arrows': one declaration covering 36 controls is cheap to fix, and six declarations of one control are six chances to miss five. It is a real 44px box rather than padding with the width handed back: a detail header runs no fit pass, and this button has a visible background, so a hit area larger than the circle would be a control bigger than it looks. The size is #55's, reached there for the same reason -- "the way out is 44px on a phone". **The explore search box was two faults.** The row was 36px *and* the input inside it was 18, so eight pixels at each edge were not a target at all: a tap near the top of the box landed on the container and did nothing. The container is 44 and the input stretches to it. **The Downloads tabs take padding rather than a min-size**, because the mark for the selected tab is its bottom border -- a min-size centres the label and leaves the underline 10px beneath it. page-action-check-now (113x29) is in that table and is not here: it is a PageAction, so #195 raised it with the rest of the header's actions and touch-targets.test.ts already covers it. **The Downloads tabs needed a min-size as well as the padding, and CI is what said so.** Padding alone made them 44px on this machine and **43px in the container**: the total is 13 + 13 + 2 + whatever line box the font gives 13px text, and ubuntu:24.04's is a pixel shorter than Arch's. A height computed from a font's line box is not a height you control -- which is #195's "stated as a property on the strength of one engine" one layer down, in the same PR that recorded it. The padding stays, because it is what keeps the underline against the label; the min-size is the floor. Caught by the new test rather than by a person, which is the half of this that worked. Verified on the device, sweeping each view the way the issue was filed: explore, downloads, autotag and artist-details now report **one** control under the floor apiece, and it is the skip link, which #186 already ruled out as keyboard-only. .search-mode-tab 89x44 and 79x44, the search input 325x44, the Downloads tabs 85x44 and 96x44, .section-toggle 174x44, .folders-menu-trigger 44x44, .back-button 44x44. All 12 new tests fail on main, the source sweep naming all six copies. make ui-test 1041 pass; make e2e 236 pass on chromium, which is half an answer -- CI had the other half, and used it. Closes #186 |
||
|
logan
|
2100f0022f |
fix(settings): raise every Settings control to the touch floor
#56 named 44px and #195 took the page header there. Settings is the other half of #186 and much the larger one: swept on the reference device (TLP301, 424x439) with all eleven config-sections expanded, **120 controls** were under the floor -- not the 93 the issue's table implies, and config-field is eight of them. The bulk is behind the disclosures, which is why nobody had counted it: 36 .column-arrow-btn 16x14 <- smallest in the app 29 .column-toggle 16x16 26 shortcut-capture button 80x25 8 download format checkbox 16x16 7 config-field select 335x30 6 wa-input / wa-button 204x20, 185x21 **The density argument, measured rather than guessed, and it is smaller than it looks.** The rows were already near the floor -- .column-item is 335x36 and .shortcut-row 335x37; it is the controls *inside* them that were 14-25px. So a control grows into the row it already occupies and the row goes 36 to 44. Measured after: the two column lists went 373->447 and 690->850, +234px over the whole page. Half a screen of extra scroll on a page that already scrolls, against 36 targets of 16x14. **Settings is cheaper than the header was, and for a stated reason.** There is no overflow fit on this page, so the header's "only width is contested" rule does not bind at all and nothing here needs padding with a negative margin. Height is a min-size, and the two square controls can simply be square. Three shapes, because one rule does not fit three kinds of control: **A native checkbox is targeted through its label.** It cannot grow its hit area without growing its paint, and a 44px checkbox is not what anyone wants -- so .column-label is a real <label for> now and the column's *name* is the target, 70x44 rather than 16x16. That is the argument config-field already makes one file over ("a real label association also makes the label text a click target, which is behaviour, not annotation"), and here it is the whole fix. The download formats already had the label; they only needed the height. **The arrows take padding, which is invisible.** They carry background: none and a transparent border, so 16x14 -> 44x44 changes nothing anyone can see until hover -- #186's Direction exactly. **Web Awesome's controls come from the library's own API.** Their height is decided inside somebody else's shadow root, and --wa-form-control-height is the variable that decides it. A custom property inherits through a shadow boundary, so a :host declaration reaches them; styles/wa-touch-floor.css.ts is that, once, adopted rather than written at :root in index.css -- a :root rule would be invisible to the component tier, which renders a component and no page stylesheet. **Two controls no sweep can see are fixed by name**, and they are the trap this issue keeps setting. config-field's toggle has an <input> that is opacity: 0; width: 0; height: 0, so a walk of every input skips it as a zero-sized node -- what a finger hits is the <label>, which measured **34x19**, smaller than anything in either of #186's tables and absent from both. It is 44x44 with the pill still painted at 2.5em x 1.4em and negative inline margins keeping it flush with the inputs above. And shortcut-capture's reset button renders only for a shortcut somebody has rebound, so a sweep of a fresh install never meets it. Verified on the device, same method as the sweep that filed it: 120 controls under the floor before, 42 after. All 42 are accounted for -- 37 are checkboxes whose labels measure 70x44 and 57x44, four are wa-input's inner input at 204x**42**, which is the control measured *inside* its own 1px border (part=base is 238x44), and one is the skip link, which #186 already ruled out as keyboard-only. The e2e suite passes, top-bar-fit and header-action-overflow included -- but that is **chromium**, which is half an answer, and saying so is the whole of what #195's second commit was about. What can be argued rather than run: library-filter is the only thing here in a container that measures itself, and its width did not change. The fit measures inline size. Two page-header screenshots are refreshed because they are this issue's own debris -- #195's taller sort control, merged last session, with its references never re-recorded. app-sidebar's and now-playing's are deliberately left: they are unrelated drift, and blessing an unrelated screenshot is how the sidebar reference came to still list a destination #27 retired. That is #196. |
||
|
logan
|
52038dc5ae | Merge pull request 'Android: raise the page header and the phone search button to the touch floor' (#195) from 186-touch-targets-page-header into main | ||
|
logan
|
0d331666d6 |
fix(shell): make the header's touch targets cost no width
The first pass grew the two square controls to 44px as boxes, which
added 22px to the header. That fit at every width Chromium was checked
at and **clipped the overflow trigger at 320x600 in WebKit** -- the
engine closest to what ships, and the one no machine here can run:
every action is reachable at 320x600 (400% zoom)
- Array []
+ Array [ "more" ]
Two things were wrong, and only one of them was the code.
**The claim was checked on one engine and stated as a property.** The
previous commit said #69's fit "does not move ... the check rather than
the assumption", on the strength of running that spec against chromium
alone. CI runs both browsers precisely because they are not the same
answer.
**And the box was the wrong thing to grow**, which the issue already
said: "reached by growing the *hit* area rather than the visual weight
where the two can differ -- padding on the control, not size on the
icon". #69's pass measures inline size, so a taller control is free and
a wider one is not.
So height stays a box -- the header has the room and nothing measures
it -- and width is padding with a negative margin handing the space
back, which is the seek bar's shape from #187. Measured in the
component tier at 320px: the arrow's rect is 45x44 and it occupies 29,
the overflow trigger 44x44 occupying 38, the search button 44x44
occupying 40. Those three occupancies are what they were before any of
this, so the fit pass sees a header identical to main's and the
320px case cannot regress.
The arrow's target is lopsided for #187's reason: the select is 6px to
its left and there is open space to its right, so it takes the side
with nothing to steal from. The overflow trigger's can be symmetric,
the actions row having an 8px gap.
`search-trigger` is border-box, so its 44px min-width is the whole
target and the margin alone gives the four pixels back.
The new assertion is the one that would have caught this: every grown
control must carry negative inline margins, because that is what keeps
the box out of the fit. The rect assertions stay -- getBoundingClientRect
includes padding, so the target is still measured directly rather than
inferred.
|
||
|
logan
|
6a5a3c33dc |
fix(shell): raise the page header's controls to the touch floor
#56 sized the playback transport for a thumb and named 44px; the queue header keeps it. Nothing else was resized, so the controls a user meets on *every* screen sat between a third and two thirds of the app's own floor. Measured on the reference device at 424x439: page-sort 99x23, page-sort-direction **28x21**, page-actions-more 38x27, and search-trigger 40x40. **Both questions the issue left open are answered by one measurement.** The header is 63px tall and its controls are 20-23px, so the vertical room was already there; the select and its direction arrow are 6px apart, so the horizontal room was not. That makes this min-size rather than padding with a negative margin, which is what the seek bar needed (#187), and the difference decides everything else. There the painted track had to stay thin, so the target was grown past its own box and had to be checked against its neighbours. Here the control *is* the target: the boxes are flex items, so the gap keeps them apart and **no two targets can overlap by construction**. From which: **There is no phone branch.** A 44px control on a desktop is merely large, and a second declaration of what a phone shows is a second thing to keep in step -- which is why this component has never had one. It also avoids a media query no tier here renders, which is exactly how the seek bar's phone rule came to be dead for months. **#69's overflow fit does not move.** That pass measures inline size, so the height costs it nothing, and only the two square controls grow the header's content -- by 22px in total. header-action-overflow.spec.ts passes unchanged at all four of its widths, which was the check rather than the assumption. Verified on the device that the count is still shown at 424px, so nothing has started yielding. search-trigger is the sharpest case and is fixed in the same pass: #57 created it as the phone's replacement for the header search box, so it exists *only* where there is a thumb, and it shipped at 40x40 under a comment calling that "the smallest a touch target should be". That was the floor restated four pixels short rather than a second opinion about it, and the comment now says so. Unlike #187 this can be measured rather than inferred: the controls are plain elements and the rule is a min-size, so it holds at every width and a real Chromium rendering a real page-header gives the actual answer. The tests fail with the device's own numbers -- 29x21, 38, 40. Verified on the device: every control in the header is now at least 44x44, and so is the phone's search button. **This is the Direction's first step, not all of it.** config-field's 93 Settings controls and explore-view's search row are the second pass; Settings is a form with one shape for every row and wants its own argument. #186 stays open for them. |
||
|
logan
|
1668b9e0d2 | Merge pull request 'Android: a seek bar you can actually hit, and the phone rule that never applied' (#193) from 187-seek-bar-hit-area into main | ||
|
logan
|
ec64dbded0 |
fix(player): give the seek bar a thumb-sized hit area
On now-playing-view -- the screen that exists so a phone has somewhere to seek from -- the slider measured 261x6 on the reference device. Six pixels is the whole of the drag target on the app's primary seeking affordance, against the 44px floor the app set for itself in #56 and holds to in the queue panel. **The phone rule had never applied**, which is why the issue read as "the thickening stops short" rather than "there is no thickening". seek-bar's stylesheet asked for a 12px track below 599px and then set 6px in a plain `wa-slider` rule *written after it*. A media query adds no specificity, so the plain rule won at every width: the source said 12 and the device said 6. That is index.css's documented rule -- "the phone section is last on purpose" -- met inside a component's own stylesheet, where nothing in any tier renders differently to say so. The block is last now, and the 12px track it always asked for is real. **And 12px is still under the floor**, so the target is built around the painted track rather than by thickening it. The two are allowed to differ and a slider is the clearest case where they should: a 44px progress bar would be wrong-looking and would cost the album art the vertical space #51 spent an issue recovering. Two things about how it is built, both settled by measurement on the device rather than by choosing a number. **The padding goes on ::part(slider), not on the host.** That is the issue's untested claim, and the answer is the pessimistic one: the inner div is what carries the gesture -- it holds the listener and the touch-action: none -- and it is exactly the host's size, so padding the host would grow a box that does not take the press. **The padding is asymmetric and the margins cancel it**, so the row does not grow by the difference. The seek row is 19px -- its clocks, not the track, decide that -- and the play button's top edge is 8px below it, while `.art` above is a non-interactive div. A symmetric 44px target reaches into the play button, and growing the row instead cost the art 25px of 143 when it was tried. So the target takes the space above. Verified on the device at 424x439: hit area 261x44 where it was 261x6, painted track 12px, seek row still 19px, album art still 143px, 7px of clearance left under the play button, a press 26px above the track seeks, and a hit test on the play button's top edge still reaches the play button. The desktop bottom bar is untouched: the rule is inside the phone query and that instance is display:none below 600px anyway. The test asserts the parsed stylesheet, on hover-affordance.test.ts's precedent and with the same limitation stated -- no tier here lays out a real wa-slider at a phone width, and a number measured on a phone is not a number CI can assert. What it holds is the shape: that the phone block is last, that padding plus track clears 44, that the margins cancel the padding, and that the growth is upward. All four are invisible on a desktop, and the first is exactly what a tidy-up undoes. Closes #187 |
||
|
logan
|
dad852a8a0 | Merge pull request 'Explore: two things that have not worked since plan 013, and the temp directory Android never had' (#192) from 189-190-explore-correctness into main | ||
|
logan
|
30c6b665f1 |
fix(system): give the process a temp directory that exists
Android has no /tmp and hands an app no TMPDIR. Go's os.TempDir() falls
back to "/tmp" when the variable is unset, so every library in this
process that wants scratch space was being handed a path that has never
existed.
SQLite is the one that noticed, and it said so precisely:
W/yellowjacket: msg="champion index rebuild failed"
explore.search-index.error="populate champion fts: disk I/O error (6410)"
6410 is not a generic I/O error. `6410 & 0xff` is 10, SQLITE_IOERR, and
`6410 >> 8` is 25 -- SQLITE_IOERR_GETTEMPPATH. SQLite could not work out
where to put a temporary file. Two measurements on the device say why:
`ls -d /tmp` does not exist, and the app process's environment carries
no TMPDIR. A shell's does (/data/local/tmp), which is why this is easy
to miss from `adb shell`.
The cost was a silent performance cliff on the slowest device this app
runs on: `championReady` stayed false, so every Explore search took the
generic path over the whole 1,079,667-row index instead of the champion
subset, and the rebuild was re-attempted on every launch.
**The class is fixed rather than the statement.** The trigger is the
*size* of the work, not that query -- anything that spills fails the
same way there, so large sorts, large joins and VACUUM were all waiting
their turn. The repair belongs at the process's one answer to "where do
temporary files go".
`PRAGMA temp_store = MEMORY` was the alternative: cheaper, more local,
and a promise that every future spill fits in RAM on a phone. The
catalog is the largest thing in this app and that is not a promise
worth making silently.
UseTempDir sits beside UseHomeOverride and carries its two rules for
the same reasons. **An empty base is a no-op**, because that is what
application.Mobile.StoragePath() returns on desktop -- so this needs no
build tag and changes nothing off mobile, where /tmp is real. And **an
explicit TMPDIR wins**, so anyone who set one deliberately gets it;
nothing sets it on the platform this exists for. It needs no new Wails
API and no Java change: StoragePath() is already what YJ_HOME is
pointed at, and the directory goes under it.
Two things beyond the rename of a variable.
**Writability is probed, not assumed.** MkdirAll on an existing
unwritable directory succeeds, so without the probe this could set
TMPDIR to a directory nothing can use -- which is the same bug one
directory over, and just as quiet.
**It returns its error, and main logs it.** A temp directory that could
not be created is the same silent failure one step earlier. A failure
is not fatal: it leaves the platform's answer in place, which is what
every release before this one ran with. That log line is readable on
the platform only because of #160.
Verified on the reference device, where the same launch that used to
print the failure now prints:
I/yellowjacket: msg="champion index rebuilt"
explore.search-index.elapsed=6.496s
Closes #190
|
||
|
logan
|
d034d6e571 |
fix(explore): resolve pending release MBIDs against the real table
The release-group MBID backfill queried `release_groups`, which plan 013
renamed to `albums`. It failed on its first statement on every launch
since
|
||
|
logan
|
25ea1f3511 | Merge pull request 'Android: make the app say what it is doing, then count what the audio path misses' (#191) from 135-android-underrun-instrumentation into main | ||
|
logan
|
842fe47e9e |
feat(player): count what the ring buffer misses
An underrun is audible and nothing counted it. When the ring is empty BufferedStreamer.Stream zeroes the caller's buffer and returns ok, so a run of zeros is spliced into the waveform and the step discontinuity at each edge is a click; a series of short ones is static. That is the one candidate in #135 whose audible signature matches the report. `starved` and `starvedSince` already existed, from #122's stall fix, and could not answer this: they are a *stall* detector, reset by every arriving sample, because their job is to end a track whose source has died and a merely slow source must not be cut short. That reset is exactly what made the audible case invisible -- a hundred 20ms underruns a minute never approach the give-up threshold, so they were invisible to the log, to the UI and to every tier. UnderrunStats counts runs, calls and samples for the life of the streamer. Runs is the number that means something audible: one episode is one pop however many callbacks it spans, and the ratio of runs to calls is what separates clicking from dropping out. Three things about the reporting are deliberate. **Counting is in Stream and reporting is not.** Stream runs on the speaker callback's real-time deadline, so a log line there would allocate, format and write on the exact path whose missed deadline is the defect -- measuring by making it worse. The count is two increments under a lock that was already held; the report is on the 1 Hz position ticker, which only runs while playing. **An unchanged count is not logged**, which is emitStatus' rule one package over. A healthy player is silent, so anything in the log is news and the line appears exactly while it is popping. **It is Info rather than Debug.** The default level is Info and a phone has no convenient way to set YJ_LOG_LEVEL, so a debug line here would be a counter nobody on the affected platform could read -- which is the shape of the bug that made #160 necessary. underrunDelta clamps at zero because the counter belongs to the streamer and the streamer is replaced on every track: a baseline carried across that boundary is the previous track's total subtracted from a fresh zero. The baseline is reset at the load as well; a negative count in a log line reads as a broken instrument and would discredit the measurement this exists to make. This is the instrument, not a fix. What it measures is on the issue. |
||
|
logan
|
168e588387 |
feat(android): route slog to logcat
Every slog line the app wrote on Android went to /dev/null, including the one naming the error it was about to os.Exit on. #52 is what that cost: a process that vanished with no tombstone, no AndroidRuntime stack and nothing in `logcat -b crash`, at Priority/Critical for months, whose entire diagnosis was one sLogger.Error main.go was already writing. backend/androidlog is a slog.Handler over __android_log_write, chosen in main() by build tag rather than by a runtime check so that a desktop binary links no cgo for a platform it cannot run on. **Everything except the write itself is untagged.** That is androidpayload.go's discipline pushed as far as it goes: the only toolchain that compiles the android tag is a cross-compiler and the only thing that runs it is a phone, so the priority mapping, the formatting, the chunking and the handler's own attr and group bookkeeping are ordinary Go that `go test` exercises everywhere, and android.go is fifteen lines that hand a string to liblog. Four things in it are load-bearing. **The tag is a fixed string, not the application id.** The debug build carries `applicationIdSuffix ".dev"` so it can be installed beside the release app, and it is the only build whose WebView can be inspected -- so a tag derived from the id is a different tag on the one build anybody debugging this app is running, and the filter meant to show these lines would hide them exactly where they were being looked for. **The priorities are android/log.h's own values, asserted twice.** android.go carries constant expressions that do not compile as uint if the header renumbers; the untagged test writes the six numbers out longhand, because comparing a constant to itself passes on any renumbering. A wrong priority is the failure that hides rather than breaks -- logcat prints whatever number it is handed, so an Error filed as Info is present, correct, and invisible to every filter. **Formatting is delegated to slog's TextHandler.** WithAttrs and WithGroup are the half of slog.Handler that is easy to get subtly wrong, and a logger whose groups are wrong is a logger nobody reads. The derived handlers share the parent's buffer *and its mutex*: a second mutex would guard nothing, and two loggers derived from one would splice their bytes into a single line under load. **A line is chunked, because liblog drops what does not fit.** The kernel logger's entry is 4068 bytes for tag and message together and the remainder goes without comment, so a long record would be truncated in the middle of the thing worth reading. Time and level are dropped from the formatted line, since logcat stamps every entry with both -- and dropping them by *key* also ate a caller's own "level" attribute, which the on-device probe caught and TestACallersOwnLevelAttrSurvives now holds. ReplaceAttr sees an empty group path for the built-ins and for every top-level attribute alike, so the kinds are what separate them. Verified on the reference device (TLP301, Android 14): a debug build logs I/W/E under the `yellowjacket` tag at the right priorities, and the first thing it surfaced was a real warning nobody could previously see -- `champion index rebuild failed ... disk I/O error (6410)`. Closes #160 |
||
|
logan
|
7eb55bd378 | Merge pull request 'Android: a Now Playing that survives a 439px screen, and the audit behind it' (#188) from 51-android-small-screens into main | ||
|
logan
|
f31331c83b |
docs(skill): what a fresh install is doing before you measure it
Three things about a fresh install cost a measurement each, and none of them was written down: it downloads the real catalog, so job-band is 103px of a 439px screen and every vertical number is wrong; stopping that build returns cleanly and it **starts again within seconds**, so it has to be stopped immediately before a measurement rather than once at the start; and a library added over the bridge does not dismiss the first-run wizard, which then sits over whatever you are looking at with a correctly disabled button, reading exactly like a swallowed tap. Also the scoped-storage path that works, the appops grant whose absence sends the app to the system "All files access" screen on launch, and why EXPR='...' cannot carry an apostrophe -- a file path with one in it fails as a JavaScript error. The positional form takes a file. |
||
|
logan
|
6a22601af7 |
docs(player): a device number is not a number CI can assert
The spec's floor on the art's height passed locally at 114 and failed in CI at 64. Both honest: the e2e app is long-lived so an earlier spec's job is still on screen, and volume-control renders in a browser where it does not on Android. Same trap as the staged-job entry above, arriving as a measurement rather than as a stuck job. |
||
|
logan
|
ce9951b93a |
test(player): assert the mechanism, not the room CI happened to have
The floor on the art's height passed locally at 114 and failed in CI at 64. Both numbers are honest and neither is about this change: the e2e app is long-lived, so a job staged by an earlier spec is still on screen, and the volume control renders here where it does not on Android. Both are chrome above and below the view, and both move the leftover. So the claim is stated as what the reflow does rather than as what it measures -- in a row the art is bounded by the row's height and fills it, where in a column it is the leftover after the names. That is the mechanism behind 53px to 143px, and it fails on the old build with "there is no row to fill". The device numbers stay on #51, which is the only tier that can honestly produce them. This is the second draft of that assertion to be thrown away; the first compared the art against the column's leftover and passed on the defect, because the subtraction goes negative exactly when the names are taller than the art. Also stops the arrangement wait from requiring the row to exist, so reverting the component to check that these tests bite still produces the crop measurements -- 263x39, 358x315, 300x36 -- rather than eight timeouts. 5 of 8 fail on the build before this change. |
||
|
logan
|
99a45401c7 |
docs(player): correct the audit's scope, and the probe's false positives
The sweep covered the detail views, Downloads and Autotag as well as the ten primary views; the note said "ten primary views plus the queue". The null result is unchanged and now covers more. Also records the two false positives the probe produced before it was right, since the next audit will write the same two checks: "painted outside the viewport" flags a horizontally scrolling carousel, so the question is whether a scrollable ancestor can bring it back; and a hit test at a control's centre flags everything below the fold in a scroll container. |
||
|
logan
|
dd76bd2fa7 |
docs(player): record the crop, the reflow, and the audit's null result
The audit #51 asks for, at 424x439 on the reference device with a real 1,577-track library: all ten primary views plus the queue. What it did *not* find is worth recording, because it is the promise plan 018 makes -- the shell does not overflow on any view, nothing is stranded outside a scrollable ancestor, and a hit test at each control's centre reaches the control. The width work of #57, #62, #55 and #59 holds; what was left was vertical. What it found is filed rather than fixed here: #186, every control that is not the transport is under the 44px floor, and #187, the seek bar's drag target is 6px. Also the two device traps that cost time despite being written down -- a fresh install downloads the real catalog and the job band then eats 103px of a 439px screen, and it *restarts* after being stopped; and the first-run wizard does not re-check for a library it did not create, so adding one over the bridge leaves it up with a correctly disabled button. Closes #51 |
||
|
logan
|
dee176c0f7 |
test(player): pin the art's shape and the short-screen arrangement
Four of these eight fail on the build before the fix, with the numbers the issue is about: 263x39 at 424x439, 358x315 at 390x700, 300x36 at 900x500, and no row at all below 500px. The fifth viewport, 412x869, passes on both -- which is the boundary landing exactly where the arithmetic says it should, since the leftover only exceeds the width above ~843. Three of them cannot fail on the old build and are said to be guards rather than evidence: that the transport does not scroll off (the old build shrank the art instead, so it did not scroll either), that a tall phone keeps its column, and -- after a first draft that passed on the defect because the subtraction went negative -- a floor on the art at the device's own viewport instead of a comparison with a layout that is no longer there. The wait is on the arrangement rather than on a non-zero box: a previous test leaves the other layout on screen and a stale column satisfies "has a size" perfectly, which showed up as one test passing alone and failing in file order. What this tier cannot see is the device's engine. Nothing here depends on Chrome 113 behaviour -- the sizing rules were chosen by measuring that engine directly, and the numbers are on the issue. |
||
|
logan
|
75a24f98b6 |
fix(player): give Now Playing a layout that survives a short screen
Two things, and the first was a defect underneath the design question rather than an answer to it. **The album art was never square.** aspect-ratio is specified not to re-derive the width when max-height clamps the height, unlike an intrinsic ratio, which is preserved under both bounds. So a definite `width: min(100%, 60vh)` kept its width while the height was clipped and object-fit: cover cropped a square cover into the band -- 264x53 on the reference device, which is what #172's "39px of art" actually looked like. It is not only the phone either: the leftover exceeds the width only above ~843px of viewport, so every height from ~500 to ~843 drew a crop. Both maxes with auto sizes is the fix, chosen by measuring four candidate rules against Chrome 113 itself at five column heights. The placeholder cannot use that rule -- with no intrinsic size it collapses to its icon, 13x58 -- so it is driven from the height, with min-width: 0 because a flex item's automatic minimum is its content, and max-height: calc(100vw - 2rem) because a non-replaced box cannot express "the largest square that fits" and went 380x484 on a tall phone without it. **Then the reflow.** The stacked budget is fixed, so the art gets `height - 386` and that is 53px at 424x439. #172 named two ways out; a floor on the art scrolls the transport off the bottom, and controls never scrolling off is #51's own Direction and plan 018's promise -- so below 500px the art and the names share a row, where the art is bounded by the row's height rather than the column's leftover. 53px to 143px on the device, nothing scrolling, the transport untouched. 500 is where the two layouts cross rather than a round number, and it is keyed on height alone because it answers vertical room: a 900x450 window has the same problem and the same fix. |
||
|
logan
|
327785e5ec |
Merge pull request 'fix(queue): draw the scrim only where it can be tapped' (#182) from fix/171-phone-queue-scrim into main
CI / check (push) Skipped
CI / e2e (push) Skipped
Build & publish the Android APK / apk (push) Successful in 1m29s
Build & publish Arch package / arch-package (push) Successful in 2m37s
Attach the desktop build to the release / linux (push) Successful in 56s
Sync Homebrew formula / sync-formula (push) Successful in 8s
|
||
|
logan
|
7ba5d321f6 |
test(queue): pin the breakpoint listener the scrim rule rests on
The scrim's existence comes from matchMedia rather than a stylesheet, which only holds if the query is listened to — and the stub's addEventListener was a no-op, so deleting the listener left all 986 tests green. The stub records its listeners now and the new case carries a panel across the breakpoint in both directions. Watched failing with the listener removed. |
||
|
logan
|
f126dd7397 |
fix(queue): draw the scrim only where it can be tapped
Below 600px `.panel-content` is `width: 100%`, so the scrim sat entirely underneath an opaque panel -- measured at 424x439, host, panel and scrim all 424x318. It dimmed nothing and dismissed nothing there while wearing `cursor: pointer`, so #24's tap-outside-to-close did not exist on the device it was drawn for. Of the issue's two directions this takes the second. A gutter is the drawer pattern and buys the affordance by taking width off a full-screen surface on a 424px viewport; #55 already made the queue a *screen* at that width, whose ways out are back and a 44px close button. So there is no scrim there rather than an unreachable one. Existence is `matchMedia` rather than `display: none`, on `job-band`'s rule: a hidden scrim is still an element carrying the handler. The 600-899 band, where the panel is a 320px column of a wider content area and the scrim has real uncovered pixels, is untouched. The e2e half asserts *absence* at 424x439 rather than clicking, because a phone-width case that clicks the scrim's centre hits the panel and passes on the broken build -- which the issue anticipates. Closes #171 |
||
|
logan
|
510d3470f9 | Merge pull request 'fix(ui): keep a touch-only affordance reachable, or absent' (#181) from fix/137-touch-only-affordances into main | ||
|
logan
|
d78830aa52 |
fix(ui): make the touch pen a corner chip, not a scrim over the art
Always-visible is not the same as always-in-the-way: the overlay is inset:0 at 50% black, so gating it on hover left every touch device with the artwork it is editing permanently darkened. It is only a hint — .cover-art-edit carries the click, so tapping the art always worked — while the × really is the only route to its action and stays. The chip borrows the remove button's size, disc and alpha. Also corrects the claim that no tier can render as a touch device: no committed one does, which is a choice about projects rather than a limit. |
||
|
logan
|
a72d1f68ed |
fix(ui): keep a touch-only affordance reachable, or absent
Three controls are revealed by :hover and are the only route to their action on a device that has none. #68 hid the home card's play button on touch, which was right because tapping the card does the same thing; these are the opposite case, so hiding them removes the action outright and leaving them costs the same long-press flash #68 was filed for -- they are visibility:hidden / opacity:0, so on touch they are invisible controls that still take taps. track-details' cover-art overlay and remove, and shortcut-capture's reset, are always visible under `@media not all and (hover: hover)`. The queue row's remove is the third case the report names and takes the other treatment, because #60 has since landed: the row's context menu is a bottom sheet carrying "Remove from Queue", so the action is one long-press away and an always-visible X would spend part of a 424px row on something already reachable. It is display:none outside `(hover: hover) and (pointer: fine)` rather than visibility:hidden, which would leave a button holding its hit area and its place in the accessibility tree -- the trap this issue is about. The rule is not extracted into styles/ yet: that leaves two call sites of the always-visible form, under the four the report names. No tier here can render as a touch device, so the tests read the parsed stylesheet the way #68's does and say so; the touch and hover renderings were measured against the running app in a hasTouch context instead. Closes #137 |
||
|
logan
|
60f1c5a6b2 | Merge pull request 'build(frontend): fail css-check on a nested rule the phone drops' (#180) from fix/154-nested-css-check into main | ||
|
logan
|
11ba7b3180 |
build(frontend): sweep every stylesheet, not index.css by name
The hook fires on frontend/**/*.{ts,css} while the script read one hardcoded path, so a second stylesheet would have been silently unswept while the hook still went green over it. There is only index.css today, which is exactly when this is cheap to fix. Watched catching a planted nested rule in a second file.
|
||
|
logan
|
7f8e185d7c |
build(frontend): fail css-check on a nested rule the phone drops
The device renders in Chrome 113, which predates relaxed CSS nesting, so
a nested rule whose selector starts with an element name is not a parse
error anyone would notice -- the rule simply does not exist, there and
nowhere else. Three were live in `index.css`, and the one that mattered
was the `text-overflow: ellipsis` on the bottom bar's title and artist,
which had therefore never truncated on the device. No tier here can see
the class at all: the component tier, the e2e tier and `make ui-visual`
all run a current engine, where the rule applies normally.
So `make css-check` carries a second script. It reads `index.css` and
the `css` literals in `src/**/*.ts` alike, since a shadow-root
stylesheet is parsed by the same engine, and it names the file, the line
and the fix -- a leading `&`, which is valid in both syntaxes.
The detection walks blocks rather than matching lines, and both things
it has to get right fall out of one rule: a rule is nested when a
*style* rule is somewhere above it, not when its immediate parent is a
block. That leaves `@media (...) { bottom-nav { ... } }` at the top
level alone, which is the majority of what a regex over the file would
report, and still flags the same rule inside an at-rule that is itself
inside a style rule. Strings and comments are read through, so a brace
in a `url()` is not a block.
The tree has no violation left, so the check would pass just as happily
over an empty glob: it refuses one, and `test/utils/css-nesting.test.ts`
pins the semantics that make the sweep mean something. The literal
scanner the two checks share is lifted into `css-literals.mjs`
unchanged, except that a `${}` substitution is now blanked keeping its
newlines so a line number survives it.
Closes #154
|
||
|
logan
|
42483c4b61 | Merge pull request 'feat(player): show progress on the phone's bar border' (#178) from feat/58-mini-player-progress-line into main | ||
|
logan
|
deea6ad06d |
test(player): pin the desktop timer gate, drop a leaked queue
Two gaps a review found. The this.phone gate on the interpolation interval is what CLAUDE.md says earns the matchMedia call, and every test passed without it — so it is asserted on the timer count now, since a desktop render is empty either way and cannot tell the two apart. Watched failing with the gate removed. The e2e spec left LONG_TRACK playing in a workers: 1 suite against one long-lived app, immediately before four other phone-* specs. Nine specs clear the queue in afterEach for that reason and phone-transport.spec.ts records the flake it caused. |
||
|
logan
|
fba608fdbd |
docs(player): attribute the phone seek bar's removal correctly
The paragraph said #59 took the seek bar off the phone's transport. It was plan 016 B2 — audio-player.ts says so in the comment above the rule that does it, and CLAUDE.md's own #59 paragraph says #59 removed shuffle, repeat and the queue button. Wrong provenance in the file whose whole value is being right about which change did what. Also stop tracking .pi/journal.md. It is a scheduled run's scratch log, and this repo's memory is CLAUDE.md and .planning/ — a session log arriving inside a feature PR is a new convention landing sideways. |
||
|
logan
|
f59490b113 |
feat(player): show progress on the phone's bar border
#59 took the seek bar off the phone's transport, so the one thing a mini player is expected to say without being opened -- how far through the song it is -- had nowhere left to be said. It is the shell's element and its own 2px grid row between `bottom-bar` and `bottom-nav`, because those two are separate components and either one drawing the line means reaching into the other's box. The fill is `scaleX()` off the same `PlaybackPositionChanged` the seek bar renders, with the same `trackChangeId`/`seq` guards and an interval that only interpolates *between* reports -- never its own clock, which is the rule that exists because a local counter drifted 30 s away from the backend across four keyboard seeks. It is `aria-hidden` and takes no pointer events at any depth: Now Playing's seek bar is what announces the position, and a 2px strip on the top edge of the tab bar is exactly where a thumb aiming at a tab lands. It renders nothing above 600px, from `matchMedia` rather than a media query, because a stylesheet cannot stop a 1 Hz interval running for the life of every desktop session about a line nobody can see. Its phone rule is at the foot of index.css beside `job-band`'s, not in the phone block above: a media query adds no specificity, so a `display: block` written before the `display: none` that takes it out of the desktop grid loses to it and the line never appears at all. Closes #58 |
||
|
logan
|
6cca57f229 | Merge pull request 'fix(explore): scroll the album page as one on a phone' (#179) from fix/66-album-page-scrolls-as-one into main | ||
|
logan
|
ea3edde697 |
fix(explore): scroll the album page as one on a phone
`explore-album-details` was a fixed header over a scrolling tracklist, which is the desktop arrangement. At the reference device's 424x439 the header owned 253 of the panel's 318px and the list scrolled inside the 64 that were left, and the header's flex row squeezed `.album-info` to 112px beside a 200px cover -- so the title drew as one ellipsised glyph and two of the album's three primary actions were clipped by the component's own `overflow: hidden`: "Shuffle album" ended at x=443 in a 424px box, reachable by no gesture. Below 600px the host is the scroller and `.content` stops being one, so the header scrolls away and the page moves together; the header stacks art over info, so the info column has the row's whole width. The tracklist is plain DOM rather than a virtualizer, so nothing inside wants a scroll window of its own. Another `min-width: 0` was not the fix and the issue's own measurement says so: `.album-info` carries one and was shrinking as asked. Nor could `layout-overflow.spec.ts` see any of this -- `body.scrollWidth` equalled the viewport throughout, because the overflow was inside a component -- so the new spec measures each header control against the host's own box, which is `top-bar-fit.spec.ts`'s shape for the same reason. The phone block is last in the stylesheet on `index.css`'s rule: a media query adds no specificity, so above the rules it overrides every declaration in it would be silently dead. Closes #66 |
||
|
logan
|
14e3ab574c | Merge pull request #176: context menus are a bottom sheet on a phone | ||
|
logan
|
4b2eec5703 | Merge remote-tracking branch 'origin/main' into 60-context-menu-action-sheet | ||
|
logan
|
3871d37fdb |
Merge pull request 'chore(agent): add the scheduled backlog-issue prompt' (#177) from pi-agent-backlog-automation into main
Reviewed-on: #177 |
||
|
logan
|
09b005557c |
chore(agent): add the scheduled backlog-issue prompt
A scheduled pi session reads this file and works one open issue end to end: pick, claim, branch, implement, verify on the tier the change demands, open a PR, stop. It declines rather than improvises where it cannot verify itself — a busy :34115 means another worktree is running the app, and a green e2e run against someone else's build is worse than no run at all. |
||
|
logan
|
ef5574d18b |
docs(shell): record the clip, and the four things only a device showed
CLAUDE.md gains the surface beside the keyboard model it shares, and NOTES.md the measurements: the 83px clip with its screenshot, the probe that established a top-layer dialog escapes paint containment from inside a view, the UA stylesheet's 354px, the focus steal a longer retry cannot beat, and the submenu this change pushed off-screen before it pulled it back. The last of those is also a note about scope: the issue was claimed saying the submenu would be measured and filed, and the measurement said fix it. |
||
|
logan
|
31dafb0ce0 |
test(shell): assert the surface, and sweep for a menu that skipped it
No tier here can reproduce the defect: this runner's Chromium and CI's WebKit both have the Popover API, so the popup is top-layered and looks perfectly correct, and a spec asserting "the menu is not clipped" would pass on the broken build. So these assert the mechanism -- that the surface is a native <dialog> at phone width -- which is the same move queue-as-a-screen.spec.ts makes about containment, for the same reason. The sweep is the more valuable half. A thirteenth menu written as a bare <wa-popup> would work in every tier here and be clipped on the device, so this reads every source file and fails on one outside a three-file allowlist, each entry carrying why. It found two call sites the by-hand conversion had missed. Four of the six behavioural tests fail on the build before this change; the two asserting the desktop popup cannot, because that behaviour was already there. Closes #60 |
||
|
logan
|
9e7e7ce5a1 |
feat(shell): put every menu in the app through the one surface
Fourteen call sites, one tag name each and nothing else -- which is what menu-surface's shape buys: the host's panel is slotted into whichever presentation is up, so no item model, no keyboard model and no styling moved. The 48px rows come from contextMenuStyles, the one stylesheet every one of these hosts already includes, because the panel is the host's own light DOM and only the host's stylesheet can reach it. Two of the fourteen were found by the source sweep rather than by the conversion: queue-panel's add-to-playlist popup, which is a real menu. now-playing's cover preview is allowlisted instead -- it is a hover affordance in the bottom bar, so a touch device never opens it and nothing clips it. The playlist submenu had to come too, and that is the one place this change made something worse before it made it better. It is a placement="right-start" flyout anchored to its row, and making the menu full-width moved that anchor to x=0 -- so the flip put the picker at x -182 to 0, entirely off-screen, and "Add to Playlist" led nowhere at all. Before the change the row started at x~245 and the same flip landed on screen. It is a sheet now and stacks over the first, which is also why menu-shown does not re-assert focus while it is open. The three hosts that do not use ContextMenuController -- page-header's overflow menu, playlist-view's hand-rolled menu, queue-panel's picker -- bind menu-dismiss themselves, or Escape would close the sheet and leave their own open flag set. page-header is included deliberately: the clipping does not bite there, since it opens downward from the top of a full-height view, but on a phone every action of an overflowing page lives in that menu at wa-dropdown-item defaults. One surface, so there is no second answer to what a menu looks like. |
||
|
logan
|
9aaa8beb99 |
feat(shell): draw a context menu where it fits, not where it is anchored
On the reference device every context menu in the app is clipped, and the two halves of that are structural rather than incidental. Chrome 113 has no Popover API, so wa-popup takes its own documented fallback and positions with strategy: "fixed"; .main-panel carries contain: layout style paint, and paint containment clips fixed descendants. Measured at 424x439 before any of this: the main panel spans 0-318, the open menu spanned 191-401, and three of its seven items were cut off with no way to reach them. Rows were 29px against a 44px floor. menu-surface is one element with two presentations -- a wa-popup above 600px, a wa-dialog bottom sheet below it -- so the host keeps rendering the panel it always rendered and ContextMenuController keeps driving .active and .anchor as though it were talking to a popup. showModal() is Chrome 37 and uses the real top layer, so the sheet is immune by construction rather than by styling. Four things needed measuring on the hardware rather than reading. "A dialog escapes containment" was the premise and was untested here: every other dialog in this app is mounted in index.html, outside .main-panel. A probe dialog appended to track-list's shadow root paints to y=439, over the mini player and the tab bar. A native dialog's UA stylesheet centres it and caps its width, which drew a 354px panel in the middle of a 424px screen -- so four declarations in this component are pure undoing. wa-dialog focuses [autofocus] or itself on the frame after showModal(), and it cannot see our first menu item to prefer it: the panel is slotted, so its own querySelector stops at the <slot>. A longer retry budget does not fix that, because the first attempt succeeds and is then overwritten -- hence menu-shown and MenuKeyboard.refocus(). The budget became time-based anyway, since what is being waited for is another component's animation. And a dismissal has to travel back: wa-dialog closes itself on Escape, which would leave the controller believing the menu is open. The failure mode there is not a stuck sheet but the *next* long-press doing nothing, which reads as the gesture breaking. |
||
|
logan
|
2e29e67664 | Merge pull request #174: Android: leave the volume to the system | ||
|
logan
|
f26b44db08 |
docs(player): close three of the four gaps with a real device
A Light Phone III (Android 14, SDK 34, arm64, Chrome 113 at 424x439) was attached after the PR was opened, so what it listed as unverifiable was checked rather than left as a caveat. SystemOwnsVolume answers true on the device -- the build tag, the constant, the field and the generated binding, end to end, which is the one thing a source sweep only approximates and which nothing else here compiles at all. The control is absent in both mount points on the real engine, and the transport measures 143px, exactly what the desktop-headless "after" predicted. A stored volume of 37 survives a session that demonstrably rewrote the row. The duck is the one that stays open, and now for a stated reason rather than for want of hardware: the foreground service omits setWillPauseWhenDucked from Oreo, so the framework attenuates us itself and never sends AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK -- the device logs `requestAudioFocus() ... flags=0x0` saying so. minSdk is 21, so that path is live code on Android 5.0 to 7.1 and unreachable above it. Asking for a modern phone will not test it. |
||
|
logan
|
b43172a60c |
docs(player): record who owns the volume, and what it gave back
CLAUDE.md's volume paragraph ended "#64 asks for it to be gone on Android outright, which is a platform question the frontend cannot currently ask", which is no longer true -- it can, and the paragraph now says why the answer is a capability rather than a viewport and what that costs. The mediacontrols entry gains the corollary: on that platform "the user's level" is a constant, and the duck is the one thing that may still move the output. NOTES.md carries the measurements: the per-element budget at 424x439 before and after, the :host([hidden]) specificity trap, the fact that the bar's centring survives the control going away, and what no tier here could check. |
||
|
logan
|
2be6fb3066 |
feat(player): draw no volume control where there is no volume
volume-control asks the player whether there is a volume of ours to
control, and renders nothing when there is not. The decision is in the
control rather than at either mount point because there are two, and
one of them -- the bottom bar's -- lives in index.html, which has no
module scope to make it conditional.
It could not have been a width, and that is the whole design decision.
Every other stand-down rule in this app is keyed on a viewport, because
a width is what a browser can answer and what every tier can test.
This one is a property of the build: keyed on width, an Android tablet
at 600px or more draws the bar's slider over a level the backend has
pinned -- a control that cannot act, on exactly the platform the rule
exists for, which library-status-indicator settled is worse than none.
The same rule is wrong the other way below 600px, where a narrow
desktop window has no hardware keys to fall back on. index.css keeps
its phone rule, which is now about room and says so.
Rendering nothing and hiding the host are both needed and are separate
assertions: an empty shadow root is what stops a by-role or positional
query finding a button that cannot act, and :host([hidden]) is what
stops the element taking a flex item's worth of the transport. The
host rule has to be written down, since :host { display: inline-flex }
outranks the UA's [hidden].
Measured at 424x439 by flipping the constant and rebuilding: the album
art goes 39px to 68px and the transport 172px to 143px -- 29px, being
the 21px control plus the 8px gap a hidden box stops drawing. The
bar's centring is unaffected, since #23's outer columns are the same
min() expression rather than content-sized.
volume-ownership.test.ts is the tier that can exercise the Android
rendering, on an ordinary Linux runner, because the predicate is a
stubbable backend answer. Both of its tests were confirmed to fail on
the build before this.
Closes #64
Closes #172
|
||
|
logan
|
867ced8c81 |
feat(player): leave the volume to the system where the system owns it
On Android the hardware keys are the volume control and the framework mixes our stream against the device level, so a second control inside the app moves something the user already moved. Where that is true the player's level sits at maximum, SetVolume / ChangeVolume / MuteToggle are refused, and nothing persists a level nobody chose: restore remembers the stored value instead of applying it, and saveState writes that same value back rather than recording the synthetic maximum. Mute is in that list because it is a level of zero by another name -- and because with no control rendered it would be the one state on such a platform the user could not get out of. The predicate is named after the capability rather than the platform, because that is what makes it testable. Only platformOwnsVolume is behind a build tag, in two files that declare nothing else; everything else is decided against Player.systemVolume, a field a test sets either way. That is mediacontrols' split, with androidpayload.go's reasoning for keeping the contract out of a tagged file, and the tagged pair is covered by a source sweep since no tier here compiles both halves. SetDuck is deliberately untouched: it applies its attenuation by re-applying the *user's* level through setVolumeLocked, so pinning that level to maximum leaves the offset arithmetic exactly as it was. It is the only thing that may still move the output on such a platform, and TestSystemVolumeStillDucks is that property rather than a comment. |
||
|
logan
|
fd71ef53c5 | Merge pull request 'The phone transport: three controls, sized for a thumb' (#173) from 59-slim-the-mini-player into main | ||
|
logan
|
e3b64f9255 |
test(player): assert the desktop bar's size by mechanism, not by pixels
WebKit draws the same button 36x24 where Chromium draws 33x21, so the literal this pinned failed in CI on a build where nothing was wrong. A button's box comes from the UA stylesheet when the author sets nothing, and what each UA sets is its own business. What must not happen is that *we* set something. So: `min-width` and `min-height` compute to 0px, the font-size still equals that of a bare button probed in the same page, and all five boxes are identical -- which is what says the desktop is neither sized context. Checked by re-introducing the `font-size: inherit` regression, which it catches in Chromium; the literal form could only be checked by hand. |
||
|
logan
|
c7e5a4f086 |
docs(player): record the phone transport, and four silent failures
The model in CLAUDE.md beside the volume rule it qualifies; the measurements and the four things that cost a cycle each in NOTES.md, dated. Three of the four are invisible to every assertion in the repo: a button not inheriting its font, a nested rule out-specifying a later one, and art whose height is bounded by nothing. |
||
|
logan
|
f65822c4b2 |
test(player): pin the phone transport, and the desktop bar not moving
Ten tests, five of which fail on the build before this. The desktop guard is meant to pass there -- that is its job, and it is the one that caught a three-pixel regression nothing else could see. `openTheQueue` moves to the fixtures, because hiding one button failed ten tests in four files about the back stack and about layout: every one of them opened the queue by clicking `#queue-button`, and so was quietly asserting *which* route exists as well as what the queue does. The route differs by width now and that is the feature. Two smaller things. The play button is named for its action, so an exact 'Play' waits out a fixture track -- 11.1s per test, passing by luck, and it would have failed outright against LONG_TRACK. And the "nothing playing" case clears the queue itself rather than trusting the app not to have played anything: `make e2e` runs one long-lived app across every spec (#168), which is how a deterministic bug first showed up as a flake. |
||
|
logan
|
32d4dc2c82 |
feat(player): slim the phone's mini player to three controls
Shuffle, repeat and the queue button leave the phone's bottom bar. They are not gone: all three are on the full-screen Now Playing view, one tap away through the mini player's art, which is the "reachable only from Now Playing" this issue asks for. #55 is what makes the queue half safe -- it is a screen with an entry in the back stack now, rather than a panel with no way out but the button being removed here. Removing a control is only allowed because it is still reachable, which is plan 018's matrix promise, so that is what the spec walks rather than counting buttons. It found that the route did not exist in the state that matters: `now-playing` renders two branches and the no-track one had no `.expand` button on its placeholder, so with nothing loaded there was no way to the full-screen view at all -- and once the queue button left the bar, no way to the queue. The queue is persisted across restarts, so "tracks queued, nothing playing" is a state the app launches into, not a corner. The favourite stays on the bar and was 18x14px, the smallest control in the app, against the 48x48 art beside it. One CSS trap, because it failed silently. The phone block is last in index.css on purpose -- a media query adds no specificity -- but the rule it overrides here is written *nested* inside `.bottom-bar`, so it builds to a descendant selector one class more specific and a bare `#queue-button` lost to it. Being last is not enough when the thing above is more specific. Closes #59 |
||
|
logan
|
218e4f5e99 |
feat(player): give the transport a context, and thumb-sized controls
Measured at the reference device's 424x439, every button here was 33x21px -- in the bottom bar and on the full-screen view alike. #56 reports them as "the most important thing in the mobile app and they are tiny", and that is the number behind it. The context is a **property, not a media query**, and that is the whole design. Everywhere else in this app a component states what it drops at phone width itself, because a media query inside a shadow root is answered by the viewport and that is the honest signal. Here the two hosts want different answers at the *same* viewport: on a phone the bar wants three controls sized for a thumb and now-playing-view wants five, larger still. So the host says which context and the viewport says which size band, and neither alone can express it. Play/pause alone goes above the 44px floor. A row of five identical squares says every action is equally likely, which is not true of play -- "large play/pause, adequate prev/next" is the Direction, and a spec caught that the first version had sized all three the same. Two things that fail silently: The desktop bar must not move, and a `<button>` does not inherit its font from its parent -- the UA stylesheet gives it one. So a generic `font-size: inherit` is not the no-op it reads as: it took every desktop control from 33x21 to 36x24. The box rules take a zero fallback and the font-size rules are scoped to the two contexts that set one. And the art on now-playing-view overflowed its own box, drawing over the header above and the title below, because `aspect-ratio: 1` with a definite width derives a height that nothing bounds -- 60vh bounds the viewport, not the room left over. `max-height: 100%`. Pre-existing; found by reading a screenshot, which is the only tier that can see it. What is left is #172: with the transport at 172px of a 439px screen the art is a 39px sliver. Closes #56 |
||
|
logan
|
56a5ff99fe | Merge pull request 'The queue is a place while it covers the content' (#169) from 55-queue-as-a-screen into main | ||
|
logan
|
af4b28b0d7 |
docs(queue): record why the queue is not a detail view
The measurement that decided it, dated, in NOTES.md -- the overlay's rect against the main panel's, the three things that were genuinely missing, and the computed containment of both candidate mounts. The model itself goes in CLAUDE.md beside the overlay rule it extends. |
||
|
logan
|
4ee5b4b473 |
test(queue): pin the back stack and the mount that was not taken
Nine tests, and the header says which of them reproduce the defect: three do, and the other six cannot. "The entry is not orphaned" and "a docked column is not in the stack" are both vacuously true of a build that pushes no entry at all. That was established by reverting the source and re-running, not assumed. The containment assertion is the one worth reading twice. It asks where the panel *is* rather than whether a menu is clipped, because CI's Chromium and WebKit both have the Popover API -- so the symptom is invisible here and a spec asserting "not clipped" is green on the broken build. `.planning/NOTES.md` states the mechanism. The rest assert the entry rather than `aria-expanded`, which is the shell's own bookkeeping and was right throughout the defect: what has to be true is that one back press closes the queue and the *next* one navigates. |
||
|
logan
|
a70a7ed9eb |
fix(queue): size the queue screen's way out for a thumb
Measured at 424x439: the three header actions were 25x21px. That matters more than it looks, because with the panel spanning the whole width the scrim underneath it has no uncovered pixels at all -- so the close button is the only pointer route out of a full-screen surface, and it was below the 24x24 floor in one dimension. Sized only in overlay mode. Inline these sit in a 320px column beside the content, where a mouse is what reaches them and 44px of header is 44px the queue does not get. |
||
|
logan
|
de2cb2693a |
feat(queue): give an overlaid queue a place in the back stack
The queue's pixels were already right. Measured at the reference device's 424x439, #24's overlay is 424x318 -- `.main-panel`'s rect exactly -- so the `DETAIL_LOADERS` mount the issue's Direction asks for would draw the same rectangle in the same place. What was missing was the navigation model: opening the queue on Artists and pressing back moved the page *underneath* to Albums and left the queue up, which is a press that changes something the user cannot see and costs them their place. So the queue is a *place* exactly while it is an overlay, and a *control* while it is a column. A column is a thing the user docked -- back must not undock it and a navigation must not take it away -- and that reuses #24's computed mode rather than adding a breakpoint, so the drag-resizable panel width keeps deciding it. It is in neither `VIEW_TAGS` nor `DETAIL_LOADERS`, because there is nothing to mount and moving it would cost something. `.main-panel > *` computes `contain: content` under a `.main-panel` that does too, and paint containment clips the `position: fixed` a `wa-popup` falls back to on Chrome 113 (#60) -- so the detail-view mount would have broken `queue-panel`'s working context menu on the one device this is about. The panel's ancestry today is paint-free to `body`. Two details that fail silently otherwise. The entry is unwound from the panel's `open` attribute in the observer that already ran for `aria-expanded`, not at each of the four ways out -- without that the entry is orphaned and the *next* back press is the one that closes the queue, which is this defect moved one press later. And the navigation writes neither `dataset.activeView` nor `searchStore.setCurrentView`, because both describe what is *in* the main panel and the queue covers that panel without replacing it. `now-playing-view`'s copy of the button went through the helper too: it set `open` directly, so on a phone it produced exactly the queue with no entry behind it that this removes. Closes #55 |
||
|
logan
|
880adff12c | Merge pull request 'Drop the phone's top bar; search becomes a button and a modal' (#167) from feat/57-drop-the-android-top-bar into main | ||
|
logan
|
d6f7412e9d |
docs(shell): the phone has no top bar, and why the modal is a dialog
CLAUDE.md's shell prose said the phone's header "controls shrink or stand down"; there is no header there now. The search box's section gains the modal and the four rules behind it, page-header gains the count as the last thing to yield, and the top-bar-fit section gains what happens below its own band. NOTES.md gets the three measured facts, dated: `contain: paint` is why a Web Awesome popup is clipped on Chrome 113 and why no tier here can reproduce it, the arithmetic that cost the page header its count at 320px, and the shared long-lived e2e app that makes an absolute coordinate a hidden assertion about background jobs. |
||
|
logan
|
1ab767a317 |
feat(shell): take the top bar out of the phone's layout
The row is deleted from the grid template below 600px, not the header hidden. That is 3.25em of a 439 CSS px viewport -- the single biggest vertical win the reference device has to give, and the reason the issue asks for the row rather than for a smaller bar. Each of the five things the bar held has somewhere else to be there: nav-history is the platform's own back gesture and was already gone from 899 down, the job indicator is <job-band> (#62, which is what this was blocked on), the search box is a modal opened from the view's own header, the library filter is Settings -> Libraries, and the wordmark stays where it is. Three things are load-bearing. **The header is visually hidden rather than display: none**, because that h1 is the document's top-level heading and several pages have no other one -- page-header renders no h1 when its heading is empty, and Settings has no page-header at all. Its four controls are display: none *inside* it, which is what keeps them out of the tab order: a visually-hidden container is still focusable, and tabbing into a search box nobody can see is worse than not having one. **The fit pass stands down**, from the bar's computed position rather than from a width. With the bar out of flow there is no content box to measure children against, and a pass that ran would collapse the wordmark on every resize and report success about a 1px box. **top-bar-fit.spec.ts keeps 390 and asserts the stronger property.** "Nothing hangs out of the bar" is trivially true of a bar with no row and would pass on a build that merely broke it, so what that width asks now is that the content starts where the row above it ends. Measuring against the window instead would have been asserting "and no background job is running", which that spec is not about and cannot arrange. Closes #57 |
||
|
logan
|
ac8f86eb00 |
fix(settings): give the library selection a home that is not the top bar
library-filter is the only control in the app that calls setSelectedLibrary, and the phone already hid it with a comment saying it was "reachable from the drawer's Settings". It was not: Settings adds, removes, renames and scans libraries, and does not set the view filter, which is a different thing -- it decides what Albums, Artists and Genres show. A phone therefore inherited whatever a desktop session last chose and could neither change nor see it, which is #24's sentence broken in the band it was written for. It is a second *placement* of the same component, not a second control, and it is at every width rather than below 600px. A phone-only copy is the cheaper answer and is the fault rather than the fix: "where do I change which library I am browsing" having two answers by viewport is exactly what one control in two places avoids. Closes #148 |
||
|
logan
|
47bd9ef211 |
fix(header): let the count yield before an action is clipped
Adding the phone's search button to this header is 43px more than the row has at 320px, which is a width the app promises and which header-action-overflow.spec.ts asks about. Measured on Playlists there, after the fit pass had already collapsed all three actions into "More actions" and truncated the title to nothing: title 0, count 50, sort 143, search 40, More 38, five 12px gaps and 32px of gutters -- 363 in 320, with the More button ending 27px past the edge. That is an action clipped, which is the exact defect this pass exists to prevent. The count is what yields, last, because it is the only item on that row that is neither an identity nor an action. The title yields first and may ellipsis away entirely, since the navigation also says which page you are on; the sort control and the buttons are each the only place they are said. An empty page says it is empty in its empty state and a full one is being looked at. With the count gone the header is 304 in 304, and the title comes back to 19px. It is rendered and hidden with an attribute rather than returned as `nothing`, for the reason the action buttons are: every pass starts from all-visible and needs a node to un-hide, or the first 320px window costs the count for the rest of the session. |
||
|
logan
|
b801fa533a |
feat(shell): make search a button and a modal where searching applies
The phone's top bar is about to go, and the search box is the one thing in it that is an action rather than chrome. It becomes a button in the row that already says which page you are on, opening a wa-dialog with the real search box in it. Three decisions worth the words. **A wa-dialog, and that is a mechanism rather than a taste.** wa-popup renders `<div popover="manual">` and feature-detects the Popover API, falling back to `strategy: "fixed"` where there is none -- which is Chrome 113, the reference device, since `popover` is Chrome 114. And `position: fixed` escapes ancestor overflow but not `contain: paint`, which `.main-panel` carries, so a popup-shaped search panel opened from a view's header is structurally clipped on that device. `<dialog>` / `showModal()` is Chrome 37 and uses the real top layer. No tier here can see the difference -- CI's Chromium and WebKit both have the Popover API -- so the component test asserts the *mechanism*, a native `<dialog>` in the tree, rather than the symptom. **An element, not a PageAction.** Two of the seven searchable views are detail views with no page-header; they filter on the term and say so in their own headers. Declaring search as an action would mean seven hosts each writing it out, which is a second list of searchable views, and it would put a phone mode for actions inside page-header, which that component documents its refusal to grow. search-store's own map is the condition, asked by one component placed three times. **The modal carries the real search-bar**, so there is still one debounce, one clear button and one view-scoped placeholder. Escape closes it and *keeps* the term -- the input treats Escape as "clear the search", which is right in a header where the box stays on screen and wrong in a surface whose dismissal would then discard the search. |
||
|
logan
|
8879192097 | Merge pull request 'Show background jobs in the phone's layout, not a popover' (#166) from feat/62-jobs-as-a-notification into main | ||
|
logan
|
f76ee96ac4 |
docs(jobs): the phone's band, and why it is in flow
CLAUDE.md's jobs section said the header indicator is "the one view of everything at once, from every page"; that is now true on a desktop only, and the band is the phone's half. NOTES.md takes the measurement that decided the shape -- an overlay band at 424x439 is a lid, not a notification -- and the corollary about which tier can see it: ui-test, tsc, lint and the Go suite all passed on the broken version, and what failed was three e2e specs that have nothing to do with jobs. Run the suite, not the spec you wrote. |
||
|
logan
|
23f5a0c53a |
feat(shell): show background jobs in the phone's layout, not a popover
The header indicator is a disclosure anchored to a bar 3.25em tall on a screen 439 CSS px tall, and it was reported as unreadable behind other UI. Background work is the one thing a phone should not make you open something to see, and #57 deletes the bar it hangs from and is blocked on it having somewhere else to live. Below 600px the indicator stands down and <job-band> takes over. It is the existing job-panel at `kinds="*"`, so pause, cancel, Details and the log come along, and so does applyJobControl. **It is in the layout, not over it**, and that was measured rather than assumed. The first version put the panel in notification-host's fixed band: it renders correctly, sits on top and stays inside the viewport, and is unusable -- at 424x439 a compact panel showing two jobs is ~216px of a 439px screen, drawn over the content and swallowing every tap under it. Four e2e specs caught it, and none of them was about jobs: two phone-shell journeys and the header's action menu, all failing on clicks the band was intercepting. As a grid row above the main panel it pushes instead, which is #24's one sentence deciding a layout question -- a band that hides the app to say the app is busy has traded the popover's fault for a worse one. It renders nothing above 600px, from matchMedia rather than a media query, because that decides whether the element exists: Settings already holds four job-panels and a fifth answering for every kind is bottom-nav's "resolved to 2 elements" trap again. index.css keeps it display:none off the phone for a second reason -- an in-flow grid child with no named area is auto-placed into one of the shell's rows, which is what the skip link is absolutely positioned to avoid. top-bar-fit's 390px case asserted the indicator was up, so that it could not pass by measuring the idle case under another name. At phone width it is now deliberately away, so the assertion takes the other branch of the same rule -- the indicator is hidden, the band has the row, and the bar still has nothing hanging out of it -- rather than the width being quietly dropped from the list. The report's own symptom is deliberately not asserted anywhere: it did not reproduce in this tier. Measured at 424x439 the popover was neither clipped nor covered, so a spec claiming a stacking fix would be asserting something that was never true here. The spec says so. Closes #62 |
||
|
logan
|
502b814a65 |
feat(jobs): let a panel answer for every kind, at either density
Three properties the phone's band needs, added here so it is the same panel rather than a second job UI -- which is what keeps `applyJobControl` and its "you will discard hours of downloading" confirmation in the picture. `kinds="*"` is every kind, which is what the header indicator was for. Spelled as a star rather than taken as the meaning of an empty attribute, because empty is what a typo and a dropped binding both produce and "show everything" is the wrong thing to do by accident; empty still shows nothing. `density` is passed to `job-row`, whose `compact` variant its own source calls "the popover density" -- which is exactly what the band replaces. `full` stays the default, so the four settings call sites are untouched. `active-only` drops terminal rows. The band is in the layout, so a finished row there holds the content down after the work is done; Settings keeps them, because that is where "did the last scan work" is asked and a finished row there dismisses itself. |
||
|
logan
|
c19a806298 | Merge pull request 'Give the seek bar's interpolation interval one owner' (#165) from fix/53-seek-bar-never-moves into main | ||
|
logan
|
67eeb75e7b |
docs(android): the device can be driven, not just looked at
The runtime call does not go over HTTP on Android — the WebView cannot deliver a fetch() POST body to shouldInterceptRequest, so v3 routes runtime calls through the addJavascriptInterface bridge. Two things follow that cost an hour each before the v3 source was read: `.playwright/init-events.js` does not transfer to the device (its outbound half hooks fetch, and a POST to /wails/runtime answers "missing object value" — which reads like a wrong payload and is the interceptor getting no body at all), and hooking fetch from an eval is too late on any platform because the bundle captured its reference at module scope. The recipe that does work goes in, along with how to get audio onto the phone (scoped storage silently swallows a push into /sdcard/Android/data/<pkg>/files, and the fixtures are 2 seconds long, which is useless for watching a seek bar) and the permission dialog a reinstall raises, which looks exactly like the app failing to start. NOTES.md takes the #53 measurements: that its frontend is byte-identical to the v0.3.1 the phone carries, that the symptom does not reproduce on main in four scenarios, and that reverting only backend/player/ to v0.3.1 reproduces #125 instead — with the shim that makes that a ten-minute experiment rather than a full checkout. |
||
|
logan
|
fe1fbefee7 |
fix(player): give the seek bar's interval one owner
`handleInput()` called `stopProgress()` and mutated no reactive state, so Lit scheduled no update, `updated()` never ran, and the tail of `updated()` that restarts the interval never executed. Only a `change` event or the next backend report could bring it back — so an `input` that never commits froze the interpolation: a drag cancelled outside the element, a pointer taken by a scroll, or a touch on the track treated as a scrub, all ordinary gestures on a phone. While playing the 1 Hz report papered over it within a second; with reports not arriving it was permanent. The drag is `@state` now and `updated()` decides whether the interval runs, so there is one place that knows. `handleChange` no longer starts it directly for the same reason. A flag set on `input` can strand, which would turn a stall of up to a second into a permanent one — the failure this removes. `change` is the ordinary end; `pointerup`/`pointercancel`/`touchend`/`touchcancel` on the document are the ends that are not, attached with the drag and dropped with it, because the pointer is routinely released outside the element it started in. The other half is that a report arriving mid-drag used to overwrite `seekValue` and pull the thumb out from under the finger once a second. It is skipped while dragging, and its seq is deliberately left unrecorded so the first report after the drag still counts as fresh. Three tests, all exercised against the fault: two fail on the old component, and the third fails if the drag flag is left set — which is the failure mode the fix introduces and the listeners exist to prevent. Verified on the device too (Chrome 113): mid-drag the bar holds its value and ignores reports, and on release it adopts the backend's real position and resumes ticking. Closes #164 |
||
|
logan
|
de04339494 |
Merge pull request 'Android: install and launch the package the APK declares' (#163) from fix/159-android-task-app-id into main
CI / check (push) Skipped
CI / e2e (push) Skipped
Build & publish the Android APK / apk (push) Successful in 1m27s
Build & publish Arch package / arch-package (push) Successful in 2m43s
Attach the desktop build to the release / linux (push) Successful in 57s
Sync Homebrew formula / sync-formula (push) Successful in 6s
|
||
|
logan
|
998ce75fb6 |
docs(android): the identity is read back, not declared twice
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. |
||
|
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. |
||
|
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
|
||
|
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 | ||
|
logan
|
8a757c9bb4 |
docs(android): record the lifecycle model and the device check
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 |
||
|
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
|
||
|
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
|
||
|
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
The button tied with the cover placeholder on paint order and lost, so it did not work for any track without artwork. Closes #150 |
||
|
logan
|
ffc9490a32 |
fix(player): put the phone's Now Playing button above the artwork
`.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 |
||
|
logan
|
dc6625d33a |
Merge pull request 'Centre the transport, and show the volume inline' (#155) from feat/42-inline-volume-and-centred-transport into main
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 |
||
|
logan
|
dc8db159f9 |
feat(player): centre the transport and show the volume inline
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 |
||
|
logan
|
86e7444603 |
Merge pull request (#157) from fix/156-queue-selection-fixture-order into main
The spec asked for the first few tracks and needed one with an album. Closes #156 |
||
|
logan
|
2365806d18 |
fix(e2e): ask the fixture for a track that can navigate
`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 |
||
|
logan
|
7d348f243a |
Merge pull request (#153) from fix/151-fuse-the-scroll-guard-and-the-write into main
Removes the window between the scroll guard and the write it guards. Closes #151 |
||
|
logan
|
ddd04623f7 |
test(e2e): fuse the scroll guard and the write it guards
`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 |
||
|
logan
|
9ad1477b1e |
Merge pull request 'Pin the queue panel'''s mouse model' (#152) from fix/43-queue-panel-selection into main
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 |
||
|
logan
|
70ab3ddf94 |
docs: record two measurements from the queue selection work
The first is a second instance of a rule CLAUDE.md already states, with numbers: a virtualized list can be repainting for a reason you are about to delete, and here there are two such reasons — so removing either alone changes nothing observable, and removing both leaves the highlight seconds late rather than absent. That is the shape a poll cannot see, which is the general lesson worth keeping. The second is the hit-scan, because it stopped a wrong fix: the queue panel is 12% link and the track list 21%, which is the opposite of the assumption the fix was being built on. |
||
|
logan
|
4f7529c315 |
test(queue): pin the panel's mouse model, and bound the highlight
Single click selects, ctrl and shift extend, double click plays from that row — all four already worked, and nothing in either tier pinned any of them, which is why the report could be made and could not be settled. `queue-reorder.spec.ts` covers the keyboard and `queue-overlay.spec.ts` the panel's mode; the pointer path had no coverage at all, so "selection is broken here" and "selection is fine here" were equally consistent with a green suite. Measured with real mouse events rather than dispatched ones, because a synthetic click aimed at the row bypasses the only thing that could be swallowing it: click row 1 selects 1, ctrl+click 4 gives 1 and 4, shift+click 7 extends to 1,4,5,6,7, a plain click collapses to one, and a double click on row 3 leaves the backend playing row 3. The three candidates the issue lists are all answered. The repaint was already correct, and already correct on the day the issue was filed. `resolveTrackIndexFromEvent` reads data-index, and DOM order matches data order. A row control does swallow the click — `explore-link` stops propagation on purpose, so a click on a name navigates and selects nothing — but a hit-scan across a row makes the queue 12% link against the track list's 21%, so the panel called broken is *less* covered by links than the list called correct. That measurement killed the fix this started out as. Two traps are written into the spec because both faked a defect while measuring. Fixture tracks are 2 seconds, so "double click row 3" read a moment later reports whatever auto-advance moved on to — recorded twice as an off-by-one that is not one, which is what `LONG_TRACK` exists for. And the selection assertions are bounded at 500ms rather than polled with the default 5s: `queue-panel` repaints two ways, the explicit `requestUpdate()` and a per-render `keyFunction` arrow, and with *both* removed the highlight still arrives — at 134ms, 3.9s and 5.8s against 5-17ms healthy. Four seconds is indistinguishable from broken to a user and invisible to a generous poll. Mutation-tested rather than trusted: `playAtIndex(index + 1)` fails both double-click tests, treating every click as ctrl+click fails both selection tests, and removing both repaint mechanisms fails all three selection tests — the last only because of the bound. Closes #43 |
||
|
logan
|
bb21072386 |
Merge pull request 'The top bar decides what it can afford to show' (#149) from fix/143-top-bar-fits-its-window into main
Fits the top bar to its window by measuring it, at every supported width and with work in flight. Closes #143 |
||
|
logan
|
ead1354e4d |
docs: record how the top bar decides what to drop
The shell section already states the three size bands and the promise that no action is unreachable at any of them; how the header chooses what to give up belongs beside them, because the promise is what decides it. Two measured facts go to NOTES.md rather than here. `scrollWidth` counts a box's left padding and not its right, so the obvious fit predicate under-reports by a gutter and passed on a bar with a control jammed against the window edge. And the overflow is 11px idle and 262px while working, which is why the issue was filed twice with different numbers — a seeded app that has finished scanning is idle by the time you resize it. Closes #143 |
||
|
logan
|
ae85df0dad |
fix(shell): give the top bar a measured fit at every width
The bar was 611px inside a 600px viewport at the bottom of the Compact band, and 862px while a scan with a real library's title ran, because `job-indicator` is `hidden` when idle and 235px wide when it is not. `body` is `overflow-x: auto`, so a user got a horizontal scrollbar on a shell #24 promised would not need one — and the band is 600 to 899 with work in flight, not the 600 to 610 the idle measurement suggested. `services/top-bar-fit.ts` is `page-header`'s treatment one bar up: a ResizeObserver, every pass starting from all-visible, hiding the lowest-priority child until it fits. Measured rather than breakpointed because three of the five children are as wide as their content — the library filter by the longest library name, the indicator by the running job's title, the search box by its view-scoped placeholder — so any width picked is right for one library, one job and one view. What yields is decided by #24's own sentence, which rules out the two cheapest candidates in the Direction. Hiding the library filter takes away an action, since it is the only control in the app that selects a library (filed as #148, which is the phone already doing it), and collapsing search to an icon is #57's, which is blocked behind #62. So the wordmark yields first — a brand the window title bar repeats, and visually-hidden rather than `display: none` because that h1 is the document's heading — and then the indicator's label, leaving the ring, which the component already does below 600px and whose live region announces the state either way. "Fits" is the children against the content box, not `scrollWidth` against `clientWidth`: `scrollWidth` counts the left padding and not the right, so the first version read 700/700 with the indicator sitting in the whole right gutter. And the bar does not resize when a job starts, which is the case this is for, so every child is observed too. Pinned before it was fixed, as the issue asks. On the unfixed build the new spec fails at 600 idle and at 600, 800 and 900 with a job, and passes at 390, 899 and 1440; `layout-overflow.spec.ts` gains 600x600 and failed there. That spec asserts on the *shell*, so it was green throughout this defect — the per-child measurement is #69's lesson, and it is what caught the gutter case above. Closes #143 |
||
|
logan
|
6e7e349e63 | Merge pull request 'Fold the Jobs tab into the places the work is started' (#147) from feat/27-jobs-into-settings into main | ||
|
logan
|
f9ba9a87d7 |
docs: note that CI's two engines share one app
Which is why a shared-selector fault can be green on chromium and red on webkit in the same run, and how to reproduce it locally. |
||
|
logan
|
e4efec6f0c |
fix(e2e): the third spec that located a disclosure by class
`config-section .header` is ambiguous once a section holds a job, and `failure-voice.spec.ts` was the one I did not grep for. It passed on chromium and failed on webkit in the same CI run, which is the tell: the two engines share one app, so the second one runs with a finished scan the first one left behind. The NOTES entry already says a class name is not a selector's contract; this is the same fix, by role and name. |