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 - Capability based personalisation flow #5375

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4a1bf11
adding base choose name fragment with UI
ouchadam Feb 9, 2022
074cde4
add click handling for the display name actions
ouchadam Feb 10, 2022
1c80914
adding tests around the onboarding view model
ouchadam Feb 10, 2022
232524d
adding barebones profile picture fragment with ability to select a us…
ouchadam Feb 16, 2022
567fd9a
extracting method for the handling of the profile picture selection
ouchadam Feb 18, 2022
6c4381f
adding dedicated camera icon for choosing profile picture
ouchadam Feb 18, 2022
0685b57
add click handling for the display name actions
ouchadam Feb 10, 2022
7ded900
add click handling for the display name actions
ouchadam Feb 10, 2022
10e4fd1
updating upstream avatar on profile picture save and continue step
ouchadam Feb 18, 2022
50740b1
extracting method for the handling of the profile picture selection
ouchadam Feb 18, 2022
3df4f1e
adding entry points for injecting and overriding the homeserver capab…
ouchadam Feb 24, 2022
537c2f5
dynamically changing the account created layout based on if the homes…
ouchadam Feb 24, 2022
a033243
adding test around account creation via dummy
ouchadam Feb 25, 2022
46be481
hiding the toolbar back button and handling system back as take the u…
ouchadam Feb 25, 2022
716928d
dynamically switching the onboarding flow based on the capabilities o…
ouchadam Feb 25, 2022
b5778bd
formatting
ouchadam Feb 25, 2022
4b9b177
forwarding to the profile picture flow when display name changing isn…
ouchadam Feb 25, 2022
ab9e440
using correct label for the avatar capability debug override
ouchadam Feb 28, 2022
7e79d7e
adding changelog entry
ouchadam Feb 28, 2022
bdedffb
taking the personalization feature flag into account when calculating…
ouchadam Mar 4, 2022
edee6ab
using consistent method naming for setting the capabilities override
ouchadam Mar 10, 2022
d89cc71
making use of binding api instead of manual findviewbyid
ouchadam Mar 10, 2022
c2fe669
extracting the personalization complete emitting to a dedicated function
ouchadam Mar 10, 2022
c84ce5a
making use of the fake overrides for testing
ouchadam Mar 10, 2022
c06d3ff
Updating changelog copy
ouchadam Mar 10, 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
1 change: 1 addition & 0 deletions changelog.d/5375.wip
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Dynamically showing/hiding onboarding personalisation screens based on the users homeserver capabilities
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,17 @@ import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.booleanPreferencesKey
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.preferencesDataStore
import im.vector.app.features.HomeserverCapabilitiesOverride
import im.vector.app.features.VectorOverrides
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.map
import org.matrix.android.sdk.api.extensions.orFalse

private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "vector_overrides")
private val keyForceDialPadDisplay = booleanPreferencesKey("force_dial_pad_display")
private val keyForceLoginFallback = booleanPreferencesKey("force_login_fallback")
private val forceCanChangeDisplayName = booleanPreferencesKey("force_can_change_display_name")
Copy link
Contributor

Choose a reason for hiding this comment

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

I have never used DataStore API but I see this so much cleaner than the SharedPreferences API. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the main "downside" is that the api forces us to use suspend functions or flows (maybe a good thing from a performance standpoint 😄 )

private val forceCanChangeAvatar = booleanPreferencesKey("force_can_change_avatar")

class DebugVectorOverrides(private val context: Context) : VectorOverrides {

Expand All @@ -40,6 +44,13 @@ class DebugVectorOverrides(private val context: Context) : VectorOverrides {
preferences[keyForceLoginFallback].orFalse()
}

override val forceHomeserverCapabilities = context.dataStore.data.map { preferences ->
HomeserverCapabilitiesOverride(
canChangeDisplayName = preferences[forceCanChangeDisplayName],
canChangeAvatar = preferences[forceCanChangeAvatar]
)
}

suspend fun setForceDialPadDisplay(force: Boolean) {
context.dataStore.edit { settings ->
settings[keyForceDialPadDisplay] = force
Expand All @@ -51,4 +62,18 @@ class DebugVectorOverrides(private val context: Context) : VectorOverrides {
settings[keyForceLoginFallback] = force
}
}

suspend fun setHomeserverCapabilities(block: HomeserverCapabilitiesOverride.() -> HomeserverCapabilitiesOverride) {
val capabilitiesOverride = block(forceHomeserverCapabilities.firstOrNull() ?: HomeserverCapabilitiesOverride(null, null))
context.dataStore.edit { settings ->
when (capabilitiesOverride.canChangeDisplayName) {
null -> settings.remove(forceCanChangeDisplayName)
else -> settings[forceCanChangeDisplayName] = capabilitiesOverride.canChangeDisplayName
}
when (capabilitiesOverride.canChangeAvatar) {
null -> settings.remove(forceCanChangeAvatar)
else -> settings[forceCanChangeAvatar] = capabilitiesOverride.canChangeAvatar
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ class DebugPrivateSettingsFragment : VectorBaseFragment<FragmentDebugPrivateSett

override fun invalidate() = withState(viewModel) {
views.forceDialPadTabDisplay.isChecked = it.dialPadVisible
views.forceChangeDisplayNameCapability.bind(it.homeserverCapabilityOverrides.displayName) { option ->
viewModel.handle(DebugPrivateSettingsViewActions.SetDisplayNameCapabilityOverride(option))
}
views.forceChangeAvatarCapability.bind(it.homeserverCapabilityOverrides.avatar) { option ->
viewModel.handle(DebugPrivateSettingsViewActions.SetAvatarCapabilityOverride(option))
}
views.forceLoginFallback.isChecked = it.forceLoginFallback
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ package im.vector.app.features.debug.settings

import im.vector.app.core.platform.VectorViewModelAction

sealed class DebugPrivateSettingsViewActions : VectorViewModelAction {
data class SetDialPadVisibility(val force: Boolean) : DebugPrivateSettingsViewActions()
data class SetForceLoginFallbackEnabled(val force: Boolean) : DebugPrivateSettingsViewActions()
sealed interface DebugPrivateSettingsViewActions : VectorViewModelAction {
data class SetDialPadVisibility(val force: Boolean) : DebugPrivateSettingsViewActions
data class SetForceLoginFallbackEnabled(val force: Boolean) : DebugPrivateSettingsViewActions
data class SetDisplayNameCapabilityOverride(val option: BooleanHomeserverCapabilitiesOverride?) : DebugPrivateSettingsViewActions
Copy link
Contributor

Choose a reason for hiding this comment

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

I am just wondering why is the option BooleanHomeserverCapabilitiesOverride a nullable? Is this a case we should handle? I thought it would be either enabled or disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's because the override allows the option to be both forced to disabled or enabled, null means don't override

2022-03-10T16:43:53,379476984+00:00

we could replace the 3 way dropdown with a checkbox wrapped with another checkbox but I felt using nullable would be less boilerplate~

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay it makes sense. I forgot it is an override setting.

data class SetAvatarCapabilityOverride(val option: BooleanHomeserverCapabilitiesOverride?) : DebugPrivateSettingsViewActions
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import im.vector.app.core.di.MavericksAssistedViewModelFactory
import im.vector.app.core.di.hiltMavericksViewModelFactory
import im.vector.app.core.extensions.exhaustive
import im.vector.app.core.platform.EmptyViewEvents
import im.vector.app.core.platform.VectorViewModel
import im.vector.app.features.debug.features.DebugVectorOverrides
import im.vector.app.features.debug.settings.DebugPrivateSettingsViewActions.SetAvatarCapabilityOverride
import im.vector.app.features.debug.settings.DebugPrivateSettingsViewActions.SetDisplayNameCapabilityOverride
import kotlinx.coroutines.launch

class DebugPrivateSettingsViewModel @AssistedInject constructor(
Expand All @@ -40,10 +43,10 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor(
companion object : MavericksViewModelFactory<DebugPrivateSettingsViewModel, DebugPrivateSettingsViewState> by hiltMavericksViewModelFactory()

init {
observeVectorDataStore()
observeVectorOverrides()
}

private fun observeVectorDataStore() {
private fun observeVectorOverrides() {
debugVectorOverrides.forceDialPad.setOnEach {
copy(
dialPadVisible = it
Expand All @@ -52,13 +55,23 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor(
debugVectorOverrides.forceLoginFallback.setOnEach {
copy(forceLoginFallback = it)
}
debugVectorOverrides.forceHomeserverCapabilities.setOnEach {
val activeDisplayNameOption = BooleanHomeserverCapabilitiesOverride.from(it.canChangeDisplayName)
val activeAvatarOption = BooleanHomeserverCapabilitiesOverride.from(it.canChangeAvatar)
copy(homeserverCapabilityOverrides = homeserverCapabilityOverrides.copy(
displayName = homeserverCapabilityOverrides.displayName.copy(activeOption = activeDisplayNameOption),
avatar = homeserverCapabilityOverrides.avatar.copy(activeOption = activeAvatarOption),
))
}
}

override fun handle(action: DebugPrivateSettingsViewActions) {
when (action) {
is DebugPrivateSettingsViewActions.SetDialPadVisibility -> handleSetDialPadVisibility(action)
is DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled -> handleSetForceLoginFallbackEnabled(action)
}
is SetDisplayNameCapabilityOverride -> handSetDisplayNameCapabilityOverride(action)
is SetAvatarCapabilityOverride -> handSetAvatarCapabilityOverride(action)
}.exhaustive
}

private fun handleSetDialPadVisibility(action: DebugPrivateSettingsViewActions.SetDialPadVisibility) {
Expand All @@ -72,4 +85,18 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor(
debugVectorOverrides.setForceLoginFallback(action.force)
}
}

private fun handSetDisplayNameCapabilityOverride(action: SetDisplayNameCapabilityOverride) {
viewModelScope.launch {
val forceDisplayName = action.option.toBoolean()
debugVectorOverrides.setHomeserverCapabilities { copy(canChangeDisplayName = forceDisplayName) }
}
}

private fun handSetAvatarCapabilityOverride(action: SetAvatarCapabilityOverride) {
viewModelScope.launch {
val forceAvatar = action.option.toBoolean()
debugVectorOverrides.setHomeserverCapabilities { copy(canChangeAvatar = forceAvatar) }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,23 @@
package im.vector.app.features.debug.settings

import com.airbnb.mvrx.MavericksState
import im.vector.app.features.debug.settings.OverrideDropdownView.OverrideDropdown

data class DebugPrivateSettingsViewState(
val dialPadVisible: Boolean = false,
val forceLoginFallback: Boolean = false,
val homeserverCapabilityOverrides: HomeserverCapabilityOverrides = HomeserverCapabilityOverrides()
) : MavericksState

data class HomeserverCapabilityOverrides(
val displayName: OverrideDropdown<BooleanHomeserverCapabilitiesOverride> = OverrideDropdown(
label = "Override display name capability",
activeOption = null,
options = listOf(BooleanHomeserverCapabilitiesOverride.ForceEnabled, BooleanHomeserverCapabilitiesOverride.ForceDisabled)
),
val avatar: OverrideDropdown<BooleanHomeserverCapabilitiesOverride> = OverrideDropdown(
label = "Override avatar capability",
activeOption = null,
options = listOf(BooleanHomeserverCapabilitiesOverride.ForceEnabled, BooleanHomeserverCapabilitiesOverride.ForceDisabled)
)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* 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.debug.settings

import android.content.Context
import android.util.AttributeSet
import android.view.Gravity
import android.view.LayoutInflater
import android.view.View
import android.widget.AdapterView
import android.widget.ArrayAdapter
import android.widget.LinearLayout
import im.vector.app.R
import im.vector.app.databinding.ViewBooleanDropdownBinding

class OverrideDropdownView @JvmOverloads constructor(
Copy link
Contributor Author

@ouchadam ouchadam Mar 10, 2022

Choose a reason for hiding this comment

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

a reusable dropdown view for the Debug menu -> Private settings (in the debug sourceset)

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! 👍 I am a big fan of custom views!

context: Context,
attrs: AttributeSet? = null
) : LinearLayout(context, attrs) {

private val binding = ViewBooleanDropdownBinding.inflate(
LayoutInflater.from(context),
this
)

init {
orientation = HORIZONTAL
gravity = Gravity.CENTER_VERTICAL
}

fun <T : OverrideOption> bind(feature: OverrideDropdown<T>, listener: Listener<T>) {
binding.overrideLabel.text = feature.label

binding.overrideOptions.apply {
val arrayAdapter = ArrayAdapter<String>(context, android.R.layout.simple_spinner_dropdown_item)
val options = listOf("Inactive") + feature.options.map { it.label }
arrayAdapter.addAll(options)
adapter = arrayAdapter

feature.activeOption?.let {
setSelection(options.indexOf(it.label), false)
}

onItemSelectedListener = object : AdapterView.OnItemSelectedListener {
override fun onItemSelected(parent: AdapterView<*>?, view: View?, position: Int, id: Long) {
when (position) {
0 -> listener.onOverrideSelected(option = null)
else -> listener.onOverrideSelected(feature.options[position - 1])
}
}

override fun onNothingSelected(parent: AdapterView<*>?) {
// do nothing
}
}
}
}

fun interface Listener<T> {
fun onOverrideSelected(option: T?)
}

data class OverrideDropdown<T : OverrideOption>(
val label: String,
val options: List<T>,
val activeOption: T?,
)
}

interface OverrideOption {
val label: String
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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.debug.settings

sealed interface BooleanHomeserverCapabilitiesOverride : OverrideOption {

companion object {
fun from(value: Boolean?) = when (value) {
null -> null
true -> ForceEnabled
false -> ForceDisabled
}
}

object ForceEnabled : BooleanHomeserverCapabilitiesOverride {
override val label = "Force enabled"
}

object ForceDisabled : BooleanHomeserverCapabilitiesOverride {
override val label = "Force disabled"
}
}

fun BooleanHomeserverCapabilitiesOverride?.toBoolean() = when (this) {
null -> null
BooleanHomeserverCapabilitiesOverride.ForceDisabled -> false
BooleanHomeserverCapabilitiesOverride.ForceEnabled -> true
}
18 changes: 18 additions & 0 deletions vector/src/debug/res/layout/fragment_debug_private_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@
android:layout_height="wrap_content"
android:text="Force login and registration fallback" />

<im.vector.app.features.debug.settings.OverrideDropdownView
android:id="@+id/forceChangeDisplayNameCapability"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginTop="4dp"
android:layout_marginEnd="16dp"
android:layout_marginBottom="4dp" />

<im.vector.app.features.debug.settings.OverrideDropdownView
android:id="@+id/forceChangeAvatarCapability"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginTop="4dp"
android:layout_marginEnd="16dp"
android:layout_marginBottom="4dp" />

</LinearLayout>

</ScrollView>
Expand Down
25 changes: 25 additions & 0 deletions vector/src/debug/res/layout/view_boolean_dropdown.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?xml version="1.0" encoding="utf-8"?>
<merge xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:parentTag="android.widget.LinearLayout">

<TextView
android:id="@+id/overrideLabel"
style="@style/Widget.Vector.TextView.Subtitle"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="8dp"
android:layout_weight="1"
android:gravity="center"
android:textColor="?vctr_content_primary"
tools:text="Login version" />

<androidx.appcompat.widget.AppCompatSpinner
android:id="@+id/overrideOptions"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_weight="1" />

</merge>
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,16 @@ import kotlinx.coroutines.flow.flowOf
interface VectorOverrides {
val forceDialPad: Flow<Boolean>
val forceLoginFallback: Flow<Boolean>
val forceHomeserverCapabilities: Flow<HomeserverCapabilitiesOverride>?
}

data class HomeserverCapabilitiesOverride(
val canChangeDisplayName: Boolean?,
val canChangeAvatar: Boolean?
)

class DefaultVectorOverrides : VectorOverrides {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a little confused about the purpose of DefaultVectorOverrides class. I see it is only used for testing purpose. Am I right? If it is the case, we may move it to the FakeVectorOverrides which is currently not used. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the missing piece of the puzzle here is that the default implementations are injected by the release variant of the app release module whereas the debug module allows for providing the runtime overridable version

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay thanks for explaining.

override val forceDialPad = flowOf(false)
override val forceLoginFallback = flowOf(false)
override val forceHomeserverCapabilities: Flow<HomeserverCapabilitiesOverride>? = null
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ sealed class OnboardingAction : VectorViewModelAction {

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

object PersonalizeProfile : OnboardingAction()
data class UpdateDisplayName(val displayName: String) : OnboardingAction()
object UpdateDisplayNameSkipped : OnboardingAction()
data class ProfilePictureSelected(val uri: Uri) : OnboardingAction()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ sealed class OnboardingViewEvents : VectorViewEvents {
object OnAccountCreated : OnboardingViewEvents()
object OnAccountSignedIn : OnboardingViewEvents()
object OnTakeMeHome : OnboardingViewEvents()
object OnPersonalizeProfile : OnboardingViewEvents()
object OnDisplayNameUpdated : OnboardingViewEvents()
object OnDisplayNameSkipped : OnboardingViewEvents()
object OnChooseDisplayName : OnboardingViewEvents()
object OnChooseProfilePicture : OnboardingViewEvents()
object OnPersonalizationComplete : OnboardingViewEvents()
object OnBack : OnboardingViewEvents()
}
Loading