Rocket.Chat.ReactNative icon indicating copy to clipboard operation
Rocket.Chat.ReactNative copied to clipboard

fix: App hanging when changing passcode on iOS

Open DSingh0304 opened this issue 2 months ago • 3 comments

Proposed changes

This PR fixes an issue where the iOS app hangs when users attempt to change their passcode in the Screen Lock settings. The hang occurs due to two modals (authentication modal and passcode change modal) opening back-to-back, causing an iOS UI thread conflict.

The fix adds a 300ms delay between the authentication modal closing and the passcode change modal opening, allowing the first modal to fully dismiss before the second one appears. Additionally, try-catch blocks have been added to gracefully handle cases where users cancel authentication.

Issue(s)

Fixes #6313

How to test or reproduce

To reproduce the original bug (on iOS):

  1. Go to Settings → Security & Privacy → Screen Lock
  2. Enable Screen Lock and set a passcode
  3. Tap "Change Passcode"
  4. Authenticate with biometric or existing passcode
  5. App would hang at this point

To verify the fix:

  1. Follow the same steps above
  2. After authentication, there should be a brief (300ms) delay
  3. The passcode change modal should appear without any hang
  4. Complete the passcode change successfully

Testing:

  • Run tests: yarn test app/views/ScreenLockConfigView.test.tsx
  • All 4 unit tests pass, covering:
    • 300ms delay between modals
    • Authentication cancellation handling
    • Direct passcode change when autoLock is disabled
    • Delay duration verification

Files modified:

  • app/views/ScreenLockConfigView.tsx - Added delay and error handling to changePasscode method
  • app/views/SecurityPrivacyView.tsx - Added delay and error handling to navigateToScreenLockConfigView method
  • app/views/ScreenLockConfigView.test.tsx - Added unit tests for the fix

Screenshots

N/A - This is a timing/modal transition fix with no visual UI changes

Types of changes

  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] Improvement (non-breaking change which improves a current function)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Documentation update (if none of the other choices apply)

Checklist

  • [x] I have read the CONTRIBUTING doc
  • [x] I have signed the CLA
  • [x] Lint and unit tests pass locally with my changes
  • [x] I have added tests that prove my fix is effective or that my feature works (if applicable)
  • [x] I have added necessary documentation (if applicable)
  • [x] Any dependent changes have been merged and published in downstream modules

Further comments

This is a minimal, targeted fix that addresses the iOS-specific modal timing issue. The 300ms delay is a standard duration used in React Native for modal transitions and should be imperceptible to users while preventing the UI thread hang.

Unit tests have been added to verify:

  • The 300ms delay is properly applied between authentication and passcode change
  • Authentication cancellation is handled gracefully without proceeding to passcode change
  • The flow works correctly when autoLock is disabled (no authentication required)
  • The delay duration constant is correct

Testing environment:

  • ✅ ESLint: Passed
  • ✅ Prettier: Passed
  • ✅ Unit Tests: 4/4 passed
  • ✅ iOS Simulator: Tested and verified - no hang occurs when changing passcode
  • ✅ Android build: Successful

Summary by CodeRabbit

  • Bug Fixes

    • Added a short delay to prevent iOS modal hangs when authentication and passcode prompts appear back-to-back.
    • Authentication failures or cancellations now abort navigation and passcode changes to avoid unintended actions.
    • Improved timing and sequencing of local authentication and security screens for more reliable transitions.
  • Chores

    • Introduced a 300ms modal-transition delay constant to centralize timing behavior.
  • Tests

    • Added tests covering authentication success/failure flows, timing of the modal delay, and passcode-change behavior.

DSingh0304 avatar Nov 12 '25 17:11 DSingh0304

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Nov 12 '25 17:11 CLAassistant

Walkthrough

Wraps local authentication calls in try/catch in two views; on successful authentication waits MODAL_TRANSITION_DELAY_MS (300ms) before proceeding; on failure/cancel returns early. Adds MODAL_TRANSITION_DELAY_MS constant and new tests for timing, cancellation, and autoLock behavior. Exports ScreenLockConfigView publicly.

Changes

