diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt index 9dc69e9..973750e 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/NotificationSettingsScreen.kt @@ -1,6 +1,14 @@ package dev.privacyllc.period.feature.settings import android.Manifest +import androidx.compose.runtime.setValue +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.mutableStateOf +import androidx.compose.material3.rememberTimePickerState +import androidx.compose.material3.TimePicker +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.AlertDialog +import android.text.format.DateFormat import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.Lifecycle @@ -114,6 +122,16 @@ private fun NotificationSettingsContent( blocked: Boolean = false, onOpenSystemSettings: () -> Unit = {}, ) { + var showTimePicker by rememberSaveable { mutableStateOf(false) } + + if (showTimePicker) { + ReminderTimePicker( + initial = prefs.reminderTime, + onDismiss = { showTimePicker = false }, + onPicked = { viewModel?.setReminderTime(it); showTimePicker = false }, + ) + } + Column( Modifier .fillMaxSize() @@ -186,6 +204,10 @@ private fun NotificationSettingsContent( ).forEach { (label, time) -> TextButton(onClick = { viewModel?.setReminderTime(time) }) { Text(label) } } + // §29 lists a custom time beside the three presets. The preference + // has always stored an arbitrary minute; only the screen insisted on + // one of three. "Around dinner" is not the same hour in every house. + TextButton(onClick = { showTimePicker = true }) { Text("Custom") } } Text( "Currently ${prefs.reminderTime.format(timeFormat)}", @@ -283,3 +305,36 @@ private fun openAppNotificationSettings(context: Context) { .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) runCatching { context.startActivity(intent) } } + +/** + * Pick any minute of the day. + * + * A dialog rather than an inline picker: this is one decision, made rarely, and + * the settings screen is a list rather than a form. The keyboard-entry toggle + * that `TimePicker` offers is left on — a dial is quick with a thumb and + * miserable with a screen reader, and this is one of the few controls somebody + * might set once and never touch again. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun ReminderTimePicker( + initial: LocalTime, + onDismiss: () -> Unit, + onPicked: (LocalTime) -> Unit, +) { + val state = rememberTimePickerState( + initialHour = initial.hour, + initialMinute = initial.minute, + is24Hour = DateFormat.is24HourFormat(LocalContext.current), + ) + + AlertDialog( + onDismissRequest = onDismiss, + title = { Text("Remind me at") }, + text = { TimePicker(state = state) }, + confirmButton = { + TextButton(onClick = { onPicked(LocalTime.of(state.hour, state.minute)) }) { Text("Set") } + }, + dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } }, + ) +} diff --git a/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt index 7b2dfac..40e359f 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt @@ -77,6 +77,15 @@ class NotificationActionHandler @Inject constructor( PeriodRecordSource.NOTIFICATION_CONFIRMATION, ) + // The day before the one asked about — not the day before + // the tap, which would drift exactly like the bug the dated + // actions fixed. + ReminderAction.STARTED_YESTERDAY -> + repository.confirmPeriodStart( + request.date!!.minusDays(1), + PeriodRecordSource.NOTIFICATION_CONFIRMATION, + ) + // Exactly what the Today screen's "Not yet" does — same // call, same censoring observation, same re-conditioned // forecast — but dated to the day that was asked about. diff --git a/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt b/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt index 98d328a..1a46c9d 100644 --- a/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt +++ b/app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt @@ -108,9 +108,17 @@ class NotificationActionHandlerTest { val end = NotificationCopy.buttons(ReminderKind.PERIOD_END_CHECK_IN, privacy) assertEquals(listOf(ReminderAction.ENDED, ReminderAction.STILL_GOING), end.map { it.action }) + // The property, not the exact list: the follow-up gained a third + // button (§30's "Yes, yesterday") and the invariant is unchanged — + // a question about a start never offers an end. listOf(ReminderKind.DID_IT_START, ReminderKind.PERIOD_EXPECTED_TODAY).forEach { kind -> - val start = NotificationCopy.buttons(kind, privacy) - assertEquals(listOf(ReminderAction.STARTED, ReminderAction.NOT_YET), start.map { it.action }) + val start = NotificationCopy.buttons(kind, privacy).map { it.action } + assertTrue("$kind offered no way to say it started", ReminderAction.STARTED in start) + assertTrue("$kind offered no way to say not yet", ReminderAction.NOT_YET in start) + assertTrue( + "$kind offered an ending for a question about a start", + start.none { it == ReminderAction.ENDED || it == ReminderAction.STILL_GOING }, + ) } (end + NotificationCopy.buttons(ReminderKind.DID_IT_START, privacy)).forEach { @@ -244,4 +252,16 @@ class NotificationActionHandlerTest { assertTrue(repo.confirmedPeriods.first().isEmpty()) } + + @Test + fun `yes-yesterday records the day before the one asked about`() = runTest { + // Not the day before the tap. The reminder asked about yesterday and is + // answered this morning: "yesterday" means the day before the question, + // or it drifts exactly like the bug dated actions fixed. + assertTrue( + handler.handle(ReminderActionRequest(ReminderAction.STARTED_YESTERDAY, today.minusDays(1))), + ) + + assertEquals(today.minusDays(2), repo.confirmedPeriods.first().single().startDate) + } } diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt index e9402c5..eb80dc5 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/NotificationCopy.kt @@ -157,12 +157,25 @@ object NotificationCopy { fun buttons(kind: ReminderKind, privacy: NotificationPrivacy): List { val direct = privacy == NotificationPrivacy.DIRECT return when (kind) { - // "Has it started?" — a start, or a censoring observation. - ReminderKind.DID_IT_START, ReminderKind.PERIOD_EXPECTED_TODAY -> listOf( + // The day it was expected: it either has or it has not. + ReminderKind.PERIOD_EXPECTED_TODAY -> listOf( ReminderButton(ReminderAction.STARTED, if (direct) "Started" else "Yes"), ReminderButton(ReminderAction.NOT_YET, "Not yet"), ) + // The follow-up, a couple of days later, where "yes" is ambiguous — + // §30 asks for the day to be pinned down. Three buttons, not §30's + // four: Android reliably renders three, and a fourth that only + // appears on some devices is worse than one that never does. The + // missing one is "Choose Date", which needs a screen rather than a + // button; logging an older start in the app is the way to that + // today. + ReminderKind.DID_IT_START -> listOf( + ReminderButton(ReminderAction.STARTED, if (direct) "Today" else "Yes, today"), + ReminderButton(ReminderAction.STARTED_YESTERDAY, if (direct) "Yesterday" else "Yes, yesterday"), + ReminderButton(ReminderAction.NOT_YET, "Not yet"), + ) + // "Is it over?" — an end, or nothing at all. Never a start. ReminderKind.PERIOD_END_CHECK_IN -> listOf( ReminderButton(ReminderAction.ENDED, if (direct) "Ended" else "Done"), diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt index e99bc59..b1787c8 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderAction.kt @@ -25,9 +25,19 @@ package dev.privacyllc.period.core.notifications * nothing" rather than guessing. */ enum class ReminderAction { - /** The period began. Confirms a start. */ + /** The period began on the day the reminder was asking about. */ STARTED, + /** + * The period began the day *before* the one the reminder asked about. + * + * §30's "Yes, Yesterday". Cheap only because the answer already carries the + * day it was about: this is that day minus one, not "yesterday" measured from + * whenever the tap happened, which would drift exactly like the bug dated + * actions fixed. + */ + STARTED_YESTERDAY, + /** It has not begun yet. Censors the forecast — §13. */ NOT_YET, diff --git a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderActionRules.kt b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderActionRules.kt index dd42c57..c3316f2 100644 --- a/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderActionRules.kt +++ b/core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderActionRules.kt @@ -53,7 +53,7 @@ object ReminderActionRules { // Already answered. She logged a start on or after the day the // notification was asking about, so the question is settled and a // second write would either duplicate it or contradict it. - ReminderAction.STARTED, ReminderAction.NOT_YET -> + ReminderAction.STARTED, ReminderAction.STARTED_YESTERDAY, ReminderAction.NOT_YET -> if (latest != null && !latest.startDate.isBefore(date)) Verdict.Stale else Verdict.Apply // Ending needs something open that began on or before the day in diff --git a/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/NotificationCopyTest.kt b/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/NotificationCopyTest.kt index e355850..222afb9 100644 --- a/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/NotificationCopyTest.kt +++ b/core/notifications/src/test/kotlin/dev/privacyllc/period/core/notifications/NotificationCopyTest.kt @@ -190,4 +190,33 @@ class NotificationCopyTest { assertTrue(NotificationCopy.stopAskingText(it).publicTitle.isNotBlank()) } } + + // ----------------------------------------------------------------------- + // §30's follow-up, within the platform's limit + // ----------------------------------------------------------------------- + + @Test fun `the follow-up pins down which day, without a fourth button`() { + NotificationPrivacy.entries.forEach { privacy -> + val buttons = NotificationCopy.buttons(ReminderKind.DID_IT_START, privacy) + + assertEquals( + listOf(ReminderAction.STARTED, ReminderAction.STARTED_YESTERDAY, ReminderAction.NOT_YET), + buttons.map { it.action }, + ) + // Three, not §30's four: Android reliably renders three, and a + // button that appears on some devices is worse than one that never + // does. + assertTrue("Android shows three actions", buttons.size <= 3) + } + } + + @Test fun `the follow-up's day labels say nothing outside Direct`() { + NotificationPrivacy.entries.filter { it != NotificationPrivacy.DIRECT }.forEach { privacy -> + val labels = NotificationCopy.buttons(ReminderKind.DID_IT_START, privacy) + .joinToString(" ") { it.label }.lowercase() + NotificationCopy.SENSITIVE_WORDS.forEach { + assertFalse("$privacy leaked \"$it\" in a button", labels.contains(it)) + } + } + } } diff --git a/docs/design/README.md b/docs/design/README.md index 9da8f72..432a81a 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -153,6 +153,21 @@ have replaced it. ## The lock screen is the one semi-public surface +### Three buttons, because Android shows three + +§30 asks the follow-up reminder for four: *Yes, Today*, *Yes, Yesterday*, +*Choose Date*, *Not Yet*. Android reliably renders three notification actions, +and a button that appears on some devices and not others is worse than one that +never appears at all — it is a feature the user learns and then loses. + +So the follow-up offers the three that fit, and the one left out is *Choose +Date*, which was never really a button: it needs a screen. Logging an older start +in the app is the route to that today, and the notification opens the app. + +"Yes, yesterday" is cheap only because the answer already carries the day it was +about. It means the day before the *question*, never the day before the tap — +otherwise it drifts exactly like the defect dated actions fixed. + ### When the copy and the platform disagree, the copy moves The reminders screen promised delivery "a few minutes either side of this time".