diff --git a/.pi/skills/yellowjacket-dev/references/android-tier.md b/.pi/skills/yellowjacket-dev/references/android-tier.md index 4b4afd3..2037395 100644 --- a/.pi/skills/yellowjacket-dev/references/android-tier.md +++ b/.pi/skills/yellowjacket-dev/references/android-tier.md @@ -524,6 +524,61 @@ Four things about it, each of which costs an hour if met cold: app.yellowjacket.dev android.permission.READ_MEDIA_AUDIO` (and `POST_NOTIFICATIONS`) ahead of the launch skips it. +### Getting the app into a state worth measuring + +A fresh install is **not** a neutral starting point, and three things +about it will each cost you a measurement. + +**It downloads the real catalog.** `YJ_CORE_INDEX_URL` is stubbed in +`dev-headless.sh` and in CI and is *real* here, so the app spends its +first minutes fetching ~0.6 GB and `job-band` is **103px of a 439px +screen** while it does. Every vertical number taken in that state is +wrong -- one #51 measurement had the album art at 0px and it was +entirely this. + +`__yj.call("explore.Service.StopIndexBuild", [])` stops it and returns +cleanly. **It then starts again within seconds.** So stop it +*immediately before* the measurement rather than once at the beginning, +and check `jobs.Service.GetJobs` afterwards -- an empty array is the +only proof. `jobs.Service.ClearFinishedJobs` tidies the finished rows +that otherwise keep the band open. + +**A library added over the bridge does not dismiss the first-run +wizard.** `library.Library.AddLibrary` works and scans, but the wizard +checks for an existing library once, on mount, and its "Get Started" +button gates on a directory chosen *in the wizard* -- so it stays up +with a correctly disabled button over everything you are trying to +measure. Nothing is broken; reload the page and it is gone. This reads +exactly like a tap being swallowed, which is the expensive part. + +**Scoped storage decides where the music can be.** `/sdcard/Music/...` +plus `pm grant android.permission.READ_MEDIA_AUDIO` works and +`AddLibrary` takes the plain path; a push into +`/sdcard/Android/data//files/` looks like it worked and then is not +there. Some builds additionally want +`appops set MANAGE_EXTERNAL_STORAGE allow`, and until they have it +the app opens the *system* "All files access" screen on launch -- so +`dumpsys window | grep mCurrentFocus` naming `com.android.settings` is +that, not a crash. + +### A note on quoting `make android-eval` + +`EXPR='...'` is a single-quoted shell word, so anything with a quote or +an apostrophe in it -- a file path like `Blazo, 49'ers - ...`, or a +snippet containing a string literal -- breaks in a way that reads as a +JavaScript error. Put the expression in a file and pass it positionally: + +```bash +node ./scripts/android-eval.mjs "$(cat /tmp/probe.js)" +``` + +That is the same script `make android-eval` wraps, so nothing is lost. +Two things worth knowing about it: it does **not** await a promise, so +an async call has to park its result (`window.__r = ...`) and be read +back in a second eval; and the shim from the section below is lost on +every reload and every app restart, along with the devtools socket, +whose name carries the pid. + ### Calling a binding on the device **The runtime call does not go over HTTP on Android**, and this is worth diff --git a/.planning/NOTES.md b/.planning/NOTES.md index b51bde7..dd6bb85 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -4785,3 +4785,130 @@ broken build. The specs assert the *mechanism* — that the surface is a native `` at phone width — which is the same move `queue-as-a-screen.spec.ts` makes about containment and for the same reason. + +## Now Playing was not drawing a small square, it was drawing a crop (measured 2026-08-21, TLP301 / Chrome 113 / 424x439) + +#172 handed #51 a design question — whether the album art gets a floor +with the block scrolling, or whether the screen reflows below some +height. Measuring it first turned up a defect underneath the question, +and the defect is bigger than the phone. + +**The 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 CSS2.1 10.4 preserves under both bounds. So +`width: min(100%, 60vh)` made the width definite, the ratio derived a +height from it, `max-height: 100%` clipped that height, and the width +stayed where it was. `object-fit: cover` then cropped a square cover +into the band. On the device: **264x53**, a 5:1 strip. #172's own table +called it "39px of art" and the missing half is that those 39px were +263 wide. + +**And it is not only the phone.** The leftover exceeds the width only +above ~843px of viewport, so every height from ~500 to ~843 drew a +crop too — most phones, and any short window. The e2e spec written for +this fails on the old build at 424x439 (263x39), 390x700 (358x315) and +900x500 (300x36), and *passes* at 412x869, which is the boundary +falling exactly where the arithmetic says it should. + +**Both maxes with auto sizes is the whole fix**, and it was chosen by +asking Chrome 113 rather than by reasoning: a probe shadow root at +column heights of 288, 300, 451, 600 and 800 measured four candidate +rules. `max-width/max-height: 100%` with `width/height: auto` is square +at all five; the shipped rule cropped at four; `aspect-ratio` on the +box cropped at the tallest. A corollary that makes it free: `auto` will +not upscale past the natural size, and the largest tier `saveCoverArt` +keeps is 400px, so nothing is lost by never exceeding it. + +**The placeholder cannot use that rule and needed its own**, which is +the part that would have shipped broken. It is not a replaced element, +so with no intrinsic size auto/auto collapses it to its icon — +measured at **13x58**, neither square nor the art's size. Three things +about the rule it did get: + +- It is driven from the **height**, which is the axis that binds + everywhere this view is reached from. +- A flex item's automatic minimum is its content, so without + `min-width: 0` the icon's own width becomes a floor and the box goes + wider than it is tall the moment the row is shorter than the icon — + which is exactly the state a job band puts this screen in. +- **A non-replaced box cannot express "the largest square that fits" at + all**, because whichever max clamps does not re-derive the other. The + height-driven rule alone went **380x484** at 412x869 — a tall phone, + #51's other named device — and `max-height: calc(100vw - 2rem)` is + what closes it. That is sound here for the reason `60vh` was not: this + is a phone-width detail view, so its content box really is the + viewport less the host's own gutters, and it is a *max*, so if that + ever stopped being true the failure is a square bounded early rather + than a crop. `rem` and not `em` — the box sets `font-size: 3rem` for + the icon, so `2em` there is 96px. + +**Then the design question, and the reflow is the answer.** The +stacked layout's budget is fixed — 48px of header, 143px of transport +since #64, 78px of names, 68px of padding and gaps — so the art gets +`height - 386`, which is 53px at 439. A floor on the art scrolls the +transport off the bottom, and "controls never scroll 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 +by the column's leftover: **53px to 143px on the device**, measured on +the shipped build, with nothing scrolling and the transport untouched. + +**500 is where the two layouts cross, not a round number.** In a row +the art is `height - 296` and the names get what is left of 392px, so +the names hold 176px at exactly 500 and less above it; stacked, the art +is `height - 386`, which passes 176px at 562. It is keyed on height +alone rather than on the phone's width because it is an answer to +vertical room — a 900x450 window has the same problem and the same fix. + +Two things the audit found that are *not* this, and are filed: +**#186**, every control that is not the transport is under the 44px +floor (the sort direction arrow is 28x21, and `search-trigger` — which +exists only on a phone — is 40x40), and **#187**, the seek bar's drag +target is 6px tall. + +**What the audit did not find is a reachability failure**, which is +worth recording because it is the promise plan 018 makes. At 424x439, +on every view -- the ten primary ones, the queue, `album-details`, +`artist-details`, Downloads and Autotag -- `documentElement.scrollWidth` +is 424 against a 424 viewport, no control sits 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, and it was this screen. + +**A number measured on the device is not a number CI can assert.** The +spec's floor on the art's height passed here at 114 and failed in CI at +**64**, and both are honest: this app is long-lived, so a job staged by +an earlier spec is still on screen, and `volume-control` renders in a +browser where it does not on Android. Both are chrome above and below +the view and both move the leftover. That is the same trap the entry +above about staged jobs describes, arriving as a *measurement* rather +than as a stuck job. The assertion is the mechanism now -- in a row the +art fills the row's height rather than being the leftover -- and the +53-to-143 stays on the issue, where it was measured. + +The probe is worth keeping in mind for the next audit, because two of +its three checks needed a second pass to mean anything. "Painted +outside the viewport" flags a horizontally scrolling carousel -- the +home shelves -- so the real question is whether a *scrollable ancestor* +can bring the element back. And a hit test at a control's centre flags +everything below the fold in a scroll container, so it only says +something once the control is on screen. Both first drafts produced +long lists of nothing. + +## Two traps that cost time on the device, both already written down (2026-08-21) + +Recorded because both are in `android-tier.md` and I met them anyway. + +**A fresh install downloads the real catalog**, so `job-band` is 103px +of a 439px screen and every vertical measurement is wrong. Worse, it +**restarts**: `explore.Service.StopIndexBuild` returns cleanly and the +job is `running` again within seconds, so it has to be stopped again +immediately before a measurement rather than once at the start. +`YJ_CORE_INDEX_URL` is stubbed in `dev-headless.sh` and in CI and is +real on a device. + +**The first-run wizard does not re-check for a library it did not +create.** Adding one through `library.Library.AddLibrary` over the +bridge leaves the wizard up with its "Get Started" button correctly +disabled — it gates on a directory chosen *in the wizard*, and the +existing-library check runs once, on mount. A reload clears it. Nothing +is broken; it cost twenty minutes of believing a tap had been swallowed. diff --git a/CLAUDE.md b/CLAUDE.md index 1f59fd7..d50d04e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2120,6 +2120,54 @@ toggled from `index.ts` would be a second expression of the same fact. The view therefore carries its own queue button, because that button lives in the bar it hides. +**And below 500px of height its art and its names share a row** (#51). +The stacked arrangement's budget is fixed — 48px of header, 143px of +transport since #64, 78px of names, 68px of padding and gaps — so the +art gets `height - 386`, which at the reference device's 424x439 is +**53px**: the one thing a Now Playing screen exists to show, smallest +on it. #172 named the two ways out and this is the second, because the +first — a floor on the art with the block scrolling — scrolls the +transport off the bottom, and *controls never scroll off* is #51's own +Direction and plan 018's promise. Sideways the art is bounded by the +row's height instead of by the column's leftover: **53px to 143px**, +measured on the device, nothing scrolling, the transport untouched. + +Three things about it are load-bearing. + +**500 is where the two layouts cross rather than a round number.** In a +row the art is `height - 296` and the names get what is left of 392px, +so the names hold 176px at exactly 500 and less above it; stacked, the +art is `height - 386`, which passes 176px at 562. Below 500 the row is +the bigger art *and* the readable one — above it the column is, which +is why a tall phone keeps the arrangement it has. It is keyed on height +alone and not on the phone's width, because it answers vertical room: a +900x450 window has the same problem and the same fix. + +**The art was not a small square, it was a crop, and that was never +only the phone.** `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 device. +The leftover exceeds the width only above ~843px of viewport, so every +height from ~500 to ~843 drew one too. `max-width`/`max-height: 100%` +with `width`/`height: auto` is the fix and is the replaced-element +path; it also never upscales past the natural size, and the largest +tier `saveCoverArt` keeps is 400px, so nothing is lost. + +**The placeholder needs its own rule, and a non-replaced box cannot +express this one.** With no intrinsic size, auto/auto collapses it to +its icon (13x58, measured). It is driven from the height instead, with +`min-width: 0` because a flex item's automatic minimum is its content — +without it the icon's width becomes a floor the moment the row is +shorter than the icon, which is exactly what a job band does to this +screen. And since whichever max clamps does not re-derive the other, a +height-driven box goes **380x484** on a tall phone; `max-height: +calc(100vw - 2rem)` closes it, which is sound here for the reason +`60vh` was not — this is a phone-width detail view, so its content box +really is the viewport less the host's gutters, and it is a *max*, so +the failure mode is a square bounded early rather than a crop. + **The playing row is a shape, not a hue.** `track-list` and `queue-panel` draw a `::before` triangle in each row's own left padding, plus `aria-current` — before, both rows were a background tint diff --git a/e2e/specs/phone-now-playing.spec.ts b/e2e/specs/phone-now-playing.spec.ts new file mode 100644 index 0000000..29d1595 --- /dev/null +++ b/e2e/specs/phone-now-playing.spec.ts @@ -0,0 +1,283 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * Now Playing on a short screen (#51). + * + * #51 asks for a layout that "survives" ~424x439 with the controls + * never scrolling off. #172 measured why it did not — the stacked + * layout's budget is fixed, so the art gets whatever is left, and that + * was 39px before #64 and 53px after it. + * + * **Two separate claims are asserted here, and only one of them is + * about the phone.** + * + * The first is that the art is *square*. It was not: `aspect-ratio` is + * specified not to re-derive the width when `max-height` clamps the + * height, so the art was drawn as a letterbox band and `object-fit: + * cover` cropped the cover to it — 264x53 on the reference device. The + * leftover only exceeds the width above ~843px of viewport, so this + * was every height from ~500 to ~843 as well: most phones, and any + * short window. That is ordinary CSS rather than a Chrome 113 quirk, + * so this tier can see it, and the heights below are chosen to cover + * the range rather than the one device. + * + * The second is the reflow: below 500px the art and the names sit side + * by side, which is what takes the art from 53px to 143px. That is + * asserted as a *relation between boxes* — the art beside the names, + * not above them — because the pixel count is a consequence of the + * arrangement and would pin this file to one device's chrome. + * + * **What this tier cannot see** is the device's engine: CI's Chromium + * and WebKit are current, and #60's clipping showed what that costs. + * Nothing here depends on Chrome 113 behaviour — the sizing rules were + * checked against the device itself, at column heights of 288, 300, + * 451, 600 and 800, and the numbers are on #51. + */ +type Page = import('@playwright/test').Page; + +/** The reference device's real viewport. */ +const DEVICE = { width: 424, height: 439 }; + +/** + * A tall phone, above the reflow's 500px. Roughly a Pixel 7, which is + * #51's other named device and was not attached — so what is checked + * here is the layout it *should* get, not that device. + */ +const TALL_PHONE = { width: 412, height: 869 }; + +/** Inside the crop's old range and above the reflow: a short window. */ +const SHORT_WINDOW = { width: 390, height: 700 }; + +/** + * The height the layout reflows at. Written down once here because the + * specs have to know which arrangement to *wait* for, not only which + * to assert. + */ +const REFLOW_AT = 500; + +/** Put a track in the player, so the view has art and names to lay out. */ +async function stageATrack(page: Page): Promise { + await page.evaluate(async () => { + const tracks = (await window.__yjEvents.call( + 'library.Library.GetTracks', + [0], + 10_000, + )) as { FilePath: string }[]; + + await window.__yjEvents.call( + 'queue.Queue.SetQueue', + [tracks.slice(0, 4).map((t) => t.FilePath), 0, false, { type: '', id: 0, label: '' }], + 10_000, + ); + }); +} + +/** Open the full-screen view and wait for the shell to say so. */ +async function openNowPlaying(page: Page): Promise { + await page.evaluate(() => { + document.dispatchEvent( + new CustomEvent('navigate', { + detail: { view: 'now-playing' }, + bubbles: true, + }), + ); + }); + + await expect(page.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'now-playing', + ); + + // The attribute is the shell's bookkeeping and lands before the view + // has a track, so measuring on it alone races the first layout -- + // which showed up as a 60x5 art on the first spec of a cold run. + // + // Waiting for a non-zero box is not enough on its own either: a + // previous test leaves the *other* arrangement on screen, and a + // stale column satisfies "has a size" perfectly. So the wait is for + // the arrangement this viewport should have, which is the thing + // every assertion below depends on. Found by this file passing one + // test at a time and failing in file order. + const wantRow = (page.viewportSize()?.height ?? 0) <= REFLOW_AT; + + await page.waitForFunction( + (row: boolean) => { + const v = document.querySelector('now-playing-view'); + const stack = v?.shadowRoot?.querySelector('.stack'); + const el = v?.shadowRoot?.querySelector('.art img, .art .placeholder'); + const t = v?.shadowRoot?.querySelector('.transport'); + + if (!el || !t) return false; + + // A build with no `.stack` at all is the one before this change, + // and the squareness assertions are still meaningful against it + // -- so this waits for the arrangement only where there is one to + // wait for. Otherwise reverting the component to check that these + // tests bite produces eight timeouts instead of the measurements + // that make the case. + if (stack) { + const dir = getComputedStyle(stack).flexDirection; + + if (dir !== (row ? 'row' : 'column')) return false; + } + + const r = el.getBoundingClientRect(); + + return r.width > 0 && r.height > 0 && t.getBoundingClientRect().height > 0; + }, + wantRow, + ); +} + +/** + * The boxes this file reasons about, read in one evaluate. + * + * It reaches into the view's shadow root rather than using locators + * because the question is geometric — where these boxes are *relative + * to each other* — and a testid per edge would be four locators and + * four round trips to say one thing. + */ +async function boxes(page: Page) { + return page.evaluate(() => { + const v = document.querySelector('now-playing-view'); + + if (!v || !v.shadowRoot) return null; + + const rect = (sel: string) => { + const el = v.shadowRoot!.querySelector(sel); + + if (!el) return null; + + const r = el.getBoundingClientRect(); + + return { + left: r.left, right: r.right, top: r.top, bottom: r.bottom, + width: r.width, height: r.height, + }; + }; + + return { + // Whichever of the two the track has; both carry the sizing. + art: rect('.art img') ?? rect('.art .placeholder'), + artBox: rect('.art'), + stack: rect('.stack'), + meta: rect('.meta'), + transport: rect('.transport'), + scrollHeight: v.scrollHeight, + clientHeight: v.clientHeight, + }; + }); +} + +test.describe('Now Playing survives a short screen', () => { + test.beforeEach(async ({ app }) => { + await stageATrack(app); + }); + + /** + * The crop, at four heights spanning the range it covered. This is + * the assertion that fails on the build before this change: at + * 424x439 the art measured 264x53. + */ + for (const vp of [DEVICE, SHORT_WINDOW, TALL_PHONE, { width: 900, height: 500 }]) { + test(`draws the art square at ${vp.width}x${vp.height}`, async ({ app }) => { + await app.setViewportSize(vp); + await openNowPlaying(app); + + const b = await boxes(app); + + expect(b, 'now-playing-view did not mount').not.toBeNull(); + expect(b!.art, 'neither art nor placeholder rendered').not.toBeNull(); + + const { width, height } = b!.art!; + + expect(width, 'the art has no width').toBeGreaterThan(0); + // One pixel of slack for sub-pixel layout, and no more: the + // defect this guards was a 5:1 band. + expect( + Math.abs(width - height), + `art is ${Math.round(width)}x${Math.round(height)}, not square`, + ).toBeLessThanOrEqual(1); + }); + } + + /** + * The promise #51 states and plan 018's matrix repeats. A floor on + * the art with the block scrolling was the other option on #172 and + * this is why it was not taken. + */ + test('never scrolls the transport off the bottom', async ({ app }) => { + await app.setViewportSize(DEVICE); + await openNowPlaying(app); + + const b = await boxes(app); + + expect(b!.transport!.bottom).toBeLessThanOrEqual(DEVICE.height); + expect( + b!.scrollHeight, + 'the view scrolls, so the transport can be moved off screen', + ).toBeLessThanOrEqual(b!.clientHeight + 1); + }); + + /** + * The reflow itself, as a relation rather than a measurement: below + * 500px the names are *beside* the art, above it they are below. + */ + test('puts the names beside the art below 500px', async ({ app }) => { + await app.setViewportSize(DEVICE); + await openNowPlaying(app); + + const b = await boxes(app); + + expect( + b!.meta!.left, + 'the names are not to the right of the art', + ).toBeGreaterThanOrEqual(b!.artBox!.right - 1); + }); + + test('keeps the names below the art on a tall phone', async ({ app }) => { + await app.setViewportSize(TALL_PHONE); + await openNowPlaying(app); + + const b = await boxes(app); + + expect( + b!.meta!.top, + 'the names are not below the art', + ).toBeGreaterThanOrEqual(b!.artBox!.bottom - 1); + }); + + /** + * What the reflow actually does, stated as a mechanism rather than + * as a number: in a row the art is bounded by the row's *height*, + * so it fills it — where in a column it is the leftover after the + * names, which is what made it 53px. + * + * **The pixel count is deliberately not asserted here.** Two drafts + * tried. The first compared the art against the column's leftover + * computed from the boxes on screen and passed on the broken build, + * because the subtraction goes negative when the names are taller + * than the art — precisely the defect. The second put a floor of + * 100px on it, passed locally at 114 and **failed in CI at 64**: this + * 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 this view, and both move + * the leftover. A test that asserts how much room CI happened to + * have is a test about the runner. + * + * The device numbers — 53px to 143px — are on #51, measured there, + * which is the only tier that can honestly produce them. + */ + test('fills the row with the art rather than the leftover', async ({ app }) => { + await app.setViewportSize(DEVICE); + await openNowPlaying(app); + + const b = await boxes(app); + + expect(b!.stack, 'there is no row to fill').not.toBeNull(); + expect( + Math.abs(b!.artBox!.height - b!.stack!.height), + 'the art does not fill the row, so it is still a leftover', + ).toBeLessThanOrEqual(1); + }); +}); diff --git a/frontend/src/components/now-playing-view/now-playing-view.ts b/frontend/src/components/now-playing-view/now-playing-view.ts index cbf1d1d..831b3c8 100644 --- a/frontend/src/components/now-playing-view/now-playing-view.ts +++ b/frontend/src/components/now-playing-view/now-playing-view.ts @@ -105,6 +105,19 @@ export class NowPlayingView extends LitElement { color: var(--yj-text-secondary, #adb5bd); } + /* The art and the names are one block, so that a short + screen can lay them out side by side without either of them + knowing about the other's box. Vertically it is exactly what + the host used to do -- same gap, art flexible, names fixed -- + so the tall layout is unchanged. */ + .stack { + display: flex; + flex-direction: column; + gap: 0.75em; + flex: 1 1 auto; + min-height: 0; + } + .art { flex: 1 1 auto; display: flex; @@ -113,38 +126,89 @@ export class NowPlayingView extends LitElement { min-height: 0; } - .art img, - .art .placeholder { - /* Square, and never taller than the room left over: the - art is the one thing here that would happily push the - transport off the bottom of a short phone. + .art img { + /* Square, and never larger than the room left over -- + where "square" is a property of what is painted and not + just of what was asked for. - **max-height is what actually keeps that promise**, and - it was missing. With a definite width and - a 1:1 aspect-ratio the height is *derived from the width* - and is bounded by nothing: at the reference device's - 424x439 that is a 263px square (60vh) in a box with far - less than 263px left, so the art overflowed its own - centred flex item and drew over the header above and the - title below it. The comment claimed this was handled; - 60vh is a bound on the *viewport*, not on the room left - over, and those differ by however much chrome is above - and below. + The previous rule asked for a square and did not get + one. width: min(100%, 60vh) makes the width definite, + aspect-ratio: 1 derives the height from it, and + max-height: 100% then clamps that height **without + re-deriving the width** -- which is how the + aspect-ratio property is specified to behave, unlike + an intrinsic ratio. So whenever the room left over was + shorter than the box was wide, the art was drawn as a + letterbox strip and object-fit: cover cropped the + cover to it. Measured on the reference device at + 424x439: **264x53**, a 5:1 band of a square image. - Pre-existing -- screenshotted on main -- and made acute - by #56, which gives the transport 95px more than it had. - Found by reading a screenshot, which is the only tier - that can see it: nothing fails, nothing overflows the - *shell*, and every control is still hittable. */ - width: min(100%, 60vh); + That is not only the phone. The leftover exceeds the + width only above ~843px of viewport, so every height + from ~500 to ~843 -- most phones, and any small window + -- drew a cropped strip too. + + Both maxes with auto sizes is the fix, and it is the + replaced-element path rather than the aspect-ratio + one: the used size preserves the ratio under *both* + bounds (CSS2.1 10.4), so the art is square at every + height. Checked against Chrome 113 itself -- the + device's engine -- at column heights of 288, 300, 451, + 600 and 800: square at all five, where the old rule + cropped at four. + + A corollary worth knowing: auto will not upscale past + the image's natural size, and the largest tier + saveCoverArt keeps is 400px. Drawing it larger was + upscaling, so nothing is lost. */ + max-width: 100%; max-height: 100%; + width: auto; + height: auto; aspect-ratio: 1; object-fit: cover; border-radius: 12px; background-color: var(--yj-bg-elevated, #343a40); } + /* The placeholder is not a replaced element, so it cannot use + the rule above: with no intrinsic size, auto/auto collapses + it to its icon -- measured at 13x58 in Chrome 113, which is + neither square nor the art's size. + + So it is sized from the height, and then bounded by the + width in the one way a box like this can be. A non-replaced + element cannot express "the largest square that fits" in a + single rule: aspect-ratio derives the second axis from the + first, and whichever max clamps it does not re-derive the + other, which is the same trap the image rule above is about. + Driving it from the height alone is right until the column + is taller than it is wide -- ~843px of viewport, which is a + tall phone and #51's other named device -- and there it went + 380x484. + + max-height in viewport units is what closes it, and it is + sound here for the reason 60vh was not: this view is a + phone-width detail view, so its content box really is the + viewport less the host's own 1rem gutters. It is a *max*, so + the failure mode if that ever stopped being true is a square + bounded slightly early rather than a crop. rem and not em -- + this box sets font-size: 3rem for the icon, so 2em here + would be 96px. */ .art .placeholder { + height: 100%; + width: auto; + max-width: 100%; + max-height: calc(100vw - 2rem); + /* A flex item's automatic minimum is its content, so + without this the icon's own width becomes a floor and + the box goes wider than it is tall the moment the row is + shorter than the icon -- which is exactly the state a + job band puts this screen in. */ + min-width: 0; + aspect-ratio: 1; + border-radius: 12px; + background-color: var(--yj-bg-elevated, #343a40); display: flex; align-items: center; justify-content: center; @@ -232,6 +296,60 @@ export class NowPlayingView extends LitElement { color: var(--yj-text-secondary, #adb5bd); text-align: center; } + + /* Below 500px of viewport the art and the names sit side by + side, and that is the whole of this screen's answer to a + short phone (#51). + + The stacked layout cannot be rescued by sizing alone. Its + budget is fixed -- 48px of header, 143px of transport since + #64, 78px of names, 68px of padding and gaps -- so the art + gets height - 386, which on the reference device's 424x439 + is **53px**. #172 measured 39px before #64 and named the + two options: give the art a floor and let the block scroll, + or reflow. A floor scrolls the transport off the bottom, + and "controls never scroll off" is #51's own Direction and + plan 018's promise -- so it is the reflow. + + Sideways the art is bounded by the row's height rather than + by the column's leftover, which is the whole gain: the same + 439px screen goes from a 53px sliver to **143px**, measured + on the device, with nothing scrolling and the transport + untouched. + + 500 is where the two layouts cross rather than a round + number. In a row the art is height - 296 and the names get + what is left of 392px, so the names hold 176px at exactly + 500 and less above it; stacked, the art is height - 386, + which passes 176px at 562. Below 500 the row is the bigger + art *and* the readable one -- above it the column is, which + is why a tall phone (a Pixel 7's ~869) keeps the layout it + has. Unverified on that device: none was attached. + + It is keyed on height alone, not on the phone's width, + because it is an answer to vertical room -- a 900x450 window + has the same problem and the same fix. */ + @media (max-height: 500px) { + .stack { + flex-direction: row; + align-items: center; + } + + /* A square of the row's height. The box has to carry the + ratio here rather than the image, because in a row the + art's width is what the ratio has to produce -- and the + image's own rule then fits it to a box that is already + square. */ + .art { + flex: 0 1 auto; + height: 100%; + aspect-ratio: 1; + } + + .meta { + flex: 1 1 auto; + } + } `]; private back() { @@ -283,55 +401,57 @@ export class NowPlayingView extends LitElement { return html` ${this.renderHeader()} -
- ${art - ? html`` - : html``} -
- -
-
-

- ${track.title || track.fileName} -

-

- ${creditLink( - creditStore.credits(track.recordingMbid), - track.artist, - track.artistMbid, - )} -

- ${track.album - ? html`

- ${albumLink( - track.album, - track.releaseGroupMbid, - undefined, - track.artist, - )} -

` - : nothing} +
+
+ ${art + ? html`` + : html``}
- +
+
+

+ ${track.title || track.fileName} +

+

+ ${creditLink( + creditStore.credits(track.recordingMbid), + track.artist, + track.artistMbid, + )} +

+ ${track.album + ? html`

+ ${albumLink( + track.album, + track.releaseGroupMbid, + undefined, + track.artist, + )} +

` + : nothing} +
+ + +