chore: convert json to lottie files#1786
Merged
transphorm merged 18 commits intodevfrom Mar 1, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughComprehensive migration from JSON-based lottie-react-native to compressed dotLottie format ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Heterogeneous changes spanning config machinery, component rewrites, asset migrations, and widespread screen updates. Multiple independent domains (Metro resolution, Vite bundling, Jest mocking, iOS/Android tooling, component API) with distinct reasoning required for each. Dense LottieAnimation implementation with legacy compatibility layer and ref forwarding. Deduplication resolver logic in metro.config.cjs requires careful validation. Patch application to external package adds complexity. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Member
Author
|
though there are savings improvements .lottie don't work on the web edit: read the message wrong. dotlottie files ARE supported but we need to change the package from lottie to dotlottie |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff6393a806
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx (1)
39-42:⚠️ Potential issue | 🟠 MajorUnhandled rejection from
onBeforeConfirmsilently aborts the confirmation flow.If
onBeforeConfirmrejects (e.g., a permission request throws or the caller's async logic fails), the error escapes theuseCallbackhandler entirely —onConfirmis never reached and the user gets no feedback.onConfirmhas its own try-catch butonBeforeConfirmhas none.🔒️ Proposed fix: guard onBeforeConfirm errors
const onPress = useCallback(async () => { - await onBeforeConfirm?.(); + try { + await onBeforeConfirm?.(); + } catch { + // onBeforeConfirm failure (e.g. permission denial) should not block confirmation + } await onConfirm(selfClient); }, [onBeforeConfirm, selfClient]);If a rejection from
onBeforeConfirmshould block confirmation (e.g., a hard requirement), surface the error to the user explicitly rather than letting it become an unhandled rejection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx` around lines 39 - 42, The onPress callback currently awaits onBeforeConfirm without error handling, so a rejection aborts the flow silently; wrap the onBeforeConfirm call in a try/catch inside the useCallback for onPress (before invoking onConfirm(selfClient)), and on catch either call an error handler prop (e.g., a new onBeforeConfirmError or existing onError callback) with the caught error or set component error state and return early to prevent calling onConfirm; update the component to accept/use that error handler or state so failures from onBeforeConfirm are surfaced to the user instead of becoming unhandled rejections (refer to onPress, onBeforeConfirm, onConfirm, useCallback, selfClient).
♻️ Duplicate comments (1)
packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx (1)
29-30: Same .lottie web-compat risk as above.
Please apply the samePlatform.OS === 'web'guard / web fallback strategy here.Does lottie-react-native (v7.2.2) / react-native-web support dotLottie (.lottie) assets on web, and what is the recommended web fallback (e.g., `@lottiefiles/dotlottie-web`)?As per coding guidelines, "Verify cross-platform compatibility for both React Native and Web environments" and "Use platform detection
Platform.OS === 'web'when adding platform-specific code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx` around lines 29 - 30, The require of the .lottie asset (passportVerifyAnimation) is not guarded for web; update the code in document-nfc-screen.tsx to detect Platform.OS === 'web' and load a web-safe fallback instead of requiring the .lottie binary on web (e.g., use a JSON animation asset or use a web lottie loader such as `@lottiefiles/dotlottie-web`), leaving the existing require('src/animations/passport_verify.lottie') for native only; ensure the symbol passportVerifyAnimation is set conditionally so components that render it use the platform-safe value.
🧹 Nitpick comments (1)
app/src/screens/app/GratificationScreen.tsx (1)
32-33: Animation won't render on web—consider using a web-compatible player or accept the empty placeholder.The
.lottieformat is not supported bylottie-react(the web runtime).DelayedLottieView.web.tsxalready provides a fallback that renders an empty<div />, so the web build succeeds and the screen remains functional. However, the animation simply won't display on web.If you need animations on web, either:
- Switch to
.json(Lottie JSON) format for web builds- Add
@lottiefiles/dotlottie-reactfor proper.lottiesupport on web with appropriate platform detectionThe current implementation is a graceful degradation; no action required unless animations are explicitly required on web.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/app/GratificationScreen.tsx` around lines 32 - 33, The require of '@selfxyz/mobile-sdk-alpha/animations/loading/youWin.lottie' assigned to youWinAnimation in GratificationScreen.tsx won't render on web because the .lottie binary format isn't supported by the web runtime; either switch the asset to a Lottie JSON (replace the .lottie require with a .json Lottie asset and update the youWinAnimation import) or add web support by installing and wiring '@lottiefiles/dotlottie-react' and platform-detecting the import so DelayedLottieView.web.tsx (or GratificationScreen.tsx) uses the dotlottie React player for web; if you accept graceful degradation, leave the require as-is since DelayedLottieView.web.tsx already falls back to an empty <div />.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/metro.config.cjs`:
- Around line 120-122: Vite is not configured to handle .lottie binary imports
used by require('@/assets/animations/*.lottie'), so web builds fail; update
vite.config.ts to add '.lottie' to the assetsInclude array (e.g., assetsInclude:
['**/*.lottie']) so Vite treats .lottie as an asset, or refactor calling sites
that use require('@/assets/animations/*.lottie') to instead load dotLottie via
`@dotlottie/dotlottie-js` with static JSON/imports; look for the require pattern
and the vite.config.ts export to make the change.
In `@app/src/screens/verification/ProofRequestStatusScreen.tsx`:
- Line 40: Rename the misspelled constant succesAnimation to successAnimation
and update every usage in this file (references such as succesAnimation in the
import/require line and any uses further down in the component) to the new name
to keep naming consistent with the other animation constants (e.g.,
successAnimation, failureAnimation) and avoid maintenance confusion.
In `@packages/mobile-sdk-alpha/package.json`:
- Around line 101-109: The package.json export keys "./animations/*.lottie" and
"./animations/loading/*.lottie" change public import specifiers and will break
consumers importing "./animations/*.json"; restore compatibility by adding
export entries for "./animations/*.json" and "./animations/loading/*.json" that
point to the same dist paths (i.e., map the .json specifiers to
"./dist/animations/*.lottie" and "./dist/animations/loading/*.lottie"), and
update the package changelog/release notes to document the breaking specifier
change and migration guidance; modify the package.json exports where
"./animations/*.lottie" and "./animations/loading/*.lottie" are defined and add
corresponding "./animations/*.json" and "./animations/loading/*.json" entries,
plus a short note in the release docs.
---
Outside diff comments:
In `@packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx`:
- Around line 39-42: The onPress callback currently awaits onBeforeConfirm
without error handling, so a rejection aborts the flow silently; wrap the
onBeforeConfirm call in a try/catch inside the useCallback for onPress (before
invoking onConfirm(selfClient)), and on catch either call an error handler prop
(e.g., a new onBeforeConfirmError or existing onError callback) with the caught
error or set component error state and return early to prevent calling
onConfirm; update the component to accept/use that error handler or state so
failures from onBeforeConfirm are surfaced to the user instead of becoming
unhandled rejections (refer to onPress, onBeforeConfirm, onConfirm, useCallback,
selfClient).
---
Duplicate comments:
In `@packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx`:
- Around line 29-30: The require of the .lottie asset (passportVerifyAnimation)
is not guarded for web; update the code in document-nfc-screen.tsx to detect
Platform.OS === 'web' and load a web-safe fallback instead of requiring the
.lottie binary on web (e.g., use a JSON animation asset or use a web lottie
loader such as `@lottiefiles/dotlottie-web`), leaving the existing
require('src/animations/passport_verify.lottie') for native only; ensure the
symbol passportVerifyAnimation is set conditionally so components that render it
use the platform-safe value.
---
Nitpick comments:
In `@app/src/screens/app/GratificationScreen.tsx`:
- Around line 32-33: The require of
'@selfxyz/mobile-sdk-alpha/animations/loading/youWin.lottie' assigned to
youWinAnimation in GratificationScreen.tsx won't render on web because the
.lottie binary format isn't supported by the web runtime; either switch the
asset to a Lottie JSON (replace the .lottie require with a .json Lottie asset
and update the youWinAnimation import) or add web support by installing and
wiring '@lottiefiles/dotlottie-react' and platform-detecting the import so
DelayedLottieView.web.tsx (or GratificationScreen.tsx) uses the dotlottie React
player for web; if you accept graceful degradation, leave the require as-is
since DelayedLottieView.web.tsx already falls back to an empty <div />.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (54)
app/metro.config.cjsapp/package.jsonapp/scripts/convert-to-dotlottie.mjsapp/src/assets/animations/launch_onboarding.jsonapp/src/assets/animations/loader.tsapp/src/assets/animations/passport_onboarding.jsonapp/src/assets/animations/passport_onboarding.lottieapp/src/assets/animations/passport_scan.jsonapp/src/assets/animations/passport_scan.lottieapp/src/assets/animations/passport_verify.jsonapp/src/assets/animations/passport_verify.lottieapp/src/assets/animations/proof_failed.jsonapp/src/assets/animations/proof_failed.lottieapp/src/assets/animations/proof_success.jsonapp/src/assets/animations/proof_success.lottieapp/src/assets/animations/qr_scan.jsonapp/src/assets/animations/qr_scan.lottieapp/src/assets/animations/splash.jsonapp/src/assets/animations/splash.lottieapp/src/assets/animations/warning.jsonapp/src/assets/animations/warning.lottieapp/src/screens/app/GratificationScreen.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/screens/app/SplashScreen.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/documents/scanning/DocumentCameraScreen.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/selection/DocumentOnboardingScreen.tsxapp/src/screens/kyc/KycSuccessScreen.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/verification/ProofRequestStatusScreen.tsxapp/src/screens/verification/QRCodeViewFinderScreen.tsxapp/tests/src/screens/kyc/KycSuccessScreen.test.tsxpackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/animations/loading/fail.jsonpackages/mobile-sdk-alpha/src/animations/loading/fail.lottiepackages/mobile-sdk-alpha/src/animations/loading/misc.jsonpackages/mobile-sdk-alpha/src/animations/loading/misc.lottiepackages/mobile-sdk-alpha/src/animations/loading/prove.jsonpackages/mobile-sdk-alpha/src/animations/loading/prove.lottiepackages/mobile-sdk-alpha/src/animations/loading/success.jsonpackages/mobile-sdk-alpha/src/animations/loading/success.lottiepackages/mobile-sdk-alpha/src/animations/loading/youWin.jsonpackages/mobile-sdk-alpha/src/animations/loading/youWin.lottiepackages/mobile-sdk-alpha/src/animations/passport_scan.jsonpackages/mobile-sdk-alpha/src/animations/passport_scan.lottiepackages/mobile-sdk-alpha/src/animations/passport_verify.jsonpackages/mobile-sdk-alpha/src/animations/passport_verify.lottiepackages/mobile-sdk-alpha/src/components/DelayedLottieView.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsxpackages/mobile-sdk-alpha/tsup.config.ts
💤 Files with no reviewable changes (12)
- packages/mobile-sdk-alpha/src/animations/loading/prove.json
- packages/mobile-sdk-alpha/src/animations/loading/fail.json
- app/src/assets/animations/proof_success.json
- app/src/assets/animations/warning.json
- packages/mobile-sdk-alpha/src/animations/loading/youWin.json
- app/src/assets/animations/proof_failed.json
- packages/mobile-sdk-alpha/src/animations/passport_scan.json
- app/src/assets/animations/splash.json
- packages/mobile-sdk-alpha/src/animations/loading/success.json
- app/src/assets/animations/passport_scan.json
- app/src/assets/animations/qr_scan.json
- packages/mobile-sdk-alpha/src/animations/loading/misc.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/metro.config.cjs (1)
120-121: LGTM — assetExts update addresses the dotLottie Metro integration requirement.Adding
'lottie'toassetExtsis the canonical approach recommended by the@lottiefiles/dotlottie-react-nativedocs. TheassetsIncludecompanion change invite.config.tsalso resolves the previously flagged Vite build concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/metro.config.cjs` around lines 120 - 121, The change to include 'lottie' in the Metro config's assetExts (update to assetExts: [...assetExts.filter(ext => ext !== 'svg'), 'lottie']) is correct and matches the `@lottiefiles/dotlottie-react-native` recommendation; no further code changes are needed here—ensure the companion assetsInclude modification in vite.config.ts remains in place and keep assetExts and its SVG filter logic intact.
🧹 Nitpick comments (2)
app/metro.config.cjs (1)
136-169: Animation dedup logic is sound — consider tightening the extension guard.The fallthrough behaviour is correct: if
animRelPathis unset (non-animation JSON), or if neither candidate path exists, Metro's default resolver continues. No misrouting risk.One optional hardening: the outer guard
/\.(json|lottie)$/intercepts all.jsonfiles originating frommobile-sdk-alphabefore attempting the animation redirect. Narrowing it to only.lottie(since that's the new asset type) or adding an explicitanimationspath check earlier would reduce the surface area and make the intent clearer.♻️ Suggested tightening (optional)
- if ( - /\.(json|lottie)$/.test(moduleName) && - context.originModulePath?.includes('mobile-sdk-alpha') - ) { + if ( + /\.(json|lottie)$/.test(moduleName) && + /\/animations\//.test(moduleName) && + context.originModulePath?.includes('mobile-sdk-alpha') + ) {This moves the
/animations/gate to the outerif, so theanimRelPathextraction is only attempted for animation paths, skipping entirely for all other JSON imports from the SDK.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/metro.config.cjs` around lines 136 - 169, The current outer guard /\. (json|lottie)$/ on moduleName is too broad and will intercept all .json imports from mobile-sdk-alpha; tighten it by moving the animations path check into the outer condition or by restricting the extension to only '.lottie' so animation-specific logic only runs for real animation imports. Update the if that references moduleName and context.originModulePath (the block that computes animRelPath and checks appAnimPath/sdkAnimPath using projectRoot and sdkAlphaPath) so it first ensures the request is an animations path (e.g., /\/animations\// or moduleName.startsWith('src/animations/')) or only matches /\.lottie$/, then keep the existing animRelPath extraction and fs.existsSync checks unchanged.app/vite.config.ts (1)
194-197:manualChunksentry for.lottieis a no-op.Once
assetsInclude: ['**/*.lottie']is set, Vite treats.lottiefiles as static binary assets — they are copied to the output directory and imports resolve to URL strings. Rollup'smanualChunksoperates exclusively on JS module graph entries; a.lottiebinary never enters the module graph, so this chunk entry has no effect. The comment "split out heavy Lottie files" is misleading because chunking does not apply here.♻️ Suggested cleanup
// Large animations - split out heavy Lottie files - 'animations-passport-onboarding': [ - './src/assets/animations/passport_onboarding.lottie', - ], -The
.lottieasset is already handled as a static file by theassetsIncludeconfig above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/vite.config.ts` around lines 194 - 197, The manualChunks entry for 'animations-passport-onboarding' is ineffective because .lottie files are treated as static assets by assetsInclude and never enter Rollup's JS module graph; remove the 'animations-passport-onboarding' key from the manualChunks object (or replace it with a clarifying comment) and rely on assetsInclude for .lottie handling; update any comment saying "split out heavy Lottie files" to reflect that these files are emitted as static assets and not chunked by manualChunks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/jest.config.cjs`:
- Around line 54-55: The moduleNameMapper entry
'^@selfxyz/mobile-sdk-alpha/(.*)\\.lottie$':
'<rootDir>/../packages/mobile-sdk-alpha/dist/$1.lottie' is unreachable because
the earlier general '\\.(png|jpg|jpeg|gif|webp|lottie)$' mapping already
captures all .lottie imports; remove the SDK-specific
'^@selfxyz/mobile-sdk-alpha/(.*)\\.lottie$' mapping (or if you prefer to keep
it, move it above the general '\\.(png|jpg|jpeg|gif|webp|lottie)$' entry) so
Jest will use the intended imageMock.js mapping instead of attempting to load
binary .lottie files from dist.
In `@app/vite.config.ts`:
- Line 20: The Vite config currently treats .lottie as static assets via
assetsInclude which causes components like DocumentOnboardingScreen (which use
require('@/assets/animations/passport_onboarding.lottie')) to resolve to a URL
that lottie-react (aliased from lottie-react-native) cannot play; fix by
installing `@lottiefiles/dotlottie-react` as a web dependency, update the Vite
alias in vite.config.ts to map "lottie-react-native" →
"@lottiefiles/dotlottie-react" (or provide a small shim module matching
LottieView props: source, loop, autoplay, cacheComposition, renderMode), and
then verify all components using .lottie (e.g., DocumentOnboardingScreen and any
LottieView usages) are API-compatible and load the dotLottie compositions rather
than treating them as raw binary URLs; also remove or ignore any manualChunks
entry for passport_onboarding.lottie since static assets aren’t JS-chunked.
---
Duplicate comments:
In `@app/metro.config.cjs`:
- Around line 120-121: The change to include 'lottie' in the Metro config's
assetExts (update to assetExts: [...assetExts.filter(ext => ext !== 'svg'),
'lottie']) is correct and matches the `@lottiefiles/dotlottie-react-native`
recommendation; no further code changes are needed here—ensure the companion
assetsInclude modification in vite.config.ts remains in place and keep assetExts
and its SVG filter logic intact.
---
Nitpick comments:
In `@app/metro.config.cjs`:
- Around line 136-169: The current outer guard /\. (json|lottie)$/ on moduleName
is too broad and will intercept all .json imports from mobile-sdk-alpha; tighten
it by moving the animations path check into the outer condition or by
restricting the extension to only '.lottie' so animation-specific logic only
runs for real animation imports. Update the if that references moduleName and
context.originModulePath (the block that computes animRelPath and checks
appAnimPath/sdkAnimPath using projectRoot and sdkAlphaPath) so it first ensures
the request is an animations path (e.g., /\/animations\// or
moduleName.startsWith('src/animations/')) or only matches /\.lottie$/, then keep
the existing animRelPath extraction and fs.existsSync checks unchanged.
In `@app/vite.config.ts`:
- Around line 194-197: The manualChunks entry for
'animations-passport-onboarding' is ineffective because .lottie files are
treated as static assets by assetsInclude and never enter Rollup's JS module
graph; remove the 'animations-passport-onboarding' key from the manualChunks
object (or replace it with a clarifying comment) and rely on assetsInclude for
.lottie handling; update any comment saying "split out heavy Lottie files" to
reflect that these files are emitted as static assets and not chunked by
manualChunks.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/jest.config.cjsapp/metro.config.cjsapp/vite.config.tspackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx
- packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx
- packages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/vite.config.ts (1)
197-200: Minor:manualChunksentry for binary asset has no effect.Rollup's
manualChunksoperates on JavaScript modules. The.lottiebinary asset is handled byassetsIncludeinstead, making this entry a no-op. Consider removing it for config clarity, though it causes no harm.🧹 Suggested cleanup
- // Large animations - split out heavy Lottie files - 'animations-passport-onboarding': [ - './src/assets/animations/passport_onboarding.lottie', - ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/vite.config.ts` around lines 197 - 200, The manualChunks entry 'animations-passport-onboarding' is a no-op because Rollup manualChunks only affects JS modules while the .lottie file is treated as a binary asset via assetsInclude; remove the 'animations-passport-onboarding' key from the manualChunks configuration (or move any intended chunking logic to a JS module import) to clean up the vite config and avoid misleading config entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/LoadingUI.tsx`:
- Around line 25-28: The AnimationSource type is missing support for React
Native require() assets (which are numbers); update the type alias
AnimationSource used in LoadingUIProps to include number (e.g., string | { uri:
string } | number) so code that assigns require('@/assets/... .lottie') to state
(see components that use setAnimationSource in LoadingScreen, DevLoadingScreen,
ProofRequestStatusScreen) type-checks correctly; ensure the declaration named
AnimationSource is changed and any imports/uses of LoadingUIProps remain
compatible.
---
Nitpick comments:
In `@app/vite.config.ts`:
- Around line 197-200: The manualChunks entry 'animations-passport-onboarding'
is a no-op because Rollup manualChunks only affects JS modules while the .lottie
file is treated as a binary asset via assetsInclude; remove the
'animations-passport-onboarding' key from the manualChunks configuration (or
move any intended chunking logic to a JS module import) to clean up the vite
config and avoid misleading config entries.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
app/Gemfile.lockis excluded by!**/*.lockapp/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
app/ios/Podfileapp/ios/Self.xcodeproj/project.pbxprojapp/metro.config.cjsapp/package.jsonapp/scripts/convert-to-dotlottie.mjsapp/src/components/LoadingUI.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/selection/DocumentOnboardingScreen.tsxapp/src/screens/verification/ProofRequestStatusScreen.tsxapp/src/screens/verification/QRCodeViewFinderScreen.tsxapp/vite.config.tspackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/components/DelayedLottieView.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsx
✅ Files skipped from review due to trivial changes (1)
- app/ios/Podfile
🚧 Files skipped from review as they are similar to previous changes (7)
- app/src/screens/dev/DevLoadingScreen.tsx
- app/src/screens/app/LoadingScreen.tsx
- packages/mobile-sdk-alpha/package.json
- app/src/screens/documents/scanning/DocumentNFCScanScreen.tsx
- app/scripts/convert-to-dotlottie.mjs
- app/src/screens/verification/QRCodeViewFinderScreen.tsx
- app/src/screens/verification/ProofRequestStatusScreen.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx (1)
85-98: Consider typing the ref callback parameter more precisely.The
instance: unknownparameter requires a cast toDotlottie | null. If theDotLottiecomponent'srefprop provides accurate typing, you could use that type directly to avoid the cast. However, this is a minor type-safety nit — the current approach works correctly at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx` around lines 85 - 98, The ref callback parameter is typed as unknown causing a cast; update the handleRef signature to accept the concrete type (e.g., change (instance: unknown) to (instance: Dotlottie | null) or the actual ref type provided by the DotLottie component) so you can remove the manual cast. Adjust the dependency/reference usages (internalRef, forwardedRef) to work with the new parameter type and preserve the existing forwardedRef handling for both function and MutableRefObject cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx`:
- Around line 85-98: The ref callback parameter is typed as unknown causing a
cast; update the handleRef signature to accept the concrete type (e.g., change
(instance: unknown) to (instance: Dotlottie | null) or the actual ref type
provided by the DotLottie component) so you can remove the manual cast. Adjust
the dependency/reference usages (internalRef, forwardedRef) to work with the new
parameter type and preserve the existing forwardedRef handling for both function
and MutableRefObject cases.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
app/package.jsonapp/src/components/LoadingUI.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/verification/ProofRequestStatusScreen.tsxpackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/components/DelayedLottieView.tsxpackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/tests/setup.tspackages/mobile-sdk-alpha/tsup.config.tsspecs/lottie-dotlottie-migration/REVIEW.md
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/components/LoadingUI.tsx
- app/package.json
- packages/mobile-sdk-alpha/package.json
- app/src/screens/dev/DevLoadingScreen.tsx
- packages/mobile-sdk-alpha/tsup.config.ts
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/screens/dev/DevLoadingScreen.tsx (1)
91-104:⚠️ Potential issue | 🟡 Minor
completedstate leaves animation unchanged—potential stale UI.When transitioning from an error state (e.g.,
failure) tocompleted, the animation source won't reset sincecompletedjustbreaks. This could leave the fail animation visible on the "completed" state, which seems unintentional for a success scenario.🛠️ Suggested fix
switch (currentState) { case 'completed': + setAnimationSource(proveLoadingAnimation); break; case 'error':Alternatively, if
completedshould show a distinct success animation, consider adding one. Since this is a dev screen for QA, consistent behavior across state transitions helps verify UI correctness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/dev/DevLoadingScreen.tsx` around lines 91 - 104, The switch on currentState leaves the animation unchanged for the 'completed' case, which can leave a stale failAnimation visible after transitioning from an error; update the 'completed' branch in the switch (where currentState is handled and setAnimationSource is called) to explicitly set the animation to the appropriate source (e.g., proveLoadingAnimation or a new successAnimation variable) so that setAnimationSource(...) is invoked for 'completed' and resets the UI consistently across state transitions.packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx (1)
39-42:⚠️ Potential issue | 🟠 MajorHandle
onBeforeConfirmfailures so confirmation doesn’t silently abortLine 40 can reject and currently short-circuits the flow, so
onConfirmis never reached and users get no graceful fallback path.Suggested fix
const onPress = useCallback(async () => { - await onBeforeConfirm?.(); - await onConfirm(selfClient); + try { + await onBeforeConfirm?.(); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : 'Unknown error'; + selfClient.trackEvent(ProofEvents.PROVING_PROCESS_ERROR, { + error: `onBeforeConfirm failed: ${message}`, + }); + // Graceful degradation: continue with confirmation flow + } + await onConfirm(selfClient); }, [onBeforeConfirm, selfClient]);As per coding guidelines, "Always use try-catch for async operations with graceful degradation when native modules fail and comprehensive error boundaries."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx` around lines 39 - 42, Wrap the await onBeforeConfirm?.() call inside a try-catch in the onPress useCallback so a rejection does not short-circuit the flow; on error log or surface the error (e.g., call a provided onBeforeConfirmError handler or processLogger) and then continue to call onConfirm(selfClient) so confirmation still proceeds with a graceful fallback; ensure the change is made inside the onPress function referenced in useCallback and keep the dependency array updated to include any new handlers (onBeforeConfirmError) you reference.
♻️ Duplicate comments (2)
packages/mobile-sdk-alpha/src/index.ts (1)
51-52:⚠️ Potential issue | 🟠 Major
DotLottieSourceis too narrow for RN bundled.lottieassets.Line 51 re-exports
DotLottieSource, but the current SDK source type excludesnumber. In React Native, bundled assets loaded viarequire('*.lottie')are numeric asset IDs, so this public type is incomplete for strict TypeScript consumers.Proposed fix
--- a/packages/mobile-sdk-alpha/src/components/LottieAnimation.tsx +++ b/packages/mobile-sdk-alpha/src/components/LottieAnimation.tsx @@ -export type DotLottieSource = string | { uri: string }; +export type DotLottieSource = string | number | { uri: string };#!/bin/bash # Verify DotLottieSource shape and .lottie require usage patterns rg -n "export type DotLottieSource" packages/mobile-sdk-alpha/src/components/LottieAnimation.tsx -C2 rg -n "require\\(.*\\.lottie" app/src packages/mobile-sdk-alpha/src -C1 rg -n "\\bDotLottieSource\\b" app/src packages/mobile-sdk-alpha/src -C1As per coding guidelines: "Type definitions must be complete and accurate across the package".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mobile-sdk-alpha/src/index.ts` around lines 51 - 52, The exported DotLottieSource type is too narrow for RN bundled .lottie assets; update the DotLottieSource type declaration in the LottieAnimation component (symbol: DotLottieSource in LottieAnimation.tsx) to include numeric RN asset IDs (add number to the union, or use ImageRequireSource/Asset type alongside string and {uri:string}) so require('*.lottie') consumers pass strict checks, then keep the existing re-export in index.ts as-is.app/src/screens/verification/ProofRequestStatusScreen.tsx (1)
39-39:⚠️ Potential issue | 🟡 MinorFix typo in
succesAnimation→successAnimation.The variable name is missing an
s. While it's used consistently within this file so there's no runtime bug, it's a maintenance hazard and inconsistent with the other animation constant names (failAnimation,loadingAnimation).✏️ Proposed fix
-const succesAnimation = require('@/assets/animations/proof_success.lottie'); +const successAnimation = require('@/assets/animations/proof_success.lottie');Also update the usage on line 105:
- setAnimationSource(succesAnimation); + setAnimationSource(successAnimation);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/verification/ProofRequestStatusScreen.tsx` at line 39, Rename the misspelled variable succesAnimation to successAnimation and update all its usages in this module (e.g., the require assignment and the reference at the existing usage around where the animation is rendered, previously referenced as succesAnimation) so names are consistent with failAnimation and loadingAnimation; ensure you update the import line (const succesAnimation = ...) and every occurrence (including the render at the usage near line 105) to the new identifier successAnimation.
🧹 Nitpick comments (5)
app/android/build.gradle (1)
64-66: Apply defensive guards for cleaner conditional logic, though current setup is safe.The condition at line 64 safely accesses
project.android.buildFeatures.composeonly whenproject.hasProperty('android')is true. Since Groovy property access returnsnullfor undefined properties, the current code works—but explicitly guarding property existence makes intent clearer and future-proofs against modules withoutbuildFeatures.Suggested improvement
- if (project.hasProperty('android') && project.android.buildFeatures.compose) { + if ( + project.hasProperty('android') && + project.android.hasProperty('buildFeatures') && + project.android.buildFeatures.compose + ) { project.apply plugin: 'org.jetbrains.kotlin.plugin.compose' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/android/build.gradle` around lines 64 - 66, Make the conditional explicit and null-safe by guarding access to project.android and its buildFeatures before checking compose: update the if that currently uses project.hasProperty('android') and project.android.buildFeatures.compose so it uses null-safe checks (safe navigation) and an explicit truth check on compose, then call project.apply plugin: 'org.jetbrains.kotlin.plugin.compose' only when that guarded condition is true; reference the existing symbols project.hasProperty('android'), project.android.buildFeatures.compose, and project.apply plugin: 'org.jetbrains.kotlin.plugin.compose' when making the change.app/src/assets/animations/loader.ts (1)
6-10: These loader functions are unused; either remove them or add platform-safe loading if intended for future use.These functions are currently never imported or called anywhere in the codebase. However, if they are intended for future use, the
require()calls will fail on web (Vite doesn't supportrequire()at runtime). Either remove these unused exports or guard them for web with platform-specific handling:+import { Platform } from 'react-native'; + export const loadMiscAnimation = () => + Platform.OS === 'web' + ? import('@selfxyz/mobile-sdk-alpha/animations/loading/misc.lottie').then( + mod => mod.default ?? mod, + ) + : Promise.resolve(require('@selfxyz/mobile-sdk-alpha/animations/loading/misc.lottie')); export const loadPassportAnimation = () => + Platform.OS === 'web' + ? import('@/assets/animations/passport_verify.lottie').then(mod => mod.default ?? mod) + : Promise.resolve(require('@/assets/animations/passport_verify.lottie'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/assets/animations/loader.ts` around lines 6 - 10, The two loader expressions (the top-level Promise.resolve(require('@selfxyz/.../misc.lottie')) and the exported loadPassportAnimation function) are unused and use runtime require() which will break on web; either remove these unused loaders or make them platform-safe by replacing runtime require() with conditional loading: for example, detect platform (Platform.OS or a build-time flag) and on web use dynamic import() (import('.../passport_verify.lottie')) while on native keep Promise.resolve(require(...)); update or remove the top-level anonymous loader and the exported loadPassportAnimation accordingly so both are either deleted if unused or rewritten to safely handle web vs native.patches/@LottieFiles+dotlottie-react-native+0.5.0.patch (1)
94-98: Reconsider the scope: this patch handles non-http(s) URIs, but the library doesn't officially supportfile://orcontent://schemes.The official dotlottie-react-native library supports only
http(s)URLs and localrequire()assets—notfile://orcontent://schemes. These URIs don't appear in the codebase either. The patch's fallback toDotLottieSource.Asset(url.substringAfterLast("/"))for non-http(s) inputs is defensive but imperfect: it assumes all other inputs are bundled asset filenames, which breaks relative paths like"animations/my-anim.lottie"(stripped to"my-anim.lottie"). Since the library doesn't document support for alternative schemes, consider whether the patch should instead reject or warn on unexpected input formats rather than silently converting them to local assets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/`@lottiefiles+dotlottie-react-native+0.5.0.patch around lines 94 - 98, The current patch silently treats any non-http(s) value as a bundled asset by setting source = DotLottieSource.Asset(url.substringAfterLast("/")), which breaks relative paths and mistakenly accepts unsupported schemes like file:// or content://; update the logic in the code that assigns source (referencing variables url and DotLottieSource) to explicitly validate the URI scheme: if url starts with http:// or https:// use DotLottieSource.Url, if it looks like a bundler require path (e.g. does not contain a scheme and should be treated as a local asset) use DotLottieSource.Asset without stripping path segments, and for unsupported schemes (file://, content://, or other unexpected formats) either return an error/throw or emit a clear warning and refuse conversion rather than silently mapping to an asset. Ensure tests (or callers) handle the new error/warning behavior.app/src/screens/verification/ProofRequestStatusScreen.tsx (1)
220-229: Consider removing unused legacy props.The
cacheComposition,renderMode, andprogressprops are explicitly dropped byLottieAnimation(they're destructured and ignored). While harmless, they add noise.♻️ Optional cleanup
<LottieAnimation autoPlay loop={animationSource === loadingAnimation} source={animationSource} style={styles.animation} - cacheComposition={false} - renderMode="HARDWARE" - speed={1} - progress={0} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/verification/ProofRequestStatusScreen.tsx` around lines 220 - 229, The LottieAnimation usage includes legacy props that are ignored; remove the unused props cacheComposition, renderMode, and progress from the LottieAnimation component instance (the JSX where LottieAnimation is rendered with props like autoPlay, loop={animationSource === loadingAnimation}, source={animationSource}, style={styles.animation}, speed={1}) so the component props list only includes the actual used props (e.g., autoPlay, loop, source, style, speed) to reduce noise.app/src/screens/app/LoadingScreen.tsx (1)
180-183: Clarify or remove the commented-out success animation case.The
completedcase has a commented-outsetAnimationSource(successAnimation)but no actual implementation. If the success animation is intentionally omitted (perhaps because the screen navigates away before it's visible), consider adding a brief comment explaining why. Otherwise, if the animation is needed, it should be imported and set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/app/LoadingScreen.tsx` around lines 180 - 183, The switch case for animationState ('completed') contains a commented-out setAnimationSource(successAnimation); either restore the success animation or explain why it's omitted: if you want the animation, import successAnimation and call setAnimationSource(successAnimation) inside the case (and ensure successAnimation is exported/available), otherwise remove the dead code and replace it with a short clarifying comment like "// intentionally no success animation because we navigate away" to make the intent clear; locate this change in the switch over animationState and the case 'completed' within LoadingScreen (setAnimationSource and successAnimation are the symbols to update).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/ios/Podfile`:
- Line 33: The PR is missing testing evidence for the iOS change
(IOS_DEPLOYMENT_TARGET in Podfile); update the PR description's "Tested" and
"How to QA" sections to state that you ran the iOS simulator build with yarn ios
(include simulator name and OS version) and verified dotLottie animations play
back correctly in the app (mention which screen/flow and a brief pass/fail
result), e.g., "Tested: ran yarn ios on iPhone 14 simulator (iOS 16.4) — build
succeeded; verified dotLottie animations on <screen> play smoothly."
In `@packages/mobile-sdk-alpha/package.json`:
- Line 219: The peer dependency entry "@lottiefiles/dotlottie-react-native": "*"
should be changed from an unbounded wildcard to a tested semver range (e.g.
"^0.5.0") in package.json to prevent unintentional major upgrades; locate the
dependency key in the peers/devDependencies block and replace "*" with the
chosen range, and if desired apply the same semver pinning approach to other
wildcard entries like "haptic-feedback", "localize", and "svg" for consistency.
In `@packages/mobile-sdk-alpha/src/components/LottieAnimation.web.tsx`:
- Around line 8-9: The web fallback LottieAnimation currently returns an inert
<div /> and never calls lifecycle callbacks or accepts style, which breaks
completion-gated flows; update the LottieAnimation component to accept props
(including style and the callbacks
onAnimationLoaded/onLoad/onAnimationFinish/onComplete), pass style through to
the wrapper element, and invoke the appropriate callback(s) immediately (or
after a microtask) to simulate a finished animation (e.g., call
onAnimationLoaded/onLoad then onAnimationFinish/onComplete) so web behavior
matches native completion-driven flows.
In `@packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx`:
- Around line 24-25: The web LottieAnimation stub currently returns an empty div
and ignores the source prop, so either implement web animation support using
lottie-web (initialize/destroy player in LottieAnimation for web, accept the
same source prop shape as native and load success.lottie via require) or add a
graceful fallback: keep a web LottieAnimation that respects the source prop,
logs a clear warning, and renders a static fallback (poster image or SVG) when
lottie-web cannot be used; ensure the component API/TypeScript types for
LottieAnimation and the source prop match the native implementation so imports
like successAnimation still type-check and animations degrade predictably on
web.
In `@packages/mobile-sdk-alpha/src/index.ts`:
- Around line 82-83: The web variant LottieAnimation.web.tsx currently returns
an empty <div /> causing breaks; implement a web playback using lottie-web (or
an equivalent) so the component matches native behavior. Update
LottieAnimation.web.tsx to import lottie-web, initialize an animation on mount
using the provided props (animation data/path, loop, autoplay, speed, renderer
settings) and attach it to a ref/container, clean up on unmount, and forward
relevant props/events so the exported LottieAnimation component behaves like the
native implementation.
In `@patches/`@lottiefiles+dotlottie-react-native+0.5.0.patch:
- Around line 30-33: Replace the unstable empty-string key in the onComplete
emission: in the onComplete function call to dataStore.onComplete(["" : ""]),
send a stable, documented key/value pair (e.g., ["event": "complete"] or
["type": "onComplete"]) so JS handlers receive a predictable payload; update the
call site in the onComplete function to use that named key and ensure any JS
listeners expect the new key.
---
Outside diff comments:
In `@app/src/screens/dev/DevLoadingScreen.tsx`:
- Around line 91-104: The switch on currentState leaves the animation unchanged
for the 'completed' case, which can leave a stale failAnimation visible after
transitioning from an error; update the 'completed' branch in the switch (where
currentState is handled and setAnimationSource is called) to explicitly set the
animation to the appropriate source (e.g., proveLoadingAnimation or a new
successAnimation variable) so that setAnimationSource(...) is invoked for
'completed' and resets the UI consistently across state transitions.
In `@packages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsx`:
- Around line 39-42: Wrap the await onBeforeConfirm?.() call inside a try-catch
in the onPress useCallback so a rejection does not short-circuit the flow; on
error log or surface the error (e.g., call a provided onBeforeConfirmError
handler or processLogger) and then continue to call onConfirm(selfClient) so
confirmation still proceeds with a graceful fallback; ensure the change is made
inside the onPress function referenced in useCallback and keep the dependency
array updated to include any new handlers (onBeforeConfirmError) you reference.
---
Duplicate comments:
In `@app/src/screens/verification/ProofRequestStatusScreen.tsx`:
- Line 39: Rename the misspelled variable succesAnimation to successAnimation
and update all its usages in this module (e.g., the require assignment and the
reference at the existing usage around where the animation is rendered,
previously referenced as succesAnimation) so names are consistent with
failAnimation and loadingAnimation; ensure you update the import line (const
succesAnimation = ...) and every occurrence (including the render at the usage
near line 105) to the new identifier successAnimation.
In `@packages/mobile-sdk-alpha/src/index.ts`:
- Around line 51-52: The exported DotLottieSource type is too narrow for RN
bundled .lottie assets; update the DotLottieSource type declaration in the
LottieAnimation component (symbol: DotLottieSource in LottieAnimation.tsx) to
include numeric RN asset IDs (add number to the union, or use
ImageRequireSource/Asset type alongside string and {uri:string}) so
require('*.lottie') consumers pass strict checks, then keep the existing
re-export in index.ts as-is.
---
Nitpick comments:
In `@app/android/build.gradle`:
- Around line 64-66: Make the conditional explicit and null-safe by guarding
access to project.android and its buildFeatures before checking compose: update
the if that currently uses project.hasProperty('android') and
project.android.buildFeatures.compose so it uses null-safe checks (safe
navigation) and an explicit truth check on compose, then call project.apply
plugin: 'org.jetbrains.kotlin.plugin.compose' only when that guarded condition
is true; reference the existing symbols project.hasProperty('android'),
project.android.buildFeatures.compose, and project.apply plugin:
'org.jetbrains.kotlin.plugin.compose' when making the change.
In `@app/src/assets/animations/loader.ts`:
- Around line 6-10: The two loader expressions (the top-level
Promise.resolve(require('@selfxyz/.../misc.lottie')) and the exported
loadPassportAnimation function) are unused and use runtime require() which will
break on web; either remove these unused loaders or make them platform-safe by
replacing runtime require() with conditional loading: for example, detect
platform (Platform.OS or a build-time flag) and on web use dynamic import()
(import('.../passport_verify.lottie')) while on native keep
Promise.resolve(require(...)); update or remove the top-level anonymous loader
and the exported loadPassportAnimation accordingly so both are either deleted if
unused or rewritten to safely handle web vs native.
In `@app/src/screens/app/LoadingScreen.tsx`:
- Around line 180-183: The switch case for animationState ('completed') contains
a commented-out setAnimationSource(successAnimation); either restore the success
animation or explain why it's omitted: if you want the animation, import
successAnimation and call setAnimationSource(successAnimation) inside the case
(and ensure successAnimation is exported/available), otherwise remove the dead
code and replace it with a short clarifying comment like "// intentionally no
success animation because we navigate away" to make the intent clear; locate
this change in the switch over animationState and the case 'completed' within
LoadingScreen (setAnimationSource and successAnimation are the symbols to
update).
In `@app/src/screens/verification/ProofRequestStatusScreen.tsx`:
- Around line 220-229: The LottieAnimation usage includes legacy props that are
ignored; remove the unused props cacheComposition, renderMode, and progress from
the LottieAnimation component instance (the JSX where LottieAnimation is
rendered with props like autoPlay, loop={animationSource === loadingAnimation},
source={animationSource}, style={styles.animation}, speed={1}) so the component
props list only includes the actual used props (e.g., autoPlay, loop, source,
style, speed) to reduce noise.
In `@patches/`@lottiefiles+dotlottie-react-native+0.5.0.patch:
- Around line 94-98: The current patch silently treats any non-http(s) value as
a bundled asset by setting source =
DotLottieSource.Asset(url.substringAfterLast("/")), which breaks relative paths
and mistakenly accepts unsupported schemes like file:// or content://; update
the logic in the code that assigns source (referencing variables url and
DotLottieSource) to explicitly validate the URI scheme: if url starts with
http:// or https:// use DotLottieSource.Url, if it looks like a bundler require
path (e.g. does not contain a scheme and should be treated as a local asset) use
DotLottieSource.Asset without stripping path segments, and for unsupported
schemes (file://, content://, or other unexpected formats) either return an
error/throw or emit a clear warning and refuse conversion rather than silently
mapping to an asset. Ensure tests (or callers) handle the new error/warning
behavior.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
app/Gemfile.lockis excluded by!**/*.lockapp/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (72)
app/android/app/build.gradleapp/android/build.gradleapp/ios/Podfileapp/ios/Self.xcodeproj/project.pbxprojapp/jest.config.cjsapp/metro.config.cjsapp/package.jsonapp/scripts/convert-to-dotlottie.mjsapp/src/assets/animations/launch_onboarding.jsonapp/src/assets/animations/loader.tsapp/src/assets/animations/passport_onboarding.jsonapp/src/assets/animations/passport_onboarding.lottieapp/src/assets/animations/passport_scan.jsonapp/src/assets/animations/passport_scan.lottieapp/src/assets/animations/passport_verify.jsonapp/src/assets/animations/passport_verify.lottieapp/src/assets/animations/proof_failed.jsonapp/src/assets/animations/proof_failed.lottieapp/src/assets/animations/proof_success.jsonapp/src/assets/animations/proof_success.lottieapp/src/assets/animations/qr_scan.jsonapp/src/assets/animations/qr_scan.lottieapp/src/assets/animations/splash.jsonapp/src/assets/animations/splash.lottieapp/src/assets/animations/warning.jsonapp/src/assets/animations/warning.lottieapp/src/components/LoadingUI.tsxapp/src/screens/app/GratificationScreen.tsxapp/src/screens/app/LoadingScreen.tsxapp/src/screens/app/SplashScreen.tsxapp/src/screens/dev/DevLoadingScreen.tsxapp/src/screens/documents/scanning/DocumentCameraScreen.tsxapp/src/screens/documents/scanning/DocumentNFCScanScreen.tsxapp/src/screens/documents/selection/DocumentOnboardingScreen.tsxapp/src/screens/kyc/KycSuccessScreen.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/verification/ProofRequestStatusScreen.tsxapp/src/screens/verification/QRCodeViewFinderScreen.tsxapp/tests/src/screens/GratificationScreen.test.tsxapp/tests/src/screens/kyc/KycSuccessScreen.test.tsxapp/vite.config.tspackages/mobile-sdk-alpha/package.jsonpackages/mobile-sdk-alpha/src/animations/loading/fail.jsonpackages/mobile-sdk-alpha/src/animations/loading/fail.lottiepackages/mobile-sdk-alpha/src/animations/loading/misc.jsonpackages/mobile-sdk-alpha/src/animations/loading/misc.lottiepackages/mobile-sdk-alpha/src/animations/loading/prove.jsonpackages/mobile-sdk-alpha/src/animations/loading/prove.lottiepackages/mobile-sdk-alpha/src/animations/loading/success.jsonpackages/mobile-sdk-alpha/src/animations/loading/success.lottiepackages/mobile-sdk-alpha/src/animations/loading/youWin.jsonpackages/mobile-sdk-alpha/src/animations/loading/youWin.lottiepackages/mobile-sdk-alpha/src/animations/passport_scan.jsonpackages/mobile-sdk-alpha/src/animations/passport_scan.lottiepackages/mobile-sdk-alpha/src/animations/passport_verify.jsonpackages/mobile-sdk-alpha/src/animations/passport_verify.lottiepackages/mobile-sdk-alpha/src/components/DelayedLottieView.tsxpackages/mobile-sdk-alpha/src/components/LottieAnimation.tsxpackages/mobile-sdk-alpha/src/components/LottieAnimation.web.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/confirm-identification.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsxpackages/mobile-sdk-alpha/src/flows/onboarding/document-nfc-screen.tsxpackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/tests/LottieAnimation.test.tsxpackages/mobile-sdk-alpha/tests/setup.tspackages/mobile-sdk-alpha/tsup.config.tspatches/@lottiefiles+dotlottie-react-native+0.5.0.patchpurple/frontend/styling.mdspecs/README.mdspecs/handoff-p1-fixes/SECURITY-HARDENING.mdspecs/lottie-dotlottie-migration/REVIEW.md
💤 Files with no reviewable changes (13)
- packages/mobile-sdk-alpha/src/animations/loading/fail.json
- packages/mobile-sdk-alpha/src/animations/loading/success.json
- app/src/assets/animations/qr_scan.json
- packages/mobile-sdk-alpha/src/animations/passport_scan.json
- app/src/assets/animations/warning.json
- app/src/assets/animations/splash.json
- packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx
- app/src/assets/animations/proof_success.json
- packages/mobile-sdk-alpha/src/animations/loading/youWin.json
- app/src/assets/animations/passport_scan.json
- app/src/assets/animations/proof_failed.json
- packages/mobile-sdk-alpha/src/animations/loading/prove.json
- packages/mobile-sdk-alpha/src/animations/loading/misc.json
transphorm
added a commit
that referenced
this pull request
Mar 12, 2026
This reverts commit 9406bac.
5 tasks
transphorm
added a commit
that referenced
this pull request
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A brief description of the changes, what and how is being changed.
Tested
Explain how the change has been tested (for example by manual testing, unit tests etc) or why it's not necessary (for example version bump).
How to QA
How can the change be tested in a repeatable manner?
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores