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

FTUE - Choose a display name #5211

Merged
merged 13 commits into from
Mar 2, 2022
Merged
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions vector/src/main/java/im/vector/app/core/di/FragmentModule.kt
Original file line number Diff line number Diff line change
@@ -99,6 +99,7 @@ import im.vector.app.features.matrixto.MatrixToRoomSpaceFragment
import im.vector.app.features.matrixto.MatrixToUserFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthAccountCreatedFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthCaptchaFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthChooseDisplayNameFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthGenericTextInputFormFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthLoginFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthResetPasswordFragment
@@ -479,6 +480,11 @@ interface FragmentModule {
@FragmentKey(FtueAuthAccountCreatedFragment::class)
fun bindFtueAuthAccountCreatedFragment(fragment: FtueAuthAccountCreatedFragment): Fragment

@Binds
@IntoMap
@FragmentKey(FtueAuthChooseDisplayNameFragment::class)
fun bindFtueAuthChooseDisplayNameFragment(fragment: FtueAuthChooseDisplayNameFragment): Fragment

@Binds
@IntoMap
@FragmentKey(UserListFragment::class)
Original file line number Diff line number Diff line change
@@ -73,4 +73,7 @@ sealed class OnboardingAction : VectorViewModelAction {
data class PostViewEvent(val viewEvent: OnboardingViewEvents) : OnboardingAction()

data class UserAcceptCertificate(val fingerprint: Fingerprint) : OnboardingAction()

data class UpdateDisplayName(val displayName: String) : OnboardingAction()
object UpdateDisplayNameSkipped : OnboardingAction()
}
Original file line number Diff line number Diff line change
@@ -52,4 +52,6 @@ sealed class OnboardingViewEvents : VectorViewEvents {
object OnAccountSignedIn : OnboardingViewEvents()
object OnTakeMeHome : OnboardingViewEvents()
object OnPersonalizeProfile : OnboardingViewEvents()
object OnDisplayNameUpdated : OnboardingViewEvents()
object OnDisplayNameSkipped : OnboardingViewEvents()
}
Original file line number Diff line number Diff line change
@@ -146,6 +146,8 @@ class OnboardingViewModel @AssistedInject constructor(
is OnboardingAction.UserAcceptCertificate -> handleUserAcceptCertificate(action)
OnboardingAction.ClearHomeServerHistory -> handleClearHomeServerHistory()
is OnboardingAction.PostViewEvent -> _viewEvents.post(action.viewEvent)
is OnboardingAction.UpdateDisplayName -> updateDisplayName(action.displayName)
OnboardingAction.UpdateDisplayNameSkipped -> _viewEvents.post(OnboardingViewEvents.OnDisplayNameSkipped)
}.exhaustive
}

