justinzeus/wolfskin
1
0
Fork 0
Code Issues 2 Pull requests 4 Releases 3 Packages Activity

Smooth Discover heroes and retain Details destination focus #58

Closed
justinzeus wants to merge 0 commits from fm/tv-rd2-discover into fm/tv-round2
pull from: fm/tv-rd2-discover
merge into: justinzeus:fm/tv-round2
Conversation 3 Commits - Files changed -
justinzeus commented 2026-11-07 18:05:24 +01:00
Owner
Copy link

AI-assisted (fork-internal)

Outcome

Smooth Discover artwork changes, retain destination focus after Details, and make Discover lane memory visit-local. The copy-only review correction restores the approved sequential handoff and first-appearance entry on both Discover and Home; it removes the overlapping text seen in the reviewed counter-fade.

Current pushed head: 79f64f6e18aba3eff2938831061d1d53532ba98e. Base: fm/tv-round2. Integration and the title follow-up are merged without rewriting published history. No merge is requested before re-review and the merged-candidate checks.

Native reviewed-versus-corrected frame strips, 16 ms alpha/translation/position series, Home proof and provenance

Copy review corrections

  • B1 — sequential copy, not a counter-fade. One copy slot exits with heroCopyOut 120 ms ease (.25,.1,.25,1). Incoming is gated on decoded art/final fallback and the heroCopyDelay 140 ms start window, then enters with heroCopyIn 200 ms ease-out and copyRise 14 px. Text is replaced only after outgoing alpha reaches zero. Interruptions retarget current alpha; pending artwork does not cancel an already-running copy entry. There can never be two painted copies together.
  • N1 — first appearance restored. Initial copy uses rowEnter 180 ms ease-out and copyEntryRise 16 px. A saveable entry marker suppresses replay only when the same saved hero is remounted. Native Discover and Home cases verify alpha 0 / rise 16 on first appearance and alpha 1 / translation 0 on saved return.
  • N6 — Home visual evidence added. The real Home hero has a 124-frame native sequence covering initial entry, short-to-wrapping title handoff, repeated selections, reduced motion and saved remount. Play bounds stay unchanged in every mounted frame.
  • Art blend/readiness, stationary controls, pager, lane memory and Details-focus fixes are unchanged by this correction. The spec rows are restored to the approved copy direction. No new motion token.

Measured copy behavior

The reviewed counter-fade at 7bfa4313 is preserved separately, not overwritten as if it were the original art baseline. These paired runs use identical fixture data, selections and timestamps; “visible” below means alpha > 0.

Discover scenario Reviewed overlapping-copy frames Corrected overlapping-copy frames
Interrupted 35 0
Slow art 12 0
Failed art 12 0
Sustained 54-press burst 26 0
Reduced motion 4 0

Corrected maximum simultaneous visible copy: 1, including the Home sequence. The approved fade-through intentionally permits zero-alpha copy: the longest exact-zero run in these sampled handoffs is 2 frames / 32 ms. Repeated input can extend the visually quiet interval while the last selection waits for its entry window. There is no minimum-copy-opacity claim. Artwork keeps an opaque base and controls remain visible.

Visible outgoing translation stays where entry stopped; incoming visible translation is monotonic. At zero alpha the slot may reset its pose for the next copy. The original upper-art probe improvements remain unchanged: peak RGB-channel steps are 35 interrupted, 31 failed and 39 sustained input, versus up to 255 in the original art baseline. These are upper-probe measurements, not full-screen maxima: the approved bounded lower backdrop still changes on art readiness.

Applied where

  1. Discover hero, artwork and pager: HeroMotion, DiscoverScreen and DiscoverArtwork retain painted art until decoded art/final fallback is ready. Each outgoing artwork layer keeps its own original token lifetime and current alpha. Copy uses the sequential slot above. Details/pager remain outside copy motion; dots retarget current values.
  2. Home hero: HomeHeader shares the readiness scope and copy primitive. Copy grows upward inside the bottom-anchored layout; controls stay stationary. Home rows, focus-loop policy, presets, merged lane and card gradient are not changed here. Resume/Play still share HomeAction.Play and navigate directly to playback without focusNav; Home Details shares the title roots below. Player playback was not exercised by this lane.
  3. Title/person destination focus: the authorized minimal TitlePage change removes forced focusNav during Loading. Down cannot enter skeletons; Back still pops without opening the picker. WolfskinTitleViewModel.load keeps already-ready catalogue and held titles Ready on same-route/same-person refresh, retaining the existing authentication guard. Four obsolete nav-focus expectations were migrated without weakening stale-cast, Down, Back or picker safety. These paths are unchanged by the copy correction.
  4. Shared artwork: TitleArtwork disables Coil's independent painter transition only for hero-controlled requests. Standalone title/episode art keeps ordinary reveal. The touched native Coil NETWORK-success edge still measures red + blue >= 254/255 over 40 frames, with no nested painter-fade gap.
  5. Discover lanes: root and per-service memories keep their own moved column and first visible item. Untouched lanes start leftmost; moved lanes restore their position. Leaving Discover or rebuilding resets memory; pushed title open/pop does not. Root/service identities cannot collide. No change to this behavior in the copy correction.
  6. Inspected and unchanged: standalone title/episode heroes and person headers have no carousel handoff to migrate. Existing initial title entry remains; the ready-refresh fix prevents replay for already-loaded content. Shared row reveals, captions, focus, skeleton pulse and frame/shelf scroll retain their token-backed motion. The title follow-up's conditional readiness band and same-remeasure compensation remain intact.

