Skip to content

Conversation

@shazarre
Copy link
Collaborator

@shazarre shazarre commented Oct 15, 2025

This PR introduces DocumentCamera screen to the SDK. It also moves all required components (biggest being the expandable bottom layout with insets handling).

Tested:

  • ran the demo app and successfuly got to NFC scanning
  • ran the app and successfuly got to NFC scanning

Summary by CodeRabbit

  • New Features

    • New expandable bottom layout with flexible top/full/bottom sections
    • New document camera screen for passport/MRZ capture with animations and cancel flow
  • Improvements

    • Animation component now falls back gracefully when native animation support is unavailable
    • Views support max-width and improved safe-area handling
    • Document scan success now triggers a dedicated success navigation path
  • Platform

    • SVG support enabled for the demo project

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Walkthrough

Replaces local UI/components with exports from @selfxyz/mobile-sdk-alpha, adds DelayedLottieView (with runtime guard) and ExpandableBottomLayout to the SDK, adjusts Metro and deps for Lottie/SVG, removes demo MRZ hook in favor of SDK DocumentCameraScreen, and adds test mocks and small layout constants. (39 words)

Changes

Cohort / File(s) Summary
Metro & Dependency Config
app/metro.config.cjs, packages/mobile-sdk-demo/metro.config.cjs, packages/mobile-sdk-alpha/package.json, packages/mobile-sdk-demo/package.json
Added/adjusted Metro resolver rules to block duplicate lottie-react-native and enable SVG as source; added lottie/react-native and react-native-svg to package deps/peerDeps.
DelayedLottieView & Consumers
packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx, packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx, app/src/components/loading/LoadingUI.tsx, app/src/screens/app/SplashScreen.tsx, app/src/screens/documents/scanning/DocumentCameraScreen.tsx, app/src/screens/documents/selection/ConfirmBelongingScreen.tsx, app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx, app/src/screens/onboarding/DisclaimerScreen.tsx
Moved DelayedLottieView into SDK, added a web stub and a runtime guard for missing Lottie; updated app imports to consume SDK export.
ExpandableBottomLayout (SDK + App wrapper)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx, app/src/layouts/ExpandableBottomLayout.tsx, packages/mobile-sdk-alpha/src/index.ts, packages/mobile-sdk-alpha/src/browser.ts
Added SDK ExpandableBottomLayout (Layout/TopSection/FullSection/BottomSection) with types and safe-area handling; app wrapper now delegates to SDK and adopts SDK prop types.
Document Camera / MRZ Flow
packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx, packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx, packages/mobile-sdk-demo/src/screens/index.ts, packages/mobile-sdk-demo/src/hooks/useMRZScanner.ts, packages/mobile-sdk-demo/tests/screens/DocumentCamera.test.tsx
Added SDK DocumentCameraScreen; removed demo useMRZScanner hook; demo DocumentCamera now renders SDK screen and forwards new onSuccess; test updated accordingly.
MRZScannerView & Layout Utilities
packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx, packages/mobile-sdk-alpha/src/components/layout/View.tsx, packages/mobile-sdk-alpha/src/constants/layout.ts
Adjusted MRZScannerView inline sizing and removed StyleSheet usage; added maxWidth prop to SDK View; introduced extraYPadding = 15.
Test Mocks / Setup
packages/mobile-sdk-alpha/tests/setup.ts
Added deterministic PixelRatio mocks and mocks for react-native-svg, react-native-svg-circle-country-flags, lottie-react-native, and react-native-haptic-feedback.
Import Consolidation
app/src/providers/selfClientProvider.tsx
Consolidated multiple type imports from @selfxyz/mobile-sdk-alpha into a single import block.

Sequence Diagram(s)

sequenceDiagram
    participant Demo as Demo App
    participant SDK as @selfxyz/mobile-sdk-alpha
    participant Scanner as MRZScannerView
    participant Client as SelfClient

    Demo->>SDK: render DocumentCameraScreen({onBack, onSuccess})
    SDK->>Scanner: mount and start scanning (animation)
    loop frame processing
        Scanner->>Scanner: analyze frames
    end

    alt MRZ detected
        Scanner->>SDK: onPassportRead(mrzData)
        SDK->>Client: emit MRZ event / process payload
        SDK->>Demo: call onSuccess()
        Demo->>Demo: navigate -> 'nfc'
    else Scanner error
        Scanner->>Client: emit SdkEvents.ERROR
    else User cancels
        Demo->>Demo: call onBack()
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • aaronmgdr
  • remicolin