@@ -881,6 +883,21 @@ class OnboardingViewModel @AssistedInject constructor(
fun getFallbackUrl(forSignIn: Boolean, deviceId: String?): String? {
return authenticationService.getFallbackUrl(forSignIn, deviceId)
}

private fun updateDisplayName(displayName: String) {
setState { copy(asyncDisplayName = Loading()) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this uses the same async state mechanism as the other parts of the onboarding state (sign in/register), the login V2 converts this to a simple isLoading: boolean but I've avoided doing the same change here to keep the diff smaller

viewModelScope.launch {
val activeSession = activeSessionHolder.getActiveSession()
try {
activeSession.setDisplayName(activeSession.myUserId, displayName)
setState { copy(asyncDisplayName = Success(Unit)) }
_viewEvents.post(OnboardingViewEvents.OnDisplayNameUpdated)
} catch (error: Throwable) {
setState { copy(asyncDisplayName = Fail(error)) }
_viewEvents.post(OnboardingViewEvents.Failure(error))
}
}
}
}

private fun LoginMode.supportsSignModeScreen(): Boolean {
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ data class OnboardingViewState(
val asyncResetPassword: Async<Unit> = Uninitialized,
val asyncResetMailConfirmed: Async<Unit> = Uninitialized,
val asyncRegistration: Async<Unit> = Uninitialized,
val asyncDisplayName: Async<Unit> = Uninitialized,

@PersistState
val onboardingFlow: OnboardingFlow? = null,
@@ -70,7 +71,8 @@ data class OnboardingViewState(
asyncHomeServerLoginFlowRequest is Loading ||
asyncResetPassword is Loading ||
asyncResetMailConfirmed is Loading ||
asyncRegistration is Loading
asyncRegistration is Loading ||
asyncDisplayName is Loading
}

fun isAuthTaskCompleted(): Boolean {
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) 2021 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.features.onboarding.ftueauth

import android.os.Bundle
import android.text.Editable
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import com.google.android.material.textfield.TextInputLayout
import im.vector.app.core.platform.SimpleTextWatcher
import im.vector.app.databinding.FragmentFtueDisplayNameBinding
import im.vector.app.features.onboarding.OnboardingAction
import im.vector.app.features.onboarding.OnboardingViewEvents
import javax.inject.Inject

class FtueAuthChooseDisplayNameFragment @Inject constructor() : AbstractFtueAuthFragment<FragmentFtueDisplayNameBinding>() {

override fun getBinding(inflater: LayoutInflater, container: ViewGroup?): FragmentFtueDisplayNameBinding {
return FragmentFtueDisplayNameBinding.inflate(inflater, container, false)
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
setupViews()
}

private fun setupViews() {
views.displayNameSubmit.isEnabled = views.displayNameInput.hasContentEmpty()
views.displayNameInput.editText?.addTextChangedListener(object : SimpleTextWatcher() {
override fun afterTextChanged(s: Editable) {
val newContent = s.toString()
views.displayNameSubmit.isEnabled = newContent.isNotEmpty()
Copy link
Member

Choose a reason for hiding this comment

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

Could be nice to reset the error here.
Because the error when setting display name could be displayed in the TextInputLayout. You can trigger an error for instance by setting a display name with @

Copy link
Contributor Author

@ouchadam ouchadam Feb 25, 2022

Choose a reason for hiding this comment

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

at the moment errors are coming back in the form of the generic login dialog, I'm not seeing any errors when using @ in the display name though 🤔

2022-02-25T12:09:55,986486346+00:00

Copy link
Member

Choose a reason for hiding this comment

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

I mixed between display name and userId, my bad.

Copy link
Member

Choose a reason for hiding this comment

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

Still it could be nice to display the error in the TextInputLayout. You can do it easily by overidding onError in FtueAuthChooseDisplayNameFragment.

For network error (in airplane mode for instance), it's maybe better if the error is displayed in a dialog (so just call super.onError in this case).

Long display name can be rejected by the homeserver, and other rules may apply, that's why it could be nice to inline the error in such case IMO, instead of

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the screenshot and explanation! I've checked in with product/design, we'd like to do a pass of all the onboarding errors and their copy

I'll create another subtask in #5200 to revisit the error handling here in case there's other types of errors we'd like to capture within the text field instead of dialog and I'll need to track down all the possible error types (and their copy)

Copy link
Member

Choose a reason for hiding this comment

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

Ack

}
})

views.displayNameSubmit.debouncedClicks {
val newDisplayName = views.displayNameInput.editText?.text.toString()
viewModel.handle(OnboardingAction.UpdateDisplayName(newDisplayName))
}

views.displayNameSkip.debouncedClicks { viewModel.handle(OnboardingAction.UpdateDisplayNameSkipped) }
}

override fun resetViewModel() {
// Nothing to do
}

override fun onBackPressed(toolbarButton: Boolean): Boolean {
viewModel.handle(OnboardingAction.PostViewEvent(OnboardingViewEvents.OnTakeMeHome))
return false
Copy link
Member

@bmarty bmarty Feb 23, 2022

Choose a reason for hiding this comment

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

return true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah 🤦 thought I'd included updates from the previous PR comments, will fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated 8dec4fd

}
}

private fun TextInputLayout.hasContentEmpty() = !editText?.text.isNullOrEmpty()
Original file line number Diff line number Diff line change
@@ -223,8 +223,10 @@ class FtueAuthVariant(
}
OnboardingViewEvents.OnAccountCreated -> onAccountCreated()
OnboardingViewEvents.OnAccountSignedIn -> onAccountSignedIn()
OnboardingViewEvents.OnPersonalizeProfile -> TODO()
OnboardingViewEvents.OnPersonalizeProfile -> onPersonalizeProfile()
OnboardingViewEvents.OnTakeMeHome -> navigateToHome(createdAccount = true)
OnboardingViewEvents.OnDisplayNameUpdated -> onDisplayNameUpdated()
OnboardingViewEvents.OnDisplayNameSkipped -> onDisplayNameUpdated()
}.exhaustive
}

@@ -389,4 +391,16 @@ class FtueAuthVariant(
activity.startActivity(intent)
activity.finish()
}

private fun onPersonalizeProfile() {
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthChooseDisplayNameFragment::class.java,
option = commonOption
)
}

private fun onDisplayNameUpdated() {
// TODO go to the real profile picture fragment
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will be the next PR

navigateToHome(createdAccount = true)
}
}
5 changes: 4 additions & 1 deletion vector/src/main/res/layout/activity_login.xml
Original file line number Diff line number Diff line change
@@ -7,13 +7,16 @@
android:layout_height="match_parent">

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/loginContainer"
android:layout_width="match_parent"
android:layout_height="match_parent">

<androidx.fragment.app.FragmentContainerView
android:id="@+id/loginFragmentContainer"
android:layout_width="match_parent"
android:layout_height="match_parent" />
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<androidx.constraintlayout.widget.Group
android:id="@+id/loginLoading"
151 changes: 151 additions & 0 deletions vector/src/main/res/layout/fragment_ftue_display_name.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.core.widget.NestedScrollView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
style="@style/LoginFormScrollView"
android:layout_height="match_parent"
android:background="?android:colorBackground"
android:fillViewport="true"
android:paddingTop="0dp"
android:paddingBottom="0dp">

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">

