Skip to content

Conversation

@paviad
Copy link
Contributor

@paviad paviad commented Aug 11, 2025

Proposed changes

For some reason, the code works fine on Android but doesn't work on iOS. I tracked it down to a weird timing issue and added a delay in the appropriate place.

Issue(s)

Resolves #6313

Types of changes

  • 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

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

Further comments

This fix feels more like a workaround than a proper fix, however creating a proper fix for this is not something that I would be able to achieve in reasonable time.

Summary by CodeRabbit

  • Bug Fixes
    • Improved rendering stability for passcode entry and selection components.
    • Enhanced iOS compatibility when changing passcode settings.

✏️ Tip: You can customize this high-level summary in your review settings.

@paviad paviad had a problem deploying to experimental_ios_build August 13, 2025 07:05 — with GitHub Actions Error
@paviad paviad had a problem deploying to experimental_android_build August 13, 2025 07:05 — with GitHub Actions Error
@paviad paviad had a problem deploying to official_android_build August 13, 2025 07:05 — with GitHub Actions Error
@paviad paviad had a problem deploying to approve_e2e_testing August 14, 2025 21:16 — with GitHub Actions Failure
@paviad paviad had a problem deploying to official_ios_build August 14, 2025 21:19 — with GitHub Actions Failure
@paviad paviad had a problem deploying to experimental_ios_build August 14, 2025 21:19 — with GitHub Actions Failure
@paviad paviad had a problem deploying to official_android_build August 14, 2025 21:19 — with GitHub Actions Failure
@paviad paviad had a problem deploying to experimental_android_build August 14, 2025 21:19 — with GitHub Actions Failure
@Rohit3523 Rohit3523 requested a deployment to approve_e2e_testing December 7, 2025 17:00 — with GitHub Actions Waiting
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

Walkthrough

Fixes an iPhone app hang when changing passcode by reusing a ref in PasscodeChoose, adding React keys to prevent re-rendering issues in passcode components, and introducing a 1-second delay workaround in ScreenLockConfigView before modal presentation.

Changes

Cohort / File(s) Change Summary
Passcode component key stabilization
app/containers/Passcode/Base/Dots.tsx, app/containers/Passcode/PasscodeChoose.tsx, app/containers/Passcode/PasscodeEnter.tsx
Added React key props to Base and View components to ensure stable element identity during state transitions and avoid rendering warnings from list items. PasscodeChoose now reuses chooseRef for both CHOOSE and CONFIRM states instead of using a separate confirmRef.
iOS modal race condition workaround
app/views/ScreenLockConfigView.tsx
Added 1-second artificial delay in changePasscode after local authentication to prevent back-to-back modal presentations from causing app hang on iOS.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that key values ("passcode-choose", "passcode-enter") are appropriate and won't cause unintended re-mounts
  • Understand the necessity and timing of the 1-second iOS delay and confirm it doesn't introduce perceptible UX lag
  • Confirm that reusing chooseRef for both CHOOSE and CONFIRM states in PasscodeChoose doesn't cause ref conflicts or state management issues

Possibly related PRs

Suggested reviewers

  • diegolmello

Poem

🐰 With keys in hand and refs now wise,
Two modals cease to synchronize,
A heartbeat's pause, an iOS mend,
Where passcodes now flow to the end.

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 'fix: passcode update issues on iOS' directly and concisely describes the main change—addressing a passcode-related bug on iOS platform.
Linked Issues check ✅ Passed The PR successfully addresses issue #6313 by fixing the app hang when changing passcode on iPhone through strategic key props and timing adjustments.
Out of Scope Changes check ✅ Passed All changes (key props in Passcode components and iOS delay workaround) are directly scoped to fixing the passcode update issue on iOS, with no extraneous modifications.
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 1bd62a2 and 576a648.

📒 Files selected for processing (4)
  • app/containers/Passcode/Base/Dots.tsx (1 hunks)
  • app/containers/Passcode/PasscodeChoose.tsx (2 hunks)
  • app/containers/Passcode/PasscodeEnter.tsx (1 hunks)
  • app/views/ScreenLockConfigView.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/ScreenLockConfigView.tsx (1)
app/lib/encryption/helpers/deferred.ts (1)
  • resolve (32-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (3)
app/containers/Passcode/PasscodeEnter.tsx (1)

82-91: Static key on Base is harmless and consistent with other passcode screens

Adding key={"passcode-enter"} here doesn’t change behavior (static key on a single element), but it’s fine for consistency with the other passcode components.

app/containers/Passcode/Base/Dots.tsx (1)

21-47: Good addition of key on list items

key={val} on the outer View is appropriate here and will eliminate React’s key warnings without side effects for this static-range list.

app/containers/Passcode/PasscodeChoose.tsx (1)

40-62: Ref unification and key usage in PasscodeChoose look good

Reusing chooseRef for both CHOOSE and CONFIRM states simplifies the component and keeps all imperative calls (clearPasscode, animate) going through a single ref, which should make the flow more predictable. The added key={"passcode-choose"} on both branches is effectively neutral here (same type, same key, never rendered as siblings) but harmless and keeps things aligned with PasscodeEnter.


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.


return (
<Base
key={"passcode-enter"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you remove this because it is not required


return (
<Base
key={"passcode-choose"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you remove this because it is not required

const marginLeft = lengthSup ? 10 - (SIZE_FULL - SIZE_EMPTY) / 2 : 10;
return (
<View style={styles.dotsView}>
<View key={val} style={styles.dotsView}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you remove this because it is not required

if (autoLock) {
await handleLocalAuthentication(true);
}
console.log('IOS workaround - waiting 1 sec...');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this one

}
console.log('IOS workaround - waiting 1 sec...');
await new Promise(resolve => setTimeout(resolve, 1000));
console.log('IOS workaround - waiting 1 sec...done');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this one

@Rohit3523
Copy link
Collaborator

Hey @paviad, can you please make the changes

@Rohit3523 Rohit3523 requested a deployment to experimental_android_build December 7, 2025 17:03 — with GitHub Actions Waiting
@Rohit3523 Rohit3523 requested a deployment to experimental_ios_build December 7, 2025 17:03 — with GitHub Actions Waiting
@Rohit3523 Rohit3523 requested a deployment to official_android_build December 7, 2025 17:03 — with GitHub Actions Waiting
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.

bug: App Stops working when change passcode on iPhone

2 participants