Motion audit and spec

Art: heroFade / easeInOut. Copy out: heroCopyOut / ease; delayed start: heroCopyDelay; copy in: heroCopyIn / easeOut. Initial entry: rowEnter / easeOut and copyEntryRise. Pager: focusMove / easeOut. Fade durations use motionDuration; reduced motion removes the copy delay and translation, snaps pager geometry, and caps each permitted fade at 80 ms.

data/tv-spec/spec.md records the approved sequential copy, restored first entry/saveable remount behavior, visit-local lane memory and destination-focus/no-refresh-replay contract. Animated artwork stays bounded to Home 1920×820 and Discover 1920×740. The full painted backdrop remains static while incoming art is pending; no full-screen animated alpha owner is added.

Untouched findings from the explicit tween / animate*AsState / spring / animateScroll / Animatable audit:

  • NavBar: literal label easing and a Shell duration rather than Motion; reduced colour tween omits explicit token easing. Spring parameters are token-backed.
  • HomeRatings / SourceRatings: rating-plate tweens omit explicit token easing.
  • TitlePage season-chip scroll, outside the loading-focus fix: animateScrollToItem uses the library default; reduced motion snaps. Its frame-scroll spec is token-backed.
  • Integrated TitleMenu, TitleShelf and readiness-height motion already use token durations/easing and reduced-motion handling.

Choices I made

  • Reuse existing tokens; keep a single copy slot rather than layering different titles.
  • Preserve the current painted art while waiting, then use the approved bounded outgoing blend.
  • Use the existing navigation/lazy-item saveable-state ownership for the entry marker, not global state or new persistence.
  • Keep root/service lane memory separate from Shell/Home/Library persistence.
  • Use deterministic decoded native fixtures; the JNI ShortBuffer failure was independently reproduced on clean integration, and no production image workaround was added.

Resolved navigation choices

  • Home lane entry: Justin answered yes; follow-up lane queued after the Home focus-loop fix. This review does not implement that separate row-navigation change.
  • Library grid: keeps standard grid navigation; no behavior change here.

Verification — approved slim scope

On 79f64f6e18aba3eff2938831061d1d53532ba98e, the touched/native copy set passes 10/10:

  • DiscoverHeroSequenceTest: six cases — interrupted, slow art, failed art, sustained burst, reduced motion, and first-entry/saved-remount.
  • HeroMotionTest: two shared/Home cases, including the bounded art region and stable outgoing-copy/Play anchors.
  • HomeHeroSequenceTest: one full native Home sequence.
  • DiscoverNetworkHeroTest: one real Coil success edge.

The earlier full gate on 7bfa4313 passed 984/984 (design 226, client 26, app 732), plus all four debug APK/branding checks. Those are historical results, not fresh full-suite/build results for this copy-only head. As Justin explicitly approved, no full make test or make apk-debug was rerun after installation; one full run plus Opt checks will be done on the merged round candidate. Firstmate confirms combined Opt bf2efcfd (Home c5dbe910 + copy 79f64f6e) is installed and launches; this lane performed no device access or installation.

Captures are actual Compose/Robolectric native 1920×1080, sampled every 16 ms, not browser approximations or TV frame-time measurements. Initial/remount and Home panels are after-only; one deliberately absent host frame exercises saveable disposal/restoration. The retained title-refresh panel is explicitly from 7bfa4313, not rerun under the slim scope. Published files are 660/directories 770; native strips were inspected locally behind the expected page auth gate.

AI-assisted (fork-internal) ## Outcome Smooth Discover artwork changes, retain destination focus after Details, and make Discover lane memory visit-local. The copy-only review correction restores the approved sequential handoff and first-appearance entry on both Discover and Home; it removes the overlapping text seen in the reviewed counter-fade. Current pushed head: `79f64f6e18aba3eff2938831061d1d53532ba98e`. Base: `fm/tv-round2`. Integration and the title follow-up are merged without rewriting published history. No merge is requested before re-review and the merged-candidate checks. [Native reviewed-versus-corrected frame strips, 16 ms alpha/translation/position series, Home proof and provenance](https://pages.justinvisser.org/wolfskin/tv-round2-discover/) ## Copy review corrections - **B1 — sequential copy, not a counter-fade.** One copy slot exits with `heroCopyOut` **120 ms ease** (`.25,.1,.25,1`). Incoming is gated on decoded art/final fallback and the `heroCopyDelay` **140 ms** start window, then enters with `heroCopyIn` **200 ms ease-out** and `copyRise` **14 px**. Text is replaced only after outgoing alpha reaches zero. Interruptions retarget current alpha; pending artwork does not cancel an already-running copy entry. There can never be two painted copies together. - **N1 — first appearance restored.** Initial copy uses `rowEnter` **180 ms ease-out** and `copyEntryRise` **16 px**. A saveable entry marker suppresses replay only when the same saved hero is remounted. Native Discover and Home cases verify alpha 0 / rise 16 on first appearance and alpha 1 / translation 0 on saved return. - **N6 — Home visual evidence added.** The real Home hero has a 124-frame native sequence covering initial entry, short-to-wrapping title handoff, repeated selections, reduced motion and saved remount. Play bounds stay unchanged in every mounted frame. - Art blend/readiness, stationary controls, pager, lane memory and Details-focus fixes are unchanged by this correction. The spec rows are restored to the approved copy direction. No new motion token. ### Measured copy behavior The reviewed counter-fade at `7bfa4313` is preserved separately, not overwritten as if it were the original art baseline. These paired runs use identical fixture data, selections and timestamps; “visible” below means alpha > 0. | Discover scenario | Reviewed overlapping-copy frames | Corrected overlapping-copy frames | |---|---:|---:| | Interrupted | 35 | 0 | | Slow art | 12 | 0 | | Failed art | 12 | 0 | | Sustained 54-press burst | 26 | 0 | | Reduced motion | 4 | 0 | Corrected maximum simultaneous visible copy: **1**, including the Home sequence. The approved fade-through intentionally permits zero-alpha copy: the longest exact-zero run in these sampled handoffs is **2 frames / 32 ms**. Repeated input can extend the visually quiet interval while the last selection waits for its entry window. There is no minimum-copy-opacity claim. Artwork keeps an opaque base and controls remain visible. Visible outgoing translation stays where entry stopped; incoming visible translation is monotonic. At zero alpha the slot may reset its pose for the next copy. The original upper-art probe improvements remain unchanged: peak RGB-channel steps are **35** interrupted, **31** failed and **39** sustained input, versus up to **255** in the original art baseline. These are upper-probe measurements, not full-screen maxima: the approved bounded lower backdrop still changes on art readiness. ## Applied where 1. **Discover hero, artwork and pager:** `HeroMotion`, `DiscoverScreen` and `DiscoverArtwork` retain painted art until decoded art/final fallback is ready. Each outgoing artwork layer keeps its own original token lifetime and current alpha. Copy uses the sequential slot above. Details/pager remain outside copy motion; dots retarget current values. 2. **Home hero:** `HomeHeader` shares the readiness scope and copy primitive. Copy grows upward inside the bottom-anchored layout; controls stay stationary. Home rows, focus-loop policy, presets, merged lane and card gradient are not changed here. Resume/Play still share `HomeAction.Play` and navigate directly to playback without `focusNav`; Home Details shares the title roots below. Player playback was not exercised by this lane. 3. **Title/person destination focus:** the authorized minimal `TitlePage` change removes forced `focusNav` during Loading. Down cannot enter skeletons; Back still pops without opening the picker. `WolfskinTitleViewModel.load` keeps already-ready catalogue and held titles Ready on same-route/same-person refresh, retaining the existing authentication guard. Four obsolete nav-focus expectations were migrated without weakening stale-cast, Down, Back or picker safety. These paths are unchanged by the copy correction. 4. **Shared artwork:** `TitleArtwork` disables Coil's independent painter transition only for hero-controlled requests. Standalone title/episode art keeps ordinary reveal. The touched native Coil NETWORK-success edge still measures red + blue >= **254/255** over 40 frames, with no nested painter-fade gap. 5. **Discover lanes:** root and per-service memories keep their own moved column and first visible item. Untouched lanes start leftmost; moved lanes restore their position. Leaving Discover or rebuilding resets memory; pushed title open/pop does not. Root/service identities cannot collide. No change to this behavior in the copy correction. 6. **Inspected and unchanged:** standalone title/episode heroes and person headers have no carousel handoff to migrate. Existing initial title entry remains; the ready-refresh fix prevents replay for already-loaded content. Shared row reveals, captions, focus, skeleton pulse and frame/shelf scroll retain their token-backed motion. The title follow-up's conditional readiness band and same-remeasure compensation remain intact. ## Motion audit and spec Art: `heroFade` / `easeInOut`. Copy out: `heroCopyOut` / `ease`; delayed start: `heroCopyDelay`; copy in: `heroCopyIn` / `easeOut`. Initial entry: `rowEnter` / `easeOut` and `copyEntryRise`. Pager: `focusMove` / `easeOut`. Fade durations use `motionDuration`; reduced motion removes the copy delay and translation, snaps pager geometry, and caps each permitted fade at 80 ms. `data/tv-spec/spec.md` records the approved sequential copy, restored first entry/saveable remount behavior, visit-local lane memory and destination-focus/no-refresh-replay contract. Animated artwork stays bounded to Home **1920×820** and Discover **1920×740**. The full painted backdrop remains static while incoming art is pending; no full-screen animated alpha owner is added. Untouched findings from the explicit `tween` / `animate*AsState` / `spring` / `animateScroll` / `Animatable` audit: - `NavBar`: literal label easing and a Shell duration rather than Motion; reduced colour tween omits explicit token easing. Spring parameters are token-backed. - `HomeRatings` / `SourceRatings`: rating-plate tweens omit explicit token easing. - `TitlePage` season-chip scroll, outside the loading-focus fix: `animateScrollToItem` uses the library default; reduced motion snaps. Its frame-scroll spec is token-backed. - Integrated `TitleMenu`, `TitleShelf` and readiness-height motion already use token durations/easing and reduced-motion handling. ## Choices I made - Reuse existing tokens; keep a single copy slot rather than layering different titles. - Preserve the current painted art while waiting, then use the approved bounded outgoing blend. - Use the existing navigation/lazy-item saveable-state ownership for the entry marker, not global state or new persistence. - Keep root/service lane memory separate from Shell/Home/Library persistence. - Use deterministic decoded native fixtures; the JNI ShortBuffer failure was independently reproduced on clean integration, and no production image workaround was added. ## Resolved navigation choices - **Home lane entry:** Justin answered yes; follow-up lane queued after the Home focus-loop fix. This review does not implement that separate row-navigation change. - **Library grid:** keeps standard grid navigation; no behavior change here. ## Verification — approved slim scope On **`79f64f6e18aba3eff2938831061d1d53532ba98e`**, the touched/native copy set passes **10/10**: - `DiscoverHeroSequenceTest`: six cases — interrupted, slow art, failed art, sustained burst, reduced motion, and first-entry/saved-remount. - `HeroMotionTest`: two shared/Home cases, including the bounded art region and stable outgoing-copy/Play anchors. - `HomeHeroSequenceTest`: one full native Home sequence. - `DiscoverNetworkHeroTest`: one real Coil success edge. The earlier full gate on **`7bfa4313`** passed **984/984** (design 226, client 26, app 732), plus all four debug APK/branding checks. Those are historical results, not fresh full-suite/build results for this copy-only head. As Justin explicitly approved, **no full `make test` or `make apk-debug` was rerun after installation**; one full run plus Opt checks will be done on the merged round candidate. Firstmate confirms combined Opt `bf2efcfd` (Home `c5dbe910` + copy `79f64f6e`) is installed and launches; this lane performed no device access or installation. Captures are actual Compose/Robolectric native **1920×1080**, sampled every **16 ms**, not browser approximations or TV frame-time measurements. Initial/remount and Home panels are after-only; one deliberately absent host frame exercises saveable disposal/restoration. The retained title-refresh panel is explicitly from `7bfa4313`, not rerun under the slim scope. Published files are 660/directories 770; native strips were inspected locally behind the expected page auth gate.
justinzeus added 7 commits 2026-11-07 18:05:25 +01:00
Retain destination focus in the initial title loading regression
Some checks failed
review/code-claude Code review: approve, no blocking finding (9 non-blocking notes)
review/design-claude Blocked: hero copy counter-fade overlaps text; approved sequential hand-off differs (B1)
Details
7bfa431350
justinzeus commented 2026-11-07 18:18:22 +01:00
Author
Owner
Copy link