Poem

✨ SDK pieces fit together neat, Lottie waits until beats repeat,
Layouts lift and cameras roam, demo screens now call the home,
Metro hides what should not clash, tests mock steady for the bash,
New exports bloom and old hooks rest — a tidier tree, now trimmed and dressed.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Move DocumentCamera screen to the SDK" directly and accurately describes the primary objective of this pull request. The changeset confirms this is the main focus: a new DocumentCameraScreen component is introduced in packages/mobile-sdk-alpha with supporting infrastructure (ExpandableBottomLayout, DelayedLottieView, etc.), and the app-level DocumentCamera screen is refactored to use this new SDK component. The title is concise, specific, and free from vague terminology or misleading claims.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch shazarre/move_expandable_bottom_layout_to_the_sdk

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20e1a03 and b5d42f7.

⛔ Files ignored due to path filters (2)
  • app/Gemfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • app/metro.config.cjs (2 hunks)
  • packages/mobile-sdk-alpha/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/metro.config.cjs
🧰 Additional context used
📓 Path-based instructions (3)
packages/mobile-sdk-alpha/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Files:

  • packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)

Files:

  • packages/mobile-sdk-alpha/package.json
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts

Files:

  • packages/mobile-sdk-alpha/package.json
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
⏰ 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). (7)
  • GitHub Check: android-build-test
  • GitHub Check: e2e-ios
  • GitHub Check: android-e2e
  • GitHub Check: ios-e2e
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
  • GitHub Check: build-deps
🔇 Additional comments (1)
packages/mobile-sdk-alpha/package.json (1)

140-140: Dependency addition aligns with DocumentCamera components. The pinned version (7.2.2) is consistent with the project's strategy of strict coupling (see react-native at line 155), and mirrors the tight version control seen elsewhere in the SDK.

Also applies to: 153-153


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@shazarre shazarre marked this pull request as ready for review October 16, 2025 15:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/screens/documents/scanning/DocumentCameraScreen.tsx (1)

31-31: Update ExpandableBottomLayout import to use SDK
This screen currently pulls the local copy; since ExpandableBottomLayout was migrated into the @selfxyz/mobile-sdk-alpha package, change the import to:

import { ExpandableBottomLayout } from '@selfxyz/mobile-sdk-alpha/layouts/ExpandableBottomLayout';
🧹 Nitpick comments (3)
packages/mobile-sdk-alpha/src/constants/layout.ts (1)

5-5: LGTM with a minor documentation suggestion.

The constant is clean and well-named. Consider adding a JSDoc comment explaining what this padding represents (e.g., additional vertical padding for bottom layouts accounting for safe area insets) to improve maintainability.

packages/mobile-sdk-alpha/package.json (1)

153-153: Consider using a version range for the peer dependency.

The peer dependency specifies an exact version (7.2.2) which may cause installation conflicts if consumers use a different version of lottie-react-native. Consider using a caret range (e.g., ^7.2.2) or minimum version (e.g., >=7.2.2) to allow flexibility while ensuring compatibility.

packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (1)

148-148: Track the TODO for style comparison.

The TODO indicates the styles may need verification against the original implementation.

Would you like me to open an issue to track comparing these styles with the original implementation to ensure visual consistency?

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d04870 and 52faaf8.

