fix(#143): review round 1 — empty group state, inset header ripple, prefs test

app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/CalendarFilterList.kt

@@ -18,6 +18,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.getValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.clip
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.semantics.Role
 import androidx.compose.ui.semantics.semantics
@@ -77,10 +78,11 @@ private fun FilterList(
 ) {
     Column(modifier = modifier.fillMaxWidth()) {
         groups.forEach { group ->
+            val ids = group.calendars.map { it.id }
             GroupHeader(
                 title = accountGroupTitle(group),
-                state = groupVisibility(group.calendars.map { it.id }, hidden),
-                onToggle = { visible -> onSetGroupVisible(group.calendars.map { it.id }, visible) },
+                state = groupVisibility(ids, hidden),
+                onToggle = { visible -> onSetGroupVisible(ids, visible) },
             )
             group.calendars.forEachIndexed { index, cal ->
                 val visible = cal.id !in hidden
@@ -120,10 +122,11 @@ private fun GroupHeader(
         verticalAlignment = Alignment.CenterVertically,
         modifier = Modifier
             .fillMaxWidth()
-            .padding(top = 4.dp)
+            .padding(horizontal = GroupedListInset / 2)
+            .clip(MaterialTheme.shapes.large)
             .clickable(onClickLabel = label, role = Role.Checkbox) { onToggle(state.nextGroupVisible()) }
             .semantics { toggleableState = state }
-            .padding(start = GroupedListInset, end = GroupedListInset + 16.dp),
+            .padding(start = GroupedListInset / 2, end = GroupedListInset / 2 + 16.dp),
     ) {
         Text(
             text = title,

app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/GroupVisibility.kt

@@ -3,14 +3,14 @@ package de.jeanlucmakiola.calendula.ui.filter
 import androidx.compose.ui.state.ToggleableState
 
 /** The drawer group header's checkbox state: on, off, or some of [calendarIds] shown. */
-fun groupVisibility(calendarIds: Collection<Long>, hidden: Set<Long>): ToggleableState {
+internal fun groupVisibility(calendarIds: Collection<Long>, hidden: Set<Long>): ToggleableState {
     val shown = calendarIds.count { it !in hidden }
     return when (shown) {
-        calendarIds.size -> ToggleableState.On
         0 -> ToggleableState.Off
+        calendarIds.size -> ToggleableState.On
         else -> ToggleableState.Indeterminate
     }
 }
 
 /** What tapping the header does: a fully shown group hides, anything else shows all. */
-fun ToggleableState.nextGroupVisible(): Boolean = this != ToggleableState.On
+internal fun ToggleableState.nextGroupVisible(): Boolean = this != ToggleableState.On

app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/CalendarPrefsTest.kt

@@ -34,6 +34,18 @@ class CalendarPrefsTest {
         assertThat(prefs.hiddenCalendarIds.first()).isEmpty()
     }
 
+    @Test
+    fun `setCalendarsHidden adds and removes only the given ids`(@TempDir tempDir: Path) = runTest {
+        val prefs = testPrefs(tempDir)
+        prefs.setHiddenCalendarIds(setOf(9L))
+        prefs.setCalendarsHidden(listOf(1L, 2L), hidden = true)
+        assertThat(prefs.hiddenCalendarIds.first()).isEqualTo(setOf(1L, 2L, 9L))
+        prefs.setCalendarsHidden(listOf(2L, 3L), hidden = false)
+        assertThat(prefs.hiddenCalendarIds.first()).isEqualTo(setOf(1L, 9L))
+        prefs.setCalendarsHidden(listOf(1L, 9L), hidden = false)
+        assertThat(prefs.hiddenCalendarIds.first()).isEmpty()
+    }
+
     @Test
     fun `garbage stored string is parsed defensively`(@TempDir tempDir: Path) = runTest {
         val store = testDataStore(tempDir, "test_prefs")

app/src/test/java/de/jeanlucmakiola/calendula/ui/filter/GroupVisibilityTest.kt

@@ -28,4 +28,9 @@ class GroupVisibilityTest {
         assertThat(state).isEqualTo(ToggleableState.Indeterminate)
         assertThat(state.nextGroupVisible()).isTrue()
     }
+
+    @Test
+    fun `empty group is off`() {
+        assertThat(groupVisibility(emptyList(), hidden = emptySet())).isEqualTo(ToggleableState.Off)
+    }
 }