Code review (Claude) of head 7bfa43135081595b7ae83ac177133286573c782d: APPROVE, no blocking finding. I judge, I do not merge.

Read: HeroMotion (art/copy/scope), DiscoverArtwork/TitleArtwork/TitleSlotArtwork readiness and Coil transition scoping, DiscoverScreen/Shelf/State lane memory, HomeHeader, TitlePage + TitleState, WolfskinTitleViewModel.load/readTitle/ConductarrTitlePages, ShellState route/tab logic, and the new and migrated tests. Ran only generate_tokens.py --check (passes; no ad-hoc easing literals added). No Gradle.

Findings:

  • Hero art: each outgoing layer owns an independent token-bounded fade from its current alpha and removes itself; pending layers are dropped on interruption; the newest layer is always an opaque base. Readiness is signalled on success, error, missing art and bitmap fixtures, so no stuck or blank hero found. Reduced motion handled.
  • Home: layout and focus unchanged (action row moved outside the copy layer; native test pins bounds).
  • Title roots: with forced focusNav removed, the title-page node takes focus on every route; Down is swallowed in Loading, Back pops without the picker, Up reaches nav, no focus dead end. Migrated tests strengthened, none weakened.
  • load fast path: same route/viewer/Ready/non-person/non-fixture plus the auth guard; a different title, person, sign-out or first load takes the full reload, so no stale data for another title/person.
  • Lane memory: leftmost on first entry, moved lane restores, tab leave/rebuild reset, pushed title keeps (rootRoute unchanged by pushes), service memory isolated, pending lanes still skipped.
  • Coil NONE transition only inside hero requests; standalone artwork unchanged.