⛔ Files ignored due to path filters (2)
  • packages/mobile-sdk-alpha/svgs/icons/passport_camera_scan.svg is excluded by !**/*.svg
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (23)
  • app/metro.config.cjs (1 hunks)
  • app/src/components/loading/LoadingUI.tsx (1 hunks)
  • app/src/layouts/ExpandableBottomLayout.tsx (3 hunks)
  • app/src/providers/selfClientProvider.tsx (1 hunks)
  • app/src/screens/app/SplashScreen.tsx (1 hunks)
  • app/src/screens/documents/scanning/DocumentCameraScreen.tsx (1 hunks)
  • app/src/screens/documents/selection/ConfirmBelongingScreen.tsx (1 hunks)
  • app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (1 hunks)
  • app/src/screens/onboarding/DisclaimerScreen.tsx (1 hunks)
  • packages/mobile-sdk-alpha/package.json (2 hunks)
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx (2 hunks)
  • packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (3 hunks)
  • packages/mobile-sdk-alpha/src/components/layout/View.tsx (2 hunks)
  • packages/mobile-sdk-alpha/src/constants/layout.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx (1 hunks)
  • packages/mobile-sdk-alpha/src/index.ts (2 hunks)
  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (1 hunks)
  • packages/mobile-sdk-demo/metro.config.cjs (1 hunks)
  • packages/mobile-sdk-demo/package.json (1 hunks)
  • packages/mobile-sdk-demo/src/hooks/useMRZScanner.ts (0 hunks)
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1 hunks)
  • packages/mobile-sdk-demo/src/screens/index.ts (1 hunks)
  • packages/mobile-sdk-demo/tests/screens/DocumentCamera.test.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/mobile-sdk-demo/src/hooks/useMRZScanner.ts
🧰 Additional context used
📓 Path-based instructions (10)
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/constants/layout.ts
  • packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx
  • packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
  • packages/mobile-sdk-alpha/src/components/layout/View.tsx
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/constants/layout.ts
  • packages/mobile-sdk-demo/src/screens/index.ts
  • packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
  • app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx
  • app/src/screens/app/SplashScreen.tsx
  • app/src/screens/documents/scanning/DocumentCameraScreen.tsx
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx
  • app/src/screens/documents/selection/ConfirmBelongingScreen.tsx
  • app/src/components/loading/LoadingUI.tsx
  • packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx
  • app/src/providers/selfClientProvider.tsx
  • app/src/screens/onboarding/DisclaimerScreen.tsx
  • app/src/layouts/ExpandableBottomLayout.tsx
  • packages/mobile-sdk-demo/tests/screens/DocumentCamera.test.tsx
  • packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
  • packages/mobile-sdk-alpha/src/components/layout/View.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/constants/layout.ts
  • packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx
  • packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx
  • packages/mobile-sdk-alpha/src/index.ts
  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
  • packages/mobile-sdk-alpha/src/components/layout/View.tsx
packages/mobile-sdk-alpha/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Files:

  • packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)

Files:

  • packages/mobile-sdk-alpha/package.json
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts

Files:

  • packages/mobile-sdk-alpha/package.json
  • packages/mobile-sdk-demo/package.json
app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (app/AGENTS.md)

Type checking must pass before PRs (yarn types)

Files:

  • app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx
  • app/src/screens/app/SplashScreen.tsx
  • app/src/screens/documents/scanning/DocumentCameraScreen.tsx
  • app/src/screens/documents/selection/ConfirmBelongingScreen.tsx
  • app/src/components/loading/LoadingUI.tsx
  • app/src/providers/selfClientProvider.tsx
  • app/src/screens/onboarding/DisclaimerScreen.tsx
  • app/src/layouts/ExpandableBottomLayout.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/onboarding/AccountVerifiedSuccessScreen.tsx
  • app/src/screens/app/SplashScreen.tsx
  • app/src/screens/documents/scanning/DocumentCameraScreen.tsx
  • app/src/screens/documents/selection/ConfirmBelongingScreen.tsx
  • app/src/components/loading/LoadingUI.tsx
  • app/src/providers/selfClientProvider.tsx
  • app/src/screens/onboarding/DisclaimerScreen.tsx
  • app/src/layouts/ExpandableBottomLayout.tsx
**/*.{test,spec}.{ts,js,tsx,jsx}

⚙️ CodeRabbit configuration file

**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:

  • Test coverage completeness
  • Test case quality and edge cases
  • Mock usage appropriateness
  • Test readability and maintainability

Files:

  • packages/mobile-sdk-demo/tests/screens/DocumentCamera.test.tsx
packages/mobile-sdk-alpha/src/index.ts

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Files:

  • packages/mobile-sdk-alpha/src/index.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies

Applied to files:

  • app/metro.config.cjs
📚 Learning: 2025-07-16T02:20:44.173Z
Learnt from: transphorm
PR: selfxyz/self#636
File: app/src/components/native/QRCodeScanner.tsx:135-142
Timestamp: 2025-07-16T02:20:44.173Z
Learning: In app/src/components/native/QRCodeScanner.tsx, the Android camera dimension multipliers (screenWidth * 3 and screenHeight * 2) are intentionally set to these values and should not be changed. These multipliers are correct and any visual issues with black areas in the camera preview are caused by other factors, not the dimension calculations.

Applied to files:

  • packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.

