Trim the wizard's comments to what carries weight (#163)

Drop the paragraphs narrating how the screens got here — the retired
visibility dialog, the preview that used to fit one screen, the
cross-fade comparison — and shorten the rest. Also correct the
ARCHITECTURE note on arming, which described the old grant-callback
route.

app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefs.kt

@@ -623,9 +623,8 @@ class SettingsPrefs @Inject constructor(
 
     /**
      * Close the wizard for good: the closing screen has been seen and the
-     * install stops counting as fresh, so a later revoke-and-re-grant of the
+     * install stops counting as fresh, so a later revoke-and-re-grant is only
-     * calendar permission is only ever the permission screen. One edit, so the
+     * the permission screen. One edit, so the plan can't see it half-closed.
-     * plan can never observe a half-closed wizard.
      */
     suspend fun finishOnboardingWizard() {
         store.edit { prefs ->

app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt

@@ -91,9 +91,8 @@ fun RootScreen(
     val onboarding: OnboardingViewModel = hiltViewModel()
     val plan by onboarding.plan.collectAsStateWithLifecycle()
     // The grant is reported however it was taken, including via Android's
-    // app-settings screen (caught by the ON_RESUME above) — arming the wizard
+    // app-settings screen (caught by the ON_RESUME above); arming the wizard
-    // hangs off that, not off the callback. Only a grant made during this
+    // hangs off that, not off the in-app callback.
-    // session owes the re-scan.
     LaunchedEffect(hasPermission) {
         onboarding.setHasPermission(hasPermission)
         if (hasPermission && !grantedAtLaunch) onboarding.onPermissionGranted()
@@ -112,9 +111,8 @@ fun RootScreen(
         else -> RootTarget.Step(current.current)
     }
 
-    // Which way the flow is moving, so a step-back slides back. The app sits
+    // Which way the flow is moving, so a step-back slides back. The app sorts
-    // after every step and the blank first frame before them, so finishing the
+    // after every step, so finishing the wizard reads as one more move forward.
-    // wizard reads as one more move forward.
     val ordinal = when (target) {
         RootTarget.Loading -> Int.MIN_VALUE
         RootTarget.App -> Int.MAX_VALUE
@@ -124,17 +122,13 @@ fun RootScreen(
     val slideDir = if (ordinal < lastOrdinal) -1 else 1
     SideEffect { lastOrdinal = ordinal }
 
-    // The wizard swipes: the steps sit side by side and the whole width travels,
-    // so the flow reads as moving through a stack of screens rather than as one
-    // screen redrawing itself.
     val slideSpec = rememberOnboardingSlideSpec()
     val fadeSpec = rememberCalendarFadeSpec()
     val reduceMotion = rememberReduceMotion()
     AnimatedContent(
         targetState = target,
         transitionSpec = {
-            // Coming off the blank first frame is not a step change — the app
+            // Coming off the blank first frame is not a step change.
-            // has only just opened, so the first screen simply appears.
             if (initialState == RootTarget.Loading) {
                 fadeIn(fadeSpec).togetherWith(fadeOut(fadeSpec))
             } else {
@@ -164,9 +158,8 @@ fun RootScreen(
                 requestedEditKey = requestedEditKey,
                 onEditKeyConsumed = onEditKeyConsumed,
             )
-            // The plan the outgoing step was drawn with is gone by the time the
+            // Both halves of a transition read their position out of the live
-            // transition ends, so the live one carries both halves; each reads
+            // plan; the one the outgoing step was drawn with is already gone.
-            // its own position out of it.
             is RootTarget.Step -> current?.let { plan ->
                 OnboardingSteps(
                     step = shown.step,

app/src/main/java/de/jeanlucmakiola/calendula/ui/common/ViewPreview.kt

@@ -39,18 +39,14 @@ internal val PREVIEW_VIEWPORT_HEIGHT = 440.dp
 
 /**
  * Renders [content] at a full phone viewport and shrinks the finished layout
- * into a box [height] tall, for the settings and onboarding choosers.
+ * into a box [height] tall, for the settings and onboarding choosers. Callers
+ * pass the *real* view composables, so a preview cannot drift from what it
+ * depicts.
  *
- * Previews render the *real* view composables rather than a drawing of them, so
+ * The [layout] below ignores the incoming constraints so the view measures at a
- * they cannot drift from the thing they depict: change a cell's shape or an
+ * phone viewport, and a [graphicsLayer] scale shrinks the result.
- * event block's colour and every preview follows. The trick is the [layout]
+ * `Modifier.requiredSize` is the obvious alternative but *centres* content that
- * below — it ignores the incoming constraints, so the view lays itself out at a
+ * overflows its constraints, leaving only the bottom-right corner in the clip.
- * plausible phone viewport, and a [graphicsLayer] scale shrinks the result.
- *
- * `Modifier.requiredSize` would be the obvious way to force the larger
- * measurement, but it *centres* content that overflows the incoming constraints
- * — which pushes the view to a negative offset and leaves only its bottom-right
- * corner inside the clip.
  */
 @Composable
 internal fun ScaledViewPreview(
@@ -99,13 +95,9 @@ internal fun ScaledViewPreview(
 
 /**
  * The stage every preview picker sets its preview on, cross-fading as [selected]
- * changes.
+ * changes. A preview draws the app's own surface, so a tonal plate gives it the
- *
+ * edge it would otherwise lack on a chooser screen; the plate takes the width
- * A preview draws the app's own surface, so on a chooser screen — which draws the
+ * and outer radius of the grouped rows underneath so the two read as one stack.
- * same surface — it had no edge of its own and dissolved into the page. A tonal
- * plate gives it one: the preview reads as a screen being shown rather than as
- * part of the screen showing it. The plate takes the width and the outer radius
- * of the grouped rows underneath, so the two read as one stack.
  *
  * Only vertical spacing is the caller's to pass — the horizontal inset is the
  * rows' own, and the alignment is the point.

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/BackupStep.kt

@@ -28,9 +28,8 @@ import de.jeanlucmakiola.floret.components.OnboardingSpace
  * Wizard step shown only when everything you write to is a device-only calendar
  * (#163): the events exist nowhere but this phone, so losing it loses them.
  *
- * The primary action picks a folder and switches automatic backup on in one go
+ * The primary action picks a folder and switches automatic backup on in one go.
- * — the step changes something rather than pointing at Settings. Everything it
+ * Everything it sets is a normal Backup setting, adjustable afterwards.
- * sets is a normal Backup setting, so it stays adjustable afterwards.
  */
 @Composable
 internal fun BackupStep(
@@ -43,8 +42,7 @@ internal fun BackupStep(
     val pickFolder = rememberLauncherForActivityResult(
         contract = ActivityResultContracts.OpenDocumentTree(),
     ) { uri ->
-        // Cancelling the picker leaves the step open: nothing was decided, and
+        // Cancelling the picker decides nothing and leaves the step open.
-        // "Not now" is right there for the user who meant to decline.
         uri?.let(onEnable)
     }
 

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/OnboardingMotion.kt

@@ -28,12 +28,9 @@ internal fun rememberOnboardingSlideSpec(): FiniteAnimationSpec<IntOffset> =
  * width swipes across, position on a spring so the arriving step settles with a
  * little give rather than stopping dead.
  *
- * No cross-fade, unlike the calendar's shared-axis paging. Adjacent months are
+ * No cross-fade, unlike the calendar's shared-axis paging: two steps have
- * near-identical grids, so there the fade does the swapping and a short slide
+ * nothing in common, so fading one through the other smears. Both are opaque and
- * only hints the direction; here the two steps have nothing in common and fading
+ * travel in lockstep, showing one screen pushing the other off.
- * one through the other read as a smear. Both steps are opaque and travel in
- * lockstep, so a full-width slide shows one screen pushing the other off with no
- * seam between them.
  *
  * @param slideDir     +1 = forward (incoming from the right), -1 = back.
  * @param spec         spatial spec, typically [rememberOnboardingSlideSpec].

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/OnboardingPlan.kt

@@ -55,31 +55,27 @@ data class OnboardingPlan(
 /**
  * Work out the flow from what is stored.
  *
- * The optional steps are only for installs that are still working through their
+ * The optional steps belong to installs still working through their first run.
- * first run — an existing user sees nothing new. Before the grant that can only
+ * Before the grant that can only be guessed, so one that has not answered the
- * be guessed, so an install that has not answered the reminder step either is
+ * reminder step either counts as fresh — the same condition
- * treated as fresh, which is the same condition
  * [de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs.armOnboardingWizard]
- * commits to storage a moment later. The flag is cleared again on the closing
+ * commits to storage a moment later, and clears again on the closing screen so
- * screen, so revoking and re-granting the permission years later is only ever
+ * a later re-grant is only the permission step.
- * the permission step.
  *
  * [backupApplies] is null until the calendar list can be read, i.e. for the
- * whole permission step. The backup step is assumed to apply until proven
+ * whole permission step; the backup step is assumed to apply until proven
  * otherwise, so the flow can only ever get shorter — never sprout a step the
  * counter had not accounted for.
  *
  * The month-style step is *not* conditional on Month being the chosen view:
- * Month is reachable from the drawer whatever opens first, and making the step
+ * Month is reachable from the drawer whatever opens first, and a step that came
- * appear and disappear as the view is picked would move the counter under the
+ * and went as the view is picked would move the counter under the user on the
- * user on the step right before it.
+ * step right before it.
  *
  * [visibilityArmed] carries the one-time notice that the device is hiding some
- * calendars (#75). It is a step rather than a dialog over the app so that an
+ * calendars (#75) — the only optional step an *existing* install can be given,
- * install owing both meets it inside the flow instead of straight after it —
+ * which is why it is not gated on `fresh`, and on its own it earns no closing
- * and it is the only optional step an *existing* install can be given, which is
+ * screen.
- * why it is not gated on `fresh`. The closing screen is: there is nothing to
- * conclude for someone who was only shown one notice.
  */
 fun onboardingPlan(
     hasPermission: Boolean,

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/OnboardingSteps.kt

@@ -26,8 +26,7 @@ fun OnboardingSteps(
     modifier: Modifier = Modifier,
 ) {
     // Collected for every step, not just the two that draw a preview: starting
-    // it when the picker composes would leave the step blank for the frames the
+    // it when the picker composes would leave that step blank as it slides in.
-    // first stored emission takes to arrive — right as it slides in.
     val choice by viewModel.viewChoice.collectAsStateWithLifecycle()
 
     // Coerced because the outgoing half of a transition may be a step the live

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/OnboardingViewModel.kt

@@ -60,9 +60,8 @@ class OnboardingViewModel @Inject constructor(
 
     /**
      * The grant as the rest of the flow sees it: published only once the wizard
-     * has been armed for it. Arming is a stored write, so announcing the grant
+     * has been armed for it, since arming is a stored write and the frames
-     * first would leave a few frames in which the install looks neither fresh
+     * before it lands would collapse the plan to its one-step form.
-     * nor permission-less and the plan collapses to its one-step form.
      */
     private val hasPermission: StateFlow<Boolean?> = permissionReported
         .mapLatest { granted ->
@@ -78,9 +77,9 @@ class OnboardingViewModel @Inject constructor(
     /**
      * Whether the backup step applies: something of yours is worth exporting and
      * nothing you write to is synced anywhere. Null while the calendar list
-     * cannot be read, which is every frame before the grant — and any frame the
+     * cannot be read: every frame before the grant, and any read that fails —
-     * provider read fails, since an empty list is also how a null cursor
+     * an empty list is also how a null cursor arrives, and guessing false there
-     * arrives. Guessing false there would drop the step for the whole session.
+     * would drop the step for the whole session.
      */
     private val backupApplies: Flow<Boolean?> = hasPermission.flatMapLatest { granted ->
         if (granted != true) {
@@ -152,8 +151,7 @@ class OnboardingViewModel @Inject constructor(
      * Arm the wizard for an install that has yet to answer the reminder step,
      * whatever route the grant took. Gating this on the in-app grant would
      * strand anyone who granted on Android's own settings screen and came back
-     * to a restarted process: they hold the permission at launch, so nothing
+     * to a restarted process, leaving them the reminder step alone.
-     * would ever arm and the flow would silently shrink to the reminder step.
      */
     private suspend fun armWizardIfFresh() {
         if (!prefs.reminderOnboardingDone.first()) prefs.armOnboardingWizard()
@@ -252,9 +250,8 @@ class OnboardingViewModel @Inject constructor(
     fun finishVisibility() {
         viewModelScope.launch {
             prefs.setOnboardingVisibilityDone()
-            // Retiring the notice drops the step from the plan, so it waits
+            // Retiring the notice drops its step, so it waits until nothing
-            // until nothing follows it — otherwise the count would fall from
+            // follows — the count must not fall on the way to a later screen.
-            // under the user on the way to the closing screen.
             if (plan.value?.steps?.last() == OnboardingStep.Visibility) retireVisibilityNotice()
         }
     }
@@ -268,9 +265,9 @@ class OnboardingViewModel @Inject constructor(
     }
 
     /**
-     * Put the one-time visibility notice away for good. Nothing else clears it
+     * Put the one-time visibility notice away for good — nothing else clears it
-     * now that the wizard has taken the notice over, and left standing it would
+     * now that the wizard owns it, and left standing it would keep its step in
-     * keep its step in every later plan — padding the counter on a re-grant.
+     * every later plan.
      */
     private suspend fun retireVisibilityNotice() {
         if (calendarPrefs.visibilityNoticePending.first() == true) {

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/ViewStep.kt

@@ -37,13 +37,9 @@ import de.jeanlucmakiola.floret.components.positionOf
 import de.jeanlucmakiola.floret.locale.currentLocale
 
 /**
- * Wizard step: which view the app opens on (#163).
+ * Wizard step: which view the app opens on (#163). Built like the Settings
- *
+ * picker it mirrors — a live preview over connected grouped rows. Selecting
- * Built like the Settings pickers it mirrors — a live preview over connected
+ * applies immediately and leaves the step open; the preview is the confirmation.
- * grouped rows — so the choice is made by looking rather than by reading four
- * names and guessing, and so the same choice looks the same wherever it is
- * made. Selecting applies immediately and leaves the step open; the preview
- * above is the confirmation.
  */
 @Composable
 internal fun ViewStep(
@@ -163,8 +159,8 @@ private fun StepPickerScaffold(
         navigationIcon = navigationIcon,
         topSpacing = OnboardingSpace.xs,
         contentPadding = 0.dp,
-        // The preview is worth more than a screenful of certainty: it gets the
+        // The preview gets the room it needs; Continue scrolls in under the
-        // room it needs and Continue scrolls in under the options with it.
+        // options rather than pinning to the bottom.
         scrollingActions = true,
         hero = {
             Text(
@@ -178,8 +174,8 @@ private fun StepPickerScaffold(
         actions = {
             Button(
                 onClick = onFinished,
-                // The step is full-bleed, so the button takes the grouped rows'
+                // Full-bleed step: the button takes the grouped rows' own inset
-                // own inset and ends up as wide as the options above it.
+                // and ends up as wide as the options above it.
                 modifier = Modifier
                     .padding(horizontal = GroupedListInset)
                     .fillMaxWidth()
@@ -196,9 +192,8 @@ private fun StepPickerScaffold(
 }
 
 /**
- * The live preview, framed like the picker's. The gap under it is wider than the
+ * The live preview, framed like the picker's. The wider gap below keeps the rows
- * one above: the rows below are the things to tap, and they need to look like a
+ * reading as a group of their own rather than as the preview's caption.
- * group of their own rather than the preview's caption.
  */
 @Composable
 private fun <T> StepPreview(selected: T, content: @Composable (T) -> Unit) {
@@ -210,9 +205,5 @@ private fun <T> StepPreview(selected: T, content: @Composable (T) -> Unit) {
     )
 }
 
-/**
+/** The same size the Settings chooser gives it. */
- * The same size the Settings chooser gives it. The step no longer tries to fit a
- * screen: showing the view properly is what the step is for, and a picture too
- * small to read would cost more than the scroll does.
- */
 private val PREVIEW_HEIGHT: Dp = 280.dp

app/src/main/java/de/jeanlucmakiola/calendula/ui/onboarding/VisibilityStep.kt

@@ -21,10 +21,8 @@ import de.jeanlucmakiola.floret.components.OnboardingSpace
  * Wizard step for the one-time notice that Calendula now follows the device's
  * own per-calendar visibility and some of yours are switched off (#75, #163).
  *
- * It used to be a dialog thrown over the app once onboarding had finished, which
+ * Nothing to decide here — the app does not switch those calendars back on,
- * put the one thing that explains a changed behaviour outside the flow that
+ * since that would un-hide them everywhere else too.
- * explains everything else. Nothing to decide here — the app does not switch
- * those calendars back on, since that would un-hide them everywhere else too.
  */
 @Composable
 internal fun VisibilityStep(

app/src/test/java/de/jeanlucmakiola/calendula/ui/onboarding/OnboardingPlanTest.kt

@@ -106,8 +106,7 @@ class OnboardingPlanTest {
     @Test
     fun `re-granting after the wizard itself ran does not re-enter it`() {
         // The closing screen clears the armed flag, so someone the wizard did
-        // onboard is in the same position as anyone else who revokes the
+        // onboard ends up where any other revoker does: one screen, no counter.
-        // permission: one screen, no counter.
         val revoked = plan(remindersDone = true, wizardArmed = false, doneShown = true)
         assertThat(revoked.steps).containsExactly(OnboardingStep.Permission)
         assertThat(revoked.showsProgress).isFalse()

docs/ARCHITECTURE.md

@@ -97,10 +97,11 @@ step it is on, and `RootScreen` renders that step. Two rules make it work:
   counter under the user, and stepping *back* is just clearing the previous
   step's answer — there is no separate back stack. The calendar grant is the
   one step with nothing to return to, so back stops there.
-- The optional steps (backup, view) only exist for an install that went
+- The optional steps (backup, view) only exist for an install still on its
-  through the grant in-app — recorded by `armOnboardingWizard()`, which
+  first run — `armOnboardingWizard()` arms on any launch holding the calendar
-  refuses to arm if the reminder step was already answered, so an existing
+  permission but refuses if the reminder step was already answered, and the
-  user who revokes and re-grants the permission is not re-onboarded.
+  closing screen clears the flag again, so revoking and re-granting the
+  permission later never re-onboards anyone.
 
 Whether the backup step applies (nothing writable is synced anywhere) cannot
 be known before the grant, so it is assumed until the calendar list can be