Non-blocking notes (full detail in the review report):

  1. Slow art: Details/Play act on the new hero while the old copy is shown; no readiness timeout.
  2. Home initial hero copy entrance removed (lane's own choice; heroCopyOut/heroCopyDelay/copyEntryRise now unused) - worth asking Justin with the PR's two questions.
  3. Catalogue fast path: auth guard checks signedIn only (not client); failed refresh can still show Unavailable; refresh test does not cover the post-lookup emission.
  4. presentation.identity not seeded for the initial art layer (narrow stale case).
  5. focus() coerce can overwrite a remembered column on an empty lane / openService first lane.
  6. Up-to-nav during Loading is pulled back to content when Ready arrives.
  7. Test-only semantics/testTag in production layers; copy crossfade ghosting by design.
  8. Parked interceptor coroutines in slow-art tests until loader reconfigure.
  9. Four non-token easing spots (NavBar, rating plates, season-chip scroll) left for a follow-up.
Code review (Claude) of head `7bfa43135081595b7ae83ac177133286573c782d`: **APPROVE, no blocking finding.** I judge, I do not merge. Read: HeroMotion (art/copy/scope), DiscoverArtwork/TitleArtwork/TitleSlotArtwork readiness and Coil transition scoping, DiscoverScreen/Shelf/State lane memory, HomeHeader, TitlePage + TitleState, WolfskinTitleViewModel.load/readTitle/ConductarrTitlePages, ShellState route/tab logic, and the new and migrated tests. Ran only `generate_tokens.py --check` (passes; no ad-hoc easing literals added). No Gradle. Findings: - Hero art: each outgoing layer owns an independent token-bounded fade from its current alpha and removes itself; pending layers are dropped on interruption; the newest layer is always an opaque base. Readiness is signalled on success, error, missing art and bitmap fixtures, so no stuck or blank hero found. Reduced motion handled. - Home: layout and focus unchanged (action row moved outside the copy layer; native test pins bounds). - Title roots: with forced focusNav removed, the title-page node takes focus on every route; Down is swallowed in Loading, Back pops without the picker, Up reaches nav, no focus dead end. Migrated tests strengthened, none weakened. - load fast path: same route/viewer/Ready/non-person/non-fixture plus the auth guard; a different title, person, sign-out or first load takes the full reload, so no stale data for another title/person. - Lane memory: leftmost on first entry, moved lane restores, tab leave/rebuild reset, pushed title keeps (rootRoute unchanged by pushes), service memory isolated, pending lanes still skipped. - Coil NONE transition only inside hero requests; standalone artwork unchanged. Non-blocking notes (full detail in the review report): 1. Slow art: Details/Play act on the new hero while the old copy is shown; no readiness timeout. 2. Home initial hero copy entrance removed (lane's own choice; heroCopyOut/heroCopyDelay/copyEntryRise now unused) - worth asking Justin with the PR's two questions. 3. Catalogue fast path: auth guard checks signedIn only (not client); failed refresh can still show Unavailable; refresh test does not cover the post-lookup emission. 4. presentation.identity not seeded for the initial art layer (narrow stale case). 5. focus() coerce can overwrite a remembered column on an empty lane / openService first lane. 6. Up-to-nav during Loading is pulled back to content when Ready arrives. 7. Test-only semantics/testTag in production layers; copy crossfade ghosting by design. 8. Parked interceptor coroutines in slow-art tests until loader reconfigure. 9. Four non-token easing spots (NavBar, rating plates, season-chip scroll) left for a follow-up.
justinzeus commented 2026-11-07 18:19:57 +01:00
Author
Owner
Copy link

Design review (Claude) - reviewed SHA 7bfa43135081595b7ae83ac177133286573c782d - verdict: BLOCK on one finding, everything else non-blocking.

Full report: /root/.treehouse/firstmate-7bab20/2/firstmate/data/review-58-design/report.md. Evidence: the published frames and series under /opt/stacks/pages/store/wolfskin/tv-round2-discover/ (after-strip.png, after/interrupted/first-06..09.png, after/rapid/press-2-02.png, press-4-00.png, burst-3-00.png, after/slow/ready-02.png, after/title-refresh/*, every after/*/series.json).

Good: art blend (no flash, dip, pop or snap-back; upper-probe step 35/31/39; series equals the 320 ms ease-in-out token curve), interrupted retarget, stationary buttons and pager (Details button/pager y constant), reduced motion (80 ms, no translation), failed-art fallback, and Details from the Discover hero (12 refresh frames pixel-identical to the loaded frame, nav not focused).

Blocking - B1 copy hand-off. The PR replaced the approved sequential copy hand-off (out 120 ms ease, in from 140 ms over 200 ms ease-out, 14 px rise) with a simultaneous 200 ms ease-out counter-fade. Frames 5-9 after each press show both titles, metadata rows and overviews overlapped (five displayed frames, overview unreadable); in the 54-press burst 3-4 titles and several "For you" kickers stack (up to 7 copy layers, composite minimum 0.546). The new copy also leads the art by about 100 ms. Justin's words (flicker, smooth easing) are about the art restart and the dip, which this PR fixes; they do not ask for overlapped text, and the approved direction A shows none. heroCopyOut and heroCopyDelay are now unused. Fix is copy-only: restore out heroCopyOut/ease, in after heroCopyDelay with heroCopyIn/easeOut + 14 px rise, still gated on art readiness, keep everything else. If a crossfade is preferred it needs Justin's explicit OK on first-08.png.

Values not equal to the page: outgoing copy 200 ms ease-out (page 120 ms ease); incoming copy start 0 ms (page 140 ms); initial Home/Discover hero copy entry removed (page 180 ms / 16 px rise). All other numbers equal the tokens (heroFade 320 easeInOut, heroCopyIn 200 easeOut, copyRise 14, pager focusMove 180 easeOut, reducedFade 80).

Non-blocking:

  1. Initial hero copy entry removed on Home/Discover (changes an approved row; no frames either way) - keep entry on first appearance, suppress only replay.
  2. Lower backdrop (below the 740/820 px bound) switches in one frame, probe step 101 at every art change; approved bounded blend, but the headline "255 -> 35" is the upper probe only - walk lane should check with real art.
  3. Hold on slow art is unbounded; pager and Details/Play act on the new title while the old copy shows.
  4. Sustained burst freezes the pager mid-width (total 66 px, min 65), chevron ~6 px off until it settles.
  5. Home button labels (Resume/Play SxEy/Details) change without a fade while copy hands off.
  6. Home hero evidence is one 8-frame bounds test, no frames; by source the Home hero cannot shift layout (bottom-anchored column, bottom-aligned copy box, fixed row height) and Play stays stationary.
  7. Dead tokens heroCopyOut/heroCopyDelay (resolved by the B1 fix).
  8. Title-refresh panel is after-only and covers refresh, not first open.

Questions for Justin (recommendation only): Home row entry - yes, adopt leftmost-until-moved on Home (his "make it universal"), keep Home's visited-row restore, do not reset on tab leave, sequence after the Home focus-loop fix since both touch HomeFocus.kt. Library grid - agree with the PR, keep standard grid navigation (one continuous grid, not lanes).

Commit status review/design-claude on this SHA: failure (B1).

**Design review (Claude) - reviewed SHA `7bfa43135081595b7ae83ac177133286573c782d` - verdict: BLOCK on one finding, everything else non-blocking.** Full report: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/review-58-design/report.md`. Evidence: the published frames and series under `/opt/stacks/pages/store/wolfskin/tv-round2-discover/` (`after-strip.png`, `after/interrupted/first-06..09.png`, `after/rapid/press-2-02.png`, `press-4-00.png`, `burst-3-00.png`, `after/slow/ready-02.png`, `after/title-refresh/*`, every `after/*/series.json`). Good: art blend (no flash, dip, pop or snap-back; upper-probe step 35/31/39; series equals the 320 ms ease-in-out token curve), interrupted retarget, stationary buttons and pager (Details button/pager y constant), reduced motion (80 ms, no translation), failed-art fallback, and Details from the Discover hero (12 refresh frames pixel-identical to the loaded frame, nav not focused). **Blocking - B1 copy hand-off.** The PR replaced the approved sequential copy hand-off (out 120 ms ease, in from 140 ms over 200 ms ease-out, 14 px rise) with a simultaneous 200 ms ease-out counter-fade. Frames 5-9 after each press show both titles, metadata rows and overviews overlapped (five displayed frames, overview unreadable); in the 54-press burst 3-4 titles and several "For you" kickers stack (up to 7 copy layers, composite minimum 0.546). The new copy also leads the art by about 100 ms. Justin's words (flicker, smooth easing) are about the art restart and the dip, which this PR fixes; they do not ask for overlapped text, and the approved direction A shows none. `heroCopyOut` and `heroCopyDelay` are now unused. Fix is copy-only: restore out `heroCopyOut`/`ease`, in after `heroCopyDelay` with `heroCopyIn`/`easeOut` + 14 px rise, still gated on art readiness, keep everything else. If a crossfade is preferred it needs Justin's explicit OK on `first-08.png`. **Values not equal to the page:** outgoing copy 200 ms ease-out (page 120 ms ease); incoming copy start 0 ms (page 140 ms); initial Home/Discover hero copy entry removed (page 180 ms / 16 px rise). All other numbers equal the tokens (heroFade 320 easeInOut, heroCopyIn 200 easeOut, copyRise 14, pager focusMove 180 easeOut, reducedFade 80). **Non-blocking:** 1. Initial hero copy entry removed on Home/Discover (changes an approved row; no frames either way) - keep entry on first appearance, suppress only replay. 2. Lower backdrop (below the 740/820 px bound) switches in one frame, probe step 101 at every art change; approved bounded blend, but the headline "255 -> 35" is the upper probe only - walk lane should check with real art. 3. Hold on slow art is unbounded; pager and Details/Play act on the new title while the old copy shows. 4. Sustained burst freezes the pager mid-width (total 66 px, min 65), chevron ~6 px off until it settles. 5. Home button labels (Resume/Play SxEy/Details) change without a fade while copy hands off. 6. Home hero evidence is one 8-frame bounds test, no frames; by source the Home hero cannot shift layout (bottom-anchored column, bottom-aligned copy box, fixed row height) and Play stays stationary. 7. Dead tokens `heroCopyOut`/`heroCopyDelay` (resolved by the B1 fix). 8. Title-refresh panel is after-only and covers refresh, not first open. **Questions for Justin (recommendation only):** Home row entry - yes, adopt leftmost-until-moved on Home (his "make it universal"), keep Home's visited-row restore, do not reset on tab leave, sequence after the Home focus-loop fix since both touch `HomeFocus.kt`. Library grid - agree with the PR, keep standard grid navigation (one continuous grid, not lanes). Commit status `review/design-claude` on this SHA: failure (B1).
justinzeus added 1 commit 2026-11-07 18:44:04 +01:00
Restore sequential hero copy and first-appearance entry
All checks were successful
review/code-claude Code APPROVE: sequential copy hand-off, no blocker
Details
review/design-claude Design PASS: copy hand-off matches approved frames/tokens
Details
79f64f6e18
justinzeus commented 2026-11-07 19:27:06 +01:00
Author
Owner
Copy link

Combined round-2 review (code + design) of the Discover PR at head 79f64f6e18aba3eff2938831061d1d53532ba98e, delta since 7bfa4313 plus integration with the Home PR.

Code verdict: APPROVE (status review/code-claude = success).
Design verdict: PASS (status review/design-claude = success). The earlier blocker B1 (overlapped copy) is fixed as approved.

B1 fix, evidence (published native frames and 16 ms series):

  • Never two copies at once: max visible copy layers = 1 in all seven series (reviewed-copy had 3 layers and 35 overlapping frames).
  • Sequential timing matches the approved tokens: old copy out over about 112-120 ms ease (704 -> 816 ms), new copy starts 140 ms after the press (848 ms) and enters over about 200 ms ease-out with the 14 px rise. The copy-less gap is 16-32 ms.
  • Interrupts and bursts: an interrupted fade-in (alpha 0.22) fades out from 0.22 holding translation 10.9 px, with no snap-back and no restart. The next title appears only after the quiet period.
  • Copy tied to art readiness: in the slow case the old copy is static for 22 pending frames and the hand-off starts 2 frames after art-ready.
  • Art blend, dots, lower backdrop, selection and nav focus are identical frame by frame to the reviewed version (0 differing frames of 266 across five scenarios). Controls, pager, lane memory and Details focus are not in the delta.
  • Initial 180 ms / 16 px copy entry is back (series 0 -> 1.0 by 176 ms) and saved remount frames stay at alpha 1 / translation 0. Reduced motion: 80 ms, no delay, no translation. Tokens (heroCopyOut/heroCopyDelay/heroCopyIn/rowEnter/copyEntryRise/copyRise), not literals; generate_tokens.py --check passes. The spec rows (spec.md:69, :71, :74) are restored.

Non-blocking notes:

  1. Stale spec sentence at spec.md:57 ("Home's nearest-centre shelves ... unchanged") contradicts the Home PR's leftmost rule once both land. Reword to say Home follows the same rule and Library is unchanged.
  2. The new title is already at 0.22-0.4 alpha while the art is still 64-50% the old picture. This is the approved prototype's offset (120/140/200 ms against 320 ms), not a defect.
  3. In a Home burst the old title holds static until art is ready, because the hand-off is gated on art readiness. By design.
  4. Whether the initial entry replays on a tab revisit depends on NavDisplay discarding saved state when the entry is popped. That matches the pre-round-2 behaviour. [INFERENCE]
  5. Integration: one conflict in DiscoverScreen.kt, resolved correctly. Pushing a title does not reset lane memory, because currentTab follows rootRoute. [INFERENCE] DiscoverState still reads the route-scoped shell.view, so the outgoing Discover entry could briefly show a hero focus jump during a lane-row Details exit. Pre-existing; worth a look in the box walk.

Full report: /root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md. Not run: Gradle/make (separate gate), nothing on the box.

Combined round-2 review (code + design) of the Discover PR at head `79f64f6e18aba3eff2938831061d1d53532ba98e`, delta since 7bfa4313 plus integration with the Home PR. **Code verdict: APPROVE (status `review/code-claude` = success).** **Design verdict: PASS (status `review/design-claude` = success).** The earlier blocker B1 (overlapped copy) is fixed as approved. **B1 fix, evidence (published native frames and 16 ms series):** - Never two copies at once: max visible copy layers = 1 in all seven series (reviewed-copy had 3 layers and 35 overlapping frames). - Sequential timing matches the approved tokens: old copy out over about 112-120 ms ease (704 -> 816 ms), new copy starts 140 ms after the press (848 ms) and enters over about 200 ms ease-out with the 14 px rise. The copy-less gap is 16-32 ms. - Interrupts and bursts: an interrupted fade-in (alpha 0.22) fades out from 0.22 holding translation 10.9 px, with no snap-back and no restart. The next title appears only after the quiet period. - Copy tied to art readiness: in the slow case the old copy is static for 22 pending frames and the hand-off starts 2 frames after art-ready. - Art blend, dots, lower backdrop, selection and nav focus are identical frame by frame to the reviewed version (0 differing frames of 266 across five scenarios). Controls, pager, lane memory and Details focus are not in the delta. - Initial 180 ms / 16 px copy entry is back (series 0 -> 1.0 by 176 ms) and saved remount frames stay at alpha 1 / translation 0. Reduced motion: 80 ms, no delay, no translation. Tokens (`heroCopyOut`/`heroCopyDelay`/`heroCopyIn`/`rowEnter`/`copyEntryRise`/`copyRise`), not literals; `generate_tokens.py --check` passes. The spec rows (`spec.md:69`, `:71`, `:74`) are restored. **Non-blocking notes:** 1. Stale spec sentence at `spec.md:57` ("Home's nearest-centre shelves ... unchanged") contradicts the Home PR's leftmost rule once both land. Reword to say Home follows the same rule and Library is unchanged. 2. The new title is already at 0.22-0.4 alpha while the art is still 64-50% the old picture. This is the approved prototype's offset (120/140/200 ms against 320 ms), not a defect. 3. In a Home burst the old title holds static until art is ready, because the hand-off is gated on art readiness. By design. 4. Whether the initial entry replays on a tab revisit depends on NavDisplay discarding saved state when the entry is popped. That matches the pre-round-2 behaviour. [INFERENCE] 5. Integration: one conflict in `DiscoverScreen.kt`, resolved correctly. Pushing a title does not reset lane memory, because `currentTab` follows `rootRoute`. [INFERENCE] `DiscoverState` still reads the route-scoped `shell.view`, so the outgoing Discover entry could briefly show a hero focus jump during a lane-row Details exit. Pre-existing; worth a look in the box walk. Full report: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md`. Not run: Gradle/make (separate gate), nothing on the box.
justinzeus closed this pull request 2026-11-07 21:14:29 +01:00
All checks were successful
review/code-claude Code APPROVE: sequential copy hand-off, no blocker
Details
review/design-claude Design PASS: copy hand-off matches approved frames/tokens
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
exploring

Idea with a proof gate; not scheduled until the gate passes

No labels
exploring
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: justinzeus/wolfskin#58
No description provided.
Delete branch "fm/tv-rd2-discover"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?