bugfix: ensure that iOS lottie animations display as expected #1218
Conversation
WalkthroughAdds a DelayedLottieView wrapper that defers autoplay by ~100ms and replaces direct LottieView usage across several screens and a loading UI. DocumentOnboardingScreen also adds an explicit 100ms delay before the initial play. No other control flow or API changes beyond the wrapper export. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Screen
participant DLV as DelayedLottieView
participant LV as LottieView (native)
Note over Screen,DLV: Render phase
Screen->>DLV: render({ autoPlay: true, ... })
DLV->>LV: render({ autoPlay: false, ... })
Note over DLV: useEffect on mount/update
DLV-->>DLV: setTimeout(100ms)
DLV->>LV: play()
sequenceDiagram
autonumber
actor User
participant DOS as DocumentOnboardingScreen
participant LV as LottieView
User->>DOS: Navigate to screen
DOS-->>DOS: setTimeout(100ms)
DOS->>LV: play() initial animation
LV-->>DOS: onAnimationFinish
DOS-->>DOS: setTimeout(220ms)
DOS->>LV: play() next animation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/screens/document/DocumentOnboardingScreen.tsx (1)
36-43: Consider using DelayedLottieView for consistency.While the manual delay implementation works correctly, it creates an inconsistent pattern compared to the rest of this PR where DelayedLottieView is used universally. This screen could use
<DelayedLottieView autoPlay={false} ref={animationRef} ... />and manually callplay()after the wrapper's initialization delay, maintaining consistency while preserving the custom looping behavior.Apply this approach for consistency:
+import { DelayedLottieView } from '@/components/DelayedLottieView'; + const DocumentOnboardingScreen: React.FC = () => { const navigation = useNavigation(); const handleCameraPress = useHapticNavigation('DocumentCamera'); const animationRef = useRef<LottieView>(null); const onCancelPress = () => { impactLight(); navigation.goBack(); }; - // iOS: Delay initial animation start to ensure native Lottie module is initialized - // This screen uses custom looping logic, so we manually trigger the first play useEffect(() => { const timer = setTimeout(() => { animationRef.current?.play(); }, 100); return () => clearTimeout(timer); }, []); return ( <ExpandableBottomLayout.Layout backgroundColor={black}> <SystemBars style="light" /> <ExpandableBottomLayout.TopSection roundTop backgroundColor={black}> - <LottieView + <DelayedLottieView ref={animationRef} autoPlay={false} loop={false} onAnimationFinish={() => { setTimeout(() => { animationRef.current?.play(); }, 220); }} source={passportOnboardingAnimation} style={styles.animation} cacheComposition={true} renderMode="HARDWARE" />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/components/DelayedLottieView.tsx(1 hunks)app/src/components/loading/LoadingUI.tsx(2 hunks)app/src/screens/document/ConfirmBelongingScreen.tsx(2 hunks)app/src/screens/document/DocumentCameraScreen.tsx(2 hunks)app/src/screens/document/DocumentOnboardingScreen.tsx(1 hunks)app/src/screens/home/DisclaimerScreen.tsx(2 hunks)app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx(2 hunks)app/src/screens/system/SplashScreen.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/screens/document/DocumentOnboardingScreen.tsxapp/src/screens/system/SplashScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/screens/document/DocumentCameraScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/document/DocumentOnboardingScreen.tsxapp/src/screens/system/SplashScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/screens/document/DocumentCameraScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/document/DocumentOnboardingScreen.tsxapp/src/screens/system/SplashScreen.tsxapp/src/components/loading/LoadingUI.tsxapp/src/screens/recovery/AccountVerifiedSuccessScreen.tsxapp/src/screens/document/DocumentCameraScreen.tsxapp/src/screens/home/DisclaimerScreen.tsxapp/src/screens/document/ConfirmBelongingScreen.tsxapp/src/components/DelayedLottieView.tsx
🧬 Code graph analysis (6)
app/src/screens/system/SplashScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/components/loading/LoadingUI.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/recovery/AccountVerifiedSuccessScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/document/DocumentCameraScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/home/DisclaimerScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
app/src/screens/document/ConfirmBelongingScreen.tsx (1)
app/src/components/DelayedLottieView.tsx (1)
DelayedLottieView(23-46)
🪛 GitHub Check: workspace-lint
app/src/components/DelayedLottieView.tsx
[warning] 23-23:
Caution: React also has a named export forwardRef. Check if you meant to write import {forwardRef} from 'react' instead
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: build-deps
🔇 Additional comments (1)
app/src/components/DelayedLottieView.tsx (1)
23-23: Static analysis warning is a false positive.The linter warning about
React.forwardRefvs importingforwardRefdirectly is a stylistic concern. SinceReactis imported at line 7,React.forwardRefis valid. The warning can be safely ignored or addressed separately as a code style cleanup.
| export const DelayedLottieView = React.forwardRef<LottieView, LottieViewProps>( | ||
| (props, forwardedRef) => { | ||
| const internalRef = useRef<LottieView>(null); | ||
| const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef; |
There was a problem hiding this comment.
Fix ref handling to support callback refs.
The current ref resolution logic only handles RefObject but React's forwardRef can receive callback refs (instance: T | null) => void. If a parent passes a callback ref, this code will incorrectly fall back to internalRef and the parent's ref won't be populated.
Apply this fix to properly handle both ref types:
export const DelayedLottieView = React.forwardRef<LottieView, LottieViewProps>(
(props, forwardedRef) => {
const internalRef = useRef<LottieView>(null);
- const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef;
+
+ // Merge forwarded ref with internal ref to support both RefObject and callback refs
+ const ref = internalRef;
+
+ useEffect(() => {
+ if (typeof forwardedRef === 'function') {
+ forwardedRef(ref.current);
+ } else if (forwardedRef) {
+ (forwardedRef as React.MutableRefObject<LottieView | null>).current = ref.current;
+ }
+ });
useEffect(() => {
// Only auto-trigger for autoPlay animations
if (props.autoPlay) {
const timer = setTimeout(() => {
ref.current?.play();
}, 100);
return () => clearTimeout(timer);
}
- }, [props.autoPlay, ref]);
+ }, [props.autoPlay]);
// For autoPlay animations, disable native autoPlay and control it ourselves
const modifiedProps = props.autoPlay
? { ...props, autoPlay: false }
: props;
return <LottieView ref={ref} {...modifiedProps} />;
},
);🧰 Tools
🪛 GitHub Check: workspace-lint
[warning] 23-23:
Caution: React also has a named export forwardRef. Check if you meant to write import {forwardRef} from 'react' instead
🤖 Prompt for AI Agents
In app/src/components/DelayedLottieView.tsx around lines 23-26, the current ref
resolution only handles RefObject and ignores callback refs; change to use a
single ref setter that updates internalRef.current and forwards to the passed
ref whether it is a callback or a RefObject. Implement a setRef = (instance) =>
{ internalRef.current = instance; if (typeof forwardedRef === 'function')
forwardedRef(instance); else if (forwardedRef && 'current' in forwardedRef)
(forwardedRef as React.MutableRefObject<any>).current = instance; } and pass
that setter as the ref to the LottieView so both internalRef and any callback or
object ref provided by the parent are populated.
transphorm
deleted the
justin/fix-ios-lottie-animations-add-slight-delay
branch
Summary by CodeRabbit
Bug Fixes
Refactor