Page MenuHomePhabricator

[native] Avoid showing iCloud password manager prompt when dismissing LogInPanel
ClosedPublic

Authored by ashoat on Fri, May 24, 1:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 13, 6:40 AM
Unknown Object (File)
Thu, Jun 6, 3:05 PM
Unknown Object (File)
Wed, Jun 5, 11:07 PM
Unknown Object (File)
Wed, Jun 5, 7:42 PM
Unknown Object (File)
Wed, Jun 5, 12:06 AM
Unknown Object (File)
Tue, Jun 4, 1:08 PM
Unknown Object (File)
Tue, Jun 4, 12:22 PM
Unknown Object (File)
Mon, Jun 3, 9:59 AM
Subscribers

Details

Summary

For some reason, this prompt now appears when the LogInPanel is dismissed. It seems that switching to Reanimated 2 syntax here changed something about how the iOS password manager logic works with our code. (One big plus is that in LogInPanel, the username and password are now autocompleted together, instead of requiring two separate steps – cc @varun, who previously talked about this with me.)

Anyways: it makes sense to show this prompt when the user attempts a login, but not when the user goes back.

To avoid showing the prompt, we can clear the password TextInput before it disappears from the screen.

Depends on D12208

Test Plan
beforeafter

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/account/logged-out-modal.react.js
326–328 ↗(On Diff #40611)

This was moved lower and renamed to finishResettingToPrompt. It's only used in one place (passed to getPanelOpacity)

ashoat edited the summary of this revision. (Show Details)
ashoat added a subscriber: varun.
native/account/logged-out-modal.react.js
398 ↗(On Diff #40611)

Is it necessary to only do this for iOS? In finishResettingToPrompt setLogInState({ passwordInputText: temporarilyHiddenPassword.current }); is called at all times, which feels inconsistent.

native/account/logged-out-modal.react.js
398 ↗(On Diff #40611)

Only iOS has this behavior. The call you mention in finishResettingToPrompt is gated on temporarilyHiddenPassword.current being set, which won't happen on Android. If for some reason in the future we do set it on Android, I think it would make sense for that call to be triggered, but this is a purely hypothetical conversation

This revision is now accepted and ready to land.Mon, May 27, 1:14 AM