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 picture #5323

Merged
merged 20 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6b62b5b
adding tests around the onboarding view model
ouchadam Feb 10, 2022
53e9e8f
adding base choose name fragment with UI
ouchadam Feb 9, 2022
281b65c
add click handling for the display name actions
ouchadam Feb 10, 2022
8172589
adding tests around the onboarding view model
ouchadam Feb 10, 2022
c4dc874
adding barebones profile picture fragment with ability to select a us…
ouchadam Feb 16, 2022
05394a9
extracting uri filename resolving to a class which can be injected
ouchadam Feb 17, 2022
dcb16dd
updating upstream avatar on profile picture save and continue step
ouchadam Feb 18, 2022
bcf84b8
adding test case for skipping profile picture setting
ouchadam Feb 18, 2022
d3589fb
taking the profile loading into account when rendering the onboarding…
ouchadam Feb 18, 2022
479bff3
extracting method for the handling of the profile picture selection
ouchadam Feb 18, 2022
efb08b1
adding dedicated camera icon for choosing profile picture
ouchadam Feb 18, 2022
146ad6f
adding toolbar to back to profile picture page
ouchadam Feb 18, 2022
9518581
changing edit/add picture icon based on if we're already selected an …
ouchadam Feb 18, 2022
989e762
making use of debounced clicks to avoid potential extra clicks
ouchadam Feb 22, 2022
c934fdc
making the avatar height and camera icon relative percentage based
ouchadam Feb 23, 2022
aa3f1b0
fixing formatting
ouchadam Feb 23, 2022
6b04efd
making use of fake session id for user id assertion
ouchadam Feb 23, 2022
deec923
using a real matrix id syntax for the fake session user id
ouchadam Mar 4, 2022
1fc1935
removing duplicated dimens
ouchadam Mar 4, 2022
3a2dea2
using self closing imageview tag
ouchadam Mar 4, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
package im.vector.lib.multipicker.utils

import android.database.Cursor
import androidx.core.database.getStringOrNull

fun Cursor.getColumnIndexOrNull(column: String): Int? {
return getColumnIndex(column).takeIf { it != -1 }
}

fun Cursor.readStringColumnOrNull(column: String): String? {
return getColumnIndexOrNull(column)?.let { getStringOrNull(it) }
}
5 changes: 5 additions & 0 deletions library/ui-styles/src/main/res/values-h480dp/dimens.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<item name="ftue_auth_profile_picture_height" format="float" type="dimen">0.15</item>
<item name="ftue_auth_profile_picture_icon_height" format="float" type="dimen">0.05</item>
Copy link
Member

Choose a reason for hiding this comment

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

Those 2 values are the same than in default resource, is it a mistake?

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 great catch 👀 ! they can be deleted, they were separated whilst checking with design about the different screen sizes, we chose to maintain the same % of screen for small screens and allow the entire avatar to be clickable instead of making the image bigger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

</resources>
3 changes: 3 additions & 0 deletions library/ui-styles/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@

<item name="ftue_auth_carousel_item_spacing" format="float" type="dimen">0.01</item>
<item name="ftue_auth_carousel_item_image_height" format="float" type="dimen">0.35</item>

<item name="ftue_auth_profile_picture_height" format="float" type="dimen">0.15</item>
<item name="ftue_auth_profile_picture_icon_height" format="float" type="dimen">0.05</item>
</resources>
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
Expand Up @@ -100,6 +100,7 @@ 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.FtueAuthChooseProfilePictureFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthGenericTextInputFormFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthLoginFragment
import im.vector.app.features.onboarding.ftueauth.FtueAuthResetPasswordFragment
Expand Down Expand Up @@ -485,6 +486,11 @@ interface FragmentModule {
@FragmentKey(FtueAuthChooseDisplayNameFragment::class)
fun bindFtueAuthChooseDisplayNameFragment(fragment: FtueAuthChooseDisplayNameFragment): Fragment

@Binds
@IntoMap
@FragmentKey(FtueAuthChooseProfilePictureFragment::class)
fun bindFtueAuthChooseProfilePictureFragment(fragment: FtueAuthChooseProfilePictureFragment): Fragment

@Binds
@IntoMap
@FragmentKey(UserListFragment::class)
Expand Down
12 changes: 12 additions & 0 deletions vector/src/main/java/im/vector/app/features/home/AvatarRenderer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.home

import android.graphics.Bitmap
import android.graphics.drawable.Drawable
import android.net.Uri
import android.widget.ImageView
import androidx.annotation.AnyThread
import androidx.annotation.ColorInt
Expand Down Expand Up @@ -48,6 +49,7 @@ import org.matrix.android.sdk.api.auth.login.LoginProfileInfo
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.content.ContentUrlResolver
import org.matrix.android.sdk.api.util.MatrixItem
import java.io.File
import javax.inject.Inject

/**
Expand Down Expand Up @@ -100,6 +102,16 @@ class AvatarRenderer @Inject constructor(private val activeSessionHolder: Active
DrawableImageViewTarget(imageView))
}

@UiThread
fun render(matrixItem: MatrixItem, localUri: Uri?, imageView: ImageView) {
val placeholder = getPlaceholderDrawable(matrixItem)
GlideApp.with(imageView)
.load(localUri?.let { File(localUri.path!!) })
.apply(RequestOptions.circleCropTransform())
.placeholder(placeholder)
.into(imageView)
}

@UiThread
fun render(mappedContact: MappedContact, imageView: ImageView) {
// Create a Fake MatrixItem, for the placeholder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package im.vector.app.features.onboarding

import android.net.Uri
import im.vector.app.core.platform.VectorViewModelAction
import im.vector.app.features.login.LoginConfig
import im.vector.app.features.login.ServerType
Expand Down Expand Up @@ -76,4 +77,7 @@ sealed class OnboardingAction : VectorViewModelAction {

data class UpdateDisplayName(val displayName: String) : OnboardingAction()
object UpdateDisplayNameSkipped : OnboardingAction()
data class ProfilePictureSelected(val uri: Uri) : OnboardingAction()
object SaveSelectedProfilePicture : OnboardingAction()
object UpdateProfilePictureSkipped : OnboardingAction()
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,6 @@ sealed class OnboardingViewEvents : VectorViewEvents {
object OnPersonalizeProfile : OnboardingViewEvents()
object OnDisplayNameUpdated : OnboardingViewEvents()
object OnDisplayNameSkipped : OnboardingViewEvents()
object OnPersonalizationComplete : OnboardingViewEvents()
object OnBack : OnboardingViewEvents()
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import org.matrix.android.sdk.api.failure.Failure
import org.matrix.android.sdk.api.failure.MatrixIdFailure
import org.matrix.android.sdk.api.session.Session
import timber.log.Timber
import java.util.UUID
import java.util.concurrent.CancellationException

/**
Expand All @@ -80,6 +81,7 @@ class OnboardingViewModel @AssistedInject constructor(
private val homeServerHistoryService: HomeServerHistoryService,
private val vectorFeatures: VectorFeatures,
private val analyticsTracker: AnalyticsTracker,
private val uriFilenameResolver: UriFilenameResolver,
private val vectorOverrides: VectorOverrides
) : VectorViewModel<OnboardingViewState, OnboardingAction, OnboardingViewEvents>(initialState) {

Expand Down Expand Up @@ -157,6 +159,9 @@ class OnboardingViewModel @AssistedInject constructor(
is OnboardingAction.PostViewEvent -> _viewEvents.post(action.viewEvent)
is OnboardingAction.UpdateDisplayName -> updateDisplayName(action.displayName)
OnboardingAction.UpdateDisplayNameSkipped -> _viewEvents.post(OnboardingViewEvents.OnDisplayNameSkipped)
OnboardingAction.UpdateProfilePictureSkipped -> _viewEvents.post(OnboardingViewEvents.OnPersonalizationComplete)
is OnboardingAction.ProfilePictureSelected -> handleProfilePictureSelected(action)
OnboardingAction.SaveSelectedProfilePicture -> updateProfilePicture()
}.exhaustive
}

Expand Down Expand Up @@ -899,14 +904,59 @@ class OnboardingViewModel @AssistedInject constructor(
val activeSession = activeSessionHolder.getActiveSession()
try {
activeSession.setDisplayName(activeSession.myUserId, displayName)
setState { copy(asyncDisplayName = Success(Unit)) }
setState {
copy(
asyncDisplayName = Success(Unit),
Copy link
Member

Choose a reason for hiding this comment

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

Can't it be asyncDisplayName = Success(displayName) instead of storing the value in personalizationState. The remark is also applicable to selectedPictureUri I guess.

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 is a good point, at the moment those asyncs are only being used for showing/hiding the loading view, in Login2 they're replaced with a single isLoading: Boolean, I was thinking of doing the same but wanted to do all the asyncs at the same time

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds like a plan! OK for me. What I want to avoid is having multiple sources of truth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will raise a PR for it 💯

personalizationState = personalizationState.copy(displayName = displayName)
)
}
_viewEvents.post(OnboardingViewEvents.OnDisplayNameUpdated)
} catch (error: Throwable) {
setState { copy(asyncDisplayName = Fail(error)) }
_viewEvents.post(OnboardingViewEvents.Failure(error))
}
}
}

private fun handleProfilePictureSelected(action: OnboardingAction.ProfilePictureSelected) {
setState {
copy(personalizationState = personalizationState.copy(selectedPictureUri = action.uri))
}
}

private fun updateProfilePicture() {
withState { state ->
when (val pictureUri = state.personalizationState.selectedPictureUri) {
null -> _viewEvents.post(OnboardingViewEvents.Failure(NullPointerException("picture uri is missing from state")))
else -> {
setState { copy(asyncProfilePicture = Loading()) }
viewModelScope.launch {
val activeSession = activeSessionHolder.getActiveSession()
try {
activeSession.updateAvatar(
activeSession.myUserId,
pictureUri,
uriFilenameResolver.getFilenameFromUri(pictureUri) ?: UUID.randomUUID().toString()
)
setState {
copy(
asyncProfilePicture = Success(Unit),
)
}
onProfilePictureSaved()
} catch (error: Throwable) {
setState { copy(asyncProfilePicture = Fail(error)) }
_viewEvents.post(OnboardingViewEvents.Failure(error))
}
}
}
}
}
}

private fun onProfilePictureSaved() {
_viewEvents.post(OnboardingViewEvents.OnPersonalizationComplete)
}
}

private fun LoginMode.supportsSignModeScreen(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package im.vector.app.features.onboarding

import android.net.Uri
import android.os.Parcelable
import com.airbnb.mvrx.Async
import com.airbnb.mvrx.Loading
import com.airbnb.mvrx.MavericksState
Expand All @@ -25,6 +27,7 @@ import com.airbnb.mvrx.Uninitialized
import im.vector.app.features.login.LoginMode
import im.vector.app.features.login.ServerType
import im.vector.app.features.login.SignMode
import kotlinx.parcelize.Parcelize

data class OnboardingViewState(
val asyncLoginAction: Async<Unit> = Uninitialized,
Expand All @@ -33,6 +36,7 @@ data class OnboardingViewState(
val asyncResetMailConfirmed: Async<Unit> = Uninitialized,
val asyncRegistration: Async<Unit> = Uninitialized,
val asyncDisplayName: Async<Unit> = Uninitialized,
val asyncProfilePicture: Async<Unit> = Uninitialized,

@PersistState
val onboardingFlow: OnboardingFlow? = null,
Expand Down Expand Up @@ -65,6 +69,9 @@ data class OnboardingViewState(
val loginModeSupportedTypes: List<String> = emptyList(),
val knownCustomHomeServersUrls: List<String> = emptyList(),
val isForceLoginFallbackEnabled: Boolean = false,

@PersistState
val personalizationState: PersonalizationState = PersonalizationState()
) : MavericksState {

fun isLoading(): Boolean {
Expand All @@ -73,7 +80,8 @@ data class OnboardingViewState(
asyncResetPassword is Loading ||
asyncResetMailConfirmed is Loading ||
asyncRegistration is Loading ||
asyncDisplayName is Loading
asyncDisplayName is Loading ||
asyncProfilePicture is Loading
}

fun isAuthTaskCompleted(): Boolean {
Expand All @@ -86,3 +94,9 @@ enum class OnboardingFlow {
SignUp,
SignInSignUp
}

@Parcelize
data class PersonalizationState(
val displayName: String? = null,
val selectedPictureUri: Uri? = null
) : Parcelable
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (c) 2022 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

import android.content.Context
import android.net.Uri
import android.provider.OpenableColumns
import im.vector.lib.multipicker.utils.readStringColumnOrNull
import javax.inject.Inject

class UriFilenameResolver @Inject constructor(private val context: Context) {

fun getFilenameFromUri(uri: Uri): String? {
val fallback = uri.path?.substringAfterLast('/')
return when (uri.scheme) {
"content" -> readResolvedDisplayName(uri) ?: fallback
else -> fallback
}
}

private fun readResolvedDisplayName(uri: Uri): String? {
return context.contentResolver.query(uri, null, null, null, null)?.use { cursor ->
cursor.takeIf { cursor.moveToFirst() }
?.readStringColumnOrNull(OpenableColumns.DISPLAY_NAME)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ 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 im.vector.app.features.onboarding.OnboardingViewState
import javax.inject.Inject

class FtueAuthChooseDisplayNameFragment @Inject constructor() : AbstractFtueAuthFragment<FragmentFtueDisplayNameBinding>() {
Expand All @@ -41,7 +42,6 @@ class FtueAuthChooseDisplayNameFragment @Inject constructor() : AbstractFtueAuth
}

private fun setupViews() {
views.displayNameSubmit.isEnabled = views.displayNameInput.hasContentEmpty()
views.displayNameInput.editText?.addTextChangedListener(object : SimpleTextWatcher() {
override fun afterTextChanged(s: Editable) {
val newContent = s.toString()
Expand All @@ -58,10 +58,7 @@ class FtueAuthChooseDisplayNameFragment @Inject constructor() : AbstractFtueAuth
}
}

views.displayNameSubmit.debouncedClicks {
updateDisplayName()
}

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

Expand All @@ -70,6 +67,11 @@ class FtueAuthChooseDisplayNameFragment @Inject constructor() : AbstractFtueAuth
viewModel.handle(OnboardingAction.UpdateDisplayName(newDisplayName))
}

override fun updateWithState(state: OnboardingViewState) {
views.displayNameInput.editText?.setText(state.personalizationState.displayName)
views.displayNameSubmit.isEnabled = views.displayNameInput.hasContentEmpty()
}

override fun resetViewModel() {
// Nothing to do
}
Expand Down
Loading