<androidx.constraintlayout.widget.Guideline
android:id="@+id/displayNameGutterStart"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:orientation="vertical"
app:layout_constraintGuide_percent="@dimen/ftue_auth_gutter_start_percent" />

<androidx.constraintlayout.widget.Guideline
android:id="@+id/displayNameGutterEnd"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:orientation="vertical"
app:layout_constraintGuide_percent="@dimen/ftue_auth_gutter_end_percent" />

<Space
android:id="@+id/headerSpacing"
android:layout_width="match_parent"
android:layout_height="52dp"
app:layout_constraintBottom_toTopOf="@id/displayNameHeaderIcon"
app:layout_constraintTop_toTopOf="parent" />

<ImageView
android:id="@+id/displayNameHeaderIcon"
android:layout_width="wrap_content"
android:layout_height="0dp"
android:adjustViewBounds="true"
android:contentDescription="@null"
android:src="@drawable/ic_user_round"
app:layout_constraintBottom_toTopOf="@id/displayNameHeaderTitle"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintHeight_percent="0.15"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toTopOf="parent"
app:tint="?colorSecondary" />

<TextView
android:id="@+id/displayNameHeaderTitle"
style="@style/Widget.Vector.TextView.Title.Medium"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="16dp"
android:gravity="center"
android:text="@string/ftue_display_name_title"
android:textColor="?vctr_content_primary"
app:layout_constraintBottom_toTopOf="@id/displayNameHeaderSubtitle"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/displayNameHeaderIcon" />

<TextView
android:id="@+id/displayNameHeaderSubtitle"
style="@style/Widget.Vector.TextView.Subtitle"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:gravity="center"
android:text="@string/ftue_display_name_subtitle"
android:textColor="?vctr_content_secondary"
app:layout_constraintBottom_toTopOf="@id/titleContentSpacing"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/displayNameHeaderTitle" />

<Space
android:id="@+id/titleContentSpacing"
android:layout_width="match_parent"
android:layout_height="0dp"
app:layout_constraintBottom_toTopOf="@id/displayNameInput"
app:layout_constraintHeight_percent="0.03"
app:layout_constraintTop_toBottomOf="@id/displayNameHeaderSubtitle" />

<com.google.android.material.textfield.TextInputLayout
android:id="@+id/displayNameInput"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:hint="@string/ftue_display_name_entry_title"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/titleContentSpacing">

<com.google.android.material.textfield.TextInputEditText
android:layout_width="match_parent"
android:layout_height="match_parent"
android:maxLines="1" />
Copy link
Member

Choose a reason for hiding this comment

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

Could be nice to set an inputType and android:imeOptions="actionDone"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great idea, will do 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added 6fb5d05


</com.google.android.material.textfield.TextInputLayout>

<TextView
android:id="@+id/displayNameEntryFooter"
style="@style/Widget.Vector.TextView.Micro"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:text="@string/ftue_display_name_entry_footer"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/displayNameInput" />

<Space
android:id="@+id/actionsSpacing"
android:layout_width="match_parent"
android:layout_height="0dp"
app:layout_constraintBottom_toTopOf="@id/displayNameSubmit"
app:layout_constraintHeight_percent="0.03"
app:layout_constraintTop_toBottomOf="@id/displayNameEntryFooter"
app:layout_constraintVertical_bias="0"
app:layout_constraintVertical_chainStyle="packed" />

<Button
android:id="@+id/displayNameSubmit"
style="@style/Widget.Vector.Button.Login"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:text="@string/ftue_personalize_submit"
android:textAllCaps="true"
app:layout_constraintBottom_toTopOf="@id/displayNameSkip"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/actionsSpacing" />

<Button
android:id="@+id/displayNameSkip"
style="@style/Widget.Vector.Button.Text.Login"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:text="@string/ftue_personalize_skip_this_step"
android:textAllCaps="true"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="@id/displayNameGutterEnd"
app:layout_constraintStart_toStartOf="@id/displayNameGutterStart"
app:layout_constraintTop_toBottomOf="@id/displayNameSubmit" />

</androidx.constraintlayout.widget.ConstraintLayout>

</androidx.core.widget.NestedScrollView>


8 changes: 8 additions & 0 deletions vector/src/main/res/values/donottranslate.xml
Original file line number Diff line number Diff line change
@@ -22,4 +22,12 @@
<string name="ftue_account_created_take_me_home" translatable="false">Take me home</string>
<string name="ftue_account_created_congratulations_title" translatable="false">Congratulations!</string>
<string name="ftue_account_created_subtitle" translatable="false">Your account %s has been created.</string>

<string name="ftue_display_name_title" translatable="false">Choose a display name</string>
<string name="ftue_display_name_subtitle" translatable="false">This will be shown when you send messages.</string>
<string name="ftue_display_name_entry_title" translatable="false">Display Name</string>
<string name="ftue_display_name_entry_footer" translatable="false">You can change this later</string>

<string name="ftue_personalize_submit" translatable="false">Save and continue</string>
<string name="ftue_personalize_skip_this_step" translatable="false">Skip this step</string>
</resources>
Loading