refactor: reorganize and rename mobile screens #1207
Conversation
WalkthroughRefactors navigation structure: adds onboarding, documents, verification, account, and shared screen groups; removes legacy system/recovery/document modules; updates imports/aliases and vite chunks; trims home routes; adds AadhaarNavBar export; adjusts several import paths and settings. Updates navigation index to compose new groups. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App Entry
participant NavIndex as navigation/index
participant Groups as Screen Groups
participant Stack as Navigator
App->>NavIndex: import navigationScreens
NavIndex->>Groups: spread app/onboarding/home/documents/verification/account/shared/dev
Note over Groups: Each group exports a { route: { screen, options, ... } } map
NavIndex->>Stack: register composed routes
Stack-->>App: renders screens per route config
sequenceDiagram
autonumber
participant User as User
participant Nav as Navigator
participant Docs as documentsScreens
participant Screen as ScreenComponent
participant Header as Custom Header
User->>Nav: navigate('IdDetails')
Nav->>Docs: lookup route config
Docs-->>Nav: { screen: IdDetails, options: { header: IdDetailsNavBar, ... } }
Nav->>Screen: mount IdDetails
Nav->>Header: render IdDetailsNavBar (if provided)
alt modal route
User->>Nav: navigate('CountryPicker', { presentation: 'modal' })
Nav->>Screen: present modal screen
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/src/screens/account/settings/SettingsScreen.tsx (1)
43-43: Consider using the new package.json alias.While the relative path correction is accurate, the PR introduces a new tsconfig alias
@/package.json(as seen in app/tsconfig.json line 13). Using this alias would improve maintainability and eliminate the need to adjust relative paths if the file moves again.Apply this diff to use the alias:
-import { version } from '../../../../package.json'; +import { version } from '@/package.json';app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (1)
17-17: LGTM with consideration for style organization.Import path correctly updated to reflect the
verificationnamespace. However, importing styles from another screen component (ProofRequestStatusScreen) creates coupling. Consider extracting shared styles to a dedicated styles module if they're used across multiple screens.app/src/screens/documents/selection/ConfirmBelongingScreen.tsx (1)
24-24: LGTM with style organization consideration.Import path correctly updated. Note that this is the second screen importing styles from
ProofRequestStatusScreen. This pattern suggests these styles should be extracted to a shared styles module to reduce cross-screen coupling.app/src/navigation/account.ts (1)
75-77: Empty nested screens configuration.The
Settingsroute includes an emptyconfig.screensobject. If nested navigation is not currently needed, consider removing this config property to reduce noise.Apply this diff if nested screens are not planned:
} as NativeStackNavigationOptions, - config: { - screens: {}, - }, },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/src/components/NavBar/index.ts(1 hunks)app/src/components/homeScreen/idCard.tsx(1 hunks)app/src/hooks/useModal.ts(1 hunks)app/src/navigation/account.ts(1 hunks)app/src/navigation/account.web.ts(1 hunks)app/src/navigation/app.tsx(2 hunks)app/src/navigation/document.ts(0 hunks)app/src/navigation/documents.ts(1 hunks)app/src/navigation/home.ts(1 hunks)app/src/navigation/index.tsx(1 hunks)app/src/navigation/onboarding.ts(1 hunks)app/src/navigation/recovery.ts(0 hunks)app/src/navigation/settings.ts(0 hunks)app/src/navigation/settings.web.ts(0 hunks)app/src/navigation/shared.ts(1 hunks)app/src/navigation/verification.ts(2 hunks)app/src/screens/account/settings/SettingsScreen.tsx(1 hunks)app/src/screens/documents/selection/ConfirmBelongingScreen.tsx(1 hunks)app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx(1 hunks)app/tsconfig.json(1 hunks)app/vite.config.ts(3 hunks)
💤 Files with no reviewable changes (4)
- app/src/navigation/settings.ts
- app/src/navigation/recovery.ts
- app/src/navigation/document.ts
- app/src/navigation/settings.web.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/navigation/app.tsxapp/src/navigation/shared.tsapp/src/components/NavBar/index.tsapp/src/navigation/documents.tsapp/src/components/homeScreen/idCard.tsxapp/vite.config.tsapp/src/navigation/onboarding.tsapp/src/navigation/index.tsxapp/src/navigation/account.web.tsapp/src/navigation/home.tsapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/documents/selection/ConfirmBelongingScreen.tsxapp/src/screens/account/settings/SettingsScreen.tsxapp/src/navigation/verification.tsapp/src/hooks/useModal.tsapp/src/navigation/account.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/navigation/app.tsxapp/src/navigation/shared.tsapp/src/components/NavBar/index.tsapp/src/navigation/documents.tsapp/src/components/homeScreen/idCard.tsxapp/vite.config.tsapp/src/navigation/onboarding.tsapp/src/navigation/index.tsxapp/src/navigation/account.web.tsapp/src/navigation/home.tsapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/documents/selection/ConfirmBelongingScreen.tsxapp/src/screens/account/settings/SettingsScreen.tsxapp/src/navigation/verification.tsapp/src/hooks/useModal.tsapp/src/navigation/account.ts
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/navigation/app.tsxapp/src/navigation/shared.tsapp/src/components/NavBar/index.tsapp/src/navigation/documents.tsapp/src/components/homeScreen/idCard.tsxapp/src/navigation/onboarding.tsapp/src/navigation/index.tsxapp/src/navigation/account.web.tsapp/src/navigation/home.tsapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/documents/selection/ConfirmBelongingScreen.tsxapp/src/screens/account/settings/SettingsScreen.tsxapp/src/navigation/verification.tsapp/src/hooks/useModal.tsapp/src/navigation/account.ts
app/**/*.{ios,android,web}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Files:
app/src/navigation/account.web.ts
🧠 Learnings (3)
📚 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 src/screens/**/*.{js,ts,tsx,jsx} : Lazy load screens using `React.lazy()`, organize screens by feature modules.
Applied to files:
app/vite.config.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/**/*.{ts,tsx} : Avoid introducing circular dependencies
Applied to files:
app/tsconfig.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/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests
Applied to files:
app/tsconfig.json
🧬 Code graph analysis (4)
app/src/navigation/documents.ts (1)
app/src/utils/colors.ts (2)
white(59-59)black(8-8)
app/vite.config.ts (3)
common/scripts/postBuild.mjs (1)
__dirname(7-7)app/scripts/find-type-import-issues.mjs (1)
__dirname(20-20)common/scripts/testExports.js (1)
__dirname(13-13)
app/src/navigation/account.web.ts (1)
app/src/utils/colors.ts (2)
white(59-59)black(8-8)
app/src/navigation/account.ts (1)
app/src/utils/colors.ts (3)
black(8-8)slate300(37-37)white(59-59)
🪛 GitHub Actions: Web CI
app/src/navigation/account.ts
[error] 1-1: Cannot assign "src/navigation/account.ts" to the "screens-recovery" chunk as it is already in the "screens-settings" chunk.
⏰ 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). (4)
- GitHub Check: analyze-android
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
🔇 Additional comments (17)
app/src/components/homeScreen/idCard.tsx (1)
5-5: LGTM!Adding
Reactto the import statement ensures JSX transforms work correctly across different build configurations. The combined import properly maintains the type-only import forFCwhile adding the runtime import for React.app/src/components/NavBar/index.ts (1)
5-5: LGTM!Adding
AadhaarNavBarto the barrel exports aligns with the navigation restructuring. This centralizes the NavBar component access.app/src/navigation/verification.ts (2)
7-10: LGTM!Import path updates correctly reflect the screen reorganization from
provetoverificationnamespace.
13-50: LGTM!The rename from
proveScreenstoverificationScreensimproves clarity and aligns with the new directory structure. Navigation configuration remains intact.app/src/navigation/account.web.ts (2)
32-34: Verify intentional empty screens configuration.The
Settingsroute includes an emptyscreensconfiguration object. If this is a placeholder for future nested navigation, consider adding a comment explaining the intent. Otherwise, this config object may be unnecessary.
1-38: Platform-specific code path detected.This is a web-specific navigation module (
.web.tsextension). Per coding guidelines, platform-specific code paths should be explained in the PR description. Ensure the PR description documents why the web platform has different account screens (includingDocumentDataNotFoundwhich was removed from shared navigation).As per coding guidelines.
app/tsconfig.json (1)
13-13: LGTM!Adding the
@/package.jsonalias improves import maintainability. Consider updating existing relative imports to package.json (like in SettingsScreen.tsx line 43) to use this new alias for consistency.app/src/navigation/app.tsx (1)
11-15: LGTM!The refactor correctly updates import paths from system screens to app screens and renames the constant accordingly. The changes align with the directory reorganization objectives.
Also applies to: 17-17, 59-59
app/src/navigation/onboarding.ts (1)
1-35: LGTM!The onboarding navigation module follows the established pattern and correctly configures the three onboarding screens with appropriate navigation options.
app/vite.config.ts (2)
35-35: LGTM!The new alias for package.json aligns with the tsconfig.json path mapping and enables proper module resolution.
177-177: LGTM!The chunk mappings correctly reflect the renamed navigation modules:
documents.tsandverification.ts.Also applies to: 181-181
app/src/navigation/account.ts (1)
1-90: Navigation configuration is well-structured.The account navigation module correctly defines routes with appropriate header styling, animations, and navigation options. The use of color constants promotes consistency across the app.
app/src/navigation/documents.ts (2)
1-155: Comprehensive document navigation module.The documents navigation configuration is well-structured with appropriate use of modals, animations, custom nav bars, and initialParams. The organization by document workflow stages (scanning, selection, management, aadhaar) is clear.
7-7: Import verified: AadhaarNavBar is exported from NavBar; no action required.app/src/navigation/index.tsx (1)
19-27: LGTM!The navigation composition correctly aggregates the new feature-oriented screen groups, replacing the previous system-based groupings. The imports and spread operator usage properly integrate all navigation modules.
Also applies to: 30-39
app/src/navigation/home.ts (1)
1-36: LGTM!The home navigation module is appropriately simplified, retaining only the core home screens. The removed screens (Disclaimer, IdDetails, Aadhaar) have been relocated to their respective feature modules (onboarding, documents), which aligns with the PR's reorganization objectives.
app/src/hooks/useModal.ts (1)
8-8: LGTM: import path and export validatedThe import path aligns with the refactor and
ModalParamsis exported fromapp/src/screens/app/ModalScreen.tsx. No runtime impact.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
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 (5)
app/package.json(1 hunks)app/src/navigation/account.ts(2 hunks)app/src/navigation/account.web.ts(1 hunks)app/src/providers/selfClientProvider.tsx(1 hunks)app/tests/src/navigation.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/navigation/account.web.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/tests/src/navigation.test.tsapp/src/providers/selfClientProvider.tsxapp/src/navigation/account.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/tests/src/navigation.test.tsapp/src/providers/selfClientProvider.tsxapp/src/navigation/account.ts
**/*.{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:
app/tests/src/navigation.test.ts
app/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests
Files:
app/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:
app/package.json
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/providers/selfClientProvider.tsxapp/src/navigation/account.ts
🧬 Code graph analysis (2)
app/src/providers/selfClientProvider.tsx (1)
app/src/navigation/index.tsx (1)
navigationRef(53-53)
app/src/navigation/account.ts (1)
app/src/utils/colors.ts (2)
black(8-8)slate300(37-37)
⏰ 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). (4)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (2)
app/tests/src/navigation.test.ts (1)
9-55: LGTM!The test correctly reflects the removal of
DocumentDataNotFoundfrom the navigation surface, and the comment on line 31 provides helpful context for future maintainers.app/src/navigation/account.ts (1)
1-83: LGTM! Well-structured navigation refactor.The reorganization into
accountScreensconsolidates account and settings navigation logically. The new recovery flow screens (AccountRecovery,AccountRecoveryChoice,RecoverWithPhrase) are properly configured with appropriate header styling, and the removal ofDocumentDataNotFoundaligns with the broader navigation changes across the codebase.
transphorm
changed the title
transphorm
force-pushed
the
codex/create-prd-for-screen-reorganization-i1rtsh
branch
from
b084723 to
8b892d2
Compare
|
need to fix the merge conflicts |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/navigation/account.ts (1)
61-76: Settings configuration is duplicated across platform files.The
Settingsroute configuration is identical in bothaccount.tsandaccount.web.ts(lines 61-76 here, lines 10-27 in web file). This duplication creates maintenance burden.Consider extracting the shared Settings configuration to a common module:
Create
app/src/navigation/account.shared.ts:import type { NativeStackNavigationOptions } from '@react-navigation/native-stack'; import SettingsScreen from '@/screens/account/settings/SettingsScreen'; import { black, white } from '@/utils/colors'; export const settingsRoute = { Settings: { screen: SettingsScreen, options: { animation: 'slide_from_bottom', title: 'Settings', headerStyle: { backgroundColor: white }, headerTitleStyle: { color: black }, } satisfies NativeStackNavigationOptions, }, };Then import and use in both
account.tsandaccount.web.ts:import { settingsRoute } from './account.shared'; const accountScreens = { ...settingsRoute, // other routes... };
♻️ Duplicate comments (1)
app/src/navigation/account.ts (1)
14-14: Verify color import path aligns with available exports.Same concern as in
account.web.ts: the color imports reference@/utils/colors, but the relevant snippets show these colors exported frompackages/mobile-sdk-alpha/src/constants/colors.ts. Verify the import path is correct given the type failures mentioned in PR objectives.
🧹 Nitpick comments (3)
app/src/navigation/account.web.ts (2)
13-22: Remove type assertion to enable TypeScript validation.The
as NativeStackNavigationOptionsassertion bypasses TypeScript's type checking. This can hide mismatches between the options object and the expected type.Remove the type assertion and let TypeScript validate the options naturally:
options: { animation: 'slide_from_bottom', title: 'Settings', headerStyle: { backgroundColor: white, }, headerTitleStyle: { color: black, }, - } as NativeStackNavigationOptions, + } satisfies NativeStackNavigationOptions,Note: Using
satisfies(TypeScript 4.9+) validates the type while preserving the literal types of the object.
23-25: Empty config object may be unnecessary.The
config: { screens: {} }object appears to be empty boilerplate. Unless required by react-navigation's API, consider removing it to reduce noise.- config: { - screens: {}, - },app/src/navigation/account.ts (1)
16-86: Replace type assertions withsatisfiesoperator.All route configurations use
as NativeStackNavigationOptionsassertions (lines 21, 27, 41, 47, 59, 72, 84), which bypass TypeScript validation. Use thesatisfiesoperator instead to enable type checking while preserving literal types.Apply this pattern to all routes:
AccountRecovery: { screen: AccountRecoveryScreen, options: { headerShown: false, - } as NativeStackNavigationOptions, + } satisfies NativeStackNavigationOptions, },Repeat for all routes: AccountRecoveryChoice, RecoverWithPhrase, DocumentDataNotFound, CloudBackupSettings, Settings, and ShowRecoveryPhrase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/navigation/account.ts(2 hunks)app/src/navigation/account.web.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/navigation/account.tsapp/src/navigation/account.web.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/navigation/account.tsapp/src/navigation/account.web.ts
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/navigation/account.tsapp/src/navigation/account.web.ts
app/**/*.{ios,android,web}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Explain platform-specific code paths in the PR description (files with .ios, .android, or .web extensions)
Files:
app/src/navigation/account.web.ts
🧬 Code graph analysis (2)
app/src/navigation/account.ts (1)
packages/mobile-sdk-alpha/src/constants/colors.ts (2)
black(8-8)slate300(37-37)
app/src/navigation/account.web.ts (1)
packages/mobile-sdk-alpha/src/constants/colors.ts (2)
white(59-59)black(8-8)
⏰ 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). (2)
- GitHub Check: build-ios
- GitHub Check: e2e-ios
🔇 Additional comments (3)
app/src/navigation/account.ts (2)
29-42: Verify headerTintColor is necessary with custom headerStyle.The
RecoverWithPhrasescreen setsheaderTintColor: black(line 32) while also usingheaderStyle: { backgroundColor: black }(lines 34-36). The tint color controls back button and icon colors, which may not be visible against a black background. Consider if this should beslate300to match the title color for visibility.
88-88: LGTM!The rename from
settingsScreenstoaccountScreensbetter reflects the broader scope of account-related screens (recovery, backup, settings) now included in this navigation group.app/src/navigation/account.web.ts (1)
8-8: No changes needed for color imports.app/src/utils/colors.tscorrectly re-exportsblackandwhitefrom@selfxyz/mobile-sdk-alpha/constants/colors.
transphorm
deleted the
codex/create-prd-for-screen-reorganization-i1rtsh
branch
Summary
Testing
https://chatgpt.com/codex/tasks/task_b_68e0b6b8684c832dadb0cb435b51af46
Summary by CodeRabbit
New Features
Refactor