use native system dialog for disable cloud backups#1863
Conversation
📝 WalkthroughWalkthroughReplaces the custom Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 (1)
app/src/screens/account/settings/CloudBackupScreen.tsx (1)
74-101: Consider whether nativeAlert.alertaligns with project conventions.The coding guidelines specify using the custom modal system with
useModalhook and callback registry. WhileAlert.alertworks, it deviates from the established pattern used elsewhere in this file (e.g.,showNoRegisteredAccountModalon lines 103-120). This inconsistency could make maintenance harder and loses the haptic feedback integration that the custom modal system provides.Additionally, on Android, users can dismiss the alert by tapping outside without triggering either callback, which would leave
iCloudPendingstuck astrue.If the native alert is intentional for this specific flow, consider adding a note explaining why, or handle the Android dismissal case explicitly using
cancelable: falsein the options:Alert.alert( 'Disable cloud backups', 'Are you sure you want to disable cloud backups? You may lose your recovery phrase.', [ { text: 'Cancel', style: 'cancel', onPress: () => setICloudPending(false), }, { text: 'Disable', style: 'destructive', onPress: async () => { /* ... */ }, }, ], + { cancelable: false }, );Based on learnings: "Screens must be organized by feature modules (passport, home, settings, etc.) with custom modal system using
useModalhook and callback registry."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/screens/account/settings/CloudBackupScreen.tsx` around lines 74 - 101, The native Alert.alert in showDisableModal deviates from the app's modal pattern and can leave iCloudPending true if dismissed on Android; replace this Alert.alert usage with the project's custom modal via useModal/callback registry (same approach as showNoRegisteredAccountModal) to present the disable confirmation, wire the Cancel and Disable handlers to call setICloudPending(false) and run loginWithBiometrics(), disableBackup(), toggleCloudBackupEnabled(), and trackEvent(...) as before, and ensure the modal is non-cancelable or that the modal’s onDismiss also clears setICloudPending(false) and triggers any needed haptic feedback; if you must keep Alert.alert, at minimum pass { cancelable: false } and document why native Alert is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/screens/account/settings/CloudBackupScreen.tsx`:
- Line 101: The dependency array on the hook ending with "},
[loginWithBiometrics, disableBackup, toggleCloudBackupEnabled, trackEvent]);"
must be formatted as a multi-line array to satisfy Prettier; update the
useEffect (or relevant hook) so the dependencies are each on their own line
(loginWithBiometrics, disableBackup, toggleCloudBackupEnabled, trackEvent) and
close the array and call on separate lines to match the project's Prettier
rules.
- Around line 84-97: The onPress handler for the "Disable" action currently uses
a try/finally and swallows errors from loginWithBiometrics(), disableBackup(),
and toggleCloudBackupEnabled(), so failures give no user feedback and may leave
state inconsistent; update the handler to wrap the async sequence in
try/catch/finally, call loginWithBiometrics(), then await disableBackup() and
only call toggleCloudBackupEnabled() if disableBackup() succeeds, and in the
catch block log/track the error (e.g., trackEvent with a failure event) and
surface an error to the user (alert/toast), while preserving the existing
setICloudPending(false) in finally; reference the onPress callback and the
functions loginWithBiometrics, disableBackup, toggleCloudBackupEnabled,
setICloudPending, and trackEvent when making changes.
---
Nitpick comments:
In `@app/src/screens/account/settings/CloudBackupScreen.tsx`:
- Around line 74-101: The native Alert.alert in showDisableModal deviates from
the app's modal pattern and can leave iCloudPending true if dismissed on
Android; replace this Alert.alert usage with the project's custom modal via
useModal/callback registry (same approach as showNoRegisteredAccountModal) to
present the disable confirmation, wire the Cancel and Disable handlers to call
setICloudPending(false) and run loginWithBiometrics(), disableBackup(),
toggleCloudBackupEnabled(), and trackEvent(...) as before, and ensure the modal
is non-cancelable or that the modal’s onDismiss also clears
setICloudPending(false) and triggers any needed haptic feedback; if you must
keep Alert.alert, at minimum pass { cancelable: false } and document why native
Alert is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cc3818f-4f6d-4d52-acb6-3cea36a1ca27
📒 Files selected for processing (1)
app/src/screens/account/settings/CloudBackupScreen.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/screens/account/settings/CloudBackupScreen.tsx`:
- Around line 74-113: The UI is toggling cloud backup state even when Android
Google-signin cancellation causes disableBackup() to return silently; update the
contract so CloudBackupScreen can reliably know outcome: change disableBackup
(in cloud-backup/index.ts) to return a boolean success flag (true = deleted,
false = user-cancelled) or alternatively throw on cancellation (matching
download()), and then update CloudBackupScreen's showDisableModal to check the
returned value (or rely on thrown error) before calling
toggleCloudBackupEnabled(); ensure callers of disableBackup are updated to
handle the new boolean/exception behavior (and keep loginWithBiometrics and
trackEvent usage intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d5c611ca-d474-4b00-98f8-eaf3d71c37f3
📒 Files selected for processing (1)
app/src/screens/account/settings/CloudBackupScreen.tsx
1 participant
Summary
useModaldialog with nativeAlert.alertfor the "Disable cloud backups" destructive action inCloudBackupScreenstyle: 'destructive'on the action button andstyle: 'cancel'on dismiss, giving users the standard platform confirmation UX for irreversible operationsChanges
React Native app
app/src/screens/account/settings/CloudBackupScreen.tsx— replaceduseModal-based custom modal with nativeAlert.alertfor the disable backup confirmationLinear Issues
Test Plan
yarn lint && yarn typespasses🤖 Generated with Claude Code
Summary by CodeRabbit