Compare commits

...
Author SHA1 Message Date
logan 0d331666d6 fix(shell): make the header's touch targets cost no width
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m31s
CI / e2e (pull_request) Successful in 9m28s
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.
2026-08-21 20:12:51 -04:00
logan 6a5a3c33dc fix(shell): raise the page header's controls to the touch floor
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Failing after 9m43s
#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.
2026-08-21 19:45:20 -04:00
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
CI / check (push) Successful in 2m31s
CI / e2e (push) Successful in 9m29s
2026-08-21 22:25:52 +00:00
logan ec64dbded0 fix(player): give the seek bar a thumb-sized hit area
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m34s
CI / e2e (pull_request) Successful in 9m28s
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
2026-08-21 18:12:29 -04:00
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
CI / check (push) Successful in 2m32s
CI / e2e (push) Successful in 9m33s
2026-08-21 21:43:38 +00:00
logan 30c6b665f1 fix(system): give the process a temp directory that exists
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 3m0s
CI / e2e (pull_request) Successful in 9m37s
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
2026-08-21 17:23:23 -04:00
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 e7748f1 and the pass returned quietly having done nothing:

    W/yellowjacket: msg="release-group mbid backfill: query failed"
      explore.error="SQL logic error: no such table: release_groups (1)"

What it does is resolve a release-level MBID (MUSICBRAINZ_ALBUMID, which
many taggers write instead of MUSICBRAINZ_RELEASEGROUPID) into the
release-group MBID everything else on the album page is keyed by. A scan
cannot afford a live MusicBrainz call, so `library.updateMBIDs` stashes
the release MBID in `pending_release_mbid` and defers to this. With this
broken the marker was written by every scan and resolved by nothing, so
those albums were untagged as far as the catalog is concerned,
permanently.

**The fix is to call the queries plan 013 already wrote.**
`GetAlbumsWithPendingReleaseMBID` and `ResolveAlbumPendingReleaseMBID`
have been in sql/queries/albums.sql since that change, generated and
never called -- the writer of the marker was repointed at `albums` and
the reader was not. So this is not a missed rename so much as a call
site left behind, and thirty lines of raw SQL and hand-rolled scanning
become three.

That is also the durable half. These two were the last raw-SQL
references to a schema table in the tree, and being raw is exactly why
013 missed them: sqlc reads sql/schemas/ and cannot generate against a
table that is not declared, which is what made every other statement in
the repo immune to the same rename.

Three smaller things.

**The LIMIT came back.** The raw statement bounded a run at
releaseGroupMBIDBackfillMaxPerRun and 013's sqlc replacement had no
LIMIT at all, so switching over as-written would have swapped a dead
pass for an unbounded one -- each row is a live MusicBrainz lookup on a
1 req/s limiter shared with every page the user can open.

**The UPDATE goes through the writer.** `ReadQueries` is a query-only
pool and an UPDATE issued on it fails at runtime with "attempt to write
a readonly database".

**The query is its own method so its failure is assertable.**
A test of the pass as a whole cannot see this bug, because a query
error and an empty library are the same early return -- which is the
whole reason it survived. `pendingReleaseMBIDs` returns the error, and
the test reproduces the device's exact message against the old
statement.

Verified on the reference device: the warning is gone from logcat.

