simplify mobile demo app screens #1183
Conversation
WalkthroughAdds reusable UI components, two hooks for document and registration flows, utilities for document formatting and catalog mutation, refactors multiple screens to use the new components/hooks, and adds unit tests for catalog and document utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as RegisterDocument
participant Hook as useRegistration
participant Client as SelfClient
participant Store as ProvingStore
participant UI as LogsPanel
User->>Screen: Tap "Start Registration"
Screen->>Hook: actions.start(documentId, document)
Hook->>Client: init(chosenCircuitType)
Client-->>Store: start proving state machine
Note over Hook,Store: Hook subscribes to SdkEvents.PROOF_EVENT
Store-->>Hook: PROOF_EVENT updates
Hook->>Hook: addLog(), update statusMessage/currentState
alt ready_to_prove (auto/confirm)
Hook->>Client: setUserConfirmed(documentId)
end
Hook-->>Screen: state {registering,statusMessage,currentState,logs,showLogs}
Screen->>UI: render logs + toggle
UI->>Screen: onToggle()
sequenceDiagram
autonumber
participant Screen as DocumentsList
participant Hook as useDocuments
participant Client as SelfClient
participant Lib as catalog.ts
Screen->>Hook: mount -> refresh()
Hook->>Client: getAllDocuments()
Client-->>Hook: documents[]
Hook-->>Screen: {documents,loading,error}
Screen->>Hook: deleteDocument(id)
Hook->>Client: deleteDocument(id)
Hook->>Client: loadDocumentCatalog()
Hook->>Lib: updateAfterDelete(catalog, id)
Hook->>Client: saveDocumentCatalog(updated)
Hook->>Hook: refresh()
Hook-->>Screen: updated state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,ts,tsx,jsx,sol,nr}📄 CodeRabbit inference engine (.cursorrules)
Files:
🧬 Code graph analysis (1)packages/mobile-sdk-demo/src/hooks/useRegistration.ts (1)
🔇 Additional comments (7)
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 (5)
packages/mobile-sdk-demo/src/components/MenuButton.tsx (2)
18-27: Consider adding accessibility props.The TouchableOpacity lacks accessibility labels and hints. For better screen reader support, add
accessibilityRole="button"andaccessibilityLabelprops.Apply this diff:
<TouchableOpacity style={[ styles.menuButton, isWorking ? styles.workingButton : styles.placeholderButton, disabled && styles.disabledButton, ]} onPress={onPress} activeOpacity={0.7} disabled={disabled} + accessibilityRole="button" + accessibilityLabel={title} + accessibilityState={{ disabled }} >
71-80: Duplicate backgroundColor and border styles.The
workingButtonandplaceholderButtonstyles are identical. Consider extracting to a shared base style to reduce duplication.Apply this diff:
+ baseButton: { + backgroundColor: '#ffffff', + borderWidth: 1, + borderColor: '#d1d9e0', + }, workingButton: { - backgroundColor: '#ffffff', - borderWidth: 1, - borderColor: '#d1d9e0', }, placeholderButton: { - backgroundColor: '#ffffff', - borderWidth: 1, - borderColor: '#d1d9e0', },Then update the style array on Line 19-23 to include
styles.baseButton.packages/mobile-sdk-demo/tests/lib/catalog.test.ts (1)
25-44: Consider adding edge case tests.The current tests cover primary scenarios well. Consider adding tests for:
- Empty catalog (no documents)
- Deleting a non-existent document ID
- Catalog with undefined
documentsarrayExample additional test cases:
it('handles empty catalog gracefully', () => { const catalog = makeCatalog([], undefined); const result = updateAfterDelete(catalog, 'nonexistent'); expect(result.documents).toEqual([]); expect(result.selectedDocumentId).toBeUndefined(); }); it('handles deleting non-existent document', () => { const catalog = makeCatalog(['a', 'b'], 'a'); const result = updateAfterDelete(catalog, 'z'); expect(result.documents.map(d => d.id)).toEqual(['a', 'b']); expect(result.selectedDocumentId).toBe('a'); });packages/mobile-sdk-demo/src/components/LogsPanel.tsx (2)
18-20: Add accessibility props to toggle.The toggle button should have accessibility labels for screen reader support.
Apply this diff:
<TouchableOpacity onPress={onToggle} style={styles.toggle} + accessibilityRole="button" + accessibilityLabel={show ? `Hide ${logs.length} log entries` : `Show ${logs.length} log entries`} >
22-28: Consider virtualization for large log lists.With
maxHeight: 200and unbounded log arrays, rendering many logs could impact performance. Consider usingFlatListinstead ofScrollViewwith.map()for better performance with large log sets.Example refactor:
{show && ( <FlatList data={logs} style={styles.logs} nestedScrollEnabled renderItem={({ item, index }) => ( <Text key={index} style={styles.entry}> {item} </Text> )} keyExtractor={(_, idx) => idx.toString()} /> )}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/mobile-sdk-demo/src/components/LogsPanel.tsx(1 hunks)packages/mobile-sdk-demo/src/components/MenuButton.tsx(1 hunks)packages/mobile-sdk-demo/src/components/PlaceholderScreen.tsx(1 hunks)packages/mobile-sdk-demo/src/components/ScreenLayout.tsx(1 hunks)packages/mobile-sdk-demo/src/hooks/useDocuments.ts(1 hunks)packages/mobile-sdk-demo/src/hooks/useRegistration.ts(1 hunks)packages/mobile-sdk-demo/src/lib/catalog.ts(1 hunks)packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx(1 hunks)packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx(1 hunks)packages/mobile-sdk-demo/src/screens/DocumentsList.tsx(4 hunks)packages/mobile-sdk-demo/src/screens/HomeScreen.tsx(3 hunks)packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx(1 hunks)packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx(5 hunks)packages/mobile-sdk-demo/src/utils/document.ts(1 hunks)packages/mobile-sdk-demo/tests/lib/catalog.test.ts(1 hunks)packages/mobile-sdk-demo/tests/utils/document.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
packages/mobile-sdk-demo/src/components/LogsPanel.tsxpackages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsxpackages/mobile-sdk-demo/src/lib/catalog.tspackages/mobile-sdk-demo/src/hooks/useDocuments.tspackages/mobile-sdk-demo/src/components/PlaceholderScreen.tsxpackages/mobile-sdk-demo/src/components/MenuButton.tsxpackages/mobile-sdk-demo/src/components/ScreenLayout.tsxpackages/mobile-sdk-demo/src/utils/document.tspackages/mobile-sdk-demo/src/hooks/useRegistration.tspackages/mobile-sdk-demo/src/screens/RegisterDocument.tsxpackages/mobile-sdk-demo/tests/lib/catalog.test.tspackages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsxpackages/mobile-sdk-demo/src/screens/DocumentCamera.tsxpackages/mobile-sdk-demo/src/screens/DocumentsList.tsxpackages/mobile-sdk-demo/src/screens/HomeScreen.tsxpackages/mobile-sdk-demo/tests/utils/document.test.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:
packages/mobile-sdk-demo/tests/lib/catalog.test.tspackages/mobile-sdk-demo/tests/utils/document.test.ts
🧠 Learnings (2)
📚 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}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-demo/tests/utils/document.test.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}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-demo/tests/utils/document.test.ts
🧬 Code graph analysis (11)
packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (1)
packages/mobile-sdk-demo/src/components/PlaceholderScreen.tsx (1)
PlaceholderScreen(17-34)
packages/mobile-sdk-demo/src/hooks/useDocuments.ts (1)
packages/mobile-sdk-demo/src/lib/catalog.ts (1)
updateAfterDelete(7-22)
packages/mobile-sdk-demo/src/components/PlaceholderScreen.tsx (1)
packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (1)
ScreenLayout(18-25)
packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (2)
packages/mobile-sdk-demo/src/components/SafeAreaScrollView.tsx (1)
SafeAreaScrollView(13-27)packages/mobile-sdk-demo/src/components/StandardHeader.tsx (1)
StandardHeader(14-24)
packages/mobile-sdk-demo/src/hooks/useRegistration.ts (1)
packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
useProvingStore(367-1417)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (3)
packages/mobile-sdk-demo/src/hooks/useRegistration.ts (1)
useRegistration(18-124)packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (1)
ScreenLayout(18-25)packages/mobile-sdk-demo/src/components/LogsPanel.tsx (1)
LogsPanel(14-32)
packages/mobile-sdk-demo/tests/lib/catalog.test.ts (1)
packages/mobile-sdk-demo/src/lib/catalog.ts (2)
updateAfterDelete(7-22)selectDocument(24-31)
packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (1)
packages/mobile-sdk-demo/src/components/PlaceholderScreen.tsx (1)
PlaceholderScreen(17-34)
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)
packages/mobile-sdk-demo/src/components/PlaceholderScreen.tsx (1)
PlaceholderScreen(17-34)
packages/mobile-sdk-demo/src/screens/DocumentsList.tsx (3)
packages/mobile-sdk-demo/src/hooks/useDocuments.ts (1)
useDocuments(17-59)packages/mobile-sdk-demo/src/utils/document.ts (1)
maskId(33-40)packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (1)
ScreenLayout(18-25)
packages/mobile-sdk-demo/tests/utils/document.test.ts (1)
packages/mobile-sdk-demo/src/utils/document.ts (3)
humanizeDocumentType(7-16)formatDataPreview(18-31)maskId(33-40)
🔇 Additional comments (9)
packages/mobile-sdk-demo/src/screens/HomeScreen.tsx (2)
6-10: LGTM!Clean refactor extracting MenuButton to a reusable component. The import changes are appropriate and the component usage is correct.
39-46: LGTM!MenuButton usage correctly maps descriptor properties to component props. The conditional logic for
statusanddisabledis preserved from the original implementation.packages/mobile-sdk-demo/src/utils/document.ts (2)
33-40: LGTM!The
maskIdimplementation correctly handles short IDs and applies appropriate masking for longer IDs with prefix/suffix preservation.
18-31: Ignore PII warning for demo
formatDataPreview only renders mock document data in this demo package—no real user PII is involved.Likely an incorrect or invalid review comment.
packages/mobile-sdk-demo/src/screens/DocumentCamera.tsx (1)
15-25: LGTM!Clean refactor to use the shared PlaceholderScreen component. The props accurately describe the intended camera-based document scanning functionality.
packages/mobile-sdk-demo/tests/lib/catalog.test.ts (1)
48-59: LGTM!The
selectDocumenttests appropriately cover the two main branches: selecting an existing ID and attempting to select a non-existent ID.packages/mobile-sdk-demo/src/screens/DocumentNFCScan.tsx (1)
15-26: LGTM!Clean refactor to PlaceholderScreen. The feature list appropriately describes NFC passport reading capabilities with security protocols.
packages/mobile-sdk-demo/src/screens/QRCodeViewFinder.tsx (1)
15-26: LGTM!Consistent refactor to PlaceholderScreen. The features accurately describe QR code scanning use cases for proof verification and partner integration.
packages/mobile-sdk-demo/src/components/LogsPanel.tsx (1)
14-32: Sanitize all log entries before rendering. Confirm the source of eachlogstring passed to LogsPanel and mask or strip any PII (names, DOB, passport numbers), credentials, tokens, or sensitive identifiers at generation or pre-render.
There was a problem hiding this comment.
Actionable comments posted: 0
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-demo/src/screens/RegisterDocument.tsx (1)
194-195: Mask or redact sensitive PII in the document data display.The document data likely contains sensitive PII such as names, DOB, passport numbers, and addresses from the MRZ data. Even in a demo app, displaying this information unmasked in the UI poses a privacy risk and violates the coding guideline requiring redaction of sensitive data.
Consider implementing a utility function to redact sensitive fields before display, using patterns like
***-***-1234for passport numbers andJ*** D***for names.As per coding guidelines.
🧹 Nitpick comments (1)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (1)
48-59: Clarify or remove the comment on line 51.The comment "// log via registration panel" doesn't correspond to any logging action in this function. If logging was intended, add it; otherwise, remove the misleading comment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx
🧬 Code graph analysis (1)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (3)
packages/mobile-sdk-demo/src/hooks/useRegistration.ts (1)
useRegistration(18-124)packages/mobile-sdk-demo/src/components/ScreenLayout.tsx (1)
ScreenLayout(18-25)packages/mobile-sdk-demo/src/components/LogsPanel.tsx (1)
LogsPanel(14-32)
🔇 Additional comments (7)
packages/mobile-sdk-demo/src/screens/RegisterDocument.tsx (7)
5-15: LGTM! Imports align with the refactor.The new imports (useRef, ScreenLayout, LogsPanel, useRegistration, humanizeDocumentType) support the modular architecture and hook-driven approach.
32-37: LGTM! Mounted ref prevents stale updates.The mounted ref with cleanup effect is a standard pattern to guard against state updates after unmount, addressing part of the previous review concern.
42-45: LGTM! Clean delegation to useRegistration hook.State variables are now derived from the centralized registration hook, improving maintainability.
90-123: LGTM! Timeout cleanup properly implemented.The completion effect now correctly clears the timeout in the cleanup function (lines 118-122) and guards state updates with
mounted.currentchecks (lines 97, 106), fully addressing the previous review concern about dangling updates.
132-132: LGTM! Refactored to use hook action.The registration is now initiated via
actions.start, aligning with the centralized hook pattern.
143-143: LGTM! Consistent use of shared layout component.The screen now uses the
ScreenLayoutcomponent for consistent UI structure across the app.
185-185: LGTM! Centralized log display.The
LogsPanelcomponent cleanly handles log display with the hook-provided state and toggle action.
Summary by CodeRabbit
New Features
Refactor
Tests