Skip to content

Commit

Permalink
Disable login button until 10 char and clear error on typing.
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawa committed Sep 20, 2023
1 parent c4a6e91 commit b2837e8
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 47 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package net.mullvad.mullvadvpn.compose.screen

import androidx.compose.animation.animateContentSize
import androidx.compose.foundation.Image
import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.defaultMinSize
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.offset
Expand Down Expand Up @@ -45,7 +43,6 @@ import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.input.ImeAction
import androidx.compose.ui.text.input.KeyboardType
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.PopupProperties
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.ActionButton
Expand Down Expand Up @@ -92,10 +89,10 @@ private fun PreviewLoginSuccess() {
@Composable
fun LoginScreen(
state: LoginUiState,
initialAccountNumber: String = "",
onLoginClick: (String) -> Unit = {},
onCreateAccountClick: () -> Unit = {},
onDeleteHistoryClick: () -> Unit = {},
onAccountNumberChange: (String) -> Unit = {},
onSettingsClick: () -> Unit = {},
) {
ScaffoldWithTopBar(
Expand All @@ -114,15 +111,15 @@ fun LoginScreen(
.background(MaterialTheme.colorScheme.primary)
.verticalScroll(scrollState)
) {
Spacer(modifier = Modifier.weight(1f).defaultMinSize(0.dp))
Spacer(modifier = Modifier.weight(1f))
LoginIcon(
state.loginState,
modifier =
Modifier.align(Alignment.CenterHorizontally)
.padding(bottom = Dimens.largePadding)
)
LoginContent(state, initialAccountNumber, onLoginClick, onDeleteHistoryClick)
Spacer(modifier = Modifier.weight(3f).defaultMinSize(0.dp))
LoginContent(state, onAccountNumberChange, onLoginClick, onDeleteHistoryClick)
Spacer(modifier = Modifier.weight(3f))
CreateAccountPanel(onCreateAccountClick, isEnabled = state.loginState is Idle)
}
}
Expand All @@ -132,38 +129,39 @@ fun LoginScreen(
@OptIn(ExperimentalMaterial3Api::class)
private fun LoginContent(
state: LoginUiState,
initialAccountNumber: String,
onAccountNumberChange: (String) -> Unit,
onLoginClick: (String) -> Unit,
onDeleteHistoryClick: () -> Unit
) {
Column(
modifier =
Modifier.animateContentSize().fillMaxWidth().padding(horizontal = Dimens.sideMargin)
) {
Column(modifier = Modifier.fillMaxWidth().padding(horizontal = Dimens.sideMargin)) {
Text(
text = state.loginState.title(),
style = MaterialTheme.typography.headlineLarge,
color = MaterialTheme.colorScheme.onPrimary,
modifier = Modifier.fillMaxWidth().padding(bottom = Dimens.mediumPadding)
)

var accountNumber by remember { mutableStateOf(initialAccountNumber) }
var expanded by remember { mutableStateOf(false) }

ExposedDropdownMenuBox(expanded = expanded, onExpandedChange = { expanded = it }) {
TextField(
modifier = Modifier.menuAnchor().fillMaxWidth(),
value = accountNumber,
value = state.accountNumberInput,
label = {
Text(
text = stringResource(id = R.string.login_description),
color = Color.Unspecified
)
},
keyboardActions = KeyboardActions(onDone = { onLoginClick(accountNumber) }),
keyboardActions =
KeyboardActions(onDone = { onLoginClick(state.accountNumberInput) }),
keyboardOptions =
KeyboardOptions(imeAction = ImeAction.Done, keyboardType = KeyboardType.Number),
onValueChange = { accountNumber = it },
KeyboardOptions(
imeAction =
if (state.loginButtonEnabled) ImeAction.Done else ImeAction.None,
keyboardType = KeyboardType.NumberPassword
),
onValueChange = onAccountNumberChange,
singleLine = true,
maxLines = 1,
visualTransformation = accountTokenVisualTransformation(),
Expand All @@ -179,11 +177,17 @@ private fun LoginContent(
unfocusedPlaceholderColor = MaterialTheme.colorScheme.primary,
focusedLabelColor = MaterialTheme.colorScheme.background,
disabledLabelColor = Color.Gray,
unfocusedLabelColor = Color.Gray,
unfocusedLabelColor = MaterialTheme.colorScheme.background,
focusedLeadingIconColor = Color.Black,
unfocusedSupportingTextColor = Color.Black,
),
isError = state.loginState.isError()
isError = state.loginState.isError(),
supportingText = {
Text(
text = state.loginState.supportingText() ?: "",
style = MaterialTheme.typography.labelMedium,
)
}
)

// If we have a previous account, show dropdown for quick re-login
Expand All @@ -205,9 +209,9 @@ private fun LoginContent(
AccountDropDownItem(
accountToken = transformedText.toString(),
onClick = {
accountNumber = token.value
onAccountNumberChange(token.value)
expanded = false
onLoginClick(accountNumber)
onLoginClick(token.value)
}
) {
onDeleteHistoryClick()
Expand All @@ -216,22 +220,10 @@ private fun LoginContent(
}
}

Text(
text = state.loginState.supportingText() ?: "",
style = MaterialTheme.typography.labelMedium,
color =
if (state.loginState.isError()) {
MaterialTheme.colorScheme.error
} else {
MaterialTheme.colorScheme.onPrimary
},
modifier = Modifier.fillMaxWidth().padding(horizontal = Dimens.mediumPadding)
)

Spacer(modifier = Modifier.size(Dimens.mediumPadding))
ActionButton(
isEnabled = state.loginState is Idle,
onClick = { onLoginClick(accountNumber) },
isEnabled = state.loginButtonEnabled,
onClick = { onLoginClick(state.accountNumberInput) },
colors =
ButtonDefaults.buttonColors(
contentColor = MaterialTheme.colorScheme.onPrimary,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ package net.mullvad.mullvadvpn.compose.state

import net.mullvad.mullvadvpn.model.AccountToken

const val MIN_ACCOUNT_LOGIN_LENGTH = 8

data class LoginUiState(
val accountNumberInput: String = "",
val lastUsedAccount: AccountToken? = null,
val loginState: LoginState = LoginState.Idle(null)
) {
val loginButtonEnabled = accountNumberInput.length >= MIN_ACCOUNT_LOGIN_LENGTH && loginState is LoginState.Idle

companion object {
val INITIAL = LoginUiState()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,11 @@ class LoginFragment : BaseFragment(), NavigationBarPainter {

// TODO: Remove this when we have a better solution for login after clearing max devices
val accountTokenArgument = arguments?.getString(ACCOUNT_TOKEN_ARGUMENT_KEY)
val initialAccountNumber =
if (accountTokenArgument != null) {
// Login and set initial TextField value
vm.login(accountTokenArgument)
accountTokenArgument
} else {
""
}
if (accountTokenArgument != null) {
// Login and set initial TextField value
vm.onAccountNumberChange(accountTokenArgument)
vm.login(accountTokenArgument)
}

return inflater.inflate(R.layout.fragment_compose, container, false).apply {
findViewById<ComposeView>(R.id.compose_view).setContent {
Expand All @@ -56,10 +53,10 @@ class LoginFragment : BaseFragment(), NavigationBarPainter {
}
LoginScreen(
loginUiState,
initialAccountNumber,
vm::login,
vm::createAccount,
vm::clearAccountHistory,
vm::onAccountNumberChange,
::openSettingsView
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,23 @@ class LoginViewModel(
private val deviceRepository: DeviceRepository,
private val dispatcher: CoroutineDispatcher = Dispatchers.IO
) : ViewModel() {
private val _loginState = MutableStateFlow<LoginState>(Idle(null))
private val _loginState = MutableStateFlow(LoginUiState.INITIAL.loginState)
private val _loginInput = MutableStateFlow(LoginUiState.INITIAL.accountNumberInput)

private val _viewActions = MutableSharedFlow<LoginViewAction>(extraBufferCapacity = 1)
val viewActions = _viewActions.asSharedFlow()

private val _uiState =
combine(
_loginInput,
accountRepository.accountHistory,
_loginState,
) { accountHistoryState, loginState ->
LoginUiState(accountHistoryState.accountToken()?.let(::AccountToken), loginState)
) { loginInput, accountHistoryState, loginState ->
LoginUiState(
loginInput,
accountHistoryState.accountToken()?.let(::AccountToken),
loginState
)
}
val uiState: StateFlow<LoginUiState> =
_uiState.stateIn(viewModelScope, SharingStarted.WhileSubscribed(), LoginUiState.INITIAL)
Expand Down Expand Up @@ -109,6 +115,12 @@ class LoginViewModel(
}
}

fun onAccountNumberChange(accountNumber: String) {
_loginInput.value = accountNumber.filter { it.isDigit() }
// If there is an error, clear it
_loginState.update { if (it is Idle) Idle() else it }
}

private suspend fun AccountCreationResult.mapToUiState(): LoginState? {
return if (this is AccountCreationResult.Success) {
_viewActions.emit(LoginViewAction.NavigateToWelcome)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,13 @@ class LoginViewModelTest {
fun testCreateAccount() =
runTest {
turbineScope {
// Arrange
val uiStates = loginViewModel.uiState.testIn(backgroundScope)
val sideEffects = loginViewModel.viewActions.testIn(backgroundScope)
coEvery { mockedAccountRepository.createAccount() } returns
AccountCreationResult.Success(DUMMY_ACCOUNT_TOKEN)

// Act, Assert
uiStates.skipDefaultItem()
loginViewModel.createAccount()
assertEquals(Loading.CreatingAccount, uiStates.awaitItem().loginState)
Expand All @@ -80,10 +82,12 @@ class LoginViewModelTest {
fun testLoginWithValidAccount() =
runTest {
turbineScope {
// Arrange
val uiStates = loginViewModel.uiState.testIn(backgroundScope)
val sideEffects = loginViewModel.viewActions.testIn(backgroundScope)
coEvery { mockedAccountRepository.login(any()) } returns LoginResult.Ok

// Act, Assert
uiStates.skipDefaultItem()
loginViewModel.login(DUMMY_ACCOUNT_TOKEN)
assertEquals(Loading.LoggingIn, uiStates.awaitItem().loginState)
Expand All @@ -96,8 +100,10 @@ class LoginViewModelTest {
fun testLoginWithInvalidAccount() =
runTest {
loginViewModel.uiState.test {
// Arrange
coEvery { mockedAccountRepository.login(any()) } returns LoginResult.InvalidAccount

// Act, Assert
skipDefaultItem()
loginViewModel.login(DUMMY_ACCOUNT_TOKEN)
assertEquals(Loading.LoggingIn, awaitItem().loginState)
Expand All @@ -112,6 +118,7 @@ class LoginViewModelTest {
fun testLoginWithTooManyDevicesError() =
runTest {
turbineScope {
// Arrange
val uiStates = loginViewModel.uiState.testIn(backgroundScope)
val sideEffects = loginViewModel.viewActions.testIn(backgroundScope)
coEvery {
Expand All @@ -125,6 +132,7 @@ class LoginViewModelTest {
coEvery { mockedAccountRepository.login(any()) } returns
LoginResult.MaxDevicesReached

// Act, Assert
uiStates.skipDefaultItem()
loginViewModel.login(DUMMY_ACCOUNT_TOKEN)
assertEquals(Loading.LoggingIn, uiStates.awaitItem().loginState)
Expand All @@ -139,8 +147,10 @@ class LoginViewModelTest {
fun testLoginWithRpcError() =
runTest {
loginViewModel.uiState.test {
// Arrange
coEvery { mockedAccountRepository.login(any()) } returns LoginResult.RpcError

// Act, Assert
skipDefaultItem()
loginViewModel.login(DUMMY_ACCOUNT_TOKEN)
assertEquals(Loading.LoggingIn, awaitItem().loginState)
Expand All @@ -155,8 +165,10 @@ class LoginViewModelTest {
fun testLoginWithUnknownError() =
runTest {
loginViewModel.uiState.test {
// Arrange
coEvery { mockedAccountRepository.login(any()) } returns LoginResult.OtherError

// Act, Assert
skipDefaultItem()
loginViewModel.login(DUMMY_ACCOUNT_TOKEN)
assertEquals(Loading.LoggingIn, awaitItem().loginState)
Expand All @@ -171,6 +183,7 @@ class LoginViewModelTest {
fun testAccountHistory() =
runTest {
loginViewModel.uiState.test {
// Act, Assert
skipDefaultItem()
accountHistoryTestEvents.emit(AccountHistory.Available(DUMMY_ACCOUNT_TOKEN))
assertEquals(
Expand All @@ -183,6 +196,7 @@ class LoginViewModelTest {
@Test
fun testClearingAccountHistory() =
runTest {
// Act, Assert
loginViewModel.clearAccountHistory()
verify { mockedAccountRepository.clearAccountHistory() }
}
Expand Down

0 comments on commit b2837e8

Please sign in to comment.