Applied to files:

  • app/src/screens/app/SplashScreen.tsx
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-08-25T14:25:57.586Z
Learnt from: aaronmgdr
PR: selfxyz/self#951
File: app/src/providers/authProvider.web.tsx:17-18
Timestamp: 2025-08-25T14:25:57.586Z
Learning: The selfxyz/mobile-sdk-alpha/constants/analytics import path is properly configured with SDK exports, Metro aliases, and TypeScript resolution. Import changes from @/consts/analytics to this path are part of valid analytics migration, not TypeScript resolution issues.

Applied to files:

  • app/src/providers/selfClientProvider.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • app/src/providers/selfClientProvider.tsx
  • packages/mobile-sdk-alpha/src/index.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-demo/package.json
🧬 Code graph analysis (7)
packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx (2)
packages/mobile-sdk-alpha/src/index.ts (1)
  • DelayedLottieView (61-61)
app/jest.setup.js (1)
  • React (687-687)
packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx (4)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (2)
  • SafeAreaInsets (85-88)
  • ExpandableBottomLayout (141-146)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (2)
  • useReadMRZ (29-96)
  • mrzReadInstructions (18-20)
packages/mobile-sdk-alpha/src/constants/colors.ts (4)
  • white (59-59)
  • black (8-8)
  • slate800 (49-49)
  • slate400 (39-39)
packages/mobile-sdk-alpha/src/constants/analytics.ts (1)
  • PassportEvents (116-136)
app/src/layouts/ExpandableBottomLayout.tsx (3)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (2)
  • LayoutProps (32-37)
  • BottomSectionProps (21-25)
packages/mobile-sdk-alpha/src/constants/colors.ts (1)
  • black (8-8)
app/src/mocks/react-native-safe-area-context.js (1)
  • useSafeAreaInsets (44-46)
packages/mobile-sdk-demo/tests/screens/DocumentCamera.test.tsx (1)
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)
  • DocumentCamera (14-20)
packages/mobile-sdk-demo/metro.config.cjs (1)
app/scripts/find-type-import-issues.mjs (1)
  • ext (71-71)
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)
packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx (1)
  • DocumentCameraScreen (29-91)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (3)
packages/mobile-sdk-alpha/src/components/layout/View.tsx (2)
  • ViewProps (49-53)
  • View (180-308)
packages/mobile-sdk-alpha/src/constants/layout.ts (1)
  • extraYPadding (5-5)
packages/mobile-sdk-alpha/src/constants/colors.ts (2)
  • black (8-8)
  • white (59-59)
🪛 Biome (2.1.2)
packages/mobile-sdk-alpha/src/components/DelayedLottieView.tsx

[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

Hooks should not be called after an early return.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)


[error] 33-33: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

Hooks should not be called after an early return.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

🪛 GitHub Actions: Web CI
app/src/layouts/ExpandableBottomLayout.tsx

[error] 15-15: Import error: 'ExpandableBottomLayout' is not exported by '@selfxyz/mobile-sdk-alpha/dist/esm/browser.js'. Imported in 'src/layouts/ExpandableBottomLayout.tsx'. Build step 'yarn workspace @selfxyz/mobile-app web:build' failed.

packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx

[error] 15-15: Import error: 'ExpandableBottomLayout' is not exported by '@selfxyz/mobile-sdk-alpha/dist/esm/browser.js'. Imported in 'src/layouts/ExpandableBottomLayout.tsx'. Build step 'yarn workspace @selfxyz/mobile-app web:build' failed.

⏰ 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: build-android
  • GitHub Check: test
  • GitHub Check: build-ios
  • GitHub Check: ios-e2e
  • GitHub Check: e2e-ios
🔇 Additional comments (14)
packages/mobile-sdk-alpha/src/components/layout/View.tsx (1)

201-201: LGTM! Clean implementation.

The maxWidth prop addition follows the exact same pattern as the existing width prop, maintaining consistency with the component's API. Type safety is preserved through the ViewStyle extension in ViewProps.

Also applies to: 230-230

packages/mobile-sdk-demo/metro.config.cjs (1)

70-74: LGTM!

SVG support is correctly configured by removing 'svg' from asset extensions and adding it to source extensions, aligning with the svg-transformer configuration.

app/src/components/loading/LoadingUI.tsx (1)

10-10: LGTM!

Import change correctly moves DelayedLottieView to the SDK package, consistent with the broader SDK migration effort.

app/metro.config.cjs (1)

69-71: LGTM!

