0d331666d68bf0119d747f22e2fef4e08930ba78
1210
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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. |