Cohort / File(s) Summary
Views — auth timing & error handling
app/views/ScreenLockConfigView.tsx, app/views/SecurityPrivacyView.tsx
Wrap handleLocalAuthentication(true) in try/catch; on success await MODAL_TRANSITION_DELAY_MS before continuing; on failure/cancel return early and do not proceed with navigation or changePasscode. Also export ScreenLockConfigView publicly.
Constants
app/lib/constants/localAuthentication.ts
Add and export MODAL_TRANSITION_DELAY_MS = 300 with comment describing its purpose to prevent iOS modal UI thread hangs during back-to-back modals.
Tests
app/views/ScreenLockConfigView.test.tsx
Add tests that mock localAuthentication and DB: verify 300ms delay after successful auth before changePasscode, verify rejection/cancellation prevents changePasscode, and verify flow when autoLock is false. Uses fake timers.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant View as SecurityPrivacyView / ScreenLockConfigView
    participant Auth as LocalAuthentication
    participant Flow as Navigation / PasscodeFlow

    User->>View: Request change/set passcode
    View->>Auth: handleLocalAuthentication(true)
    rect rgb(220,240,255)
      Note over View,Auth: try/catch around auth call
      Auth-->>View: Success
      View->>View: wait MODAL_TRANSITION_DELAY_MS (300ms)
      Note over View: Prevent iOS modal overlap before next modal
      View->>Flow: navigate / changePasscode
    end
    rect rgb(255,235,235)
      Auth-->>View: Failure / Cancel
      Note over View: return early — abort navigation / passcode change
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Check try/catch correctness and that promise rejections are handled.
  • Verify the delay constant is imported/used consistently.
  • Inspect new tests for reliable fake-timer usage and proper mocks of localAuthentication and DB.
  • Confirm the exported ScreenLockConfigView change doesn't conflict with existing imports.

Suggested reviewers

  • diegolmello

Poem

🐰 I hopped where modals nearly crashed,
I paused a beat so screens unlash.
Three hundred ticks of gentle hush,
Passcodes change without the rush. ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change—fixing an app hang when changing passcode on iOS by addressing modal conflict issues.
Linked Issues check ✅ Passed All coding requirements from #6313 are met: 300ms delay between modals added in both ScreenLockConfigView and SecurityPrivacyView, error handling for cancellation implemented, and comprehensive tests added.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the iOS passcode hang: delay constant, modal transition delay logic, error handling, and related tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f23c341d7dae3725114d8244ac0484c40be1a4b2 and 4cb54a7a946a59179ac8693bdaec23530d8b9a9c.

📒 Files selected for processing (4)
  • app/lib/constants/localAuthentication.ts (1 hunks)
  • app/views/ScreenLockConfigView.test.tsx (1 hunks)
  • app/views/ScreenLockConfigView.tsx (3 hunks)
  • app/views/SecurityPrivacyView.tsx (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/views/ScreenLockConfigView.test.tsx (1)
app/views/ScreenLockConfigView.tsx (1)
  • ScreenLockConfigView (307-307)
app/views/SecurityPrivacyView.tsx (2)
app/lib/methods/helpers/localAuthentication.ts (1)
  • handleLocalAuthentication (113-125)
app/lib/constants/localAuthentication.ts (1)
  • MODAL_TRANSITION_DELAY_MS (16-16)
app/views/ScreenLockConfigView.tsx (2)
app/lib/methods/helpers/localAuthentication.ts (1)
  • handleLocalAuthentication (113-125)
app/lib/constants/localAuthentication.ts (1)
  • MODAL_TRANSITION_DELAY_MS (16-16)
🔇 Additional comments (4)
app/lib/constants/localAuthentication.ts (1)

15-16: Centralized modal delay constant.

Moving this delay into a shared constant keeps every caller aligned and makes future tuning trivial.

app/views/SecurityPrivacyView.tsx (1)

61-70: Guarded navigation on auth failure.

The try/catch plus shared delay avoids stacking modals and cleanly bails when auth is cancelled—nice containment of the original hang.

app/views/ScreenLockConfigView.tsx (1)

135-143: Modal sequencing safeguard.

The post-auth delay with graceful early return keeps the passcode flow stable and prevents the iOS hang we were seeing.

app/views/ScreenLockConfigView.test.tsx (1)

37-61: Regression test covers async gap.

Exercising the real component instance and waiting on fake timers ensures we won’t regress the modal spacing behavior.


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.

coderabbitai[bot] avatar Nov 12 '25 17:11 coderabbitai[bot]

Hey maintainers, I would love it if someone could review the code and tell me if anything needs to be changed.

DSingh0304 avatar Nov 17 '25 10:11 DSingh0304