feat(hero-transition): shared-element flight from category grid to PDP#1473
feat(hero-transition): shared-element flight from category grid to PDP#1473zrgrvsh wants to merge 5 commits into
Conversation
Add a FLIP-based hero transition: clicking a product image in the grid flies a fixed-position clone to the PDP hero position while the routed views crossfade. A module-level coordinator connects three call sites — grid (capture source), PDP slider (register target + hide/reveal hero), and a lazily-mounted Overlay (render clone + run FLIP). Flight duration 350ms. Honors prefers-reduced-motion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…in adapters Replace the shallow coordinator with a deep flight state-machine module (flight.js) that owns the full lifecycle behind a small interface (capture/settle/registerClone/registerHero). The Overlay becomes a thin clone adapter (FLIP DOM only) and ProductImageSlider a thin hero adapter. Flight timing is consolidated into timing.js. The interface is the test surface: flight.spec.js drives whole flights with fake adapters, no DOM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e adapter C1: make the engage View crossfade duration the single source of truth. A new pure leaf libraries/engage/components/View/constants.js owns FADE_DURATION; the theme's timing.js derives FLIGHT_DURATION from it via the legal theme→engage dependency, replacing the duplicated 350ms held in sync only by a comment. C2: put the real clone adapter on the test surface. flight.spec.js drove fake adapters; the real Overlay FLIP adapter (deferred-fly via pendingFlyRef + useLayoutEffect — where the historical BLOCKER lived) had no test. New Overlay/__tests__/index.spec.jsx mounts it and drives render/fly (immediate + deferred)/fade/remove/onTransitionEnd against the real DOM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vigation #AppHeader is an in-flow flex element, so its height positions the content below it. On navigation the outgoing page's AppBar unmounts the instant its route turns invisible, but the outgoing content lingers for the view crossfade and the incoming AppBar only commits after the next page's cold first render. In that gap the in-flow header collapsed to 0 and the still-visible content jumped upward (visible on the first navigation, before render caches warm). Reserve one AppBar's height (minHeight) on the header so it can never collapse while momentarily empty, independent of why (cold render, network, cold start). Every route already renders a >=44px header, so there is no steady-state effect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Capture the stable route-change behaviors that repeatedly affect feature work: body crossfade vs immediate header unmount, lazy cold-render dominating first navigation, and the in-flow header positioning content. Each section points at the relevant source files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mfriedewald
left a comment
mfriedewald
left a comment
There was a problem hiding this comment.
Toller, sorgfältig gebauter PR — die State-Machine + Adapter-Aufteilung und die Doku sind stark, und die Tests decken die kniffligen Race-Pfade gut ab. Ich habe vier Punkte inline angemerkt: ein konkreter Timer-Bug in registerHero (#1, der einzige echte Defekt), zwei Altitude-/Robustheitsfragen (#2 shared-View, #3 Header-Floor) und ein kleines Cleanup (#4 toter reset-Export). Nichts davon ist ein Blocker — v. a. #1 würde ich aber gerne vor dem Merge adressiert sehen.
| }, HERO_REVEAL_SAFETY); | ||
|
|
||
| return () => { | ||
| if (heroRevealTimer !== null) { |
There was a problem hiding this comment.
Timer-Leak: heroRevealTimer-Clear ohne Identitäts-Guard.
Die Adapter-Löschung direkt darunter hat einen Guard (heroAdapter.setHidden === setHidden), das Timer-Clear hier aber nicht. Bei zwei überlappenden Slidern (Out-PDP + In-PDP während des 350-ms-Crossfades):
- Hero A registriert sich →
heroRevealTimer = timerA - Hero B registriert sich → löscht A's Timer, setzt
heroRevealTimer = timerB,heroAdapter = B - A meldet sich ab →
clearTimeout(heroRevealTimer)löscht B's Timer
B verliert damit seinen Reveal-Backstop. Settlet B's Flight nicht (z. B. verpasstes transitionend), bleibt B's PDP-Hero dauerhaft opacity:0.
Vorschlag: das Timer-Clear an dieselbe Identitätsprüfung koppeln wie die Adapter-Löschung, z. B. nur löschen, wenn heroAdapter && heroAdapter.setHidden === setHidden.
| } | ||
|
|
||
| // Keeps the element in layout (display:flex) while it fades out, then drops it once invisible. | ||
| const [rendered, setRendered] = useState(visible); |
There was a problem hiding this comment.
Altitude-Frage: theme-spezifisches Lingering in der geteilten View.
Das 350-ms-Lingering (rendered) liegt in der shared engage-View, dient aber nur der theme-ios11-Hero-Animation. Dadurch halten alle Themes ausgehende Views jetzt 350 ms gemountet (display:flex; opacity:0; pointerEvents:none) — inkl. extra DOM, einer pointerEvents:none-Totzone und genau des Header-Collapse-Artefakts, das den Viewport-Fix nötig macht.
Ursache (shared View) und Pflaster (Theme) liegen damit auf verschiedenen Ebenen. Wäre ein theme-/opt-in-gesteuertes Lingering (z. B. per Prop/Flag) statt global hier nicht sauberer?
| // lingers for the view crossfade and the incoming page's AppBar only appears | ||
| // after that page's cold first-render mounts; without this floor the in-flow | ||
| // header collapses in that gap and the still-visible content jumps upward. | ||
| minHeight: 'calc(44px + var(--safe-area-inset-top))', |
There was a problem hiding this comment.
Header-Floor deckt nur die nackte AppBar-Höhe ab.
Der reservierte minHeight entspricht genau einer 44-px-AppBar. Die AppBar kann aber via below-Prop / APP_BAR_BELOW-Portale (libraries/ui-ios/AppBar) Zeilen über der Bar rendern, also einen Steady-State-Header > 44 px. Bei der ersten Navigation von so einem Route klappt der Header im Crossfade-Gap von z. B. ~88 px auf 44 px → derselbe Content-Sprung, nur partiell gefixt.
Ich habe keinen konkreten ios11-Route gefunden, der den below-Slot heute persistent füllt (der Filter-Bar lebt im Body via ScrollHeader), daher kein Blocker — aber die Annahme „jeder Header ist mit 44 px floor-fähig" gilt nicht allgemein. Evtl. als bekannte Einschränkung im Kommentar vermerken.
| /** | ||
| * External cancel: an alias for settle(). | ||
| */ | ||
| export const reset = () => { |
There was a problem hiding this comment.
Cleanup: reset hat keine Caller.
reset ist ein reiner Alias auf settle() und wird nirgends aufgerufen; es gibt auch keinen Route-Change-Listener, der einen gestrandeten Flight abbricht.
Folge: Tap im Grid ohne nachfolgende PDP (Back-Nav, Nicht-PDP-Route, Navigations-Abbruch) → Teardown nur per Timer (Fade nach 600 ms, settle ~950 ms). Bis dahin schwebt der Klon über der UI und isFlightPending() bleibt true. Selbstheilend in <1 s und pointerEvents:none, also kosmetisch — aber entweder reset() an einen Route-Wechsel hängen (dann ist die Funktion auch begründet) oder den toten Export entfernen.
2 participants
Summary
Adds a shared-element ("hero") transition: tapping a product image in the grid animates that image as a FLIP flight into its position on the PDP, crossfading with the routed view. Also includes the architecture deepening that followed and a layout fix surfaced by the new crossfade.
Base is develop7 (not master), per frontend dev guidance.
What's included
feat(hero-transition)— the shared-element flight from category grid → PDP.refactor(hero-transition): deepen into a flight state machine with thin adapters— the shallow coordinator becomes a deepflightstate-machine module (capture/settle/registerClone/registerHero); the Overlay is a thin clone (FLIP) adapter andProductImageSlidera thin hero adapter. The interface is the test surface:flight.spec.jsdrives whole flights with fake adapters, no DOM.refactor(hero-transition): single timing source + cover the real clone adapter— the engage View crossfade duration becomes the single source of truth (View/constants.js), with the theme deriving its flight duration via the legal theme→engage dependency. Adds a test for the real Overlay FLIP adapter (the deferred-fly path where a historical blocker lived).fix(theme): reserve header height to prevent content jump on first navigation— see below.The header-jump fix
#AppHeaderis an in-flow flex element, so its height positions the content below it. On navigation the outgoing page's AppBar unmounts the instant its route turns invisible, but the outgoing content lingers for the view crossfade and the incoming AppBar only commits after the next page's cold first render. In that gap the in-flow header collapsed to 0 and the still-visible content jumped upward (only visible on the first navigation, before render caches warm). Fix: reserve one AppBar's height (min-height) on the header so it can never collapse while momentarily empty — every route already renders a ≥44px header, so there is no steady-state effect.Diagnosed by measuring the live navigation: the chunk loaded in ~10ms; the ~236ms gap was dev-mode cold render, which is why prefetch/Suspense-fallback approaches were rejected in favour of the layout reserve.
Testing
🤖 Generated with Claude Code