Closes #189
2026-08-21 17:23:05 -04:00
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
CI / check (push) Successful in 2m36s
CI / e2e (push) Successful in 9m21s
2026-08-21 20:47:45 +00:00
13 changed files with 1018 additions and 51 deletions
+2 -1
View File
@@ -40,7 +40,8 @@ WHERE id = ? AND (mbid IS NULL OR mbid = '');
-- name: GetAlbumsWithPendingReleaseMBID :many
SELECT id, pending_release_mbid FROM albums
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
AND (mbid IS NULL OR mbid = '');
AND (mbid IS NULL OR mbid = '')
LIMIT ?;
-- name: DeleteAlbum :exec
DELETE FROM albums WHERE id = ?;
+3 -2
View File
@@ -331,6 +331,7 @@ const getAlbumsWithPendingReleaseMBID = `-- name: GetAlbumsWithPendingReleaseMBI
SELECT id, pending_release_mbid FROM albums
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
AND (mbid IS NULL OR mbid = '')
LIMIT ?
`
type GetAlbumsWithPendingReleaseMBIDRow struct {
@@ -338,8 +339,8 @@ type GetAlbumsWithPendingReleaseMBIDRow struct {
PendingReleaseMbid sql.NullString
}
func (q *Queries) GetAlbumsWithPendingReleaseMBID(ctx context.Context) ([]GetAlbumsWithPendingReleaseMBIDRow, error) {
rows, err := q.db.QueryContext(ctx, getAlbumsWithPendingReleaseMBID)
func (q *Queries) GetAlbumsWithPendingReleaseMBID(ctx context.Context, limit int64) ([]GetAlbumsWithPendingReleaseMBIDRow, error) {
rows, err := q.db.QueryContext(ctx, getAlbumsWithPendingReleaseMBID, limit)
if err != nil {
return nil, err
}
+35 -31
View File
@@ -2,6 +2,7 @@ package explore
import (
"context"
"database/sql"
"log/slog"
"math"
"sort"
@@ -12,6 +13,7 @@ import (
"golang.org/x/sync/singleflight"
"yellowjacket/backend/database"
"yellowjacket/backend/database/sql/sqlcgen"
"yellowjacket/backend/events"
"yellowjacket/backend/jobs"
)
@@ -279,37 +281,32 @@ func (e *Service) BackfillReleaseGroupMBIDs() {
go e.backfillReleaseGroupMBIDs(e.ctx)
}
func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
rows, err := e.db.QueryContext(
"SELECT id, pending_release_mbid FROM release_groups "+
"WHERE (mbid IS NULL OR mbid = '') "+
"AND pending_release_mbid IS NOT NULL AND pending_release_mbid != '' "+
"LIMIT ?",
releaseGroupMBIDBackfillMaxPerRun,
// pendingReleaseMBIDs is the albums this pass has work to do on.
//
// It is separate from the pass, and returns its error rather than
// logging it, so that a test can assert the statement runs against the
// real schema. That is not a general preference -- it is this
// statement's history: it named `release_groups`, a table plan 013
// renamed to `albums`, so it failed on every launch since e7748f1 and
// the pass returned quietly having done nothing. A test of the pass
// as a whole cannot see that, because a query error and an empty
// library are the same early return.
func (e *Service) pendingReleaseMBIDs(
ctx context.Context,
) ([]sqlcgen.GetAlbumsWithPendingReleaseMBIDRow, error) {
return e.db.ReadQueries.GetAlbumsWithPendingReleaseMBID(
ctx, releaseGroupMBIDBackfillMaxPerRun,
)
}
func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
pending, err := e.pendingReleaseMBIDs(ctx)
if err != nil {
e.logger.Warn("release-group mbid backfill: query failed", "error", err)
return
}
type pendingRow struct {
id int64
releaseMBID string
}
var pending []pendingRow
for rows.Next() {
var p pendingRow
if err := rows.Scan(&p.id, &p.releaseMBID); err == nil {
pending = append(pending, p)
}
}
_ = rows.Close()
if len(pending) == 0 {
return
}
@@ -335,7 +332,7 @@ func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
job.progress(i, len(pending))
release, err := e.mb.LookupRelease(ctx, p.releaseMBID)
release, err := e.mb.LookupRelease(ctx, p.PendingReleaseMbid.String)
if err != nil || release.ReleaseGroupMBID == "" {
// Left alone rather than cleared: LookupRelease caches its
// answer (success or a release with no group) for 7 days,
@@ -344,12 +341,19 @@ func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
continue
}
_, err = e.db.ExecContext(
"UPDATE release_groups SET mbid = ?, pending_release_mbid = NULL "+
"WHERE id = ? AND (mbid IS NULL OR mbid = '')",
release.ReleaseGroupMBID, p.id,
)
if err != nil {
// The writer, not ReadQueries: an UPDATE issued on the
// query-only pool fails at runtime with "attempt to write a
// readonly database".
if err := e.db.Queries.ResolveAlbumPendingReleaseMBID(
ctx,
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
Mbid: sql.NullString{
String: release.ReleaseGroupMBID,
Valid: true,
},
ID: p.ID,
},
); err != nil {
e.logger.Warn("release-group mbid backfill: update failed", "error", err)
}
}
+250
View File
@@ -0,0 +1,250 @@
package explore
import (
"database/sql"
"log/slog"
"strconv"
"testing"
"yellowjacket/backend/database"
"yellowjacket/backend/database/sql/sqlcgen"
)
// The release-group MBID backfill queried `release_groups`, a table
// plan 013 renamed to `albums`, so it failed on its first statement on
// every launch from e7748f1 until #189 -- and the pass swallowed that,
// because a query error and an empty library are the same early
// return. Nothing noticed for two reasons worth keeping in mind:
//
// - the statement was **raw SQL**, so sqlc never read it. Every other
// statement in the repo was renamed by the same change because sqlc
// reads sql/schemas/ and cannot generate against a table that is not
// declared. The two sqlc queries this now calls were written by 013
// and left uncalled.
// - it needs no network and no fixture library to reproduce. The
// failure is at prepare time.
// seedPendingAlbum inserts an album whose files carried a release MBID
// but no release-group MBID, which is what `library.updateMBIDs`
// leaves behind for this pass to resolve.
func seedPendingAlbum(
t *testing.T,
db *database.DB,
name, pendingMBID string,
) int64 {
t.Helper()
res, err := db.ExecContext(
"INSERT INTO albums (name, artist_credit, pending_release_mbid) "+
"VALUES (?, ?, ?)",
name, "Test Artist", pendingMBID,
)
if err != nil {
t.Fatalf("insert albums row: %v", err)
}
id, err := res.LastInsertId()
if err != nil {
t.Fatalf("last insert id: %v", err)
}
return id
}
func newPendingTestService(db *database.DB) *Service {
return &Service{db: db, logger: slog.Default()}
}
// TestPendingReleaseMBIDsRunsAgainstTheRealSchema is the regression.
//
// It asserts the statement *runs*, which is the whole of what was
// broken: against the old raw SQL this returns
// "no such table: release_groups" rather than a row.
func TestPendingReleaseMBIDsRunsAgainstTheRealSchema(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
e := newPendingTestService(db)
want := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
pending, err := e.pendingReleaseMBIDs(db.Ctx)
if err != nil {
t.Fatalf("the backfill's query failed: %v", err)
}
if len(pending) != 1 {
t.Fatalf("got %d pending albums, want 1", len(pending))
}
if pending[0].ID != want {
t.Errorf("got album id %d, want %d", pending[0].ID, want)
}
if got := pending[0].PendingReleaseMbid.String; got != "release-mbid-1" {
t.Errorf("got pending mbid %q, want %q", got, "release-mbid-1")
}
}
// TestOnlyUnresolvedAlbumsAreReturned pins the two conditions that make
// the pass idempotent, since between them they are what stops it doing
// the same MusicBrainz lookups on every launch forever.
func TestOnlyUnresolvedAlbumsAreReturned(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
e := newPendingTestService(db)
pendingID := seedPendingAlbum(t, db, "Still Pending", "release-mbid-1")
// Already resolved: it has a real MBID, so there is nothing to
// look up even though a marker is still sitting on it.
resolved := seedPendingAlbum(t, db, "Already Resolved", "release-mbid-2")
if err := db.Queries.SetAlbumMBID(db.Ctx, sqlcgen.SetAlbumMBIDParams{
Mbid: sql.NullString{String: "rg-mbid", Valid: true},
ID: resolved,
}); err != nil {
t.Fatalf("set album mbid: %v", err)
}
// Never had a release MBID to resolve in the first place, which is
// most of a library.
seedPendingAlbum(t, db, "Nothing Pending", "")
pending, err := e.pendingReleaseMBIDs(db.Ctx)
if err != nil {
t.Fatalf("the backfill's query failed: %v", err)
}
if len(pending) != 1 || pending[0].ID != pendingID {
t.Fatalf(
"got %d albums %v, want only the unresolved one (%d)",
len(pending), pending, pendingID,
)
}
}
// TestResolvingClearsTheMarker is the other half: once the lookup has
// answered, the album must stop being a candidate, or the pass repeats
// the same live MusicBrainz call on every launch.
func TestResolvingClearsTheMarker(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
e := newPendingTestService(db)
id := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
// The writer, deliberately: this is an UPDATE, and the read pool
// would refuse it at runtime.
if err := db.Queries.ResolveAlbumPendingReleaseMBID(
db.Ctx,
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
Mbid: sql.NullString{String: "resolved-rg-mbid", Valid: true},
ID: id,
},
); err != nil {
t.Fatalf("resolve pending release mbid: %v", err)
}
pending, err := e.pendingReleaseMBIDs(db.Ctx)
if err != nil {
t.Fatalf("the backfill's query failed: %v", err)
}
if len(pending) != 0 {
t.Fatalf("a resolved album is still a candidate: %v", pending)
}
album, err := db.ReadQueries.GetAlbum(db.Ctx, id)
if err != nil {
t.Fatalf("get album: %v", err)
}
if album.Mbid.String != "resolved-rg-mbid" {
t.Errorf("album mbid = %q, want the resolved one", album.Mbid.String)
}
if album.PendingReleaseMbid.Valid &&
album.PendingReleaseMbid.String != "" {
t.Errorf(
"the pending marker survived as %q",
album.PendingReleaseMbid.String,
)
}
}
// TestAResolvedMBIDIsNeverOverwritten covers the guard in the UPDATE.
//
// The pass runs against rows it read earlier, and a rescan can resolve
// an album from its tags in between -- a real MBID from the file must
// win over one this pass inferred from a release.
func TestAResolvedMBIDIsNeverOverwritten(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
id := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
if err := db.Queries.SetAlbumMBID(db.Ctx, sqlcgen.SetAlbumMBIDParams{
Mbid: sql.NullString{String: "from-the-tags", Valid: true},
ID: id,
}); err != nil {
t.Fatalf("set album mbid: %v", err)
}
if err := db.Queries.ResolveAlbumPendingReleaseMBID(
db.Ctx,
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
Mbid: sql.NullString{String: "from-the-backfill", Valid: true},
ID: id,
},
); err != nil {
t.Fatalf("resolve pending release mbid: %v", err)
}
album, err := db.ReadQueries.GetAlbum(db.Ctx, id)
if err != nil {
t.Fatalf("get album: %v", err)
}
if album.Mbid.String != "from-the-tags" {
t.Errorf(
"album mbid = %q, want the tagged one to have won",
album.Mbid.String,
)
}
}
// TestThePassIsBounded checks the LIMIT.
//
// Each row costs a live MusicBrainz lookup on a 1 req/s limiter shared
// with every page the user can open, so an unbounded read is a run that
// lasts as long as the library is untagged. The sqlc query 013 wrote
// had no LIMIT; the raw statement it was replacing did.
func TestThePassIsBounded(t *testing.T) {
t.Parallel()
db := database.NewTestDB(t)
e := newPendingTestService(db)
for i := range releaseGroupMBIDBackfillMaxPerRun + 10 {
seedPendingAlbum(
t, db,
"Album "+string(rune('A'+i%26))+strconv.Itoa(i),
"release-mbid-"+strconv.Itoa(i),
)
}
pending, err := e.pendingReleaseMBIDs(db.Ctx)
if err != nil {
t.Fatalf("the backfill's query failed: %v", err)
}
if len(pending) != releaseGroupMBIDBackfillMaxPerRun {
t.Errorf(
"got %d albums, want the run bounded at %d",
len(pending), releaseGroupMBIDBackfillMaxPerRun,
)
}
}
+69
View File
@@ -52,6 +52,75 @@ func UseHomeOverride(base string) {
_ = os.Setenv(envHomeOverride, base)
}
// envTempDir is the variable Go's os.TempDir() reads, and through it
// every library in the process that asks for a temporary file.
const envTempDir = "TMPDIR"
// tempDirName is the subdirectory of the app's own storage that
// becomes that answer.
const tempDirName = "tmp"
// UseTempDir gives the process a temporary directory that exists.
//
// **Android has no /tmp and hands an app no TMPDIR**, and Go's
// os.TempDir() falls back to "/tmp" when the variable is unset -- so
// every library in the process that wants scratch space is handed a
// path that has never existed. SQLite is the one that noticed: an
// INSERT ... SELECT large enough to spill returned
// SQLITE_IOERR_GETTEMPPATH (disk I/O error 6410), which is how the
// champion search index came to fail its rebuild on every launch while
// the app otherwise looked healthy (#190).
//
// It is the *class* that is fixed here rather than that statement.
// Anything that spills fails the same way on that platform -- large
// sorts, large joins, VACUUM -- so the repair belongs at the process's
// one answer to the question rather than at each caller. The
// alternative considered was PRAGMA temp_store = MEMORY, which is
// cheaper and more local and is 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.
//
// The rules are UseHomeOverride's, 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
// what they asked for; nothing sets it on the platform this exists for.
//
// It returns its error rather than swallowing it because a temp
// directory that could not be created is the same silent failure one
// step earlier, and since #160 a log line on that platform is
// something a person can actually read.
func UseTempDir(base string) error {
if base == "" || os.Getenv(envTempDir) != "" {
return nil
}
dir := filepath.Join(base, tempDirName)
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("could not make the temp directory %s: %w", dir, err)
}
// Writability is checked rather than assumed: the whole failure
// this repairs is a directory that is named and cannot be used, and
// MkdirAll on an existing unwritable directory succeeds.
probe, err := os.CreateTemp(dir, "probe")
if err != nil {
return fmt.Errorf("temp directory %s is not writable: %w", dir, err)
}
name := probe.Name()
_ = probe.Close()
_ = os.Remove(name)
if err := os.Setenv(envTempDir, dir); err != nil {
return fmt.Errorf("could not set %s: %w", envTempDir, err)
}
return nil
}
// getUserDirPath returns and creates the path for a user directory.
func getUserDirPath(dt dirType) (string, error) {
path, err := resolveUserDirPath(dt)
+113
View File
@@ -87,3 +87,116 @@ func TestUseHomeOverride(t *testing.T) {
})
}
}
// UseTempDir carries UseHomeOverride's two rules for the same reasons,
// plus one of its own: the directory it names has to be usable.
//
// **The only tier that can compile the platform this exists for is a
// phone**, so everything decidable off one is decided here -- which is
// androidpayload.go's discipline, and is why the platform call is a
// parameter rather than something this package reaches for. The
// device's half is a single measurement: no /tmp, no TMPDIR (#190).
func TestUseTempDir(t *testing.T) {
t.Run("an empty base is a no-op", func(t *testing.T) {
// This is the desktop case in full: StoragePath() answers ""
// off mobile, where /tmp is real and must be left alone.
t.Setenv(envTempDir, "")
if err := UseTempDir(""); err != nil {
t.Fatalf("UseTempDir(\"\") = %v, want nil", err)
}
if got := os.Getenv(envTempDir); got != "" {
t.Errorf("%s = %q, want it untouched", envTempDir, got)
}
})
t.Run("an explicit TMPDIR wins", func(t *testing.T) {
const chosen = "/somewhere/deliberate"
// The base is taken before TMPDIR moves, because t.TempDir()
// reads TMPDIR too -- which is the same fact this function is
// about, met from the other side.
base := t.TempDir()
t.Setenv(envTempDir, chosen)
if err := UseTempDir(base); err != nil {
t.Fatalf("UseTempDir = %v, want nil", err)
}
if got := os.Getenv(envTempDir); got != chosen {
t.Errorf("%s = %q, want the explicit %q", envTempDir, got, chosen)
}
})
t.Run("points at a real directory under the base", func(t *testing.T) {
base := t.TempDir()
t.Setenv(envTempDir, "")
if err := UseTempDir(base); err != nil {
t.Fatalf("UseTempDir = %v, want nil", err)
}
got := os.Getenv(envTempDir)
want := filepath.Join(base, tempDirName)
if got != want {
t.Fatalf("%s = %q, want %q", envTempDir, got, want)
}
// The whole failure being repaired is a temp directory that is
// named and does not exist, so naming one is not enough.
info, err := os.Stat(got)
if err != nil {
t.Fatalf("the temp directory was named but not created: %v", err)
}
if !info.IsDir() {
t.Fatalf("%s is not a directory", got)
}
})
t.Run("os.TempDir then answers with it", func(t *testing.T) {
// The point of setting the variable at all: this is what every
// library in the process reads, SQLite's driver included.
base := t.TempDir()
t.Setenv(envTempDir, "")
if err := UseTempDir(base); err != nil {
t.Fatalf("UseTempDir = %v, want nil", err)
}
if got := os.TempDir(); got != filepath.Join(base, tempDirName) {
t.Errorf("os.TempDir() = %q, want the directory we made", got)
}
})
t.Run("an unwritable directory is an error, not a silent success", func(t *testing.T) {
if os.Getuid() == 0 {
t.Skip("root can write anywhere, so there is nothing to refuse")
}
base := t.TempDir()
// MkdirAll on an existing directory succeeds whatever its
// mode, so without the write probe this case would set TMPDIR
// to a directory nothing can use -- which is the bug again,
// one directory over.
if err := os.Mkdir(filepath.Join(base, tempDirName), 0o500); err != nil {
t.Fatalf("prepare the unwritable directory: %v", err)
}
t.Setenv(envTempDir, "")
if err := UseTempDir(base); err == nil {
t.Fatal("UseTempDir accepted a directory it cannot write to")
}
if got := os.Getenv(envTempDir); got != "" {
t.Errorf("%s was set to %q despite the failure", envTempDir, got)
}
})
}
@@ -45,18 +45,6 @@ export class SeekBar extends LitElement {
private showRemaining: boolean = true;
static override styles = [designTokens, waSliderLabel, css`
/* 12px below the phone breakpoint. The bottom bar's seek bar is
display:none there (016 B2 phase 1), so the only instance a
viewport media query can reach at that width is the full-screen
now-playing view's -- which is exactly the one a thumb uses.
The track size lives on wa-slider inside this shadow root, so a
custom property set by the host would not reach it. */
@media (max-width: 599px) {
wa-slider {
--track-size: 12px;
}
}
wa-slider {
--track-size: 6px;
flex: 1;
@@ -80,6 +68,57 @@ export class SeekBar extends LitElement {
background: var(--yj-bg-base, black);
}
/* The phone's seek bar, and this block is last on purpose.
A media query adds no specificity, so this lived above the plain
"wa-slider" rule and lost to it at every width: the 12px track it
asks for had never once applied, and the bar measured 261x6 on
the device while the source said 12. That is index.css's rule
("the phone section is last on purpose") met inside a component's
own stylesheet, and nothing renders differently in any tier here
to say so.
The bottom bar's seek bar is display:none below this width (016
B2 phase 1), so the only instance a viewport media query can
reach is the full-screen now-playing view's -- which is exactly
the one a thumb uses. The desktop bar keeps its 6px, where a
mouse is precise and the thickness is right.
The painted track and the thing you can hit are allowed to
differ, and a slider is the clearest case where they should: 12px
is a progress bar you can see, and 44px is the app's touch floor
(#56). A 44px-*thick* bar would be wrong-looking and would cost
the album art the vertical space #51 spent an issue recovering.
Two things about how the target is built.
The padding goes on ::part(slider) rather than on the host,
because that inner div is what carries the gesture -- it has 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. Both halves are measured: the
seek row is 19px (its clocks, not the track, decide that) and the
play button's top edge is 8px below it, so the target takes the
space *above*, where .art is a non-interactive div. Growing the
row instead cost the art 25px of 143. Verified on the device at
424x439: hit area 44px, painted track 12px, row still 19px, art
still 143px, 8px 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. */
@media (max-width: 599px) {
wa-slider {
--track-size: 12px;
}
wa-slider::part(slider) {
padding-block: 28px 4px;
margin-block: -28px -4px;
}
}
#seek-bar-container {
display: flex;
justify-content: space-between;
@@ -298,6 +298,47 @@ export class PageHeader extends LitElement {
flex-shrink: 0;
}
/* Every control in this header meets the app's 44px touch
floor -- the number #56 set for the transport and the
queue header already keeps (#186).
It is min-size rather than padding with a negative
margin, which is what the seek bar needed (#187), and
the difference is worth stating because it decides
whether targets can collide. 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 can overlap by
construction.
There is no phone branch. With the target being the box,
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 the reason this
component has never had one. It also avoids a media
query that no tier here renders, which is exactly how
the seek bar's phone rule came to be dead for months.
**The height is the box and the width is not**, and that
asymmetry is the whole of what the overflow fit below
cares about. That pass measures inline size, so a taller
control costs it nothing and a wider one costs it
directly. Growing the two square controls to 44px wide
added 22px, which fits at every width Chromium was
checked at and clipped the overflow trigger at 320px in
**WebKit** -- the engine closest to what actually ships,
and the one no machine here can run. So the horizontal
half is padding with the margin cancelling it, which is
what the issue asked for in the first place: the target
grows and the layout does not.
The cost is that a horizontal target can now overlap a
neighbour, which the box version could not. The arrow's
is deliberately lopsided for the seek bar's reason
(#187): 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. */
.sort select {
font: inherit;
color: inherit;
@@ -306,6 +347,7 @@ export class PageHeader extends LitElement {
border-radius: 4px;
padding: 3px 6px;
cursor: pointer;
min-block-size: 44px;
}
.sort-dir {
@@ -318,6 +360,18 @@ export class PageHeader extends LitElement {
color: inherit;
cursor: pointer;
padding: 3px 5px;
/* 28x21 before this, the smallest control in the
header and the only one that failed the floor in
both directions.
Vertically the box grows, because the header has the
room and nothing measures it. Horizontally the box
must not: 28 + 2 + 14 is a 44px target over a 28px
layout box, weighted right because the select is 6px
to the left. */
min-block-size: 44px;
padding-inline: 5px 21px;
margin-inline: 0 -16px;
}
.sort-dir:hover {
@@ -377,10 +431,20 @@ export class PageHeader extends LitElement {
gap: 6px;
white-space: nowrap;
flex-shrink: 0;
justify-content: center;
min-block-size: 44px;
}
.more-button {
padding: 6px 10px;
/* 38x27, and it is the route to every collapsed
action, so it is the last control that should be
hard to hit -- and the one WebKit clipped at 320px
when this was 6px wider as a box. 38 + 3 + 3 is a
44px target over a 38px layout box; the actions row
has an 8px gap, so this one can be symmetric. */
padding-inline: 13px;
margin-inline: -3px;
}
/* The display: flex above outranks the UA stylesheet's
@@ -61,11 +61,32 @@ export class SearchTrigger extends LitElement {
display: inline-flex;
align-items: center;
justify-content: center;
/* The smallest a touch target should be. The header's
own action buttons are smaller because they carry a
label; this one is a glyph. */
min-width: 40px;
min-height: 40px;
/* The app's touch floor, from #56 -- and this is the
control that should least have to argue for it: #57
created it as the phone's replacement for the header
search box, so it exists *only* where there is a
thumb.
It shipped at 40px under a comment calling that "the
smallest a touch target should be", which was the
floor being restated four pixels short rather than a
second opinion about it (#186). The rest of that
comment said the header's own action buttons are
smaller because they carry a label; they are 44px
now too, so that no longer distinguishes anything.
The extra width is a target rather than a box, for
page-header's reason: this button sits in that
header, whose overflow fit (#69) measures inline
size, and four pixels there is four pixels the
trigger for every collapsed action does not get at
320px. Height is free -- nothing measures it. */
min-width: 44px;
min-height: 44px;
/* Border-box, so the 44 above is the whole target and
the margin is what hands the four extra pixels back
to the row. */
margin-inline: -2px;
padding: 0;
background: none;
border: 1px solid var(--yj-border-subtle, #555);
@@ -99,6 +99,25 @@ describe('<search-trigger>', () => {
}
});
it('meets the touch floor it was shipped four pixels under', async () => {
stubPhone(true);
// #57 created this 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", which was the app's own 44px floor (#56)
// restated short rather than a second opinion about it. #186.
const el = await fixture('search-trigger');
const button = shadow<HTMLButtonElement>(el, '[data-testid="search-trigger"]');
expect(button).not.toBeNull();
const box = button!.getBoundingClientRect();
expect(Math.round(box.width)).toBeGreaterThanOrEqual(44);
expect(Math.round(box.height)).toBeGreaterThanOrEqual(44);
});
it('names what the button will search', async () => {
stubPhone(true);
@@ -0,0 +1,211 @@
/**
* The seek bar's painted track and the thing you can hit are allowed to
* differ, and a slider is the clearest case where they should.
*
* On `now-playing-view` — the screen that exists so a phone has
* somewhere to seek from — the slider measured 261x6 on the reference
* device (#187). Six pixels is the whole of the drag target on the
* app's primary seeking affordance, against a 44px floor the app set
* for itself in #56 and holds to in the queue panel.
*
* Two separate faults, and the first is why the second was not obvious.
*
* **The phone rule had never applied.** `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 — which is `index.css`'s documented
* rule ("the phone section is last on purpose") reproduced inside a
* component's own stylesheet. The source said 12 and the device said 6.
*
* **And 12px would still be under the floor**, so the target is built
* around the track rather than by thickening it: padding on the part
* that carries the gesture, with margins cancelling it so the row does
* not grow.
*
* This is asserted against the *parsed stylesheet*, on
* `hover-affordance.test.ts`'s precedent and with the same limitation
* stated rather than hidden: no tier here renders at a phone width with
* a real `wa-slider` laid out, so what can be checked is the shape the
* browser built from the css`` literal. The pixel measurements that
* chose these numbers were taken on the device and are recorded on
* #187 and in the stylesheet's own comment — a number measured on a
* phone is not a number CI can assert.
*
* Which is the regression worth catching anyway. Both failures are
* invisible on a desktop: hoisting the block back above the plain rule
* renders identically at every width CI runs at, and it is exactly what
* a tidy-up does.
*/
import { describe, expect, it } from 'vitest';
import '@components/audio-player/seekbar/seek-bar';
import { fixture } from '@test/support/render';
/** The app's touch floor, from #56. */
const TOUCH_FLOOR = 44;
/** The width below which the phone's rules apply. */
const PHONE_QUERY = /max-width:\s*599px/;
type Rule = { text: string; condition: string | null };
/**
* Every rule in the element's own adopted stylesheets, flattened **in
* order**, which is the whole point here: the fault being guarded is a
* rule sitting in the wrong place, not a rule being absent.
*/
function rulesOf(host: Element): Rule[] {
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
const out: Rule[] = [];
for (const sheet of sheets) {
for (const rule of Array.from(sheet.cssRules)) {
if (rule instanceof CSSMediaRule) {
for (const inner of Array.from(rule.cssRules)) {
out.push({ text: inner.cssText, condition: rule.conditionText });
}
continue;
}
out.push({ text: rule.cssText, condition: null });
}
}
return out;
}
/**
* The two px numbers of a `*-block` declaration, as [start, end].
*
* A symmetric pair is **serialised back as one value** — `padding-block:
* 16px 16px` reads as `padding-block: 16px` — so a naive pair-reader
* fails on the shorthand rather than on the thing it is checking, and
* says the wrong thing about why. That is not hypothetical: it is what
* the symmetric-padding reversion did while this test was being
* proved.
*/
function blockPair(text: string, property: string): [number, number] | null {
const declaration = new RegExp(`${property}:\\s*([^;]+)`).exec(text)?.[1];
if (declaration === undefined) {
return null;
}
const values = [...declaration.matchAll(/(-?[\d.]+)px/g)].map((m) =>
Number(m[1]),
);
const [start, end] = values;
if (start === undefined) {
return null;
}
return [start, end ?? start];
}
describe("the seek bar's phone rules", () => {
it('are last, so they are not silently overridden', async () => {
const el = await fixture('seek-bar', {});
const rules = rulesOf(el);
// A sweep that read nothing passes vacuously — the same first
// assertion icon-language.test.ts makes, for the same reason.
expect(rules.length).toBeGreaterThan(0);
const declaresTrackSize = (r: Rule) => /--track-size:/.test(r.text);
const lastUnconditional = rules.findLastIndex(
(r) => r.condition === null && declaresTrackSize(r),
);
const phoneOverride = rules.findLastIndex(
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
&& declaresTrackSize(r),
);
expect(lastUnconditional).toBeGreaterThanOrEqual(0);
expect(phoneOverride).toBeGreaterThanOrEqual(0);
// A media query adds no specificity. Written first, it loses.
expect(phoneOverride).toBeGreaterThan(lastUnconditional);
});
it('give the slider a pointer target of at least the touch floor', async () => {
const el = await fixture('seek-bar', {});
const rules = rulesOf(el);
const track = rules.find(
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
&& /--track-size:/.test(r.text),
);
const target = rules.find(
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
&& r.text.includes('::part(slider)'),
);
expect(track).toBeDefined();
expect(target).toBeDefined();
const trackSize = Number(
/--track-size:\s*(-?[\d.]+)px/.exec(track!.text)?.[1],
);
const padding = blockPair(target!.text, 'padding-block');
expect(padding).not.toBeNull();
// The padding is on ::part(slider) rather than on the host because
// that inner div is what carries the gesture: it has the listener
// and the touch-action, and it is exactly the host's size, so
// padding the host grows a box that does not take the press.
const hitArea = trackSize + padding![0] + padding![1];
expect(hitArea).toBeGreaterThanOrEqual(TOUCH_FLOOR);
});
it('do not grow the row they sit in', async () => {
const el = await fixture('seek-bar', {});
const target = rulesOf(el).find(
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
&& r.text.includes('::part(slider)'),
);
expect(target).toBeDefined();
const padding = blockPair(target!.text, 'padding-block');
const margin = blockPair(target!.text, 'margin-block');
expect(padding).not.toBeNull();
expect(margin).not.toBeNull();
// now-playing-view's vertical budget is fixed and #51 measured
// every pixel of it: letting the row grow by the difference cost
// the album art 25px of 143 when it was tried on the device.
expect(margin![0]).toBe(-padding![0]);
expect(margin![1]).toBe(-padding![1]);
});
it('take the space above, because what is below is the transport', async () => {
const el = await fixture('seek-bar', {});
const target = rulesOf(el).find(
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
&& r.text.includes('::part(slider)'),
);
expect(target).toBeDefined();
const pair = blockPair(target!.text, 'padding-block');
expect(pair).not.toBeNull();
const [above, below] = pair!;
// Measured at 424x439: the seek row is 19px and the play button's
// top edge is 8px below it, while `.art` above is a non-interactive
// div. A symmetric target would reach into the play button — the
// most important control on the screen — so the growth is upward.
expect(above).toBeGreaterThan(below);
});
});
@@ -0,0 +1,162 @@
/**
* Every control a finger meets is at least 44px (#186).
*
* #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 — the sort control, its direction
* button, the page actions, the overflow trigger and the phone's search
* button — sat between a third and two thirds of the app's own floor.
* Measured on the reference device (TLP301, 424x439): `page-sort` 99x23,
* `page-sort-direction` **28x21**, `page-actions-more` 38x27,
* `search-trigger` 40x40.
*
* Unlike the seek bar's target (#187), this one can be measured here
* rather than inferred from the stylesheet. There the painted track had
* to stay thin, so the hit area was grown past its own box and only a
* phone-width layout of a third-party slider could show it. Here the
* control *is* the target, so a real Chromium rendering a real
* `page-header` gives the actual answer — and because it is a `min-size`
* rather than a media query, the answer is the same at every width,
* which is what makes it checkable in this tier at all.
*
* That is also why there is no phone branch to test: 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.
*/
import { describe, expect, it } from 'vitest';
import type { PageAction, PageHeader } from '@components/page-header/page-header';
import '@components/page-header/page-header';
import { fixture, shadowAll } from '@test/support/render';
/** The app's touch floor, from #56. */
const FLOOR = 44;
const SORTS = [
{ id: 'name', label: 'Name' },
{ id: 'tracks', label: 'Tracks' },
];
function actions(): PageAction[] {
return [
{ id: 'import', label: 'Import', icon: 'file-import', priority: 0, onSelect: () => {} },
{ id: 'new', label: 'New Playlist', icon: 'plus', priority: 2, onSelect: () => {} },
];
}
/** Every visible control in the header's own shadow root. */
function controlsOf(el: PageHeader): { name: string; el: HTMLElement }[] {
return shadowAll<HTMLElement>(el, 'button, select')
.filter((c) => !(c as HTMLButtonElement).hidden)
.map((c) => ({
name: c.dataset.testid ?? (c.className || c.tagName.toLowerCase()),
el: c,
}));
}
function tooSmall(controls: { name: string; el: HTMLElement }[]): string[] {
return controls
.map(({ name, el }) => {
const b = el.getBoundingClientRect();
return { name, w: Math.round(b.width), h: Math.round(b.height) };
})
.filter((c) => c.w < FLOOR || c.h < FLOOR)
.map((c) => `${c.name} ${c.w}x${c.h}`);
}
describe("the page header's controls", () => {
it('all meet the touch floor', async () => {
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
count: 50,
countNoun: 'playlist',
sortOptions: SORTS,
sortField: 'name',
sortDirection: 'asc',
actions: actions(),
});
const controls = controlsOf(el);
// A sweep that found no controls passes vacuously — the same first
// assertion icon-language.test.ts makes, for the same reason.
expect(controls.length).toBeGreaterThan(0);
// The two that were smallest, named so a regression says which.
expect(controls.map((c) => c.name)).toContain('page-sort-direction');
expect(controls.map((c) => c.name)).toContain('page-sort');
expect(tooSmall(controls)).toEqual([]);
});
it('grows the target without growing the box, so the overflow fit is untouched', async () => {
// The regression this exists for, and it was a real one: growing
// the two square controls to 44px *wide* added 22px to the header,
// which 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. #69's fit pass
// measures inline size, so a taller control is free and a wider one
// is not.
//
// Negative inline margins are what keep the box out of it: the
// padding makes the target, and the margin gives the space back.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
sortOptions: SORTS,
sortField: 'name',
actions: actions(),
});
el.style.width = '320px';
for (let frame = 0; frame < 3; frame += 1) {
await new Promise((r) => requestAnimationFrame(r));
await el.updateComplete;
}
for (const selector of ['.sort-dir', '.more-button']) {
const control = shadowAll<HTMLElement>(el, selector).filter(
(c) => !(c as HTMLButtonElement).hidden,
)[0];
expect(control, selector).toBeTruthy();
const style = getComputedStyle(control!);
const added =
parseFloat(style.marginInlineStart) + parseFloat(style.marginInlineEnd);
expect(added, `${selector} gives its extra width back`).toBeLessThan(0);
}
});
it('includes the overflow trigger, which is the route to the rest', async () => {
// At 320px the fit pass collapses actions into the menu, so the
// trigger is rendered — and it is then the only way to reach them,
// which makes it the last control that should be hard to hit.
const el = await fixture<PageHeader>('page-header', {
heading: 'Playlists',
sortOptions: SORTS,
sortField: 'name',
actions: actions(),
});
el.style.width = '320px';
for (let frame = 0; frame < 3; frame += 1) {
await new Promise((r) => requestAnimationFrame(r));
await el.updateComplete;
}
const more = shadowAll<HTMLButtonElement>(el, '.more-button').filter(
(b) => !b.hidden,
);
expect(more.length).toBe(1);
const box = more[0]!.getBoundingClientRect();
expect(Math.round(box.width)).toBeGreaterThanOrEqual(FLOOR);
expect(Math.round(box.height)).toBeGreaterThanOrEqual(FLOOR);
});
});
+13
View File
@@ -113,6 +113,19 @@ func main() {
slog.SetDefault(sLogger)
sLogger.Info("starting yellowjacket", "version", version, "commit", commit)
// Android has no /tmp and gives an app no TMPDIR, so anything in
// this process that spills to a temporary file is handed a path that
// does not exist -- see system.UseTempDir. It runs here rather than
// beside UseHomeOverride above because it has something to say when
// it fails and the logger does not exist up there; what matters is
// that it is before NewYellowJacketApp, which opens the database.
//
// A failure is not fatal: it leaves the platform's own answer in
// place, which is what every release before this one ran with.
if err := system.UseTempDir(application.Mobile.StoragePath()); err != nil {
sLogger.Error("could not set up a temp directory", "err", err.Error())
}
// Start profiling server (pprof + trace). In production builds this
// is a no-op — the compiler eliminates all profiling code.
stopProfiler := profiling.Start(sLogger)