The blockList entry correctly prevents duplicate lottie-react-native module instances by blocking the SDK's node_modules copy, ensuring the app uses a single instance from its own node_modules. This aligns with the existing pattern for React and React Native dependencies.

app/src/screens/documents/scanning/DocumentCameraScreen.tsx (1)

10-14: LGTM!

Import change correctly moves DelayedLottieView to the SDK package.

app/src/screens/app/SplashScreen.tsx (1)

10-14: Import change is valid and properly supported by the SDK.

The export verification confirms that DelayedLottieView is correctly exported from @selfxyz/mobile-sdk-alpha at the package root (index.ts), and the component implementation uses the standard forwardRef pattern. No issues found.

packages/mobile-sdk-demo/package.json (1)

41-42: Referenced file not found: packages/mobile-sdk-demo/package.json does not exist in this repo.

Likely an incorrect or invalid review comment.

packages/mobile-sdk-alpha/src/components/MRZScannerView.tsx (2)

114-115: Verify the iOS dimension increase from 100% to 130%.

The width and height have been increased from 100% to 130%. This may cause visual overflow or unintended clipping of the scanner view. Please confirm this change was intentional and tested on iOS devices.


131-131: Verify the Android width reduction from 800 to 400 layout pixels.

The width has been halved from 800 to 400 layout pixels. This is a significant change that could affect the scanner's field of view and accuracy. Please confirm this was intentional and tested on Android devices.

Based on learnings

app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (1)

10-10: LGTM! Import migration to SDK package.

The DelayedLottieView import has been successfully migrated from local to the SDK package, aligning with the broader consolidation effort.

app/src/screens/onboarding/DisclaimerScreen.tsx (1)

11-11: LGTM! Import migration to SDK package.

Consistent with the SDK consolidation effort across the codebase.

packages/mobile-sdk-alpha/src/index.ts (1)

53-61: LGTM! New SDK exports added correctly.

The ExpandableBottomLayout and DelayedLottieView exports follow the SDK's public API conventions. However, note that the web build failure for ExpandableBottomLayout needs to be resolved (see review comment on ExpandableBottomLayout.tsx).

As per coding guidelines

app/src/providers/selfClientProvider.tsx (1)

9-18: LGTM! Import consolidation improves code organization.

The import reorganization consolidates types and values from the SDK package, with proper type-only annotations. No functional changes.

app/src/screens/documents/selection/ConfirmBelongingScreen.tsx (1)

10-14: LGTM! Import consolidation aligns with SDK migration.

Consistent with the broader effort to consolidate SDK imports across the app.

