diff --git a/app/src/main/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeScreen.kt b/app/src/main/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeScreen.kt index f3800f01..b7caa7a7 100644 --- a/app/src/main/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeScreen.kt +++ b/app/src/main/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeScreen.kt @@ -1,14 +1,17 @@ package hrln.interactive.sidekit.apps.settings import android.view.KeyEvent +import androidx.activity.compose.BackHandler +import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.rememberLazyListState @@ -18,19 +21,23 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.drawWithContent import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.focus.onFocusChanged import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.key.onPreviewKeyEvent import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import hrln.interactive.sidekit.apps.sideport.registry.PortedApps +import hrln.interactive.sidekit.design.action.ActionMotion import hrln.interactive.sidekit.design.action.ActionTone import hrln.interactive.sidekit.design.action.SideAction import hrln.interactive.sidekit.design.components.* import hrln.interactive.sidekit.design.icons.SidekitIcon import hrln.interactive.sidekit.design.theme.SidekitTheme +import hrln.interactive.sidekit.design.theme.drawFocusRing import hrln.interactive.sidekit.home.HomeArranger import hrln.interactive.sidekit.home.HomePreferences import hrln.interactive.sidekit.navigation.SubApp @@ -41,9 +48,10 @@ import hrln.interactive.sidekit.navigation.SubApp * * Up/Down walk the app list like Home. Reordering is a selection mode, keypad-first: Confirm on a * focused app selects it (highlighted), then Up/Down moves it through the order. The selected row - * also shows up/down arrows on its right as a direct toggle, and tapping a row selects it too. The dock is automatic — the first - * [HomePreferences.DOCK_MAX] apps are "In dock" — so reordering an app into/out of the top four - * pins/unpins it. Hidden apps are omitted so they cannot take a dock slot here. + * also shows up/down arrows on its right as a direct toggle, and tapping a row selects it too. The + * dock is automatic — the first [HomePreferences.DOCK_MAX] apps are "In dock" — so reordering an + * app into/out of the top four pins/unpins it. Hidden apps are omitted so they cannot take a dock + * slot here. */ @Composable internal fun ArrangeHomeScreen(back: () -> Unit) { @@ -59,22 +67,76 @@ internal fun ArrangeHomeScreen(back: () -> Unit) { val arranger = remember(home, known) { HomeArranger(home, known) } val order = arranger.order val selected = arranger.selected + var confirmingDiscard by remember { mutableStateOf(false) } fun done() { arranger.commit() back() } fun cancel() = back() // buffer never committed, so Back leaves the launcher unchanged + fun requestBack() { + if (arranger.hasPendingChanges) confirmingDiscard = true else cancel() + } + fun discard() { + confirmingDiscard = false + cancel() + } + BackHandler { + if (confirmingDiscard) confirmingDiscard = false else requestBack() + } // Start on the first app, not on the screen itself: a focused screen has no D-pad target, so // Up/Down would do nothing. A frame is needed because LazyColumn composes rows during layout. val firstRow = remember { FocusRequester() } + val selectedRow = remember { FocusRequester() } val list = rememberLazyListState() - LaunchedEffect(Unit) { withFrameNanos { }; runCatching { firstRow.requestFocus() } } - // Moving the selected app reorders the keyed row, but focus alone does not re-run bring-into-view. - // Keep the list on the selected app so Up/Down never walks the selection off the visible page. + LaunchedEffect(Unit) { + withFrameNanos { } + runCatching { firstRow.requestFocus() } + } + // Keep the selected app in context while it moves: scroll to the row just before it, so the + // selected app sits in the second slot with at least one neighbour above and the rest below. + // requestScrollToItem applies the scroll in the same measure pass as the reorder, so the row + // never draws at its old position for a frame (which looked like it sat on top of its neighbour). + val scrollTarget = selected?.let(order::indexOf)?.takeIf { it >= 0 } + var lastScrollTarget by remember { mutableStateOf(null) } + SideEffect { + if (scrollTarget == null) { + lastScrollTarget = null + } else if (scrollTarget != lastScrollTarget) { + list.requestScrollToItem(scrollTarget) + lastScrollTarget = scrollTarget + } + } LaunchedEffect(selected, order) { - val index = selected?.let(order::indexOf) ?: return@LaunchedEffect - if (index >= 0) list.animateScrollToItem(index + 1) + if (selected == null) return@LaunchedEffect + // Let the requested scroll land first, then restore focus to the keyed row that moved. + withFrameNanos { } + runCatching { selectedRow.requestFocus() } + } + val actions = if (confirmingDiscard) { + listOf( + SideAction( + "cancel", + "Cancel", + icon = SidekitIcon.ArrowLeftAlt, + tone = ActionTone.Muted, + motion = ActionMotion.Back, + onClick = { confirmingDiscard = false } + ), + SideAction("discard", "Discard", icon = SidekitIcon.Undo, tone = ActionTone.Destructive, motion = ActionMotion.Up, onClick = ::discard) + ) + } else { + listOf( + SideAction.back("back", "Settings", ::requestBack), + SideAction.forward("done", "Done", ::done, icon = SidekitIcon.Check) + ) + } + val options = if (confirmingDiscard) { + emptyList() + } else { + listOf( + SideAction("reset", "Reset", icon = SidekitIcon.Undo, tone = ActionTone.Muted, onClick = { arranger.reset() }) + ) } SidekitScaffold( "Arrange apps", @@ -82,30 +144,33 @@ internal fun ArrangeHomeScreen(back: () -> Unit) { // list like Home. Once an app is selected, Up/Down move that app instead (T/B on the // keypad); Left/Right, Confirm and Back are untouched so the user can still reach the // arrows and the action bar. Deselect (Confirm on the selected row) to browse freely again. + // Repeats are consumed too: otherwise holding a direction walks focus away while the row + // stays selected, leaving the highlight and the actual focus out of sync. modifier = Modifier.onPreviewKeyEvent { event -> val native = event.nativeKeyEvent - if (selected != null && native.action == KeyEvent.ACTION_DOWN && native.repeatCount == 0) { + if (selected != null && !confirmingDiscard && native.action == KeyEvent.ACTION_DOWN) { when (native.keyCode) { - KeyEvent.KEYCODE_DPAD_UP -> { arranger.moveSelected(-1); true } - KeyEvent.KEYCODE_DPAD_DOWN -> { arranger.moveSelected(+1); true } + KeyEvent.KEYCODE_DPAD_UP -> { + arranger.moveSelected(-1) + true + } + + KeyEvent.KEYCODE_DPAD_DOWN -> { + arranger.moveSelected(+1) + true + } + else -> false } - } else false + } else { + false + } }, - actions = listOf( - SideAction.back("back", "Settings", ::cancel), - SideAction.forward("done", "Done", ::done, icon = SidekitIcon.Check), - SideAction("reset", "Reset", icon = SidekitIcon.Undo, tone = ActionTone.Muted, onClick = { arranger.reset() }), - ), + actions = actions, + options = options ) { padding -> SidekitList(padding, state = list) { - item { - SidekitText( - "Select apps, then move them up or down. First 4 apps are also shown in the dock", - style = SidekitTheme.typography.label, - maxLines = Int.MAX_VALUE, - ) - } + item { ArrangeHint() } val last = order.lastIndex itemsIndexed(order, key = { _, id -> id }) { index, id -> val label = subsById[id]?.label ?: portedById[id]?.label ?: return@itemsIndexed @@ -117,12 +182,16 @@ internal fun ArrangeHomeScreen(back: () -> Unit) { Row( Modifier.fillMaxWidth().height(76.dp).listFade(list, index + 1), verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(SidekitTheme.spacing.itemContentGap), + horizontalArrangement = Arrangement.spacedBy(SidekitTheme.spacing.itemContentGap) ) { SidekitItem( title = label, modifier = Modifier.weight(1f).then( - if (index == 0) Modifier.focusRequester(firstRow) else Modifier, + when { + isSelected -> Modifier.focusRequester(selectedRow) + index == 0 -> Modifier.focusRequester(firstRow) + else -> Modifier + } ), secondary = when { isSelected -> "Selected" @@ -130,7 +199,7 @@ internal fun ArrangeHomeScreen(back: () -> Unit) { else -> null }, colour = if (isSelected) ItemColour.Secondary else ItemColour.Primary, - onClick = { arranger.select(id) }, + onClick = { arranger.select(id) } ) if (isSelected) { Column { @@ -144,6 +213,59 @@ internal fun ArrangeHomeScreen(back: () -> Unit) { } } +/** + * The keypad hint, with key names in small grey caps so they read as keys, not prose. + * + * It is focusable even though it does nothing: otherwise Up from the first app stops there and the + * hint can never be brought back into view after the list has scrolled down. Focusing it lets the + * list's bring-into-view scroll it back to the top; Down returns to the first app. + */ +@Composable +private fun ArrangeHint() { + var focused by remember { mutableStateOf(false) } + Column( + Modifier + .fillMaxWidth() + .focusable() + .onFocusChanged { focused = it.isFocused } + .drawWithContent { + drawContent() + if (focused) drawFocusRing(1f, cornerRadius = 12.dp.toPx()) + } + .padding(horizontal = 8.dp, vertical = 6.dp), + verticalArrangement = Arrangement.spacedBy(4.dp) + ) { + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(6.dp) + ) { + KeyCap("Enter") + SidekitText(": Select App", style = SidekitTheme.typography.label) + } + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(6.dp) + ) { + KeyCap("Up") + SidekitText("/", style = SidekitTheme.typography.label) + KeyCap("Down") + SidekitText(": Move App", style = SidekitTheme.typography.label) + } + } +} + +@Composable +private fun KeyCap(label: String) { + Box( + Modifier + .clip(RoundedCornerShape(6.dp)) + .background(SidekitTheme.colors.surfaceA3) + .padding(horizontal = 8.dp, vertical = 2.dp) + ) { + SidekitText(label, style = SidekitTheme.typography.small, color = SidekitTheme.colors.light) + } +} + /** A small round arrow button; `up=false` flips the upward glyph to point down. Disabled state is dimmed. */ @Composable private fun ItemArrow(up: Boolean, enabled: Boolean, contentDescription: String, onClick: () -> Unit) { @@ -153,12 +275,12 @@ private fun ItemArrow(up: Boolean, enabled: Boolean, contentDescription: String, .clip(RoundedCornerShape(50)) .alpha(if (enabled) 1f else 0.3f) .then(if (enabled) Modifier.clickable(onClickLabel = contentDescription, onClick = onClick) else Modifier), - contentAlignment = Alignment.Center, + contentAlignment = Alignment.Center ) { SidekitIconImage( SidekitIcon.ArrowUpward, contentDescription = contentDescription, - modifier = if (up) Modifier else Modifier.graphicsLayer { rotationZ = 180f }, + modifier = if (up) Modifier else Modifier.graphicsLayer { rotationZ = 180f } ) } } diff --git a/app/src/main/java/hrln/interactive/sidekit/home/HomeArranger.kt b/app/src/main/java/hrln/interactive/sidekit/home/HomeArranger.kt index 3f0ef55a..575cfa01 100644 --- a/app/src/main/java/hrln/interactive/sidekit/home/HomeArranger.kt +++ b/app/src/main/java/hrln/interactive/sidekit/home/HomeArranger.kt @@ -26,9 +26,14 @@ class HomeArranger(private val home: HomePreferences, known: Iterable) { /** Ported (or otherwise freshly-seen) ids that aren't part of the built-in default list. */ private val appendedDefaults: List = knownIds.filter { it !in builtinIds } - var order by mutableStateOf(home.homeOrderFor(knownIds)) + private val initialOrder = home.homeOrderFor(knownIds) + + var order by mutableStateOf(initialOrder) private set + /** True when the buffer differs from the order it was seeded with (Reset counts as a change). */ + val hasPendingChanges: Boolean get() = order != initialOrder + /** The app currently selected for reordering, or null when not in selection mode. */ var selected by mutableStateOf(null) private set @@ -51,7 +56,10 @@ class HomeArranger(private val home: HomePreferences, known: Iterable) { if (index < 0) return val target = (index + delta).coerceIn(0, order.lastIndex) if (target == index) return - order = order.toMutableList().apply { removeAt(index); add(target, id) } + order = order.toMutableList().apply { + removeAt(index) + add(target, id) + } } /** Moves the currently selected app by [delta]; no-op when nothing is selected. */ @@ -69,4 +77,4 @@ class HomeArranger(private val home: HomePreferences, known: Iterable) { fun commit() { home.saveOrder(order) } -} \ No newline at end of file +} diff --git a/app/src/test/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeTest.kt b/app/src/test/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeTest.kt index 17b10b65..ccab377f 100644 --- a/app/src/test/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeTest.kt +++ b/app/src/test/java/hrln/interactive/sidekit/apps/settings/ArrangeHomeTest.kt @@ -69,9 +69,13 @@ class ArrangeHomeTest { } @Test - fun `renders every app with the four pinned dock apps marked In dock`() { + fun `renders every app with the two-row keypad hint and dock labels`() { arrange() - compose.onNodeWithText("Select apps, then move them up or down. First 4 apps are also shown in the dock").assertIsDisplayed() + compose.onNodeWithText("Enter").assertIsDisplayed() + compose.onNodeWithText(": Select App").assertIsDisplayed() + compose.onNodeWithText("Up").assertIsDisplayed() + compose.onNodeWithText("Down").assertIsDisplayed() + compose.onNodeWithText(": Move App").assertIsDisplayed() compose.onAllNodesWithText("In dock").assertCountEquals(HomePreferences.DOCK_MAX) SubApp.dock.forEach { compose.onNodeWithText(it.label).assertIsDisplayed() } compose.onNodeWithText("Photos").assertExists() @@ -117,6 +121,20 @@ class ArrangeHomeTest { compose.onNodeWithContentDescription("Move Sideticks up").assertDoesNotExist() } + @Test + fun `keypad can reach the hint above the first app`() { + arrangeWithKeypad() + compose.onNodeWithText("Sidemessage").assertIsFocused() + + compose.onRoot().performKeyInput { pressKey(Key.DirectionUp) } + compose.waitForIdle() + compose.onNode(hasText("Enter") and hasAnyAncestor(isFocused())).assertExists() + + compose.onRoot().performKeyInput { pressKey(Key.DirectionDown) } + compose.waitForIdle() + compose.onNodeWithText("Sidemessage").assertIsFocused() + } + @Test fun `hidden apps are omitted from the rearrangement list`() { SidekitPreferences(context).hiddenApps = setOf("builtin:Sidenotes") diff --git a/app/src/test/java/hrln/interactive/sidekit/home/HomeArrangerTest.kt b/app/src/test/java/hrln/interactive/sidekit/home/HomeArrangerTest.kt index 763c83c3..b452d154 100644 --- a/app/src/test/java/hrln/interactive/sidekit/home/HomeArrangerTest.kt +++ b/app/src/test/java/hrln/interactive/sidekit/home/HomeArrangerTest.kt @@ -2,8 +2,8 @@ package hrln.interactive.sidekit.home import android.content.Context import androidx.test.core.app.ApplicationProvider -import hrln.interactive.sidekit.apps.sideport.registry.PortedApp import hrln.interactive.sidekit.apps.settings.visibilityId +import hrln.interactive.sidekit.apps.sideport.registry.PortedApp import hrln.interactive.sidekit.navigation.SubApp import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -43,7 +43,7 @@ class HomeArrangerTest { @Test fun `seeds from stored order, dock derived, freshly-seen apps appended`() { val home = HomePreferences(context) - home.move(photos, 2) // persisted order change + home.move(photos, 2) // persisted order change val arranger = HomeArranger(home, known) // home.homeOrder is the raw stored list; the arranger appends the freshly-seen ported id. @@ -79,16 +79,16 @@ class HomeArrangerTest { val home = HomePreferences(context) val arranger = HomeArranger(home, known) val defaultDock = SubApp.dock.map { it.visibilityId() } - assertEquals(defaultDock, arranger.dock) // defaults == first four (Photos at 4 is out) + assertEquals(defaultDock, arranger.dock) // defaults == first four (Photos at 4 is out) assertFalse(arranger.isInDock(photos)) assertFalse(arranger.isInDock(matrix)) - arranger.move(photos, -1) // photos 4 -> 3, enters the dock + arranger.move(photos, -1) // photos 4 -> 3, enters the dock assertTrue(arranger.isInDock(photos)) assertEquals(HomePreferences.DOCK_MAX, arranger.dock.size) assertEquals(photos, arranger.dock.last()) - arranger.move(photos, 1) // back out of the dock + arranger.move(photos, 1) // back out of the dock assertFalse(arranger.isInDock(photos)) assertEquals(defaultDock, arranger.dock) @@ -108,20 +108,47 @@ class HomeArrangerTest { assertEquals(photos, arranger.selected) assertTrue(arranger.isSelected(photos)) - arranger.moveSelected(-1) // photos 4 -> 3 + arranger.moveSelected(-1) // photos 4 -> 3 assertEquals(3, arranger.order.indexOf(photos)) - assertTrue(arranger.isSelected(photos)) // selection follows the app + assertTrue(arranger.isSelected(photos)) // selection follows the app - arranger.moveSelected(0) // no-op keeps selection and order + arranger.moveSelected(0) // no-op keeps selection and order assertEquals(3, arranger.order.indexOf(photos)) - arranger.select(photos) // deselect + arranger.select(photos) // deselect assertNull(arranger.selected) val before = arranger.order.toList() - arranger.moveSelected(1) // nothing selected -> no-op + arranger.moveSelected(1) // nothing selected -> no-op assertEquals(before, arranger.order) } + @Test + fun `pending changes track the buffer, not the selection`() { + val home = HomePreferences(context) + val arranger = HomeArranger(home, known) + assertFalse(arranger.hasPendingChanges) + + arranger.select(photos) + assertFalse(arranger.hasPendingChanges) // selecting is not an edit + + arranger.moveSelected(-1) + assertTrue(arranger.hasPendingChanges) + + arranger.moveSelected(+1) // back to the seeded order + assertFalse(arranger.hasPendingChanges) + } + + @Test + fun `reset is a pending change when it differs from the seeded order`() { + val home = HomePreferences(context) + home.move(photos, 2) + val arranger = HomeArranger(home, known) + assertFalse(arranger.hasPendingChanges) + + arranger.reset() + assertTrue(arranger.hasPendingChanges) + } + @Test fun `reset keeps only the known apps, so filtered apps stay out`() { val home = HomePreferences(context) @@ -151,7 +178,7 @@ class HomeArrangerTest { fun `commit persists the order and the dock follows the top four`() { val home = HomePreferences(context) val arranger = HomeArranger(home, known) - arranger.move(photos, -1) // bring Photos into the dock + arranger.move(photos, -1) // bring Photos into the dock arranger.commit() val stored = HomePreferences(context) @@ -161,4 +188,4 @@ class HomeArrangerTest { assertTrue(stored.isInDock(photos)) assertEquals(SubApp.dock.map { it.visibilityId() }.dropLast(1) + listOf(photos), arranger.dock) } -} \ No newline at end of file +}