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

feat: app lock UI and UX adjustments [WPB-4695] #2335

Merged
merged 17 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import kotlinx.coroutines.flow.map
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds

@Singleton
class ObserveAppLockConfigUseCase @Inject constructor(
Expand All @@ -37,12 +39,12 @@
}
}

sealed class AppLockConfig(open val timeoutInSeconds: Int = DEFAULT_TIMEOUT) {
sealed class AppLockConfig(open val timeout: Duration = DEFAULT_TIMEOUT) {
data object Disabled : AppLockConfig()
data object Enabled : AppLockConfig()
data class EnforcedByTeam(override val timeoutInSeconds: Int) : AppLockConfig(timeoutInSeconds)
data class EnforcedByTeam(override val timeout: Duration) : AppLockConfig(timeout)

Check warning on line 45 in app/src/main/kotlin/com/wire/android/feature/ObserveAppLockConfigUseCase.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/com/wire/android/feature/ObserveAppLockConfigUseCase.kt#L45

Added line #L45 was not covered by tests

companion object {
const val DEFAULT_TIMEOUT = 60
val DEFAULT_TIMEOUT = 60.seconds
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ private fun NavOptionsBuilder.popUpTo(
getNavBackStackEntry: () -> NavBackStackEntry?,
) {
getNavBackStackEntry()?.let { entry ->
appLogger.d("[$TAG] -> popUpTo:${entry.destination.route?.obfuscateId()} inclusive:${getInclusive(entry)}")
popUpTo(entry.destination.id) {
this.inclusive = getInclusive(entry)
}
Expand Down
12 changes: 4 additions & 8 deletions app/src/main/kotlin/com/wire/android/ui/WireActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
setUpNavigation(navigator.navController, onComplete, scope)
isLoaded = true
handleScreenshotCensoring()
handleAppLock()
handleAppLock(navigator::navigate)

Check warning on line 190 in app/src/main/kotlin/com/wire/android/ui/WireActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/com/wire/android/ui/WireActivity.kt#L190

Added line #L190 was not covered by tests
handleDialogs(navigator::navigate)
}
}
Expand Down Expand Up @@ -239,7 +239,7 @@
}

@Composable
private fun handleAppLock() {
private fun handleAppLock(navigate: (NavigationCommand) -> Unit) {
LaunchedEffect(Unit) {
lockCodeTimeManager.isLocked()
.filter { it }
Expand All @@ -249,13 +249,9 @@
.canAuthenticate(BiometricManager.Authenticators.BIOMETRIC_STRONG)

if (canAuthenticateWithBiometrics == BiometricManager.BIOMETRIC_SUCCESS) {
navigationCommands.emit(
NavigationCommand(AppUnlockWithBiometricsScreenDestination)
)
navigate(NavigationCommand(AppUnlockWithBiometricsScreenDestination, BackStackMode.UPDATE_EXISTED))

Check warning on line 252 in app/src/main/kotlin/com/wire/android/ui/WireActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/com/wire/android/ui/WireActivity.kt#L252

Added line #L252 was not covered by tests
} else {
navigationCommands.emit(
NavigationCommand(EnterLockCodeScreenDestination)
)
navigate(NavigationCommand(EnterLockCodeScreenDestination, BackStackMode.UPDATE_EXISTED))

Check warning on line 254 in app/src/main/kotlin/com/wire/android/ui/WireActivity.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/com/wire/android/ui/WireActivity.kt#L254

Added line #L254 was not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class CreateAccountDetailsViewModel @Inject constructor(
detailsState = detailsState.copy(loading = true, continueEnabled = false)
viewModelScope.launch {
val detailsError = when {
!validatePasswordUseCase(detailsState.password.text) ->
!validatePasswordUseCase(detailsState.password.text).isValid ->
CreateAccountDetailsViewState.DetailsError.TextFieldError.InvalidPasswordError

detailsState.password.text != detailsState.confirmPassword.text ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fun AppUnlockWithBiometricsScreen(
navigator.navigate(
NavigationCommand(
EnterLockCodeScreenDestination(),
BackStackMode.CLEAR_WHOLE
BackStackMode.REMOVE_CURRENT
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,20 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.Icon
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Surface
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.res.vectorResource
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.testTagsAsResourceId
import androidx.compose.ui.text.input.ImeAction
Expand All @@ -46,16 +51,18 @@ import com.ramcosta.composedestinations.annotation.Destination
import com.ramcosta.composedestinations.annotation.RootNavGraph
import com.wire.android.R
import com.wire.android.navigation.Navigator
import com.wire.android.navigation.rememberNavigator
import com.wire.android.ui.common.button.WireButtonState
import com.wire.android.ui.common.button.WirePrimaryButton
import com.wire.android.ui.common.dimensions
import com.wire.android.ui.common.rememberBottomBarElevationState
import com.wire.android.ui.common.scaffold.WireScaffold
import com.wire.android.ui.common.textfield.WirePasswordTextField
import com.wire.android.ui.common.textfield.WireTextFieldState
import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar
import com.wire.android.ui.theme.WireTheme
import com.wire.android.ui.theme.wireColorScheme
import com.wire.android.ui.theme.wireDimensions
import com.wire.android.ui.theme.wireTypography
import com.wire.android.util.ui.PreviewMultipleThemes
import java.util.Locale

@RootNavGraph
Expand Down Expand Up @@ -94,19 +101,14 @@ fun EnterLockCodeScreenContent(
onBackPress()
}

WireScaffold(topBar = {
WireCenterAlignedTopAppBar(
onNavigationPressed = onBackPress,
elevation = dimensions().spacing0x,
title = stringResource(id = R.string.settings_enter_lock_screen_title)
)
}) { internalPadding ->
WireScaffold { internalPadding ->
Column(
modifier = Modifier
.fillMaxSize()
.padding(internalPadding)
) {
Column(
horizontalAlignment = Alignment.CenterHorizontally,
modifier = Modifier
.weight(weight = 1f, fill = true)
.verticalScroll(scrollState)
Expand All @@ -115,11 +117,27 @@ fun EnterLockCodeScreenContent(
testTagsAsResourceId = true
}
) {
Icon(
imageVector = ImageVector.vectorResource(id = R.drawable.ic_wire_logo),
tint = MaterialTheme.colorScheme.onBackground,
contentDescription = stringResource(id = R.string.content_description_welcome_wire_logo),
modifier = Modifier.padding(top = MaterialTheme.wireDimensions.spacing56x)
)

Text(
text = stringResource(id = R.string.settings_enter_lock_screen_title),
style = MaterialTheme.wireTypography.title02,
color = MaterialTheme.colorScheme.onBackground,
modifier = Modifier.padding(
top = MaterialTheme.wireDimensions.spacing32x,
bottom = MaterialTheme.wireDimensions.spacing56x
)
)

WirePasswordTextField(
value = state.password,
onValueChange = onPasswordChanged,
labelMandatoryIcon = true,
descriptionText = stringResource(R.string.create_account_details_password_description),
imeAction = ImeAction.Done,
modifier = Modifier
.testTag("password"),
Expand Down Expand Up @@ -174,3 +192,18 @@ private fun ContinueButton(
)
}
}

@Composable
@PreviewMultipleThemes
fun PreviewEnterLockCodeScreen() {
WireTheme(isPreview = true) {
EnterLockCodeScreenContent(
navigator = rememberNavigator {},
state = EnterLockCodeViewState(),
scrollState = rememberScrollState(),
onPasswordChanged = {},
onBackPress = {},
onContinue = {}
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class EnterLockScreenViewModel @Inject constructor(
error = EnterLockCodeError.None,
password = password
)
state = if (validatePassword(password.text)) {
state = if (validatePassword(password.text).isValid) {
state.copy(
continueEnabled = true,
isUnlockEnabled = true
Expand All @@ -65,7 +65,7 @@ class EnterLockScreenViewModel @Inject constructor(
state = state.copy(continueEnabled = false)
// the continue button is enabled iff the password is valid
// this check is for safety only
if (!validatePassword(state.password.text)) {
if (!validatePassword(state.password.text).isValid) {
state = state.copy(isUnlockEnabled = false)
} else {
viewModelScope.launch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package com.wire.android.ui.home.appLock

import com.wire.android.appLogger
import com.wire.android.di.ApplicationScope
import com.wire.android.feature.AppLockConfig
import com.wire.android.feature.ObserveAppLockConfigUseCase
Expand All @@ -27,6 +28,7 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.flatMapLatest
Expand All @@ -51,6 +53,7 @@ class LockCodeTimeManager @Inject constructor(
runBlocking {
observeAppLockConfigUseCase().firstOrNull()?.let { appLockConfig ->
if (appLockConfig !is AppLockConfig.Disabled) {
appLogger.i("$TAG app initially locked")
isLockedFlow.value = true
}
}
Expand All @@ -62,16 +65,23 @@ class LockCodeTimeManager @Inject constructor(
observeAppLockConfigUseCase(),
currentScreenManager.isAppVisibleFlow(),
::Pair
).flatMapLatest { (appLockConfig, isInForeground) ->
)
.distinctUntilChanged()
.flatMapLatest { (appLockConfig, isInForeground) ->
when {
appLockConfig is AppLockConfig.Disabled -> flowOf(false)

!isInForeground && !isLockedFlow.value -> flow {
delay(appLockConfig.timeoutInSeconds * 1000L)
appLogger.i("$TAG lock is enabled and app in the background, lock count started")
delay(appLockConfig.timeout.inWholeMilliseconds)
appLogger.i("$TAG lock count ended, app state is locked")
emit(true)
}

else -> emptyFlow()
else -> {
appLogger.i("$TAG no change to lock state, isInForeground: $isInForeground, isLocked: ${isLockedFlow.value}")
emptyFlow()
}
}
}.collectLatest {
isLockedFlow.value = it
Expand All @@ -80,8 +90,13 @@ class LockCodeTimeManager @Inject constructor(
}

fun appUnlocked() {
appLogger.i("$TAG app unlocked")
isLockedFlow.value = false
}

fun isLocked(): Flow<Boolean> = isLockedFlow

companion object {
private const val TAG = "LockCodeTimeManager"
}
}
Loading