Comment on lines +24 to +48
export const DelayedLottieView = forwardRef<LottieView, LottieViewProps>((props, forwardedRef) => {
// If LottieView is undefined (peer dependency not installed), return null
if (typeof LottieView === 'undefined') {
return null;
}

const internalRef = useRef<LottieView>(null);
const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef;

useEffect(() => {
// Only auto-trigger for autoPlay animations
if (props.autoPlay) {
const timer = setTimeout(() => {
ref.current?.play();
}, 100);

return () => clearTimeout(timer);
}
}, [props.autoPlay, ref]);

// For autoPlay animations, disable native autoPlay and control it ourselves
const modifiedProps = props.autoPlay ? { ...props, autoPlay: false } : props;

return <LottieView ref={ref} {...modifiedProps} />;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Fix React hooks rule violation.

The early return on line 27 precedes the useRef (line 30) and useEffect (line 33) hooks, violating the Rules of Hooks. This can cause unpredictable behavior and bugs when the peer dependency state changes between renders.

Restructure to move the guard outside the component or after all hooks:

 export const DelayedLottieView = forwardRef<LottieView, LottieViewProps>((props, forwardedRef) => {
-  // If LottieView is undefined (peer dependency not installed), return null
-  if (typeof LottieView === 'undefined') {
-    return null;
-  }
-
   const internalRef = useRef<LottieView>(null);
   const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef;
 
   useEffect(() => {
     // Only auto-trigger for autoPlay animations
     if (props.autoPlay) {
       const timer = setTimeout(() => {
         ref.current?.play();
       }, 100);
 
       return () => clearTimeout(timer);
     }
   }, [props.autoPlay, ref]);
 
   // For autoPlay animations, disable native autoPlay and control it ourselves
   const modifiedProps = props.autoPlay ? { ...props, autoPlay: false } : props;
 
+  // If LottieView is undefined (peer dependency not installed), return null
+  if (typeof LottieView === 'undefined') {
+    return null;
+  }
+
   return <LottieView ref={ref} {...modifiedProps} />;
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const DelayedLottieView = forwardRef<LottieView, LottieViewProps>((props, forwardedRef) => {
// If LottieView is undefined (peer dependency not installed), return null
if (typeof LottieView === 'undefined') {
return null;
}
const internalRef = useRef<LottieView>(null);
const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef;
useEffect(() => {
// Only auto-trigger for autoPlay animations
if (props.autoPlay) {
const timer = setTimeout(() => {
ref.current?.play();
}, 100);
return () => clearTimeout(timer);
}
}, [props.autoPlay, ref]);
// For autoPlay animations, disable native autoPlay and control it ourselves
const modifiedProps = props.autoPlay ? { ...props, autoPlay: false } : props;
return <LottieView ref={ref} {...modifiedProps} />;
});
export const DelayedLottieView = forwardRef<LottieView, LottieViewProps>((props, forwardedRef) => {
const internalRef = useRef<LottieView>(null);
const ref = (forwardedRef as React.RefObject<LottieView>) || internalRef;
useEffect(() => {
// Only auto-trigger for autoPlay animations
if (props.autoPlay) {
const timer = setTimeout(() => {
ref.current?.play();
}, 100);
return () => clearTimeout(timer);
}
}, [props.autoPlay, ref]);
// For autoPlay animations, disable native autoPlay and control it ourselves
const modifiedProps = props.autoPlay ? { ...props, autoPlay: false } : props;
// If LottieView is undefined (peer dependency not installed), return null
if (typeof LottieView === 'undefined') {
return null;
}
return <LottieView ref={ref} {...modifiedProps} />;
});
🧰 Tools
🪛 Biome (2.1.2)

[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

Hooks should not be called after an early return.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)


[error] 33-33: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

Hooks should not be called after an early return.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

Comment on lines +34 to +41
const handleMRZDetected = useCallback(
(mrzData: MRZInfo) => {
onPassportRead(null, mrzData);

onSuccess?.();
},
[onPassportRead],
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Include onSuccess in the memoized callback.

handleMRZDetected captures onSuccess but doesn’t list it in the dependency array, so prop changes won’t propagate. Add onSuccess to the deps (or drop useCallback) to avoid stale handlers.

🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/flows/onboarding/document-camera-screen.tsx
around lines 34 to 41, the handleMRZDetected callback captures onSuccess but
only lists onPassportRead in its dependency array causing stale closures; update
the useCallback dependency array to include onSuccess (or remove useCallback) so
changes to the onSuccess prop are respected by the memoized handler.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/mobile-sdk-alpha/package.json (1)

154-154: Consider relaxing the lottie-react-native peer dependency constraint.

Using an exact version (7.2.2) in peerDependencies is very strict and could cause installation conflicts if consumers need a slightly different version (e.g., 7.2.1 or 7.3.0).

Consider using a caret or tilde range instead:

-    "lottie-react-native": "7.2.2",
+    "lottie-react-native": "^7.2.2",

This allows patch and minor updates while maintaining compatibility.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52faaf8 and dfdb9dc.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (4)
  • packages/mobile-sdk-alpha/package.json (1 hunks)
  • packages/mobile-sdk-alpha/src/browser.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx (1 hunks)
  • packages/mobile-sdk-alpha/tests/setup.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/tests/setup.ts
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/tests/setup.ts
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/browser.ts
  • packages/mobile-sdk-alpha/tests/setup.ts
  • packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx
packages/mobile-sdk-alpha/tests/setup.ts

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise

Files:

  • packages/mobile-sdk-alpha/tests/setup.ts
packages/mobile-sdk-alpha/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Files:

  • packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)

Files:

  • packages/mobile-sdk-alpha/package.json
**/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Use Yarn only for package management (yarn install/add/remove); do not use npm or pnpm in scripts

Files:

  • packages/mobile-sdk-alpha/package.json
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-alpha/src/browser.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/jest.setup.js : Provide comprehensive Jest setup in app/jest.setup.js with required mocks

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-09-22T11:10:22.019Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-09-22T11:10:22.019Z
Learning: Applies to jest.setup.js : Comprehensive mocks for all native modules must be set up in `jest.setup.js`.

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/tests/setup.ts : Provide Vitest setup file at packages/mobile-sdk-alpha/tests/setup.ts to suppress console noise

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Write integration tests that exercise the real validation logic (not mocks)

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors

Applied to files:

  • packages/mobile-sdk-alpha/tests/setup.ts
🧬 Code graph analysis (1)
packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx (1)
packages/mobile-sdk-alpha/src/browser.ts (1)
  • DelayedLottieView (45-45)
⏰ 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: build-android
  • GitHub Check: build-ios
  • GitHub Check: test
  • GitHub Check: e2e-ios
  • GitHub Check: ios-e2e
🔇 Additional comments (6)
packages/mobile-sdk-alpha/src/browser.ts (1)

37-48: LGTM! Clean API surface expansion.

The new exports follow TypeScript best practices with explicit type keywords for type-only exports, and the component exports are properly structured. The .web suffix for DelayedLottieView is appropriate for browser builds.

packages/mobile-sdk-alpha/package.json (1)

140-140: Verify lottie-react-native compatibility with RN 0.76.9.

The SDK pins lottie-react-native to 7.2.2. According to the library's documentation, there can be compatibility quirks with RN 0.71+. Since this project uses RN 0.76.9 (line 144), ensure you've tested lottie functionality across iOS and Android in the demo app.

Based on learnings

packages/mobile-sdk-alpha/tests/setup.ts (4)

70-75: LGTM! Deterministic PixelRatio mock.

The PixelRatio mock provides deterministic values that are appropriate for test stability. The implementation correctly covers all common PixelRatio methods.


202-227: LGTM! Comprehensive SVG component coverage.

The react-native-svg mock covers all common SVG elements needed for testing. Mapping components to their corresponding HTML element names is the right approach for test environments.


234-237: LGTM! Simple and appropriate Lottie mock.

Mocking lottie-react-native as a 'div' is appropriate for tests where animation behavior isn't being tested. This keeps tests fast and deterministic.


239-253: LGTM! Complete haptic feedback mock.

The mock correctly implements both the trigger function and HapticFeedbackTypes enum, covering the full API surface needed for testing haptic interactions.

Comment on lines +8 to +10
export const DelayedLottieView = () => {
return <div />;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add React import and TypeScript types.

The component uses JSX syntax but doesn't import React. While newer React versions may not strictly require this with JSX transform, it's best practice to include it for clarity. Additionally, the function lacks TypeScript type annotations.

Apply this diff:

+import React from 'react';
+
 /**
  * DelayedLottieView for web placeholder component.
  */
-export const DelayedLottieView = () => {
+export const DelayedLottieView: React.FC = () => {
   return <div />;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const DelayedLottieView = () => {
return <div />;
};
import React from 'react';
/**
* DelayedLottieView for web placeholder component.
*/
export const DelayedLottieView: React.FC = () => {
return <div />;
};
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/components/DelayedLottieView.web.tsx around
lines 8 to 10, add an explicit React import and TypeScript annotations: import
React from 'react'; annotate the component signature with React types (for
example export const DelayedLottieView: React.FC = () => { ... } or export const
DelayedLottieView = (): JSX.Element => { ... }) and ensure the return type is
JSX.Element; keep the existing returned <div /> and update the file to use these
imports and type annotations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (3)

39-45: Layout component accepts but doesn't use safeAreaTop/safeAreaBottom props.

The LayoutProps interface includes safeAreaTop and safeAreaBottom, but the Layout component doesn't destructure or apply them. While child sections handle their own safe areas, accepting unused props can confuse consumers about where safe area handling occurs.

Consider either:

  1. Remove safeAreaTop and safeAreaBottom from LayoutProps if Layout shouldn't handle them
  2. Or add a comment explaining that Layout passes these to children via context/composition pattern

47-65: Type definition appears after usage.

TopSectionProps is used here but defined at lines 134-139. While TypeScript's interface hoisting makes this work, it reduces readability.

Move the TopSectionProps interface definition to before line 47, grouping it with the other prop type definitions (lines 21-37).


114-119: Magic numbers reduce maintainability.

The values 0.38 (38% of screen height) and 1.3 (font scale threshold) are hardcoded without explanation.

Extract to named constants at the file level for better maintainability:

+const BOTTOM_SECTION_HEIGHT_PERCENTAGE = 0.38;
+const LARGE_TEXT_FONT_SCALE_THRESHOLD = 1.3;
+
 const SAFE_AREA_TOP_DEFAULT = 0;
 const SAFE_AREA_BOTTOM_DEFAULT = 0;

Then use them:

-  const isLargerTextEnabled = fontScale > 1.3;
+  const isLargerTextEnabled = fontScale > LARGE_TEXT_FONT_SCALE_THRESHOLD;
...
-    panelHeight = windowHeight * 0.38;
+    panelHeight = windowHeight * BOTTOM_SECTION_HEIGHT_PERCENTAGE;
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfdb9dc and 20e1a03.

📒 Files selected for processing (1)
  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/mobile-sdk-alpha/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies

Files:

  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:

  • API consistency with core SDK
  • Platform-neutral abstractions
  • Performance considerations
  • Clear experimental notes or TODOs

Files:

  • packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Update the app to consume the mobile-sdk-alpha package and replace local modules with SDK imports
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
🧬 Code graph analysis (1)
packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx (3)
packages/mobile-sdk-alpha/src/components/layout/View.tsx (2)
  • ViewProps (49-53)
  • View (180-308)
packages/mobile-sdk-alpha/src/constants/layout.ts (1)
  • extraYPadding (5-5)
packages/mobile-sdk-alpha/src/constants/colors.ts (2)
  • black (8-8)
  • white (59-59)
⏰ 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). (7)
  • GitHub Check: build-deps
  • GitHub Check: android-build-test
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-ios
  • GitHub Check: ios-e2e
  • GitHub Check: android-e2e
  • GitHub Check: analyze-android

Comment on lines +16 to +19
// Get the current font scale factor
const fontScale = PixelRatio.getFontScale();
// fontScale > 1 means the user has increased text size in accessibility settings
const isLargerTextEnabled = fontScale > 1.3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Font scale is calculated once at module load, breaking runtime accessibility updates.

The fontScale value is computed when the module loads and stored in a const, so it won't update if the user changes text size settings during app usage. Users with accessibility needs would have to restart the app for changes to take effect.

Move the font scale check inside the BottomSection component:

-// Get the current font scale factor
-const fontScale = PixelRatio.getFontScale();
-// fontScale > 1 means the user has increased text size in accessibility settings
-const isLargerTextEnabled = fontScale > 1.3;
-
 export interface BottomSectionProps extends ViewProps {
   children: React.ReactNode;
   backgroundColor: string;
   safeAreaBottom?: number;
 }

Then inside BottomSection:

 const BottomSection: React.FC<BottomSectionProps> = ({ children, style, ...props }) => {
+  // Get the current font scale factor
+  const fontScale = PixelRatio.getFontScale();
+  // fontScale > 1 means the user has increased text size in accessibility settings
+  const isLargerTextEnabled = fontScale > 1.3;
+
   const incomingBottom = props.paddingBottom ?? 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get the current font scale factor
const fontScale = PixelRatio.getFontScale();
// fontScale > 1 means the user has increased text size in accessibility settings
const isLargerTextEnabled = fontScale > 1.3;
// Remove module‐level fontScale so it can be recalculated at render time:
//
//- // Get the current font scale factor
//- const fontScale = PixelRatio.getFontScale();
//- // fontScale > 1 means the user has increased text size in accessibility settings
//- const isLargerTextEnabled = fontScale > 1.3;
//
export interface BottomSectionProps extends ViewProps {
children: React.ReactNode;
backgroundColor: string;
safeAreaBottom?: number;
}
// …
const BottomSection: React.FC<BottomSectionProps> = ({ children, style, ...props }) => {
// Get the current font scale factor
const fontScale = PixelRatio.getFontScale();
// fontScale > 1 means the user has increased text size in accessibility settings
const isLargerTextEnabled = fontScale > 1.3;
const incomingBottom = props.paddingBottom ?? 0;
// …rest of implementation
};
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/layouts/ExpandableBottomLayout.tsx around lines
16 to 19, the fontScale is computed at module load which prevents runtime
updates when the user changes accessibility text size; move the
PixelRatio.getFontScale() call and the isLargerTextEnabled check into the
BottomSection component (compute fontScale inside the component render or via a
local state updated in useEffect) so the value is evaluated at runtime, and
update any places that reference the module-level constant to use the new
in-component variable.

@shazarre shazarre merged commit 757ac58 into dev Oct 16, 2025
30 checks passed
@shazarre shazarre deleted the shazarre/move_expandable_bottom_layout_to_the_sdk branch October 16, 2025 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants