Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash on PIN code settings screen #6980

Merged
merged 1 commit into from
Aug 31, 2022

Conversation

jmartinesp
Copy link
Member

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Made VectorSettingsPinFragment.biometricHelper lazy so it's not instantiated before biometricHelperFactory is injected.

Motivation and context

See #6979.

Tests

  • Go to Settings > Security & Privacy > Protect access.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 12.

Checklist

@jmartinesp jmartinesp requested a review from a team August 31, 2022 14:06
@jmartinesp jmartinesp self-assigned this Aug 31, 2022
@jmartinesp jmartinesp requested review from Florian14 and removed request for a team August 31, 2022 14:06
@jmartinesp jmartinesp enabled auto-merge (rebase) August 31, 2022 14:19
private val biometricHelper = biometricHelperFactory.create(defaultLockScreenConfiguration.copy(mode = LockScreenMode.CREATE))
private val biometricHelper by lazy {
biometricHelperFactory.create(defaultLockScreenConfiguration.copy(mode = LockScreenMode.CREATE))
}
Copy link
Member

@bmarty bmarty Aug 31, 2022

Choose a reason for hiding this comment

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

it could also be a lateinit var and be created in fun onCreate(). Not sure if it's better, but at least it will remove the overhead of using by lazy.

Copy link
Member

Choose a reason for hiding this comment

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

(IMO we should use by lazy when on object would eventually be not used - which is not the case here)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, when it was approved I enabled automatic merge and by the time I read that message it was already merged. I can open a new PR changing lazy to lateinit and instantiate it in onCreate if you think it's worth it.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, this is not urgent.

@sonarcloud
Copy link

sonarcloud bot commented Aug 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@jmartinesp jmartinesp merged commit 3ef3e37 into develop Aug 31, 2022
@jmartinesp jmartinesp deleted the fix/jorgem/crash-in-pin-code-settings branch August 31, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants