[FTUE] Forgot password #6263

ouchadam · 2022-06-08T10:41:08Z

Type of change

WIP Feature
Bugfix
Technical
Other :

Content

Introduces reset password screens for email entry, confirmation breaker and new password entry.

Motivation and context

To add support for resetting passwords in the FTUE flow #5284 element-hq/element-meta#135

Screenshots / GIFs

SUCCESS GIF

Tests

Enable the combined login feature flag
Follow the reset password flow

Tested devices

Physical
Emulator
OS version(s): 28

github-actions · 2022-06-08T11:38:38Z

Unit Test Results

124 files ±0 124 suites ±0 2m 19s ⏱️ ±0s
224 tests +2 224 ✔️ +2 0 💤 ±0 0 ❌ ±0
742 runs +8 742 ✔️ +8 0 💤 ±0 0 ❌ ±0

Results for commit 03c7748. ± Comparison against base commit e2abbc9.

…he next step

… emitting view events to move the flow forwards

bmarty

Nice work, some remarks.

As a user I would expect to have sort of a confirmation that the password of my account has been updated, maybe a dialog, or inline, but clearly visible in the screen to sign in.

bmarty · 2022-06-28T14:09:42Z

...main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordBreakerFragment.kt

+    private fun setupUi() {
+        views.resetPasswordBreakerGradientContainer.setBackgroundResource(
+                when (themeProvider.isLightTheme()) {
+                    true  -> R.drawable.bg_waiting_for_email_verification


Can this be extracted and mutualize with https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthWaitForEmailFragment.kt#L62 ?

extracted c603b78

perfect and you've renamed the bg, nice!

bmarty · 2022-06-28T14:11:07Z

...n/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordEmailEntryFragment.kt

+    private fun setupViews() {
+        views.emailEntryInput.associateContentStateWith(button = views.emailEntrySubmit)
+        views.emailEntryInput.setOnImeDoneListener { startPasswordReset() }
+        views.emailEntrySubmit.debouncedClicks { startPasswordReset() }


Nit: maybe move this line after all the things about emailEntryInput to group them.

bmarty · 2022-06-28T14:12:26Z

...c/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordEntryFragment.kt

+        views.newPasswordInput.editText().textChanges()
+                .onEach {
+                    views.newPasswordInput.error = null
+                    views.newPasswordSubmit.isEnabled = it.isEmail()


it.isEmail() looks wrong here. newPasswordInput is the field to enter the new password, no?

Also the notice suggest that the password should have 8 chars min. But some homeserver may require other requirements. This is just to let you know that this part should be made easy to be customized for forks.

argh this is definitely wrong, great catch! the reason why the button still works correctly is because https://github.com/vector-im/element-android/pull/6263/files#diff-c29312b48a988ddf4691b69ec9126155953a737ea5d6d5eade6a8db7bc322797R52 is doing the empty content check

yes, thanks, I was wondering why it was working :)

bmarty · 2022-06-28T14:13:03Z

...c/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordEntryFragment.kt

+    private fun setupViews() {
+        views.newPasswordInput.associateContentStateWith(button = views.newPasswordSubmit)
+        views.newPasswordInput.setOnImeDoneListener { resetPassword() }
+        views.newPasswordSubmit.debouncedClicks { resetPassword() }


Nit: same remark about statement ordering, move this to the end of the fun.

bmarty · 2022-06-28T14:15:15Z

...c/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthResetPasswordEntryFragment.kt

+        views.signedOutAllGroup.isVisible = state.resetState.supportsLogoutAllDevices
+
+        if (state.isLoading) {
+            views.newPasswordInput.editText().hidePassword()


bmarty · 2022-06-28T14:17:32Z

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt

+                addLoginStageFragmentToBackstack(FtueAuthResetPasswordSuccessFragment::class.java)
+            }
+            OnboardingViewEvents.OnResetPasswordComplete                 -> {
+                activity.popBackstack()


I am wondering if we should try to (silently, i.e. not complain in case of error) login the user here. We have their email and password. Maybe check with product. Also we may want the user to actively use their new password, so there is pros and cons to automatically sign them in.

yeah sounds like something to check with product, I can't think of any other services off the top of my head that auto login on password reset 🤔 but there might be some!

Yes, me neither, there must be a good reason not to do it. Maybe to give a chance to any password manager to store the password along with the login.

bmarty · 2022-06-28T14:21:15Z

vector/src/main/res/values/donottranslate.xml

+    <string name="ftue_auth_new_password_title">Choose a new password</string>
+    <string name="ftue_auth_new_password_subtitle">Make sure it\'s 8 characters or more.</string>
+    <string name="ftue_auth_reset_password">Reset password</string>
+    <string name="ftue_auth_sign_out_all_devices">Sign out all devices</string>


Maybe change to

Suggested change

<string name="ftue_auth_sign_out_all_devices">Sign out all devices</string>

<string name="ftue_auth_sign_out_all_devices">Sign out all your other devices</string>

for clarity

bmarty · 2022-06-28T14:21:56Z

vector/src/main/res/values/donottranslate.xml

@@ -36,12 +36,20 @@
    <string name="ftue_auth_email_title">Enter your email address</string>
    <string name="ftue_auth_email_subtitle">This will help verify your account and enables password recovery.</string>
    <string name="ftue_auth_email_entry_title">Email Address</string>
+    <string name="ftue_auth_reset_password_email_subtitle">We will send you a verification link.</string>


Here we mention a link, and in the next screen we mention a button:

<string name="ftue_auth_email_verification_subtitle">To confirm your email address, tap the button in the email we just sent to %s</string>

will bring up in the FTUE room (there's a session planned to go over all of the FTUE copy)

…is check is wrong but also not needed due to associateContentStateWith above

…ith and extracts the resetting of errors on content change to an extension

ouchadam · 2022-06-30T09:45:18Z

As a user I would expect to have sort of a confirmation that the password of my account has been updated, maybe a dialog, or inline, but clearly visible in the screen to sign in.

Matches iOS by adding a confirmation toast
0d08464

sonarqubecloud · 2022-06-30T10:50:57Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

29.8% Coverage
0.0% Duplication

bmarty · 2022-06-30T13:44:49Z

As a user I would expect to have sort of a confirmation that the password of my account has been updated, maybe a dialog, or inline, but clearly visible in the screen to sign in.

Matches iOS by adding a confirmation toast 0d08464

This is better than nothing, but most users will miss it I guess. A dialog would be better, or a inline message in the form. Also the wording is not very user friendly. Something like "Your password has been successfully changed!" is probably a bit better. Not a blocker but clearly something to polish.

bmarty

Thanks for the update!
Nothing blocking, but I hope we will have the opportunity to polish some parts before enabling the whole feature.

ouchadam · 2022-06-30T14:38:41Z

the password reset flow will be included in the community session test cases, will see what the users think 🤞

ouchadam added the Z-FTUE Issue is relevant to the first time use project or experience label Jun 8, 2022

ouchadam force-pushed the feature/adm/sdk-signout-all-devices branch from e2abbc9 to 3c3bd77 Compare June 8, 2022 11:04

ouchadam mentioned this pull request Jun 8, 2022

SDK - Logout all devices #6207

Merged

6 tasks

ouchadam force-pushed the feature/adm/ftue-forgot-password branch from 03c7748 to 8f854ee Compare June 9, 2022 11:36

ouchadam force-pushed the feature/adm/sdk-signout-all-devices branch from 7a633a4 to e655a54 Compare June 24, 2022 14:10

Base automatically changed from feature/adm/sdk-signout-all-devices to develop June 27, 2022 07:30

ouchadam added 17 commits June 27, 2022 12:28

adding forgot password button to login screen

dcffc35

adding updated forgot password email input and breaker screens

9abf6e3

adding a password entry confirmation page for the reset password flow

7a4a603

taking into account the servers ability to sign out all devices

4200b4b

adding test around resending reset password email

3c03bae

fixing reset fragments not removing previous screens when moving to t…

7ef8193

…he next step

hiding the reset password input by default

47cedfb

handling the reset password completion step within the view model and…

16481df

… emitting view events to move the flow forwards

updating xml ids to reflect their context

0d80bdf

formatting

1d3b4e9

making the new password visibility toggleable

f40f838

updating test names to read better

eb84072

adding changelog entry

71d1024

fixing code quality warnings

7971a74

adding extra padding between reset button and checkbox to match designs

0a0c322

using next instead of done copy when confirming reset email link

5bb397f

fixing missing fake given

ef4889a

ouchadam force-pushed the feature/adm/ftue-forgot-password branch from b849c17 to ef4889a Compare June 27, 2022 14:25

ouchadam marked this pull request as ready for review June 27, 2022 14:39

ouchadam requested review from a team, onurays and bmarty and removed request for a team June 27, 2022 14:40

ignoring negative margin required by the checkbox alignment

2a46fbe

bmarty requested changes Jun 28, 2022

View reviewed changes

ouchadam added 3 commits June 29, 2022 13:35

fixing method call grouping

382a936

removing new password isEnabled check using the isEmail validator, th…

80ec0aa

…is check is wrong but also not needed due to associateContentStateWith above

formatting

7c0d340

ouchadam force-pushed the feature/adm/ftue-forgot-password branch from 956104b to 9d5b399 Compare June 29, 2022 13:14

triggering an initial enabled state when using associateContentStateW…

1361852

…ith and extracts the resetting of errors on content change to an extension

ouchadam force-pushed the feature/adm/ftue-forgot-password branch from 9d5b399 to 1361852 Compare June 29, 2022 13:15

ouchadam added 2 commits June 29, 2022 17:47

extracting common breaker background selection to ftue extensions

c603b78

showing a toast on password reset confirmation

0d08464

ouchadam requested a review from bmarty June 30, 2022 09:46

bmarty approved these changes Jun 30, 2022

View reviewed changes

ouchadam merged commit 72c4af0 into develop Jun 30, 2022

ouchadam deleted the feature/adm/ftue-forgot-password branch June 30, 2022 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FTUE] Forgot password #6263

[FTUE] Forgot password #6263

ouchadam commented Jun 8, 2022 •

edited

Loading

github-actions bot commented Jun 8, 2022

bmarty left a comment

bmarty Jun 28, 2022

ouchadam Jun 29, 2022

bmarty Jun 30, 2022

bmarty Jun 28, 2022

ouchadam Jun 29, 2022

bmarty Jun 28, 2022 •

edited

Loading

ouchadam Jun 29, 2022

bmarty Jun 29, 2022

bmarty Jun 28, 2022

bmarty Jun 28, 2022

bmarty Jun 28, 2022

ouchadam Jun 29, 2022

bmarty Jun 30, 2022

bmarty Jun 28, 2022 •

edited

Loading

bmarty Jun 28, 2022

ouchadam Jun 29, 2022

ouchadam commented Jun 30, 2022 •

edited

Loading

sonarqubecloud bot commented Jun 30, 2022

bmarty commented Jun 30, 2022

bmarty left a comment

ouchadam commented Jun 30, 2022

	<string name="ftue_auth_sign_out_all_devices">Sign out all devices</string>
	<string name="ftue_auth_sign_out_all_devices">Sign out all your other devices</string>

[FTUE] Forgot password #6263

[FTUE] Forgot password #6263

Conversation

ouchadam commented Jun 8, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

github-actions bot commented Jun 8, 2022

Unit Test Results

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam commented Jun 30, 2022 • edited Loading

sonarqubecloud bot commented Jun 30, 2022

bmarty commented Jun 30, 2022

bmarty left a comment

Choose a reason for hiding this comment

ouchadam commented Jun 30, 2022

ouchadam commented Jun 8, 2022 •

edited

Loading

bmarty Jun 28, 2022 •

edited

Loading

bmarty Jun 28, 2022 •

edited

Loading

ouchadam commented Jun 30, 2022 •

edited

Loading