refactor(nav): converge Login/SignUp/CreateProfile onto the majority navigateTo idiom (kills the C-NAV-002 shape)
Login/SignUp/CreateProfile drove navigation off a raw `success: Boolean` (and
`googleSuccess`) in a LaunchedEffect with no reset — the C-NAV-002 re-fire shape.
Correctness depended on a distant popUpTo; pressing system-back mid-onboarding
re-fired the stale flag and bounced the user forward.
The fix UNIFIES rather than fragments. Three nav patterns coexisted: 18 screens
on nullable `navigateTo` + `onNavigated()`, 3 on SharedFlow events, and these 3
on the raw boolean. Their own sibling — OnboardingViewModel — already uses the
nullable pattern, so these were the odd ones out within their own package.
Converted them to it: `navigateTo: String?` set on success (SignUp's two targets
collapse to one field — email→CreateProfile, google→Onboarding), cleared by
`onNavigated()` in the effect. Self-clearing means the flag cannot re-fire on
return, no matter the back stack — the loop is impossible by construction, not
by a remote popUpTo. Pattern count drops 3→2; no SharedFlow added (that would
have made a third variant — deliberately avoided).
Error handling unchanged and already unified: `uiState.error` + the house
snackbar (LaunchedEffect(state.error)). A failed op leaves navigateTo null and
sets error; success sets navigateTo once. The bare boolean never modeled the
failure path; this does.
Also deleted a stale exploration reference — `HomeViewModel.clearRecovery()`
does not exist in the tree (B1/HomeScreen needsRecovery stays as-is; it's
pop-safe via c636749c and touching it risks a keyless-Home frame).
Verified: unit suite green; Login navigateTo proven LIVE (sign-in routes
correctly through Onboarding, and system-back does not bounce back to Login —
the flag self-cleared). SignUp/CreateProfile are the identical conversion and
suite-green; exercising them live needs fresh accounts, avoided per standing
cost guidance. A Detekt rule enforcing the two blessed idioms comes in Tier 2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
884cf9f614
commit
e02b6a9c9a
|
|
@ -70,8 +70,8 @@ fun LoginScreen(
|
|||
val context = LocalContext.current
|
||||
val scope = rememberCoroutineScope()
|
||||
|
||||
LaunchedEffect(state.success) {
|
||||
if (state.success) onNavigate(AppRoute.ONBOARDING)
|
||||
LaunchedEffect(state.navigateTo) {
|
||||
state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() }
|
||||
}
|
||||
LaunchedEffect(state.error) {
|
||||
state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() }
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package app.closer.ui.auth
|
|||
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import app.closer.core.navigation.AppRoute
|
||||
import app.closer.domain.repository.AuthRepository
|
||||
import app.closer.domain.usecase.GoogleProfileMerger
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
|
|
@ -18,7 +19,9 @@ data class LoginUiState(
|
|||
val isPasswordVisible: Boolean = false,
|
||||
val isLoading: Boolean = false,
|
||||
val error: String? = null,
|
||||
val success: Boolean = false
|
||||
// Nullable one-shot nav target + onNavigated() reset — the majority idiom (OnboardingViewModel,
|
||||
// 18 screens). Self-clearing, so returning to a stale Login entry can't re-fire (C-NAV-002 class).
|
||||
val navigateTo: String? = null
|
||||
)
|
||||
|
||||
@HiltViewModel
|
||||
|
|
@ -34,6 +37,7 @@ class LoginViewModel @Inject constructor(
|
|||
fun updatePassword(pw: String) = _uiState.update { it.copy(password = pw, error = null) }
|
||||
fun togglePasswordVisibility() = _uiState.update { it.copy(isPasswordVisible = !it.isPasswordVisible) }
|
||||
fun dismissError() = _uiState.update { it.copy(error = null) }
|
||||
fun onNavigated() = _uiState.update { it.copy(navigateTo = null) }
|
||||
|
||||
fun signIn() {
|
||||
val state = _uiState.value
|
||||
|
|
@ -50,7 +54,7 @@ class LoginViewModel @Inject constructor(
|
|||
authRepository.signInWithGoogle(idToken)
|
||||
.onSuccess { result ->
|
||||
googleProfileMerger.merge(result)
|
||||
_uiState.update { it.copy(isLoading = false, success = true) }
|
||||
_uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) }
|
||||
}
|
||||
.onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } }
|
||||
}
|
||||
|
|
@ -62,7 +66,7 @@ class LoginViewModel @Inject constructor(
|
|||
_uiState.update { it.copy(isLoading = true, error = null) }
|
||||
viewModelScope.launch {
|
||||
action()
|
||||
.onSuccess { _uiState.update { it.copy(isLoading = false, success = true) } }
|
||||
.onSuccess { _uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) } }
|
||||
.onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -79,11 +79,8 @@ fun SignUpScreen(
|
|||
val context = LocalContext.current
|
||||
val scope = rememberCoroutineScope()
|
||||
|
||||
LaunchedEffect(state.success) {
|
||||
if (state.success) onNavigate(AppRoute.CREATE_PROFILE)
|
||||
}
|
||||
LaunchedEffect(state.googleSuccess) {
|
||||
if (state.googleSuccess) onNavigate(AppRoute.ONBOARDING)
|
||||
LaunchedEffect(state.navigateTo) {
|
||||
state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() }
|
||||
}
|
||||
LaunchedEffect(state.error) {
|
||||
state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() }
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope
|
|||
import app.closer.domain.AgeGate
|
||||
import app.closer.domain.SignupHandoff
|
||||
import app.closer.ui.brand.CloserCopy
|
||||
import app.closer.core.navigation.AppRoute
|
||||
import app.closer.domain.repository.AuthRepository
|
||||
import app.closer.domain.usecase.GoogleProfileMerger
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
|
|
@ -24,10 +25,10 @@ data class SignUpUiState(
|
|||
val isPasswordVisible: Boolean = false,
|
||||
val isLoading: Boolean = false,
|
||||
val error: String? = null,
|
||||
val success: Boolean = false,
|
||||
// Google sign-up: profile already comes from Google, so route through ONBOARDING
|
||||
// (which decides HOME vs CREATE_PROFILE) rather than the email CREATE_PROFILE step.
|
||||
val googleSuccess: Boolean = false
|
||||
// One nullable nav target for both paths (majority navigateTo+onNavigated idiom; self-clearing,
|
||||
// loop-proof): email success → CREATE_PROFILE; google success → ONBOARDING (which decides
|
||||
// HOME vs CREATE_PROFILE, since Google already supplies the profile).
|
||||
val navigateTo: String? = null
|
||||
)
|
||||
|
||||
@HiltViewModel
|
||||
|
|
@ -46,6 +47,7 @@ class SignUpViewModel @Inject constructor(
|
|||
fun updateBirthDate(millis: Long) = _uiState.update { it.copy(birthDate = millis, error = null) }
|
||||
fun togglePasswordVisibility() = _uiState.update { it.copy(isPasswordVisible = !it.isPasswordVisible) }
|
||||
fun dismissError() = _uiState.update { it.copy(error = null) }
|
||||
fun onNavigated() = _uiState.update { it.copy(navigateTo = null) }
|
||||
|
||||
fun signUp() {
|
||||
val state = _uiState.value
|
||||
|
|
@ -73,7 +75,7 @@ class SignUpViewModel @Inject constructor(
|
|||
authRepository.currentUserId?.let { uid ->
|
||||
signupHandoff.setPendingBirthDate(uid, dob)
|
||||
}
|
||||
_uiState.update { it.copy(isLoading = false, success = true) }
|
||||
_uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.CREATE_PROFILE) }
|
||||
}
|
||||
.onFailure { e ->
|
||||
val msg = friendlyError(e)
|
||||
|
|
@ -88,7 +90,7 @@ class SignUpViewModel @Inject constructor(
|
|||
authRepository.signInWithGoogle(idToken)
|
||||
.onSuccess { result ->
|
||||
googleProfileMerger.merge(result)
|
||||
_uiState.update { it.copy(isLoading = false, googleSuccess = true) }
|
||||
_uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.ONBOARDING) }
|
||||
}
|
||||
.onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -91,8 +91,8 @@ fun CreateProfileScreen(
|
|||
val focusManager = LocalFocusManager.current
|
||||
val context = LocalContext.current
|
||||
|
||||
LaunchedEffect(state.success) {
|
||||
if (state.success) onNavigate(AppRoute.PAIR_PROMPT)
|
||||
LaunchedEffect(state.navigateTo) {
|
||||
state.navigateTo?.let { onNavigate(it); viewModel.onNavigated() }
|
||||
}
|
||||
LaunchedEffect(state.error) {
|
||||
state.error?.let { snackbar.showSnackbar(it); viewModel.dismissError() }
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package app.closer.ui.onboarding
|
|||
|
||||
import android.net.Uri
|
||||
import androidx.lifecycle.ViewModel
|
||||
import app.closer.core.navigation.AppRoute
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import app.closer.data.remote.FirebaseStorageDataSource
|
||||
import app.closer.domain.AgeGate
|
||||
|
|
@ -38,7 +39,8 @@ data class CreateProfileUiState(
|
|||
val birthDateError: String? = null,
|
||||
val isLoading: Boolean = false,
|
||||
val error: String? = null,
|
||||
val success: Boolean = false,
|
||||
// Majority navigateTo+onNavigated idiom (self-clearing, loop-proof): set to PAIR_PROMPT on save.
|
||||
val navigateTo: String? = null,
|
||||
val nameError: String? = null,
|
||||
val sexError: String? = null
|
||||
)
|
||||
|
|
@ -191,7 +193,7 @@ class CreateProfileViewModel @Inject constructor(
|
|||
}.onSuccess {
|
||||
// DOB is now on the doc — drop the in-memory handoff so a later sign-up can't inherit it.
|
||||
signupHandoff.clear(uid)
|
||||
_uiState.update { it.copy(isLoading = false, success = true) }
|
||||
_uiState.update { it.copy(isLoading = false, navigateTo = AppRoute.PAIR_PROMPT) }
|
||||
}.onFailure { e ->
|
||||
_uiState.update { it.copy(isLoading = false, error = e.message ?: "Couldn't save your profile. Please try again.") }
|
||||
}
|
||||
|
|
@ -199,4 +201,5 @@ class CreateProfileViewModel @Inject constructor(
|
|||
}
|
||||
|
||||
fun dismissError() = _uiState.update { it.copy(error = null) }
|
||||
fun onNavigated() = _uiState.update { it.copy(navigateTo = null) }
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue