diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountMessage.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountMessage.kt index c1f950a..592dc48 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountMessage.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountMessage.kt @@ -1,4 +1,4 @@ -package de.jeanlucmakiola.agendula.ui.accounts +package de.jeanlucmakiola.agendula.ui.accounts.add import de.jeanlucmakiola.caldav.ServerQuirk diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountParts.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountParts.kt new file mode 100644 index 0000000..66a1e5e --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountParts.kt @@ -0,0 +1,172 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.Button +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringArrayResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardCapitalization +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.text.input.VisualTransformation +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.caldav.ServerQuirk +import de.jeanlucmakiola.floret.components.GroupedListInset +import de.jeanlucmakiola.floret.components.GroupedSurface +import de.jeanlucmakiola.floret.components.InstructionSteps +import de.jeanlucmakiola.floret.components.InlineTextField +import de.jeanlucmakiola.floret.components.Position + +/** + * The family's text input: a tonal grouped surface with a borderless field in + * it, never Material's outlined box. + * + * The label sits above the value rather than floating into a notch, because a + * `GroupedSurface` has no outline for a notch to interrupt. + */ +@Composable +internal fun FieldRow( + label: String, + value: String, + onValueChange: (String) -> Unit, + modifier: Modifier = Modifier, + position: Position = Position.Alone, + placeholder: String = "", + error: String? = null, + hint: String? = null, + keyboardType: KeyboardType = KeyboardType.Text, + onImeAction: (() -> Unit)? = null, +) { + // ⚠️ KeyboardType.Password only tells the IME to drop suggestions; it does + // not mask anything. Without the transformation the app password renders in + // the clear on screen. + val masked = keyboardType == KeyboardType.Password + Column(modifier) { + GroupedSurface(position = position) { + Column(Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 12.dp)) { + Text( + label, + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + InlineTextField( + value = value, + onValueChange = onValueChange, + placeholder = placeholder, + keyboardType = keyboardType, + // A server address and a password are both case-sensitive, + // and sentence-casing either is a support ticket. + capitalization = KeyboardCapitalization.None, + imeAction = ImeAction.Go, + onImeAction = onImeAction, + visualTransformation = if (masked) { + PasswordVisualTransformation() + } else { + VisualTransformation.None + }, + ) + } + } + // ⚠️ Outside the surface, not inside it. An error rendered within the + // field's own card reads as part of the value the user typed. + (error ?: hint)?.let { + Text( + it, + style = MaterialTheme.typography.bodySmall, + color = if (error != null) { + MaterialTheme.colorScheme.error + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 6.dp), + ) + } + } +} + +/** The shell's primary action shape, so every step ends identically. */ +@Composable +internal fun PrimaryAction(label: String, enabled: Boolean, onClick: () -> Unit) { + Button( + onClick = onClick, + enabled = enabled, + modifier = Modifier.fillMaxWidth().height(56.dp), + ) { + Text(label, style = MaterialTheme.typography.titleMedium) + } +} + +/** + * What the chosen service requires, before it is required. + * + * ⚠️ Steps, not a paragraph. Fastmail and iCloud both reject the account + * password with a plain 401, and the fix is a four-action errand in someone + * else's web app — the exact shape of instruction that gets skimmed and missed + * when it is written as prose. Google is the odd one out and stays a sentence: + * there is no procedure, because there is nothing the user can do. + * + * ⚠️ Collapsed. By the time this draws, the errand has had a whole screen of its + * own — [SetupStep] — so what is left to do here is let someone re-read it + * without going back, not print it a second time under the field they are + * trying to fill in. + */ +@Composable +internal fun QuirkGuidance(quirk: ServerQuirk) { + when (quirk) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> InstructionSteps( + title = stringResource(R.string.add_account_setup_fastmail_title), + steps = stringArrayResource(R.array.add_account_setup_fastmail_steps).asList(), + footnote = stringResource(R.string.add_account_setup_fastmail_footnote), + collapsible = true, + ) + + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> InstructionSteps( + title = stringResource(R.string.add_account_setup_icloud_title), + steps = stringArrayResource(R.array.add_account_setup_icloud_steps).asList(), + footnote = stringResource(R.string.add_account_setup_icloud_footnote), + collapsible = true, + ) + + ServerQuirk.GOOGLE_UNSUPPORTED -> + QuirkNote(stringResource(R.string.add_account_quirk_google)) + + // A pre-flight warning for the engine, never something to read. + ServerQuirk.NEXTCLOUD_BRUTE_FORCE_PROTECTED -> Unit + } +} + +/** + * A single fact the user has to read — one that is not a procedure, so it is a + * card rather than an [InstructionSteps] list of one. + * + * ⚠️ Inset to `GroupedListInset`, like a grouped row. Without it the note ran to + * the screen edge while every row above it stopped 16dp short, so a caller that + * is already inside a padded column must not add its own — see [BrowserStep]. + */ +@Composable +internal fun QuirkNote(text: String) { + GroupedSurface( + position = Position.Alone, + modifier = Modifier.padding(horizontal = GroupedListInset), + // Not tertiaryContainer: under dynamic colour that is the low-chroma + // role, and against a light wallpaper-derived surface the card's own + // edge disappears even though its text pairing is fine. + color = MaterialTheme.colorScheme.secondaryContainer, + gapBelow = false, + ) { + Text( + text, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSecondaryContainer, + modifier = Modifier.padding(GroupedListInset), + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountScreen.kt index fe847f3..f64d4b2 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountScreen.kt @@ -1,28 +1,24 @@ -package de.jeanlucmakiola.agendula.ui.accounts +package de.jeanlucmakiola.agendula.ui.accounts.add import android.content.Intent +import androidx.activity.compose.BackHandler import androidx.browser.customtabs.CustomTabsIntent -import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.background -import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.ArrowBack -import androidx.compose.material.icons.rounded.CloudOff import androidx.compose.material.icons.rounded.Checklist +import androidx.compose.material.icons.rounded.CloudOff import androidx.compose.material.icons.rounded.CloudSync import androidx.compose.material.icons.rounded.Lock import androidx.compose.material.icons.rounded.OpenInBrowser -import androidx.compose.material3.Button -import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme @@ -37,40 +33,32 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign -import androidx.compose.ui.text.input.ImeAction -import androidx.compose.ui.text.input.KeyboardCapitalization -import androidx.compose.ui.text.input.KeyboardType -import androidx.compose.ui.text.input.PasswordVisualTransformation -import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.unit.dp import androidx.core.net.toUri -import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.agendula.R -import de.jeanlucmakiola.caldav.CalDavDiscovery -import de.jeanlucmakiola.caldav.ServerQuirk -import de.jeanlucmakiola.floret.components.CollapsingScaffold -import de.jeanlucmakiola.floret.components.GroupedRow -import de.jeanlucmakiola.floret.components.GroupedSurface -import de.jeanlucmakiola.floret.components.InlineTextField +import de.jeanlucmakiola.agendula.ui.accounts.ProviderLogo import de.jeanlucmakiola.floret.components.OnboardingProgress import de.jeanlucmakiola.floret.components.OnboardingScaffold import de.jeanlucmakiola.floret.components.OnboardingSpace -import de.jeanlucmakiola.floret.components.Position -import de.jeanlucmakiola.floret.components.SelectedCheck -import de.jeanlucmakiola.floret.components.positionOf /** * Adding a CalDAV account: one flow, one back-stack entry. * - * A stepper rather than four destinations, because the steps are not + * A stepper rather than several destinations, because the steps are not * independently reachable — you cannot pick lists before signing in, and going * "back" from the browser step means abandoning a server-side flow rather than - * popping a screen. + * popping a screen. The steps that *have* written nothing do step back, in the + * flow rather than out of it; [AddAccountViewModel.onBackWithin] decides which, + * and the shell falls out only when it says there is nowhere left to go. The + * back arrow and the system back gesture are the same action and run the same + * code — a stepper whose gesture skips the steps is a stepper with one way in + * and no way back. * * [stepOffset] and [totalSteps] let a longer flow host this one inline — first * run offers the account before it has finished onboarding — so the wizard's - * three steps report their place in the *outer* progress rather than restarting + * four steps report their place in the *outer* progress rather than restarting * the count at one. Left at their defaults it stands alone. */ @Composable @@ -78,7 +66,10 @@ internal fun AddAccountScreen( onDone: () -> Unit, onBack: () -> Unit, stepOffset: Int = 0, - totalSteps: Int = ADD_ACCOUNT_STEPS, + // Null when the wizard stands alone: it then counts its own steps, which + // vary by provider. A host that draws one bar for a longer flow passes the + // whole denominator, having already asked the wizard how long it is. + totalSteps: Int? = null, viewModel: AddAccountViewModel = hiltViewModel(), ) { val state by viewModel.state.collectAsStateWithLifecycle() @@ -124,23 +115,40 @@ internal fun AddAccountScreen( onBack() } + // One action, two ways to ask for it: step back inside the flow, and fall + // out of it only when there is nowhere left to go. + val goBack = { if (!viewModel.onBackWithin()) abandon() } + + // ⚠️ The system gesture has to be caught here, not left to the host. The + // hosts only put the section away — `SettingsScreen` sets `section = parent` + // — while this ViewModel is scoped to the Settings back-stack entry and + // outlives that. So a gesture that fell through left the flow *loaded*: + // reopening "Add account" landed back on the previous server's list picker, + // still holding its username, password and collections. Worse in + // `WaitingForBrowser`, where the two-second poll kept running with no screen + // attached for the rest of the twenty-minute window, and a password minted + // after that point was held unrevoked until Settings itself was left. + // Consuming it means [AddAccountViewModel.onStartOver] runs on every exit. + BackHandler(onBack = goBack) + OnboardingScaffold( hero = { StepHero(state) }, // Grouped rows and fields carry their own inset, so the column adds none. contentPadding = 0.dp, topSpacing = OnboardingSpace.md, progress = { - val position = state.step.position?.plus(stepOffset) + val position = state.step.position(state.hasSetupStep)?.plus(stepOffset) + val total = totalSteps ?: state.totalSteps if (position != null) { OnboardingProgress( step = position, - total = totalSteps, - label = stringResource(R.string.add_account_step_of, position, totalSteps), + total = total, + label = stringResource(R.string.add_account_step_of, position, total), ) } }, navigationIcon = { - IconButton(onClick = abandon) { + IconButton(onClick = goBack) { Icon( Icons.AutoMirrored.Rounded.ArrowBack, contentDescription = stringResource(R.string.back), @@ -164,14 +172,34 @@ internal fun AddAccountScreen( StepTitle(state.step) when (val step = state.step) { - is AddAccountStep.EnterServer -> ServerStep(step, state.quirk, viewModel) + is AddAccountStep.ChooseProvider -> ProviderStep(step, viewModel) + is AddAccountStep.PrepareAccess -> SetupStep(step) + is AddAccountStep.EnterAddress -> AddressStep(step, viewModel) is AddAccountStep.Working -> WorkingStep(step) is AddAccountStep.EnterCredentials -> CredentialsStep(step, viewModel) - is AddAccountStep.WaitingForBrowser -> BrowserStep(step, viewModel) + is AddAccountStep.WaitingForBrowser -> BrowserStep(step) is AddAccountStep.ChooseLists -> ListsStep(step, viewModel) + is AddAccountStep.Summary -> SummaryStep(step) AddAccountStep.Done -> Unit } + // What is known about the chosen service, named before the attempt rather + // than after a 401 the user cannot act on. + // + // ⚠️ Not on the errand step — that step *is* this, and drawing it again + // underneath would print the same instructions twice. On the address step + // it is the fallback for the one route that skips the errand: picking + // "Other server" and then typing an address that turns out to be a + // Fastmail or iCloud one. + val note = when (state.step) { + is AddAccountStep.ChooseProvider, is AddAccountStep.EnterAddress -> state.quirk + else -> null + } + note?.let { + Spacer(Modifier.height(OnboardingSpace.md)) + QuirkGuidance(it) + } + // Learned during the browser step and shown from there on, whichever // step follows: the address it blames is one the user has to go and // correct on the server, and it is just as true once the lists load. @@ -187,23 +215,51 @@ internal fun AddAccountScreen( } } -/** Which of the flow's visible steps this is, or null for one that has no place. */ -private val AddAccountStep.position: Int? - get() = when (this) { - is AddAccountStep.EnterServer -> 1 - is AddAccountStep.EnterCredentials, is AddAccountStep.WaitingForBrowser -> 2 - is AddAccountStep.ChooseLists -> 3 +/** + * Which of the flow's visible steps this is, or null for one that has no place. + * + * [hasSetup] shifts everything after the errand down by one, because the errand + * is a step of its own rather than a screen borrowing the address's number. + */ +private fun AddAccountStep.position(hasSetup: Boolean): Int? { + val errand = if (hasSetup) 1 else 0 + return when (this) { + is AddAccountStep.ChooseProvider -> 1 + is AddAccountStep.PrepareAccess -> 2 + is AddAccountStep.EnterAddress -> 2 + errand + // One slot, because only one of the two ever happens: a server either + // hands the sign-in to a browser or asks for a password. + is AddAccountStep.EnterCredentials, is AddAccountStep.WaitingForBrowser -> 3 + errand + is AddAccountStep.ChooseLists -> 4 + errand + // The receipt keeps the bar full rather than dropping it: the flow is + // finished, and a chrome that vanishes on the last screen reads as a + // step lost rather than a step done. + is AddAccountStep.Summary -> ADD_ACCOUNT_STEPS + errand // Working is a moment inside whichever step spawned it, and Done is gone // before it draws — neither is a place the user can be. is AddAccountStep.Working, AddAccountStep.Done -> null } +} -/** The wizard's own visible steps, and the default denominator of its progress. */ -internal const val ADD_ACCOUNT_STEPS = 3 - -/** The mark for the step, in the family's tonal circle. */ +/** + * The mark for the step, in the family's tonal circle. + * + * Once a service has been chosen it wears *that service's* logo instead — the + * same mark the accounts list will show — so the flow keeps saying what is being + * set up rather than restating that an account is being added. + */ @Composable private fun StepHero(state: AddAccountUiState) { + if (state.step is AddAccountStep.Summary) { + ProviderLogo(state.step.provider, size = 72.dp) + return + } + val chosen = (state.step as? AddAccountStep.EnterAddress)?.choice?.provider + ?: (state.step as? AddAccountStep.PrepareAccess)?.choice?.provider + if (chosen != null && state.fatal == null) { + ProviderLogo(chosen, size = 72.dp) + return + } val icon = when { state.fatal != null -> Icons.Rounded.CloudOff state.step is AddAccountStep.EnterCredentials -> Icons.Rounded.Lock @@ -231,12 +287,21 @@ private fun StepHero(state: AddAccountUiState) { @Composable private fun StepTitle(step: AddAccountStep) { val (title, body) = when (step) { - is AddAccountStep.EnterServer -> + is AddAccountStep.ChooseProvider -> + R.string.add_account_provider_title to R.string.add_account_provider_body + is AddAccountStep.PrepareAccess -> + step.quirk.setupTitle to R.string.add_account_setup_body + is AddAccountStep.EnterAddress -> if (step.choice.provider?.hosted == true) { + R.string.add_account_email_title to R.string.add_account_email_body + } else { R.string.add_account_server_title to R.string.add_account_server_body + } is AddAccountStep.EnterCredentials -> R.string.add_account_credentials_title to R.string.add_account_credentials_body is AddAccountStep.ChooseLists -> R.string.add_account_lists_title to R.string.add_account_lists_body + is AddAccountStep.Summary -> + R.string.add_account_summary_title to R.string.add_account_summary_body else -> return } Text( @@ -256,197 +321,12 @@ private fun StepTitle(step: AddAccountStep) { Spacer(Modifier.height(OnboardingSpace.lg)) } -@Composable -private fun ServerStep( - step: AddAccountStep.EnterServer, - quirk: ServerQuirk?, - viewModel: AddAccountViewModel, -) { - Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { - FieldRow( - label = stringResource(R.string.add_account_server_label), - value = step.input, - onValueChange = viewModel::onServerInputChanged, - placeholder = stringResource(R.string.add_account_server_hint), - error = step.error?.let { stringResource(it.message) }, - keyboardType = KeyboardType.Uri, - onImeAction = viewModel::onServerSubmitted, - ) - - // Named before the attempt, not after a 401 the user cannot act on. - quirk?.let { QuirkNote(it) } - - } -} - -@Composable -private fun CredentialsStep(step: AddAccountStep.EnterCredentials, viewModel: AddAccountViewModel) { - Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { - // Two fields in one connected group, which is what the family does with - // a pair that is filled in together. - Column { - FieldRow( - label = stringResource(R.string.add_account_username_label), - value = step.username, - onValueChange = viewModel::onUsernameChanged, - position = Position.Top, - ) - FieldRow( - label = stringResource(R.string.add_account_password_label), - value = step.password, - onValueChange = viewModel::onPasswordChanged, - position = Position.Bottom, - error = step.error?.text(), - hint = stringResource(R.string.add_account_password_hint), - keyboardType = KeyboardType.Password, - onImeAction = viewModel::onCredentialsSubmitted, - ) - } - } -} - -/** - * A discovery failure, in words rather than a status code. - * - * ⚠️ The server's own message never reaches the screen. The most common failure - * here is a **405** — what an ordinary web server answers to `PROPFIND`, and so - * what someone typing their *website* instead of their CalDAV address gets — and - * "HTTP 405 Method Not Allowed" tells them nothing they can act on. It is also - * untranslatable, and frequently not in a language they read. - */ -private val CalDavDiscovery.Outcome.Cause.message: Int - get() = when (this) { - CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS -> R.string.add_account_error_not_an_address - CalDavDiscovery.Outcome.Cause.NOT_A_DAV_SERVER -> R.string.add_account_error_not_dav - CalDavDiscovery.Outcome.Cause.NO_CALENDAR_SUPPORT -> R.string.add_account_error_no_calendar - CalDavDiscovery.Outcome.Cause.UNREACHABLE -> R.string.add_account_error_unreachable - CalDavDiscovery.Outcome.Cause.INSECURE -> R.string.add_account_error_insecure - CalDavDiscovery.Outcome.Cause.NO_CALENDARS -> R.string.add_account_error_no_calendars - CalDavDiscovery.Outcome.Cause.SERVER_ERROR -> R.string.add_account_error_server - } - -/** - * The same rule as [message], for the messages the flow itself produces. - * - * Kept here rather than on [AddAccountMessage] so the type stays a plain Kotlin - * one and the resource ids stay where the resources are. - */ -@Composable -private fun AddAccountMessage.text(): String = when (this) { - AddAccountMessage.Progress.Discovering -> - stringResource(R.string.add_account_working_discovering) - AddAccountMessage.Progress.SigningIn -> - stringResource(R.string.add_account_working_signing_in) - AddAccountMessage.Progress.ReadingLists -> - stringResource(R.string.add_account_working_reading_lists) - AddAccountMessage.Progress.Saving -> - stringResource(R.string.add_account_working_saving) - AddAccountMessage.GoogleUnsupported -> - stringResource(R.string.add_account_error_google_unsupported) - AddAccountMessage.AlreadyExists -> - stringResource(R.string.add_account_error_already_exists) - AddAccountMessage.NoUsableLists -> - stringResource(R.string.add_account_error_no_usable_lists) - AddAccountMessage.NotSaved -> stringResource(R.string.add_account_error_not_saved) - AddAccountMessage.KeystoreRefused -> stringResource(R.string.add_account_error_keystore) - AddAccountMessage.CredentialsRejected -> - stringResource(R.string.add_account_error_credentials_rejected) - AddAccountMessage.BrowserApprovalExpired -> - stringResource(R.string.add_account_browser_error_expired) - AddAccountMessage.BrowserRateLimited -> - stringResource(R.string.add_account_browser_error_rate_limited) - AddAccountMessage.BrowserMaintenance -> - stringResource(R.string.add_account_browser_error_maintenance) - AddAccountMessage.BrowserFailed -> - stringResource(R.string.add_account_browser_error_failed) - AddAccountMessage.BrowserUnavailable -> - stringResource(R.string.add_account_browser_error_unavailable) - is AddAccountMessage.OutsideCredentialScope -> - stringResource(R.string.add_account_error_cross_domain, host) - is AddAccountMessage.Quirk -> when (quirk) { - ServerQuirk.FASTMAIL_APP_PASSWORD -> - stringResource(R.string.add_account_quirk_hint_fastmail) - ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> - stringResource(R.string.add_account_quirk_hint_icloud) - // Neither reaches a credentials step: Google is refused before it, and - // the brute-force note is a pre-flight warning. - else -> "" - } -} - -/** - * The family's text input: a tonal grouped surface with a borderless field in - * it, never Material's outlined box. - * - * The label sits above the value rather than floating into a notch, because a - * `GroupedSurface` has no outline for a notch to interrupt. - */ -@Composable -private fun FieldRow( - label: String, - value: String, - onValueChange: (String) -> Unit, - modifier: Modifier = Modifier, - position: Position = Position.Alone, - placeholder: String = "", - error: String? = null, - hint: String? = null, - keyboardType: KeyboardType = KeyboardType.Text, - onImeAction: (() -> Unit)? = null, -) { - // ⚠️ KeyboardType.Password only tells the IME to drop suggestions; it does - // not mask anything. Without the transformation the app password renders in - // the clear on screen. - val masked = keyboardType == KeyboardType.Password - Column(modifier) { - GroupedSurface(position = position) { - Column(Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 12.dp)) { - Text( - label, - style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - InlineTextField( - value = value, - onValueChange = onValueChange, - placeholder = placeholder, - keyboardType = keyboardType, - // A server address and a password are both case-sensitive, - // and sentence-casing either is a support ticket. - capitalization = KeyboardCapitalization.None, - imeAction = ImeAction.Go, - onImeAction = onImeAction, - visualTransformation = if (masked) { - PasswordVisualTransformation() - } else { - VisualTransformation.None - }, - ) - } - } - // ⚠️ Outside the surface, not inside it. An error rendered within the - // field's own card reads as part of the value the user typed. - (error ?: hint)?.let { - Text( - it, - style = MaterialTheme.typography.bodySmall, - color = if (error != null) { - MaterialTheme.colorScheme.error - } else { - MaterialTheme.colorScheme.onSurfaceVariant - }, - modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 6.dp), - ) - } - } -} - /** * The step's call to action, pinned at the bottom the way the onboarding shell * puts it. * * One primary action per step, always in the same place, so the flow reads as - * one screen advancing rather than four different ones. A step that commits + * one screen advancing rather than five different ones. A step that commits * nothing renders no button rather than a disabled one. */ @Composable @@ -461,10 +341,22 @@ private fun ColumnScope.StepActions(state: AddAccountUiState, viewModel: AddAcco } when (val step = state.step) { - is AddAccountStep.EnterServer -> PrimaryAction( + // ⚠️ No action at all. Tapping a row *is* the choice and advances on the + // spot, so a Continue here would be a second press for the same decision + // — and a disabled one, before anything is picked, that looks like the + // screen is broken. + is AddAccountStep.ChooseProvider -> Unit + + is AddAccountStep.PrepareAccess -> PrimaryAction( + label = stringResource(R.string.add_account_continue), + enabled = true, + onClick = viewModel::onSetupAcknowledged, + ) + + is AddAccountStep.EnterAddress -> PrimaryAction( label = stringResource(R.string.add_account_continue), enabled = step.input.isNotBlank(), - onClick = viewModel::onServerSubmitted, + onClick = viewModel::onAddressSubmitted, ) is AddAccountStep.EnterCredentials -> PrimaryAction( @@ -485,6 +377,12 @@ private fun ColumnScope.StepActions(state: AddAccountUiState, viewModel: AddAcco onClick = viewModel::onSave, ) + is AddAccountStep.Summary -> PrimaryAction( + label = stringResource(R.string.add_account_summary_done), + enabled = true, + onClick = viewModel::onSummaryDone, + ) + is AddAccountStep.WaitingForBrowser -> { // A failed flow gets the primary action: there is nothing left to // wait for, and the way forward is a password. @@ -505,134 +403,3 @@ private fun ColumnScope.StepActions(state: AddAccountUiState, viewModel: AddAcco is AddAccountStep.Working, AddAccountStep.Done -> Unit } } - -/** The shell's primary action shape, so every step ends identically. */ -@Composable -private fun PrimaryAction(label: String, enabled: Boolean, onClick: () -> Unit) { - Button( - onClick = onClick, - enabled = enabled, - modifier = Modifier.fillMaxWidth().height(56.dp), - ) { - Text(label, style = MaterialTheme.typography.titleMedium) - } -} - -@Composable -private fun BrowserStep(step: AddAccountStep.WaitingForBrowser, viewModel: AddAccountViewModel) { - Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { - // ⚠️ The title changes too. Swapping only the body left every failure in - // this phase — a burnt credential, a 429, a maintenance page — sitting - // under the heading "Waiting for your browser", which reads as "still - // working" when nothing is working and nothing else will happen. - Message( - icon = { - if (step.error == null) { - CircularProgressIndicator(Modifier.size(24.dp)) - } else { - Icon(Icons.Rounded.CloudOff, contentDescription = null) - } - }, - title = if (step.error == null) { - stringResource(R.string.add_account_browser_title) - } else { - stringResource(R.string.add_account_browser_failed_title) - }, - text = step.error?.text() ?: stringResource(R.string.add_account_browser_body), - ) - - step.hostMismatch?.let { mismatch -> - QuirkNote( - text = stringResource( - R.string.add_account_browser_host_mismatch, - mismatch.actual, - mismatch.expected, - ), - ) - } - - } -} - -@Composable -private fun WorkingStep(step: AddAccountStep.Working) { - Column( - Modifier.fillMaxWidth().padding(32.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(16.dp), - ) { - CircularProgressIndicator() - Text(step.message.text(), style = MaterialTheme.typography.bodyLarge) - } -} - -@Composable -private fun ListsStep(step: AddAccountStep.ChooseLists, viewModel: AddAccountViewModel) { - // The heading is the shell's; this step only lists. - step.collections.forEachIndexed { index, collection -> - val checked = collection.url in step.selected - GroupedRow( - title = collection.displayName ?: collection.url.encodedPath, - summary = when { - collection.readOnly -> stringResource(R.string.add_account_lists_read_only) - collection.isShared -> stringResource(R.string.add_account_lists_shared) - else -> null - }, - position = positionOf(index, step.collections.size), - selected = checked, - // The family marks a chosen row with a check, not a Material - // checkbox — same affordance every picker in the app uses. - trailing = { if (checked) SelectedCheck() }, - onClick = { viewModel.onListToggled(collection.url) }, - ) - } - Spacer(Modifier.height(16.dp)) -} - -@Composable -private fun QuirkNote(quirk: ServerQuirk) { - val text = when (quirk) { - ServerQuirk.FASTMAIL_APP_PASSWORD -> stringResource(R.string.add_account_quirk_fastmail) - ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> - stringResource(R.string.add_account_quirk_icloud) - ServerQuirk.GOOGLE_UNSUPPORTED -> stringResource(R.string.add_account_quirk_google) - ServerQuirk.NEXTCLOUD_BRUTE_FORCE_PROTECTED -> return - } - QuirkNote(text) -} - -@Composable -private fun QuirkNote(text: String) { - GroupedSurface( - position = Position.Alone, - color = MaterialTheme.colorScheme.tertiaryContainer, - gapBelow = false, - ) { - Text( - text, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onTertiaryContainer, - modifier = Modifier.padding(16.dp), - ) - } -} - -@Composable -private fun Message( - icon: @Composable () -> Unit, - text: String, - title: String? = null, -) { - Column( - Modifier.fillMaxWidth().padding(horizontal = 16.dp), - verticalArrangement = Arrangement.spacedBy(12.dp), - ) { - icon() - title?.let { Text(it, style = MaterialTheme.typography.titleMedium) } - Text( - text, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } -} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountText.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountText.kt new file mode 100644 index 0000000..770dc44 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountText.kt @@ -0,0 +1,76 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.runtime.Composable +import androidx.compose.ui.res.stringResource +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.ServerQuirk + +/** + * A discovery failure, in words rather than a status code. + * + * ⚠️ The server's own message never reaches the screen. The most common failure + * here is a **405** — what an ordinary web server answers to `PROPFIND`, and so + * what someone typing their *website* instead of their CalDAV address gets — and + * "HTTP 405 Method Not Allowed" tells them nothing they can act on. It is also + * untranslatable, and frequently not in a language they read. + */ +internal val CalDavDiscovery.Outcome.Cause.message: Int + get() = when (this) { + CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS -> R.string.add_account_error_not_an_address + CalDavDiscovery.Outcome.Cause.NOT_A_DAV_SERVER -> R.string.add_account_error_not_dav + CalDavDiscovery.Outcome.Cause.NO_CALENDAR_SUPPORT -> R.string.add_account_error_no_calendar + CalDavDiscovery.Outcome.Cause.UNREACHABLE -> R.string.add_account_error_unreachable + CalDavDiscovery.Outcome.Cause.INSECURE -> R.string.add_account_error_insecure + CalDavDiscovery.Outcome.Cause.NO_CALENDARS -> R.string.add_account_error_no_calendars + CalDavDiscovery.Outcome.Cause.SERVER_ERROR -> R.string.add_account_error_server + } + +/** + * The same rule as [message], for the messages the flow itself produces. + * + * Kept here rather than on [AddAccountMessage] so the type stays a plain Kotlin + * one and the resource ids stay where the resources are. + */ +@Composable +internal fun AddAccountMessage.text(): String = when (this) { + AddAccountMessage.Progress.Discovering -> + stringResource(R.string.add_account_working_discovering) + AddAccountMessage.Progress.SigningIn -> + stringResource(R.string.add_account_working_signing_in) + AddAccountMessage.Progress.ReadingLists -> + stringResource(R.string.add_account_working_reading_lists) + AddAccountMessage.Progress.Saving -> + stringResource(R.string.add_account_working_saving) + AddAccountMessage.GoogleUnsupported -> + stringResource(R.string.add_account_error_google_unsupported) + AddAccountMessage.AlreadyExists -> + stringResource(R.string.add_account_error_already_exists) + AddAccountMessage.NoUsableLists -> + stringResource(R.string.add_account_error_no_usable_lists) + AddAccountMessage.NotSaved -> stringResource(R.string.add_account_error_not_saved) + AddAccountMessage.KeystoreRefused -> stringResource(R.string.add_account_error_keystore) + AddAccountMessage.CredentialsRejected -> + stringResource(R.string.add_account_error_credentials_rejected) + AddAccountMessage.BrowserApprovalExpired -> + stringResource(R.string.add_account_browser_error_expired) + AddAccountMessage.BrowserRateLimited -> + stringResource(R.string.add_account_browser_error_rate_limited) + AddAccountMessage.BrowserMaintenance -> + stringResource(R.string.add_account_browser_error_maintenance) + AddAccountMessage.BrowserFailed -> + stringResource(R.string.add_account_browser_error_failed) + AddAccountMessage.BrowserUnavailable -> + stringResource(R.string.add_account_browser_error_unavailable) + is AddAccountMessage.OutsideCredentialScope -> + stringResource(R.string.add_account_error_cross_domain, host) + is AddAccountMessage.Quirk -> when (quirk) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> + stringResource(R.string.add_account_quirk_hint_fastmail) + ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD -> + stringResource(R.string.add_account_quirk_hint_icloud) + // Neither reaches a credentials step: Google is refused before it, and + // the brute-force note is a pre-flight warning. + else -> "" + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModel.kt index bbccd79..669935a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModel.kt @@ -1,4 +1,4 @@ -package de.jeanlucmakiola.agendula.ui.accounts +package de.jeanlucmakiola.agendula.ui.accounts.add import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -8,6 +8,7 @@ import de.jeanlucmakiola.agendula.data.sync.AccountRepository import de.jeanlucmakiola.agendula.data.sync.CalDavGateway import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.CalDavProvider import de.jeanlucmakiola.caldav.NextcloudLoginFlow import de.jeanlucmakiola.caldav.ServerQuirk import de.jeanlucmakiola.caldav.ServiceDiscovery @@ -27,11 +28,51 @@ import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import javax.inject.Inject +/** + * Which service the account lives on. + * + * Picked before anything is typed, because it is the only thing that makes the + * next question answerable: a hosted service is reached by the email address the + * user already knows, while a server they run is reached by a URL only they have. + * Asking for "an address" without knowing which of the two it is was the flow's + * first dead end. + */ +sealed interface ProviderChoice { + + /** The service itself, or null for a server we know nothing about yet. */ + val provider: CalDavProvider? + + /** A service we know by name, and therefore by domain, quirk and sign-in route. */ + data class Service(override val provider: CalDavProvider) : ProviderChoice + + /** Anything else that speaks CalDAV. Identified after discovery, if at all. */ + data object OtherServer : ProviderChoice { + override val provider: CalDavProvider? get() = null + } +} + /** Where the user is in adding an account. */ sealed interface AddAccountStep { - /** Type an address. The whole flow starts from one field. */ - data class EnterServer( + /** Step 1: pick the service. */ + data class ChooseProvider(val choice: ProviderChoice? = null) : AddAccountStep + + /** + * Step 2, first half: what to go and do before any of this will work. + * + * Its own screen rather than a note under the address field, because it is + * an errand in someone else's web app — the user leaves, does four things, + * and comes back — and an instruction you are meant to act on before typing + * does not belong underneath the thing you type into. + */ + data class PrepareAccess( + val choice: ProviderChoice, + val quirk: ServerQuirk, + ) : AddAccountStep + + /** Step 2, second half: type the one address the choice above asks for. */ + data class EnterAddress( + val choice: ProviderChoice, val input: String = "", /** A cause, not a message — the screen owns the wording. */ val error: CalDavDiscovery.Outcome.Cause? = null, @@ -62,11 +103,27 @@ sealed interface AddAccountStep { val selected: Set, ) : AddAccountStep + /** + * What the flow just built, before it hands back. + * + * The account is already saved by the time this draws — it is a receipt, not + * a confirmation. It exists because the wizard used to vanish on success, + * leaving no answer to "which lists did it take, and under what name?" short + * of going and finding the account again. + */ + data class Summary( + val provider: CalDavProvider?, + val title: String, + val secondary: String?, + val username: String, + val lists: List, + ) : AddAccountStep + data object Done : AddAccountStep } data class AddAccountUiState( - val step: AddAccountStep = AddAccountStep.EnterServer(), + val step: AddAccountStep = AddAccountStep.ChooseProvider(), /** * A warning the user should read before going further — the three providers * whose real failure is not "wrong password", and a server that sent the @@ -82,11 +139,26 @@ data class AddAccountUiState( * survive into the steps after it. */ val originMismatch: NextcloudLoginFlow.HostMismatch? = null, + /** + * Whether this run includes the errand step, which not every service needs. + * + * The flow's length is therefore a property of the *choice*, not a constant: + * a service with an app password to mint is one screen longer than one + * without, and a counter that says "of 4" through a five-screen flow is + * simply wrong. + */ + val hasSetupStep: Boolean = false, /** Set when the flow cannot continue at all; the UI offers only "start over". */ val fatal: AddAccountMessage? = null, /** Non-null once the browser flow has a URL to open. */ val openInBrowser: HttpUrl? = null, -) +) { + /** How many numbered steps this run has, which the errand can push to five. */ + val totalSteps: Int get() = if (hasSetupStep) ADD_ACCOUNT_STEPS + 1 else ADD_ACCOUNT_STEPS +} + +/** The wizard's own visible steps, before a service adds an errand to them. */ +const val ADD_ACCOUNT_STEPS = 4 @HiltViewModel class AddAccountViewModel @Inject constructor( @@ -102,6 +174,14 @@ class AddAccountViewModel @Inject constructor( private val _state = MutableStateFlow(AddAccountUiState()) val state: StateFlow = _state.asStateFlow() + private var choice: ProviderChoice? = null + + /** + * What is in the address field, as opposed to [typedInput], which is what was + * last *submitted*. Kept so stepping back to the picker and forward again + * returns to a filled field rather than an empty one. + */ + private var addressInput: String = "" private var typedInput: String = "" private var serverRoot: HttpUrl? = null private var username: String = "" @@ -123,24 +203,78 @@ class AddAccountViewModel @Inject constructor( */ private var minted: CalDavGateway.Credentials? = null - fun onServerInputChanged(value: String) = _state.update { - it.copy(step = AddAccountStep.EnterServer(value), quirk = ServerQuirk.forInput(value)) + /** + * Step 1 → step 2, on the tap itself. + * + * ⚠️ No Continue. A picker whose rows already say what they are does not + * need a second confirmation of the same tap — the back arrow is the undo, + * and it costs one press rather than two. + * + * The exception is a service we cannot sync with at all. Google stays on the + * picker with its reason under it rather than ending the flow: picking the + * wrong service should cost one tap to correct, not a restart. + */ + fun onProviderChosen(picked: ProviderChoice) { + // A different service asks a different question, so the answer to the old + // one is not an answer to this one. + if (picked != choice) addressInput = "" + choice = picked + val quirk = picked.provider?.let(ServerQuirk::forProvider) + val setup = quirk?.hasSetupSteps == true + _state.update { + it.copy( + step = when { + quirk?.isFatal == true -> AddAccountStep.ChooseProvider(picked) + setup -> AddAccountStep.PrepareAccess(picked, quirk) + else -> addressStep(addressInput) + }, + quirk = quirk, + // Set on the choice, so the counter is right on the very screen + // the errand appears on rather than one step later. + hasSetupStep = setup, + fatal = null, + ) + } + } + + /** The errand is read; on to the address. */ + fun onSetupAcknowledged() { + if (_state.value.step !is AddAccountStep.PrepareAccess) return + _state.update { it.copy(step = addressStep(addressInput), fatal = null) } + } + + fun onAddressChanged(value: String) { + // The fallback matters: a step reached from a failure carries a choice + // the field would otherwise have no way to read, and without it the + // field goes dead rather than merely unlabelled. + val picked = choice ?: ProviderChoice.OtherServer + choice = picked + addressInput = value + _state.update { + it.copy( + step = AddAccountStep.EnterAddress(picked, value), + // The service's own rule outranks the one the typed host implies: + // it was chosen deliberately, and it is known before a key is hit. + quirk = picked.provider?.let(ServerQuirk::forProvider) + ?: ServerQuirk.forInput(value), + ) + } } fun onUsernameChanged(value: String) = updateCredentials { it.copy(username = value) } fun onPasswordChanged(value: String) = updateCredentials { it.copy(password = value) } - /** Step 1 → discovery, unauthenticated. */ - fun onServerSubmitted() { - val input = (_state.value.step as? AddAccountStep.EnterServer)?.input?.trim().orEmpty() + /** Step 2 → discovery, unauthenticated. */ + fun onAddressSubmitted() { + val input = (_state.value.step as? AddAccountStep.EnterAddress)?.input?.trim().orEmpty() if (input.isEmpty()) return // ⚠️ The retry path, and the one that actually leaks. A post-approval // failure sends the user back here with an error, and the screen offers // *Continue*, not start over — so a second attempt would mint a second // password on top of the first without this. discardMintedPassword() - // ⚠️ And the credential itself, not just the minted one. backToServer is + // ⚠️ And the credential itself, not just the minted one. backToAddress is // reachable *after* a successful approval — a post-approval discovery // failure lands on an ordinary address step with a live Continue button // — and only onStartOver cleared these. So: approve on server A, @@ -157,6 +291,7 @@ class AddAccountViewModel @Inject constructor( // about that server's settings which was never measured. _state.update { it.copy(originMismatch = null) } typedInput = input + addressInput = input val quirk = ServerQuirk.forInput(input) if (quirk?.isFatal == true) { @@ -164,7 +299,7 @@ class AddAccountViewModel @Inject constructor( // explanation beats a 401 the user cannot act on. _state.update { it.copy( - step = AddAccountStep.EnterServer(input = input), + step = addressStep(input), fatal = AddAccountMessage.GoogleUnsupported, quirk = quirk, ) @@ -190,14 +325,14 @@ class AddAccountViewModel @Inject constructor( // would make it permanently unreachable. CalDavDiscovery.Outcome.Unauthenticated -> offerSignIn() - is CalDavDiscovery.Outcome.NotCalDav -> backToServer(outcome.cause) + is CalDavDiscovery.Outcome.NotCalDav -> backToAddress(outcome.cause) - is CalDavDiscovery.Outcome.Failed -> backToServer(outcome.cause) + is CalDavDiscovery.Outcome.Failed -> backToAddress(outcome.cause) } } } - /** Step 2a → re-run discovery with the password the user typed. */ + /** Step 3a → re-run discovery with the password the user typed. */ fun onCredentialsSubmitted() { val step = _state.value.step as? AddAccountStep.EnterCredentials ?: return if (step.username.isBlank() || step.password.isEmpty()) return @@ -212,7 +347,7 @@ class AddAccountViewModel @Inject constructor( // resulting 401 as "credentials rejected" about a password nothing // had tried. val root = serverRoot - ?: return@launch backToServer(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS) + ?: return@launch backToAddress(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS) val credentials = CalDavGateway.Credentials(username, appPassword, root) when (val outcome = gateway.discover(typedInput, credentials)) { is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome) @@ -234,8 +369,8 @@ class AddAccountViewModel @Inject constructor( CalDavDiscovery.Outcome.Unauthenticated -> credentialsRejected() - is CalDavDiscovery.Outcome.NotCalDav -> backToServer(outcome.cause) - is CalDavDiscovery.Outcome.Failed -> backToServer(outcome.cause) + is CalDavDiscovery.Outcome.NotCalDav -> backToAddress(outcome.cause) + is CalDavDiscovery.Outcome.Failed -> backToAddress(outcome.cause) } } } @@ -251,9 +386,9 @@ class AddAccountViewModel @Inject constructor( ) } - /** Step 2b → the browser flow, if this looks like a Nextcloud. */ + /** Step 3b → the browser flow, if this looks like a Nextcloud. */ private suspend fun offerSignIn() { - val root = serverRoot ?: return backToServer(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS) + val root = serverRoot ?: return backToAddress(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS) val flow = gateway.startLoginFlow(root) if (flow == null) { @@ -360,8 +495,8 @@ class AddAccountViewModel @Inject constructor( // outcome as NO_CALENDARS tells someone whose server // named an unresolvable host that their account holds // no task lists, which is both wrong and unactionable. - is CalDavDiscovery.Outcome.Failed -> backToServer(outcome.cause) - is CalDavDiscovery.Outcome.NotCalDav -> backToServer(outcome.cause) + is CalDavDiscovery.Outcome.Failed -> backToAddress(outcome.cause) + is CalDavDiscovery.Outcome.NotCalDav -> backToAddress(outcome.cause) // ⚠️ These two carry no Cause, so they used to land // on "signed in, but no task lists" — said of a // credential the server had just rejected. A 401 here @@ -375,12 +510,12 @@ class AddAccountViewModel @Inject constructor( if (stranger != null) { fatal(AddAccountMessage.OutsideCredentialScope(stranger)) } else { - backToServer(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) + backToAddress(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) } } CalDavDiscovery.Outcome.Unauthenticated -> - backToServer(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) + backToAddress(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) } return@launch } @@ -491,7 +626,7 @@ class AddAccountViewModel @Inject constructor( // Cleared before the state update: an exception there must not // leave a saved account's own credential queued for revoking. if (minted?.password == appPassword) minted = null else discardMintedPassword() - _state.update { it.copy(step = AddAccountStep.Done) } + _state.update { it.copy(step = summaryOf(discovered, chosen)) } } AccountRepository.Outcome.AlreadyExists -> @@ -508,6 +643,63 @@ class AddAccountViewModel @Inject constructor( } } + /** The receipt is read; hand back to whoever hosts the flow. */ + fun onSummaryDone() = _state.update { it.copy(step = AddAccountStep.Done) } + + /** + * Step back inside the flow, or report that there is nowhere left to go. + * + * Only the two steps that have written nothing can be stepped back from. + * From the browser step onwards, "back" means abandoning a flow the server + * is still holding open, and from the list picker it would mean re-running + * discovery — both are the host's business, which is what `false` asks for. + * + * ⚠️ The credentials go with it. `EnterCredentials` is reachable *from* an + * approved browser flow, so stepping back to the address without this leaves + * a minted password behind and a typed address that could carry it to a + * different server — the same leak `onAddressSubmitted` guards. + */ + fun onBackWithin(): Boolean = when (_state.value.step) { + // Back into the errand when there was one, so the steps can be re-read + // without starting the flow again — that is the screen people return to. + is AddAccountStep.EnterAddress -> { + _state.update { it.copy(step = providerOrSetupStep(), fatal = null) } + true + } + + is AddAccountStep.PrepareAccess -> { + _state.update { + it.copy(step = AddAccountStep.ChooseProvider(choice), fatal = null) + } + true + } + + is AddAccountStep.EnterCredentials -> { + discardMintedPassword() + username = "" + appPassword = "" + found = null + hostsNeedingAuth = emptyList() + _state.update { + it.copy(step = addressStep(typedInput), fatal = null, originMismatch = null) + } + true + } + + else -> false + } + + /** Whatever precedes the address: the errand if the service sets one, else the picker. */ + private fun providerOrSetupStep(): AddAccountStep { + val picked = choice ?: return AddAccountStep.ChooseProvider(null) + val quirk = picked.provider?.let(ServerQuirk::forProvider) + return if (quirk?.hasSetupSteps == true) { + AddAccountStep.PrepareAccess(picked, quirk) + } else { + AddAccountStep.ChooseProvider(picked) + } + } + /** * Full reset, including the credentials. * @@ -520,9 +712,11 @@ class AddAccountViewModel @Inject constructor( */ fun onStartOver() { discardMintedPassword() + choice = null pollJob?.cancel() pollJob = null found = null + addressInput = "" typedInput = "" serverRoot = null username = "" @@ -589,7 +783,7 @@ class AddAccountViewModel @Inject constructor( * lying about what it is doing. */ private fun fatal(reason: AddAccountMessage) = _state.update { - it.copy(step = AddAccountStep.EnterServer(input = typedInput), fatal = reason) + it.copy(step = addressStep(typedInput), fatal = reason) } /** @@ -600,8 +794,43 @@ class AddAccountViewModel @Inject constructor( * The screen turns the cause into a translated sentence; nothing renders the * detail. */ - private fun backToServer(cause: CalDavDiscovery.Outcome.Cause) = _state.update { - it.copy(step = AddAccountStep.EnterServer(input = typedInput, error = cause)) + private fun backToAddress(cause: CalDavDiscovery.Outcome.Cause) = _state.update { + it.copy(step = addressStep(typedInput, cause)) + } + + /** + * The address step, carrying the choice that framed the question. + * + * The choice can only be null for a flow that never had a step 1 — nothing + * reaches this without one — and an unnamed server is the honest fallback. + */ + private fun addressStep( + input: String, + error: CalDavDiscovery.Outcome.Cause? = null, + ) = AddAccountStep.EnterAddress( + choice = choice ?: ProviderChoice.OtherServer, + input = input, + error = error, + ) + + /** What was actually set up, named the way the accounts list will name it. */ + private fun summaryOf( + discovered: CalDavDiscovery.Outcome.Found, + chosen: Set, + ): AddAccountStep.Summary { + // The principal, not the typed address: it is what the account stores and + // what identifies self-hosted software, so the receipt and the accounts + // list cannot disagree about what this account is. + val provider = CalDavProvider.forPrincipal(discovered.principal) + val host = discovered.principal.host.removePrefix("www.") + val hosted = provider?.hosted == true + return AddAccountStep.Summary( + provider = provider, + title = if (hosted) provider.label else host, + secondary = if (hosted) host else provider?.label, + username = username, + lists = chosen.map { it.displayName ?: it.url.encodedPath }, + ) } private fun browserFailed(reason: AddAccountMessage) = _state.update { @@ -620,7 +849,10 @@ class AddAccountViewModel @Inject constructor( /** The provider-specific reason a correct-looking password gets rejected. */ private fun quirkHint(): AddAccountMessage? = - when (val quirk = ServerQuirk.forInput(typedInput)) { + when ( + val quirk = choice?.provider?.let(ServerQuirk::forProvider) + ?: ServerQuirk.forInput(typedInput) + ) { ServerQuirk.FASTMAIL_APP_PASSWORD, ServerQuirk.ICLOUD_APP_SPECIFIC_PASSWORD, -> AddAccountMessage.Quirk(quirk) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddressStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddressStep.kt new file mode 100644 index 0000000..2e28328 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddressStep.kt @@ -0,0 +1,43 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.padding +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R + +/** + * Step 2: the one address the chosen service actually needs. + * + * A hosted service is reached by the email address the user already knows, so it + * asks for that and nothing else; a server they run is reached by a URL only + * they have. The old single field had to ask for either, which is why its hint + * had to show both. + */ +@Composable +internal fun AddressStep(step: AddAccountStep.EnterAddress, viewModel: AddAccountViewModel) { + val hosted = step.choice.provider?.hosted == true + Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + FieldRow( + label = if (hosted) { + stringResource(R.string.add_account_email_label) + } else { + stringResource(R.string.add_account_server_label) + }, + value = step.input, + onValueChange = viewModel::onAddressChanged, + placeholder = step.choice.provider?.primaryDomain?.let { "you@$it" } + ?: stringResource(R.string.add_account_server_hint), + error = step.error?.let { stringResource(it.message) }, + // An email address is still typed with the URI keyboard: it is the + // one that carries "@" and "." without a shift, and the field takes + // a URL too whenever a hosted service is reached by one. + keyboardType = KeyboardType.Uri, + onImeAction = viewModel::onAddressSubmitted, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ListsStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ListsStep.kt new file mode 100644 index 0000000..4b366df --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ListsStep.kt @@ -0,0 +1,36 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.SelectedCheck +import de.jeanlucmakiola.floret.components.positionOf + +/** Step 4: which of the account's collections to keep in sync. */ +@Composable +internal fun ListsStep(step: AddAccountStep.ChooseLists, viewModel: AddAccountViewModel) { + // The heading is the shell's; this step only lists. + step.collections.forEachIndexed { index, collection -> + val checked = collection.url in step.selected + GroupedRow( + title = collection.displayName ?: collection.url.encodedPath, + summary = when { + collection.readOnly -> stringResource(R.string.add_account_lists_read_only) + collection.isShared -> stringResource(R.string.add_account_lists_shared) + else -> null + }, + position = positionOf(index, step.collections.size), + selected = checked, + // The family marks a chosen row with a check, not a Material + // checkbox — same affordance every picker in the app uses. + trailing = { if (checked) SelectedCheck() }, + onClick = { viewModel.onListToggled(collection.url) }, + ) + } + Spacer(Modifier.height(16.dp)) +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ProviderStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ProviderStep.kt new file mode 100644 index 0000000..0f2a300 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/ProviderStep.kt @@ -0,0 +1,73 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.agendula.ui.accounts.ProviderLogo +import de.jeanlucmakiola.caldav.CalDavProvider +import de.jeanlucmakiola.caldav.ServerQuirk +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.GroupedSectionHeader +import de.jeanlucmakiola.floret.components.Position +import de.jeanlucmakiola.floret.components.SelectedCheck +import de.jeanlucmakiola.floret.components.positionOf + +/** + * Step 1: which service the tasks live on. + * + * The services we know by name, each by its own mark, and then the escape hatch + * for everything else. Choosing here is what lets step 2 ask one clear question + * instead of "an email address, or a server address, whichever you have" — and + * it is what puts a provider's app-password rule in front of the user *before* + * the 401 rather than after it. + */ +@Composable +internal fun ProviderStep(step: AddAccountStep.ChooseProvider, viewModel: AddAccountViewModel) { + val services = CalDavProvider.selectable + services.forEachIndexed { index, provider -> + val choice = ProviderChoice.Service(provider) + val chosen = step.choice == choice + // ⚠️ On the row, not behind a tap. Google is listed because people come + // looking for it — an absent row reads as the app being unfinished + // rather than as Google's own limitation — but a row that only reveals + // why it cannot be used *after* being selected is the worst of both: it + // looks available, then quietly refuses. + val unusable = ServerQuirk.forProvider(provider)?.isFatal == true + GroupedRow( + title = provider.label, + summary = when { + unusable -> stringResource(R.string.add_account_provider_unsupported) + else -> provider.primaryDomain + ?: stringResource(R.string.add_account_provider_self_hosted) + }, + position = positionOf(index, services.size), + selected = chosen && !unusable, + dimmed = unusable, + leading = { ProviderLogo(provider) }, + trailing = { if (chosen && !unusable) SelectedCheck() }, + // Still tappable: the summary is the short reason, and the note the + // tap brings up is the long one. + onClick = { viewModel.onProviderChosen(choice) }, + ) + } + + GroupedSectionHeader(stringResource(R.string.add_account_provider_other_header)) + val other = ProviderChoice.OtherServer + val otherChosen = step.choice == other + GroupedRow( + title = stringResource(R.string.add_account_provider_other), + summary = stringResource(R.string.add_account_provider_other_summary), + position = Position.Alone, + selected = otherChosen, + // Null is the family's "a server, unnamed" mark — exactly what this row + // is choosing. + leading = { ProviderLogo(provider = null) }, + trailing = { if (otherChosen) SelectedCheck() }, + onClick = { viewModel.onProviderChosen(other) }, + ) + Spacer(Modifier.height(16.dp)) +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SetupStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SetupStep.kt new file mode 100644 index 0000000..f0f3971 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SetupStep.kt @@ -0,0 +1,56 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringArrayResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.caldav.ServerQuirk +import de.jeanlucmakiola.floret.components.InstructionSteps + +/** + * Step 2, first half: the errand the chosen service requires, before anything is + * typed. + * + * A screen of its own rather than a note under the address field. What it asks + * for happens somewhere else entirely — open a browser, sign in to the provider, + * mint an app password, come back — so it has to be read *before* the fields it + * makes answerable, and it needs the room to be four numbered actions rather + * than a paragraph. Underneath an input, it was neither. + * + * The list carries no header of its own: the step's headline already names the + * requirement, and repeating it above the rows says it twice. + */ +@Composable +internal fun SetupStep(step: AddAccountStep.PrepareAccess) { + InstructionSteps( + steps = stringArrayResource(step.quirk.steps).asList(), + footnote = stringResource(step.quirk.footnote), + ) + Spacer(Modifier.height(16.dp)) +} + +/** The headline for the errand — what the service requires, in one line. */ +internal val ServerQuirk.setupTitle: Int + get() = when (this) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> R.string.add_account_setup_fastmail_title + else -> R.string.add_account_setup_icloud_title + } + +private val ServerQuirk.steps: Int + get() = when (this) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> R.array.add_account_setup_fastmail_steps + // Only two quirks reach this screen — ServerQuirk.hasSetupSteps is what + // decides — so the branch that cannot happen takes the other one rather + // than inventing a third set of instructions to be wrong with. + else -> R.array.add_account_setup_icloud_steps + } + +private val ServerQuirk.footnote: Int + get() = when (this) { + ServerQuirk.FASTMAIL_APP_PASSWORD -> R.string.add_account_setup_fastmail_footnote + else -> R.string.add_account_setup_icloud_footnote + } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SignInStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SignInStep.kt new file mode 100644 index 0000000..bc760a4 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SignInStep.kt @@ -0,0 +1,135 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.CloudOff +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.floret.components.Position + +/** + * Step 3a: a username and a password. + * + * The generic CalDAV route, taken whenever the server has no browser sign-in to + * hand off to. + */ +@Composable +internal fun CredentialsStep( + step: AddAccountStep.EnterCredentials, + viewModel: AddAccountViewModel, +) { + Column(Modifier.padding(horizontal = 16.dp), verticalArrangement = Arrangement.spacedBy(16.dp)) { + // Two fields in one connected group, which is what the family does with + // a pair that is filled in together. + Column { + FieldRow( + label = stringResource(R.string.add_account_username_label), + value = step.username, + onValueChange = viewModel::onUsernameChanged, + position = Position.Top, + ) + FieldRow( + label = stringResource(R.string.add_account_password_label), + value = step.password, + onValueChange = viewModel::onPasswordChanged, + position = Position.Bottom, + error = step.error?.text(), + hint = stringResource(R.string.add_account_password_hint), + keyboardType = KeyboardType.Password, + onImeAction = viewModel::onCredentialsSubmitted, + ) + } + } +} + +/** + * Step 3b: the server is signing the user in, in a browser we do not own. + * + * The same slot as [CredentialsStep] because only one of the two ever happens — + * but its own title, mark and body, since "approve this in your browser" and + * "type your password" have nothing in common but their place in the flow. + */ +@Composable +internal fun ColumnScope.BrowserStep(step: AddAccountStep.WaitingForBrowser) { + // ⚠️ The title changes too. Swapping only the body left every failure in + // this phase — a burnt credential, a 429, a maintenance page — sitting + // under the heading "Waiting for your browser", which reads as "still + // working" when nothing is working and nothing else will happen. + Message( + icon = { + if (step.error == null) { + CircularProgressIndicator(Modifier.size(24.dp)) + } else { + Icon(Icons.Rounded.CloudOff, contentDescription = null) + } + }, + title = if (step.error == null) { + stringResource(R.string.add_account_browser_title) + } else { + stringResource(R.string.add_account_browser_failed_title) + }, + text = step.error?.text() ?: stringResource(R.string.add_account_browser_body), + ) + + // ⚠️ A sibling of the message, not a child of its padded column: QuirkNote + // carries the grouped-list inset itself, and nesting it inside one that + // already pads by the same amount insets it twice. + step.hostMismatch?.let { mismatch -> + Spacer(Modifier.height(16.dp)) + QuirkNote( + text = stringResource( + R.string.add_account_browser_host_mismatch, + mismatch.actual, + mismatch.expected, + ), + ) + } +} + +@Composable +internal fun WorkingStep(step: AddAccountStep.Working) { + Column( + Modifier.fillMaxWidth().padding(32.dp), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(16.dp), + ) { + CircularProgressIndicator() + Text(step.message.text(), style = MaterialTheme.typography.bodyLarge) + } +} + +@Composable +private fun Message( + icon: @Composable () -> Unit, + text: String, + title: String? = null, +) { + Column( + Modifier.fillMaxWidth().padding(horizontal = 16.dp), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + icon() + title?.let { Text(it, style = MaterialTheme.typography.titleMedium) } + Text( + text, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SummaryStep.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SummaryStep.kt new file mode 100644 index 0000000..746e149 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/add/SummaryStep.kt @@ -0,0 +1,81 @@ +package de.jeanlucmakiola.agendula.ui.accounts.add + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.Check +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.pluralStringResource +import androidx.compose.ui.unit.dp +import de.jeanlucmakiola.agendula.R +import de.jeanlucmakiola.agendula.ui.accounts.ProviderLogo +import de.jeanlucmakiola.floret.components.GroupedRow +import de.jeanlucmakiola.floret.components.GroupedSectionHeader +import de.jeanlucmakiola.floret.components.positionOf + +/** + * The receipt: what the flow just set up, under the name the accounts list will + * use for it. + * + * Not a confirmation — the account is already saved, and there is nothing here + * to undo. It exists because the wizard used to vanish on success, leaving "did + * it take the right lists, and under which name?" answerable only by going and + * finding the account again. + */ +@Composable +internal fun SummaryStep(step: AddAccountStep.Summary) { + Row( + modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + ProviderLogo(step.provider, size = 48.dp) + Spacer(Modifier.width(16.dp)) + Column { + Text(step.title, style = MaterialTheme.typography.titleMedium) + // The account's own two supporting facts, in the order the accounts + // list gives them: what it is, then who you are on it. + listOfNotNull(step.secondary, step.username.takeIf { it.isNotBlank() }) + .forEach { + Text( + it, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } + + Spacer(Modifier.height(24.dp)) + + GroupedSectionHeader( + pluralStringResource( + R.plurals.add_account_summary_lists, + step.lists.size, + step.lists.size, + ), + ) + step.lists.forEachIndexed { index, name -> + GroupedRow( + title = name, + position = positionOf(index, step.lists.size), + leading = { + Icon( + Icons.Rounded.Check, + contentDescription = null, + tint = MaterialTheme.colorScheme.primary, + ) + }, + ) + } + Spacer(Modifier.height(16.dp)) +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingFlow.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingFlow.kt index 9e7c422..ce1592b 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingFlow.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingFlow.kt @@ -12,6 +12,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource @@ -22,7 +23,8 @@ import androidx.compose.ui.unit.sp import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.agendula.R -import de.jeanlucmakiola.agendula.ui.accounts.AddAccountScreen +import de.jeanlucmakiola.agendula.ui.accounts.add.AddAccountScreen +import de.jeanlucmakiola.agendula.ui.accounts.add.AddAccountViewModel import de.jeanlucmakiola.floret.components.OnboardingProgress import de.jeanlucmakiola.floret.components.OnboardingScaffold import de.jeanlucmakiola.floret.components.OnboardingSpace @@ -34,7 +36,7 @@ import de.jeanlucmakiola.floret.components.OnboardingSpace * * [OnboardingStep.Account] hands the whole screen to the add-account wizard, * which already speaks `OnboardingScaffold` — it just needs to be told where in - * *this* flow its own three steps sit, so the progress stays one bar. + * *this* flow its own steps sit, so the progress stays one bar. */ @Composable fun OnboardingFlow( @@ -44,11 +46,22 @@ fun OnboardingFlow( val state by viewModel.state.collectAsStateWithLifecycle() if (state.step == OnboardingStep.Account) { + // The same instance the wizard resolves for itself, so its length can be + // read here without keeping a second copy of its state. + val account: AddAccountViewModel = hiltViewModel() + val accountState by account.state.collectAsStateWithLifecycle() + // ⚠️ The wizard grows a step for a provider that needs an app password + // minted first, and the outer bar has to grow with it — otherwise first + // run counts one flow while the screen inside it counts another. + LaunchedEffect(accountState.totalSteps) { + viewModel.onAccountStepsChanged(accountState.totalSteps) + } AddAccountScreen( onDone = { viewModel.onAccountFinished(added = true) }, onBack = { viewModel.onAccountFinished(added = false) }, stepOffset = state.position - 1, totalSteps = state.total, + viewModel = account, ) return } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingViewModel.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingViewModel.kt index fc7f567..eeaeb91 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/onboarding/OnboardingViewModel.kt @@ -6,7 +6,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.agendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.agendula.data.tasks.TasksRepository import de.jeanlucmakiola.agendula.data.tasks.recoveringFromProviderFailure -import de.jeanlucmakiola.agendula.ui.accounts.ADD_ACCOUNT_STEPS +import de.jeanlucmakiola.agendula.ui.accounts.add.ADD_ACCOUNT_STEPS import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -31,8 +31,15 @@ enum class OnboardingStep { QuickAdd, ; - /** How many progress segments the step occupies. */ - val slots: Int get() = if (this == Account) ADD_ACCOUNT_STEPS else 1 + /** + * How many progress segments the step occupies, given the length the inline + * wizard currently reports. + * + * ⚠️ Not a constant for [Account]. The wizard grows a step for a service + * that needs an app password minted first, so its length is only known once + * a provider has been picked — and every step after it shifts with it. + */ + fun slots(accountSlots: Int): Int = if (this == Account) accountSlots else 1 } data class OnboardingUiState( @@ -74,6 +81,19 @@ class OnboardingViewModel @Inject constructor( private val step = MutableStateFlow(OnboardingStep.Welcome) + /** + * How long the inline add-account wizard says it is. + * + * Seeded with its shortest form and corrected by the flow itself once a + * provider is chosen, so the bar is honest from the first frame and stays + * honest when the choice makes the flow longer. + */ + private val accountSlots = MutableStateFlow(ADD_ACCOUNT_STEPS) + + fun onAccountStepsChanged(count: Int) { + accountSlots.value = count + } + /** Answered at [OnboardingStep.SyncOffer]; null while it is still the question. */ private val connectsAccount = MutableStateFlow(null) @@ -90,12 +110,18 @@ class OnboardingViewModel @Inject constructor( } val state: StateFlow = - combine(step, connectsAccount, needsList, prefs.settings) { step, sync, needsList, settings -> + combine( + step, + connectsAccount, + needsList, + prefs.settings, + accountSlots, + ) { step, sync, needsList, settings, slots -> val plan = plan(sync ?: false, needsList) OnboardingUiState( step = step, - position = plan.takeWhile { it != step }.sumOf { it.slots } + 1, - total = plan.sumOf { it.slots }, + position = plan.takeWhile { it != step }.sumOf { it.slots(slots) } + 1, + total = plan.sumOf { it.slots(slots) }, canGoBack = canGoBack(step, sync), bottomAddBar = settings.bottomAddBar, ) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt index 53b66be..4a9cea4 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/ui/settings/SettingsScreen.kt @@ -44,7 +44,7 @@ import de.jeanlucmakiola.agendula.R import de.jeanlucmakiola.agendula.ui.accounts.AccountDetailScreen import de.jeanlucmakiola.agendula.ui.accounts.AccountsScreen import de.jeanlucmakiola.agendula.ui.accounts.AccountsViewModel -import de.jeanlucmakiola.agendula.ui.accounts.AddAccountScreen +import de.jeanlucmakiola.agendula.ui.accounts.add.AddAccountScreen import de.jeanlucmakiola.agendula.ui.export.ExportScreen import de.jeanlucmakiola.agendula.ui.licences.LicencesScreen import de.jeanlucmakiola.floret.components.AboutCard diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c3812b5..82b51b4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -326,6 +326,10 @@ Save as a zip file Exporting… Open + + Syncing %1$d list + Syncing %1$d lists + Exported %1$d list Exported %1$d lists @@ -373,8 +377,21 @@ Sync now Sign-in didn\u2019t finish Step %1$d of %2$d + Where are your tasks? + Pick the service your task lists live on. Agendula syncs with anything that speaks CalDAV. + You run the server + Doesn\u2019t support tasks over CalDAV + Any CalDAV server + Other CalDAV server + Baïkal, DAViCal, SOGo, Radicale, or one you run yourself + What\u2019s your address there? + The email address you use with this service. Agendula finds the server itself. + Email address + You\u2019re set up + Agendula will keep these lists in sync from now on. You can change what syncs in Settings. + Done Where are your tasks? - Enter the address of your CalDAV server, or the email address you use with it. + Enter the address of your CalDAV server. An email address works too, if the server publishes itself under its domain. Sign in Use an app password if your provider offers one \u2014 it can be revoked without changing your account password. Pick the task lists to keep in sync. You can change this later. @@ -403,6 +420,23 @@ This device has no browser that can open the sign-in page. Use a password instead. Fastmail needs an app password, not your account password \u2014 and CalDAV is not on the Basic plan. iCloud needs an app-specific password, which you create at appleid.apple.com with two-factor on. + You\u2019ll do this in your browser, then come back here to finish. + iCloud needs an app-specific password + + Sign in at appleid.apple.com. + Turn on two-factor authentication if it isn\u2019t already \u2014 Apple only offers app passwords with it on. + Under Sign-In and Security, open App-Specific Passwords and create one. + Come back here and use that password with your iCloud address. + + Tasks kept in iCloud won\u2019t show up in Apple\u2019s Reminders app. + Fastmail needs an app password + + Sign in at fastmail.com. + Open Settings, then Privacy & Security, then Integrations. + Under App Passwords, create one and give it CalDAV access. + Come back here and use that password with your Fastmail address. + + CalDAV isn\u2019t included in Fastmail\u2019s Basic plan. Fastmail needs an app password, and CalDAV is not available on the Basic plan. iCloud needs an app-specific password, created at appleid.apple.com with two-factor on. Tasks stored there will not appear in Reminders. Google Calendar does not support tasks over CalDAV. @@ -413,8 +447,8 @@ Syncing tasks Add an account - Email address or server address - you@example.com, or https://cloud.example.com + Server address + https://cloud.example.com Continue Start over diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModelTest.kt index f69703f..a7dfd2c 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModelTest.kt @@ -1,4 +1,4 @@ -package de.jeanlucmakiola.agendula.ui.accounts +package de.jeanlucmakiola.agendula.ui.accounts.add import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.agendula.data.sync.AccountCreator @@ -6,7 +6,9 @@ import de.jeanlucmakiola.agendula.data.sync.AccountRepository import de.jeanlucmakiola.agendula.data.sync.CalDavGateway import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord import de.jeanlucmakiola.caldav.CalDavDiscovery +import de.jeanlucmakiola.caldav.CalDavProvider import de.jeanlucmakiola.caldav.NextcloudLoginFlow +import de.jeanlucmakiola.caldav.ServerQuirk import de.jeanlucmakiola.caldav.TaskCollection import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -54,14 +56,109 @@ class AddAccountViewModelTest { private fun viewModel() = AddAccountViewModel(creator, gateway, record, appScope) + @Nested + inner class TheProviderStep { + + @Test + fun `a service with an errand puts it on its own screen first`() = + runTest(dispatcher) { + val vm = viewModel() + vm.onProviderChosen(ProviderChoice.Service(CalDavProvider.FASTMAIL)) + + // The whole reason the picker earns a step: Fastmail rejects the + // account password with a plain 401, and saying so afterwards is + // saying it too late. It is a screen rather than a note because + // it is four actions in someone else's web app. + val step = vm.state.value.step as AddAccountStep.PrepareAccess + assertThat(step.quirk).isEqualTo(ServerQuirk.FASTMAIL_APP_PASSWORD) + + vm.onSetupAcknowledged() + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.EnterAddress::class.java) + } + + @Test + fun `a service we cannot sync with does not advance`() = runTest(dispatcher) { + val vm = viewModel() + vm.onProviderChosen(ProviderChoice.Service(CalDavProvider.GOOGLE)) + + // Picking the wrong service is a tap to undo, so it stays on the + // picker rather than ending the flow the way a typed address does. + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.ChooseProvider::class.java) + assertThat(vm.state.value.quirk).isEqualTo(ServerQuirk.GOOGLE_UNSUPPORTED) + } + + @Test + fun `a service with nothing to prepare goes straight to the address`() = + runTest(dispatcher) { + val vm = viewModel() + val choice = ProviderChoice.Service(CalDavProvider.POSTEO) + // One tap, no Continue: the row is the decision. + vm.onProviderChosen(choice) + + // The step carries the choice because the question it asks + // depends on it: a hosted service is reached by an email address, + // a server by a URL. + assertThat((vm.state.value.step as AddAccountStep.EnterAddress).choice) + .isEqualTo(choice) + } + + @Test + fun `the address steps back to the picker`() = runTest(dispatcher) { + val vm = viewModel() + vm.enterAddress("https://cloud.example.com/") + + assertThat(vm.onBackWithin()).isTrue() + assertThat(vm.state.value.step) + .isEqualTo(AddAccountStep.ChooseProvider(ProviderChoice.OtherServer)) + } + + @Test + fun `the address steps back into the errand when there was one`() = + runTest(dispatcher) { + val vm = viewModel() + vm.onProviderChosen(ProviderChoice.Service(CalDavProvider.ICLOUD)) + vm.onSetupAcknowledged() + + // Back to the instructions, not past them: that is the screen + // someone returns to when they lose the app password. + assertThat(vm.onBackWithin()).isTrue() + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.PrepareAccess::class.java) + assertThat(vm.onBackWithin()).isTrue() + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.ChooseProvider::class.java) + } + + @Test + fun `the picker is the end of the road, so the host takes the back press`() = + runTest(dispatcher) { + val vm = viewModel() + vm.onProviderChosen(ProviderChoice.OtherServer) + vm.onBackWithin() + + // False is what makes the screen abandon rather than swallow the + // gesture — and abandoning is what runs onStartOver, which is the + // only thing that stops a poll and hands back a minted password. + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.ChooseProvider::class.java) + assertThat(vm.onBackWithin()).isFalse() + } + + @Test + fun `there is nowhere to step back to from the list picker`() = runTest(dispatcher) { + val vm = signedIn() + + // Back there would mean re-running discovery, so the host is left to + // decide what leaving means. + assertThat(vm.onBackWithin()).isFalse() + } + } + @Nested inner class TheAddressStep { @Test fun `Google is refused before any request is made`() = runTest(dispatcher) { val vm = viewModel() - vm.onServerInputChanged("me@gmail.com") - vm.onServerSubmitted() + vm.enterAddress("me@gmail.com") + vm.onAddressSubmitted() advanceUntilIdle() // It supports neither VTODO nor MKCALENDAR — its own docs say so — so @@ -73,7 +170,7 @@ class AddAccountViewModelTest { @Test fun `a provider quirk is named while the user is still typing`() = runTest(dispatcher) { val vm = viewModel() - vm.onServerInputChanged("me@fastmail.com") + vm.enterAddress("me@fastmail.com") assertThat(vm.state.value.quirk).isNotNull() } @@ -87,8 +184,8 @@ class AddAccountViewModelTest { gateway.loginFlow = null val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() assertThat(vm.state.value.step) @@ -103,11 +200,11 @@ class AddAccountViewModelTest { ) val vm = viewModel() - vm.onServerInputChanged("https://example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://example.com/") + vm.onAddressSubmitted() advanceUntilIdle() - val step = vm.state.value.step as AddAccountStep.EnterServer + val step = vm.state.value.step as AddAccountStep.EnterAddress // ⚠️ A cause, never the server's own words — the screen owns the // wording, and a raw status line bypasses strings.xml entirely. assertThat(step.error).isEqualTo(CalDavDiscovery.Outcome.Cause.NO_CALENDAR_SUPPORT) @@ -139,8 +236,8 @@ class AddAccountViewModelTest { gateway.discoveryOutcomes += found(collection("Tasks")) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() assertThat(gateway.discoveries.last().target).isEqualTo("https://dav.example.com/") @@ -169,8 +266,8 @@ class AddAccountViewModelTest { gateway.discoveryOutcomes += found(collection("Tasks")) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() // Sticky: it is learned mid-flow, but the setting it blames is @@ -202,14 +299,14 @@ class AddAccountViewModelTest { ) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() // ⚠️ Collapsing this to NO_CALENDARS tells someone with a DNS // failure that their account holds no task lists — wrong, and // nothing they can act on. - val step = vm.state.value.step as AddAccountStep.EnterServer + val step = vm.state.value.step as AddAccountStep.EnterAddress assertThat(step.error).isEqualTo(CalDavDiscovery.Outcome.Cause.UNREACHABLE) } @@ -219,8 +316,8 @@ class AddAccountViewModelTest { gateway.loginFlow = flow() val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() // runCurrent, not advanceUntilIdle: the poll loop's first act is a // delay, so this settles discovery without consuming the flow. runCurrent() @@ -238,8 +335,8 @@ class AddAccountViewModelTest { // The fake never approves, so only the loop's own bound ends it. val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() val step = vm.state.value.step as AddAccountStep.WaitingForBrowser @@ -252,8 +349,8 @@ class AddAccountViewModelTest { gateway.loginFlow = null val vm = viewModel() - vm.onServerInputChanged("https://baikal.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://baikal.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() assertThat(vm.state.value.step) @@ -273,8 +370,8 @@ class AddAccountViewModelTest { gateway.loginFlow = flow() val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() // Not advanceUntilIdle: that would run the whole twenty-minute // poll loop out to its expiry, which forgets the flow again. runCurrent() @@ -300,8 +397,8 @@ class AddAccountViewModelTest { gateway.discoveryOutcomes += found(collection("Tasks")) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() // The server deleted the flow row before answering, so the token now @@ -318,8 +415,8 @@ class AddAccountViewModelTest { gateway.pollResults += NextcloudLoginFlow.PollResult.Expired("window closed") val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() vm.onStartOver() advanceUntilIdle() @@ -336,6 +433,23 @@ class AddAccountViewModelTest { @Nested inner class AbandoningTheBrowserFlow { + @Test + fun `stepping back from the credentials hands the minted password back`() = + runTest(dispatcher) { + val vm = approveThen( + CalDavDiscovery.Outcome.NeedsAuthentication(listOf("cloud.example.com")), + ) + // The credentials step is reachable *from* an approved browser + // flow, so stepping back off it is one of the routes that leaks a + // one-shot password if it does not clean up. + vm.onBrowserCancelled() + gateway.revoked.clear() + vm.onBackWithin() + advanceUntilIdle() + + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.EnterAddress::class.java) + } + private fun TestScope.approveThen(outcome: CalDavDiscovery.Outcome): AddAccountViewModel { gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication( listOf("cloud.example.com"), @@ -350,8 +464,8 @@ class AddAccountViewModelTest { ) gateway.discoveryOutcomes += outcome val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() return vm } @@ -373,7 +487,7 @@ class AddAccountViewModelTest { CalDavDiscovery.Outcome.Cause.UNREACHABLE, "no route to host", ) - vm.onServerSubmitted() + vm.onAddressSubmitted() advanceUntilIdle() // Nextcloud hands it over exactly once. Walking away leaves it valid @@ -439,7 +553,7 @@ class AddAccountViewModelTest { gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication( listOf("cloud.example.com"), ) - vm.onServerSubmitted() + vm.onAddressSubmitted() advanceUntilIdle() // The retry itself hands back the first password. @@ -460,7 +574,7 @@ class AddAccountViewModelTest { advanceUntilIdle() assertThat(vm.state.value.fatal).isNull() - assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.EnterServer::class.java) + assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.ChooseProvider::class.java) } @Test @@ -471,7 +585,7 @@ class AddAccountViewModelTest { // "Signed in, but no task lists" said of a credential the server had // just refused. Same registrable domain, so retrying may help. - val step = vm.state.value.step as AddAccountStep.EnterServer + val step = vm.state.value.step as AddAccountStep.EnterAddress assertThat(step.error).isEqualTo(CalDavDiscovery.Outcome.Cause.SERVER_ERROR) } @@ -505,15 +619,15 @@ class AddAccountViewModelTest { gateway.loginFlow = null val vm = viewModel() - vm.onServerInputChanged("cloud.example.com/nextcloud") - vm.onServerSubmitted() + vm.enterAddress("cloud.example.com/nextcloud") + vm.onAddressSubmitted() advanceUntilIdle() vm.onUsernameChanged("me") vm.onPasswordChanged("pw") vm.onCredentialsSubmitted() advanceUntilIdle() - val step = vm.state.value.step as AddAccountStep.EnterServer + val step = vm.state.value.step as AddAccountStep.EnterAddress assertThat(step.error).isEqualTo(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS) // One discovery, the anonymous one. Nothing was sent blind. assertThat(gateway.discoveries).hasSize(1) @@ -532,8 +646,8 @@ class AddAccountViewModelTest { ) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() vm.onUsernameChanged("me") vm.onPasswordChanged("pw") @@ -557,8 +671,8 @@ class AddAccountViewModelTest { ) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() val step = vm.state.value.step as AddAccountStep.ChooseLists @@ -572,8 +686,8 @@ class AddAccountViewModelTest { gateway.discoveryOutcomes += found() val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() assertThat(vm.state.value.fatal).isEqualTo(AddAccountMessage.NoUsableLists) @@ -591,8 +705,8 @@ class AddAccountViewModelTest { gateway.loginFlow = null val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() vm.onUsernameChanged("me") vm.onPasswordChanged("first-pw") @@ -602,8 +716,8 @@ class AddAccountViewModelTest { assertThat(vm.state.value).isEqualTo(AddAccountUiState()) // And a save attempt now has nothing to save, rather than reusing it. gateway.discoveryOutcomes += found(collection("Tasks")) - vm.onServerInputChanged("https://other.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://other.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() vm.onSave() advanceUntilIdle() @@ -634,14 +748,14 @@ class AddAccountViewModelTest { "no route to host", ) val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() gateway.loginFlow = null gateway.discoveryOutcomes += found(collection("Tasks")) - vm.onServerInputChanged("https://other.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://other.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() vm.onSave() advanceUntilIdle() @@ -676,6 +790,15 @@ class AddAccountViewModelTest { advanceUntilIdle() assertThat(creator.created).containsExactly("me@cloud.example.com") + + // The receipt, not the exit: the flow says what it built before it + // hands back, and only the user's acknowledgement ends it. + val summary = vm.state.value.step as AddAccountStep.Summary + assertThat(summary.title).isEqualTo("cloud.example.com") + assertThat(summary.username).isEqualTo("me") + assertThat(summary.lists).containsExactly("Tasks") + + vm.onSummaryDone() assertThat(vm.state.value.step).isEqualTo(AddAccountStep.Done) } } @@ -691,8 +814,8 @@ class AddAccountViewModelTest { gateway.loginFlow = null val vm = viewModel() - vm.onServerInputChanged("https://cloud.example.com/") - vm.onServerSubmitted() + vm.enterAddress("https://cloud.example.com/") + vm.onAddressSubmitted() advanceUntilIdle() gateway.discoveryOutcomes += found(collection("Tasks")) @@ -703,6 +826,16 @@ class AddAccountViewModelTest { return vm } + /** + * The taps and typing that put a bare address into the flow: step 1's "Other + * server", then the field. An unnamed server is what a typed address *is*, so + * every test that only cares about discovery takes this route. + */ + private fun AddAccountViewModel.enterAddress(input: String) { + onProviderChosen(ProviderChoice.OtherServer) + onAddressChanged(input) + } + // ------------------------------------------------------------- fixtures private fun collection(name: String, readOnly: Boolean = false) = TaskCollection( diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavProvider.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavProvider.kt index 5422653..eb3b904 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavProvider.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavProvider.kt @@ -56,8 +56,33 @@ enum class CalDavProvider( YANDEX("Yandex", hosted = true, domains = setOf("yandex.ru", "yandex.com")), ; + /** + * The domain to put in an address hint, for a service the user reaches by + * email address. Null for software they run themselves, whose host is theirs. + */ + val primaryDomain: String? get() = domains.firstOrNull() + companion object { + /** + * The services worth offering by name on a picker, in the order they + * should be offered. + * + * Everything identified by domain, plus Nextcloud — picked for its + * browser sign-in rather than for its host. The rest are self-hosted + * software recognised *after* discovery, from the principal path, so + * naming them here would only add rows that all ask the same question. + * + * A service that cannot be synced at all sorts to the **end**. It is + * still listed — people come looking for Google, and an absent row reads + * as the app being unfinished rather than as Google's own limitation — + * but a dead row belongs under the live ones, not in the middle of them. + * The sort is stable, so everything else keeps declaration order. + */ + val selectable: List + get() = entries.filter { it.domains.isNotEmpty() || it == NEXTCLOUD } + .sortedBy { ServerQuirk.forProvider(it)?.isFatal == true } + /** The service a host belongs to, if it is one we know by name. */ fun forHost(host: String): CalDavProvider? { val lower = host.lowercase().trimEnd('.') diff --git a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt index 868eeee..0b19aa2 100644 --- a/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt +++ b/caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServerQuirks.kt @@ -45,6 +45,9 @@ enum class ServerQuirk { fun forHost(host: String): ServerQuirk? = CalDavProvider.forHost(host)?.quirk + /** What is known about a service the user picked by name rather than typed. */ + fun forProvider(provider: CalDavProvider): ServerQuirk? = provider.quirk + fun forUrl(url: HttpUrl): ServerQuirk? = forHost(url.host) private val CalDavProvider.quirk: ServerQuirk? @@ -59,4 +62,16 @@ enum class ServerQuirk { /** True when discovery should not even be attempted. */ val isFatal: Boolean get() = this == GOOGLE_UNSUPPORTED + + /** + * True when the quirk is an errand the user can actually run — go to the + * provider, mint an app password, come back — rather than a refusal or a + * note for the engine. + * + * What separates the two is whether there is anything to *do*. Google is a + * dead end and the brute-force note is never shown, so neither earns the + * screen that walks through the steps. + */ + val hasSetupSteps: Boolean + get() = this == FASTMAIL_APP_PASSWORD || this == ICLOUD_APP_SPECIFIC_PASSWORD } diff --git a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt index 283544c..35026bc 100644 --- a/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt +++ b/caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerQuirksTest.kt @@ -28,6 +28,20 @@ class ServerQuirksTest { assertThat(ServerQuirk.forHost("icloud.com.example.org")).isNull() } + @Test + fun `a service that cannot be synced is offered last, not omitted`() { + val offered = CalDavProvider.selectable + // Listed, because people go looking for it and an absent row reads as + // the app being unfinished rather than as Google's own limitation. + assertThat(offered).contains(CalDavProvider.GOOGLE) + assertThat(offered.last()).isEqualTo(CalDavProvider.GOOGLE) + // Everything else keeps declaration order — the sort only moves the + // dead rows, so Nextcloud stays the first thing offered. + assertThat(offered.first()).isEqualTo(CalDavProvider.NEXTCLOUD) + assertThat(offered.none { ServerQuirk.forProvider(it)?.isFatal == true && it != CalDavProvider.GOOGLE }) + .isTrue() + } + @Test fun `a self-hosted server has no quirk`() { assertThat(ServerQuirk.forInput("https://cloud.example.de/remote.php/dav/")).isNull() diff --git a/floret-kit b/floret-kit index 97b4d24..103a37c 160000 --- a/floret-kit +++ b/floret-kit @@ -1 +1 @@ -Subproject commit 97b4d24724170b1f15a4a401d7a908cea5e3b05e +Subproject commit 103a37cb28ea5f3adee9cad33ddea48285ecccf9