From adc9b3b43cab6a0fd8531a9e5bff237e65f231f9 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 11 Jul 2026 13:25:24 +0200 Subject: [PATCH] Preserve loader behavior while reducing hidden work (#1996) * fix(app): preserve loader behavior in retained panels Keep the running footer mounted across panel switches. Inactive loaders stop their animation work, then realign to the original wall-clock phase before they become visible. * perf(app): remove inactive loader wrapper worklet * perf(app): pause hidden turn elapsed timers * perf(app): reduce loader animation work Keep one wall-clock-aligned clock for visible loaders and detach retained hidden loaders without unmounting them. Preserve the 950 ms sequence and reduced-motion behavior. --- docs/coding-standards.md | 2 +- docs/mobile-panels.md | 3 + packages/app/src/agent-stream/turn-footer.tsx | 8 +- packages/app/src/agent-stream/view.tsx | 2 - packages/app/src/components/message.tsx | 10 +- .../components/synced-loader-clock.test.ts | 33 ---- .../app/src/components/synced-loader-clock.ts | 36 ---- .../components/synced-loader-state.test.ts | 32 ++++ .../app/src/components/synced-loader-state.ts | 22 +++ packages/app/src/components/synced-loader.tsx | 157 +++++++++++++----- 10 files changed, 182 insertions(+), 123 deletions(-) delete mode 100644 packages/app/src/components/synced-loader-clock.test.ts delete mode 100644 packages/app/src/components/synced-loader-clock.ts create mode 100644 packages/app/src/components/synced-loader-state.test.ts create mode 100644 packages/app/src/components/synced-loader-state.ts diff --git a/docs/coding-standards.md b/docs/coding-standards.md index 5f2602097..f33f77f15 100644 --- a/docs/coding-standards.md +++ b/docs/coding-standards.md @@ -87,7 +87,7 @@ For testing rules, see [testing.md](testing.md). - Collection rows do not independently subscribe to a high-frequency global store. The collection owner selects structurally shared indexes once, derives a keyed row model with `useMemo`, and passes entries to rows. This keeps retained hidden collections current without running one selector per row on every store update. - Equality functions prevent React renders; they do not prevent selector callbacks from running. A selector attached to a hot store must be O(1) when its relevant source references have not changed. - Retained native panels use `RetainedPanel`. If an existing gesture/layout wrapper must own visibility, wrap its contents in `RetainedPanelActivity` instead. Keep keyed panel roots in a stable sibling order, include the newly active panel in the same render, centralize subscriptions, and gate genuine effects through `useRetainedPanelActive`. Do not use `Suspense` or render freezing for this on native: those techniques change native tree ownership instead of merely stopping work. -- Infinite animations are subscriptions. Start them only while their retained panel is active, and cancel a shared clock when its final active consumer leaves. Match scheduler cadence to the actual visual cadence; do not run a 60 fps interpolation when the rendered value changes only a few times per second. +- Infinite animations are subscriptions. Start them only while their retained panel is active, and cancel a shared clock when its final active consumer leaves. Synchronized animations use one clock per animation family and feed active instances through local shared values; retained hidden instances stay mounted but unsubscribed. Match state updates to the actual visual cadence; do not run every style worklet at 60 fps when the rendered value changes only a few times per second. - Stable references for props that cross `memo` boundaries or feed dependency arrays. Static literals at module scope `as const`; derived with `useMemo`; handlers with `useCallback` only when there's a memoized beneficiary. - Use stable ids for `key`, never array index for reorderable/filterable lists. - Context for stable values (theme, auth). Store with selectors for state that changes. diff --git a/docs/mobile-panels.md b/docs/mobile-panels.md index 1d577abe9..46c385d20 100644 --- a/docs/mobile-panels.md +++ b/docs/mobile-panels.md @@ -85,6 +85,9 @@ definition, no longer eligible to begin. - Panels whose gesture wrapper already owns visibility use `RetainedPanelActivity` to provide the same active signal without adding another layout root. Persistent animations, timers, polling, and shared clocks must subscribe to that signal and stop when their final visible consumer leaves. +- Synchronized step animations use one wall-clock-aligned source. Register a local shared value only + while its retained panel is active so hidden animated styles remain mounted without receiving clock + updates. Do not give every instance its own loop or leave hidden styles subscribed to the source. - Retention order and render order are separate concerns. LRU metadata may change on every switch; keyed retained roots must keep a stable sibling order. Moving large retained roots triggered Fabric Differ failures (`addViewAt` / `removeViewAt` view reuse) on Android. diff --git a/packages/app/src/agent-stream/turn-footer.tsx b/packages/app/src/agent-stream/turn-footer.tsx index 2ed53dad8..2f7bf3c8b 100644 --- a/packages/app/src/agent-stream/turn-footer.tsx +++ b/packages/app/src/agent-stream/turn-footer.tsx @@ -18,6 +18,7 @@ import { } from "@/components/message"; import type { TurnFooterHost } from "./layout"; import { SyncedLoader } from "@/components/synced-loader"; +import { useRetainedPanelActive } from "@/components/retained-panel"; const ThemedSyncedLoader = withUnistyles(SyncedLoader); const workingIndicatorColorMapping = (theme: Theme) => ({ @@ -35,23 +36,18 @@ export type AssistantTurnForkHandler = (input: { export const TurnFooter = memo(function TurnFooter({ isRunning, - isActive, inFlightTurnStartedAt, host, strategy, onForkAssistantTurn, }: { isRunning: boolean; - isActive: boolean; inFlightTurnStartedAt: Date | null; host: TurnFooterHost | null; strategy: TurnContentStrategy; onForkAssistantTurn?: AssistantTurnForkHandler; }) { if (isRunning) { - if (!isActive) { - return null; - } return ( @@ -103,6 +99,7 @@ const WorkingIndicator = memo(function WorkingIndicator({ }: { inFlightTurnStartedAt?: Date | null; }) { + const active = useRetainedPanelActive(); return ( @@ -111,6 +108,7 @@ const WorkingIndicator = memo(function WorkingIndicator({ {inFlightTurnStartedAt ? ( diff --git a/packages/app/src/agent-stream/view.tsx b/packages/app/src/agent-stream/view.tsx index fad5cc460..17afca016 100644 --- a/packages/app/src/agent-stream/view.tsx +++ b/packages/app/src/agent-stream/view.tsx @@ -772,7 +772,6 @@ const AgentStreamViewComponent = forwardRef; testID?: string; } @@ -702,23 +703,28 @@ interface LiveElapsedProps { */ export const LiveElapsed = memo(function LiveElapsed({ startedAt, + active = true, style, testID, }: LiveElapsedProps) { const startedAtMs = startedAt.getTime(); const [elapsedMs, setElapsedMs] = useState(() => Math.max(0, Date.now() - startedAtMs)); + const visibleElapsedMs = active ? Math.max(0, Date.now() - startedAtMs) : elapsedMs; useEffect(() => { + if (!active) { + return; + } setElapsedMs(Math.max(0, Date.now() - startedAtMs)); const handle = setInterval(() => { setElapsedMs(Math.max(0, Date.now() - startedAtMs)); }, 1000); return () => clearInterval(handle); - }, [startedAtMs]); + }, [active, startedAtMs]); return ( - {formatDuration(elapsedMs)} + {formatDuration(visibleElapsedMs)} ); }); diff --git a/packages/app/src/components/synced-loader-clock.test.ts b/packages/app/src/components/synced-loader-clock.test.ts deleted file mode 100644 index 7cb79a162..000000000 --- a/packages/app/src/components/synced-loader-clock.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; -import { createSharedStepClock } from "@/components/synced-loader-clock"; - -afterEach(() => { - vi.useRealTimers(); -}); - -describe("createSharedStepClock", () => { - it("advances a shared cadence only while at least one consumer is subscribed", () => { - vi.useFakeTimers(); - const clock = createSharedStepClock(6, 960); - const firstListener = vi.fn(); - const secondListener = vi.fn(); - - const unsubscribeFirst = clock.subscribe(firstListener); - const unsubscribeSecond = clock.subscribe(secondListener); - vi.advanceTimersByTime(320); - - expect(clock.getSnapshot()).toBe(2); - expect(firstListener).toHaveBeenCalledTimes(2); - expect(secondListener).toHaveBeenCalledTimes(2); - - unsubscribeFirst(); - vi.advanceTimersByTime(160); - expect(firstListener).toHaveBeenCalledTimes(2); - expect(secondListener).toHaveBeenCalledTimes(3); - - unsubscribeSecond(); - vi.advanceTimersByTime(640); - expect(clock.getSnapshot()).toBe(3); - expect(secondListener).toHaveBeenCalledTimes(3); - }); -}); diff --git a/packages/app/src/components/synced-loader-clock.ts b/packages/app/src/components/synced-loader-clock.ts deleted file mode 100644 index e46915ab3..000000000 --- a/packages/app/src/components/synced-loader-clock.ts +++ /dev/null @@ -1,36 +0,0 @@ -export interface SharedStepClock { - getSnapshot: () => number; - subscribe: (listener: () => void) => () => void; -} - -export function createSharedStepClock(stepCount: number, cycleDurationMs: number): SharedStepClock { - const normalizedStepCount = Math.max(1, Math.floor(stepCount)); - const stepDurationMs = Math.max(1, Math.round(cycleDurationMs / normalizedStepCount)); - const listeners = new Set<() => void>(); - let step = 0; - let timer: ReturnType | null = null; - - function advance(): void { - step = (step + 1) % normalizedStepCount; - for (const listener of listeners) { - listener(); - } - } - - return { - getSnapshot: () => step, - subscribe: (listener) => { - listeners.add(listener); - if (timer === null) { - timer = setInterval(advance, stepDurationMs); - } - return () => { - listeners.delete(listener); - if (listeners.size === 0 && timer !== null) { - clearInterval(timer); - timer = null; - } - }; - }, - }; -} diff --git a/packages/app/src/components/synced-loader-state.test.ts b/packages/app/src/components/synced-loader-state.test.ts new file mode 100644 index 000000000..b01c1c9cd --- /dev/null +++ b/packages/app/src/components/synced-loader-state.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, test } from "vitest"; +import { getSyncedLoaderDotOpacity, getSyncedLoaderStep } from "./synced-loader-state"; + +describe("synced loader state", () => { + test("advances through six wall-clock-aligned steps every 950 milliseconds", () => { + const sampleTimes = [0, 158, 159, 316, 317, 474, 475, 633, 634, 791, 792, 949, 950]; + + const steps = sampleTimes.map(getSyncedLoaderStep); + + expect(steps).toEqual([0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 0]); + }); + + test("preserves the six visible snake states", () => { + const states: number[][] = []; + for (let step = 0; step < 6; step += 1) { + const dotOpacities: number[] = []; + for (let dot = 0; dot < 6; dot += 1) { + dotOpacities.push(getSyncedLoaderDotOpacity(step, dot)); + } + states.push(dotOpacities); + } + + expect(states).toEqual([ + [1, 0, 0.78, 0, 0.56, 0.34], + [0.78, 1, 0.56, 0, 0.34, 0], + [0.56, 0.78, 0.34, 1, 0, 0], + [0.34, 0.56, 0, 0.78, 0, 1], + [0, 0.34, 0, 0.56, 1, 0.78], + [0, 0, 1, 0.34, 0.78, 0.56], + ]); + }); +}); diff --git a/packages/app/src/components/synced-loader-state.ts b/packages/app/src/components/synced-loader-state.ts new file mode 100644 index 000000000..dba96baa2 --- /dev/null +++ b/packages/app/src/components/synced-loader-state.ts @@ -0,0 +1,22 @@ +const SYNCED_LOADER_DURATION_MS = 950; +export const SYNCED_LOADER_DOT_COUNT = 6; + +const SYNCED_LOADER_OPACITY_STATES = [ + [1, 0, 0.78, 0, 0.56, 0.34], + [0.78, 1, 0.56, 0, 0.34, 0], + [0.56, 0.78, 0.34, 1, 0, 0], + [0.34, 0.56, 0, 0.78, 0, 1], + [0, 0.34, 0, 0.56, 1, 0.78], + [0, 0, 1, 0.34, 0.78, 0.56], +] as const; + +export function getSyncedLoaderStep(nowMs: number): number { + "worklet"; + const elapsedMs = nowMs % SYNCED_LOADER_DURATION_MS; + return Math.floor((elapsedMs * SYNCED_LOADER_DOT_COUNT) / SYNCED_LOADER_DURATION_MS); +} + +export function getSyncedLoaderDotOpacity(step: number, dot: number): number { + "worklet"; + return SYNCED_LOADER_OPACITY_STATES[step]?.[dot] ?? 0; +} diff --git a/packages/app/src/components/synced-loader.tsx b/packages/app/src/components/synced-loader.tsx index c8464f819..9a1e9a492 100644 --- a/packages/app/src/components/synced-loader.tsx +++ b/packages/app/src/components/synced-loader.tsx @@ -1,26 +1,101 @@ -import { useMemo, useSyncExternalStore } from "react"; +import { useLayoutEffect, useMemo, useState } from "react"; import { View } from "react-native"; +import Animated, { + makeMutable, + type SharedValue, + useAnimatedStyle, + useReducedMotion, + useSharedValue, +} from "react-native-reanimated"; +import { scheduleOnUI } from "react-native-worklets"; import { useRetainedPanelActive } from "@/components/retained-panel"; -import { createSharedStepClock } from "@/components/synced-loader-clock"; +import { + SYNCED_LOADER_DOT_COUNT, + getSyncedLoaderDotOpacity, + getSyncedLoaderStep, +} from "@/components/synced-loader-state"; -const SYNCED_LOADER_DURATION_MS = 6_000; -const DOT_SEQUENCE = [0, 1, 3, 5, 4, 2] as const; -const DOT_COUNT = DOT_SEQUENCE.length; const GRID_COLUMNS = 2; -const SNAKE_SEGMENT_OFFSETS = [0, -1, -2, -3, -4] as const; -const SNAKE_OPACITIES = [1, 0.78, 0.56, 0.34, 0] as const; -const DOT_INDEXES = Array.from({ length: DOT_COUNT }, (_, dotIndex) => dotIndex); -const DOT_KEYS = DOT_INDEXES.map((dotIndex) => `dot-${dotIndex}`); -const sharedStepClock = createSharedStepClock(DOT_COUNT, SYNCED_LOADER_DURATION_MS); -const subscribePaused = () => () => {}; +const DOT_KEYS = Array.from({ length: SYNCED_LOADER_DOT_COUNT }, (_, i) => `dot-${i}`); +const sharedStep = makeMutable(0); +const activeLoaderCount = makeMutable(0); +const clockRunning = makeMutable(false); +let nextStepListenerId = 1; + +function advanceSharedStep(): void { + "worklet"; + if (activeLoaderCount.value === 0) { + clockRunning.value = false; + return; + } + + const nextStep = getSyncedLoaderStep(Date.now()); + if (sharedStep.value !== nextStep) { + sharedStep.value = nextStep; + } + requestAnimationFrame(advanceSharedStep); +} + +function registerStepListener( + step: SharedValue, + registered: SharedValue, + listenerId: number, +): void { + "worklet"; + if (registered.value) { + return; + } + + registered.value = true; + step.value = getSyncedLoaderStep(Date.now()); + sharedStep.addListener(listenerId, (nextStep) => { + step.value = nextStep; + }); + activeLoaderCount.value += 1; + + if (!clockRunning.value) { + clockRunning.value = true; + sharedStep.value = step.value; + requestAnimationFrame(advanceSharedStep); + } +} + +function unregisterStepListener(registered: SharedValue, listenerId: number): void { + "worklet"; + if (!registered.value) { + return; + } + + registered.value = false; + sharedStep.removeListener(listenerId); + activeLoaderCount.value -= 1; +} + +function useSyncedLoaderStep(active: boolean, reduceMotion: boolean): SharedValue { + // The local value lets retained loaders detach from the app-wide clock without + // unmounting their animated views or leaving hidden style worklets subscribed. + const step = useSharedValue(reduceMotion ? 0 : getSyncedLoaderStep(Date.now())); + const registered = useSharedValue(false); + const [listenerId] = useState(() => nextStepListenerId++); + + useLayoutEffect(() => { + if (!active || reduceMotion) { + return; + } + + scheduleOnUI(registerStepListener, step, registered, listenerId); + return () => { + scheduleOnUI(unregisterStepListener, registered, listenerId); + }; + }, [active, listenerId, reduceMotion, registered, step]); + + return step; +} export function SyncedLoader({ size = 10, color }: { size?: number; color: string }) { const active = useRetainedPanelActive(); - const step = useSyncExternalStore( - active ? sharedStepClock.subscribe : subscribePaused, - sharedStepClock.getSnapshot, - sharedStepClock.getSnapshot, - ); + const reduceMotion = useReducedMotion(); + const step = useSyncedLoaderStep(active, reduceMotion); const gap = Math.max(1, Math.round(size * 0.12)); const dotSize = Math.max(2, Math.floor((size - gap * 2) / 3)); @@ -45,17 +120,16 @@ export function SyncedLoader({ size = 10, color }: { size?: number; color: strin return ( - {DOT_INDEXES.map((dotIndex) => { + {Array.from({ length: SYNCED_LOADER_DOT_COUNT }).map((_, dotIndex) => { const rowIndex = Math.floor(dotIndex / GRID_COLUMNS); const columnIndex = dotIndex % GRID_COLUMNS; - const sequenceIndex = DOT_SEQUENCE.indexOf(dotIndex as (typeof DOT_SEQUENCE)[number]); return ( ; left: number; top: number; }) { - let opacity = 0; - - for (let segmentIndex = 0; segmentIndex < SNAKE_SEGMENT_OFFSETS.length; segmentIndex += 1) { - const activeSequenceIndex = - (step + SNAKE_SEGMENT_OFFSETS[segmentIndex] + DOT_COUNT) % DOT_COUNT; - if (sequenceIndex === activeSequenceIndex) { - opacity = SNAKE_OPACITIES[segmentIndex] ?? 0; - break; - } - } + const animatedStyle = useAnimatedStyle(() => ({ + opacity: getSyncedLoaderDotOpacity(step.value, dotIndex), + })); const dotStyle = useMemo( - () => ({ - width: dotSize, - height: dotSize, - borderRadius: dotSize / 2, - backgroundColor: color, - position: "absolute" as const, - left, - top, - opacity, - }), - [color, dotSize, left, opacity, top], + () => [ + animatedStyle, + { + width: dotSize, + height: dotSize, + borderRadius: dotSize / 2, + backgroundColor: color, + position: "absolute" as const, + left, + top, + }, + ], + [animatedStyle, dotSize, color, left, top], ); - return ; + return ; }