diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/HomeScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/HomeScreen.kt index 6f525349..638c6497 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/HomeScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/HomeScreen.kt @@ -243,7 +243,18 @@ fun HomeScreen( key(true) { chatRoomListViewModel.RenderFeed( - onNavigateToDirectMessageDetail = onOpenChatRoom + onNavigateToDirectMessageDetail = onOpenChatRoom, + // Only where there is a pane to be selected + // into. The selection outlives a window + // that narrows -- it is `rememberSaveable`, + // and a tablet turned to portrait keeps it + // -- so this is read from the layout rather + // than from whether a room is selected. + isSelected = if (listPaneWidth == null) { + null + } else { + { id -> id == selectedChatRoomId } + }, ) } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomListViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomListViewModel.kt index e3a988f9..3bd3ff61 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomListViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatRoomListViewModel.kt @@ -10,6 +10,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.LoadingIndicator import androidx.compose.material3.MaterialTheme @@ -20,6 +21,8 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.selected +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.lifecycle.ViewModel @@ -144,7 +147,20 @@ class ChatRoomListViewModel( @OptIn(ExperimentalMaterial3ExpressiveApi::class) @Composable fun RenderFeed( - onNavigateToDirectMessageDetail: (press.mantra.compose.ui.composable.navigation.routes.Route) -> Unit + onNavigateToDirectMessageDetail: (press.mantra.compose.ui.composable.navigation.routes.Route) -> Unit, + /** + * Which room the detail pane beside this list is showing, or `null` where this + * list is the whole screen. + * + * A predicate rather than an id because the two layouts disagree about whether + * selection is a concept at all, not merely about its value. Where tapping a room + * navigates to it, no room is "the selected one" -- the list is about to be gone -- + * and a highlight left behind on the way out, or a reader told "not selected" + * about every room in the list, would both be describing a pane that is not there. + * `null` says that; a predicate that answers false for every room would not, and + * is also what a two-pane layout looks like before the first tap. + */ + isSelected: ((String) -> Boolean)?, ) { Column( modifier = Modifier.fillMaxWidth(), @@ -174,8 +190,30 @@ class ChatRoomListViewModel( items = chatRoomListUIState.chatRoomList, key = { it.chatRoom.id } ) { localChatRoom -> + // Null where there is no pane, so the row says nothing + // about selection at all rather than saying "no". + val selected = isSelected?.invoke(localChatRoom.chatRoom.id) + Card( - modifier = Modifier.fillMaxWidth(), + modifier = Modifier + .fillMaxWidth() + .then( + if (selected == null) Modifier + else Modifier.semantics { this.selected = selected } + ), + // M3's container for a selected list item. The + // colour is the only signal a reader scanning the + // list gets about which room the pane is answering + // with -- the pane's own app bar repeats the title, + // but that is the half of the window they are not + // looking at while choosing the next room. + colors = if (selected == true) { + CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.secondaryContainer + ) + } else { + CardDefaults.cardColors() + }, onClick = { onNavigateToDirectMessageDetail.invoke( press.mantra.compose.ui.composable.navigation.routes.ChatRoomMessagingRoute( diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/ChatPaneLayoutJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/ChatPaneLayoutJvmTest.kt index 937a68aa..4d436864 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/ChatPaneLayoutJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/ChatPaneLayoutJvmTest.kt @@ -2,7 +2,11 @@ package press.mantra.compose.ui.composable import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotSelected +import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.assertWidthIsEqualTo +import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.isSelectable import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText @@ -32,7 +36,7 @@ import kotlin.test.assertEquals /** * The chat list-detail split, measured in windows of the widths the phase names. * - * Four claims, and none of them can be read off the source: + * Five claims, and none of them can be read off the source: * * - below expanded the home screen is unchanged. That is the promise that let the split * land as one commit -- the chat room is a navigation destination reached from eleven @@ -42,9 +46,11 @@ import kotlin.test.assertEquals * a `ListDetailPaneScaffold` would; * - the detail pane says what it is for while nothing is selected, rather than being an * unexplained empty half of the window; - * - and a second tap in the list changes what the detail pane shows. That one is not a + * - a second tap in the list changes what the detail pane shows. That one is not a * layout claim at all -- it is about view model scope -- but it is only reachable - * through this layout, and it is the way the split visibly failed on a tablet. + * through this layout, and it is the way the split visibly failed on a tablet; + * - and the list says which room the pane is answering with, in the one layout where + * that question exists. * * The repositories are the no-op ones with the two reads this screen makes delegated to a * fixed answer. Kotlin's interface delegation makes that ten lines rather than a @@ -123,6 +129,49 @@ class ChatPaneLayoutJvmTest { assertEquals(1, nodesSaying(roomOne), "room one is listed, not shown") } + @Test + fun `the list marks the room the detail pane is showing`() = + runDesktopComposeUiTest(1000, 900) { + setContent { Home(chatRepository = TwoRooms) } + + // Selectable, and nothing selected. A pane that has been asked nothing yet is + // a different state from a room that is not the one being shown, and the list + // is allowed to say so. `card` only matches a selectable node, so finding both + // rooms through it is half of the claim. + card(roomOne).assertIsNotSelected() + card(roomTwo).assertIsNotSelected() + + card(roomOne).performClick() + waitUntil("room one opens", OpenTimeout) { nodesSaying(roomOne) == 2 } + card(roomOne).assertIsSelected() + card(roomTwo).assertIsNotSelected() + + card(roomTwo).performClick() + waitUntil("room two opens", OpenTimeout) { nodesSaying(roomTwo) == 2 } + card(roomTwo).assertIsSelected() + + // The mark moves rather than accumulating. Two highlighted rows beside one + // transcript would say the pane was showing both. + card(roomOne).assertIsNotSelected() + } + + @Test + fun `a one-pane list says nothing about selection, having nothing to select into`() = + runDesktopComposeUiTest(700, 900) { + setContent { Home(chatRepository = TwoRooms) } + + // Tapping navigates here, and the test's callback is a no-op, so the list is + // still on screen afterwards -- which is the only reason this can be asked at + // all. A row answering "not selected" would be describing a pane that is not + // there, and a highlight left behind would outlive the screen it belonged to. + onNodeWithText(roomOne).performClick() + + // Both halves, because either alone passes for the wrong reason: a list that + // never rendered has no selectable rows either. + onNodeWithText(roomOne).assertExists() + card(roomOne).assertDoesNotExist() + } + @androidx.compose.runtime.Composable private fun Home(chatRepository: ChatRepository = NoRooms) { MantraTheme { @@ -202,4 +251,15 @@ class ChatPaneLayoutJvmTest { private fun ComposeUiTest.nodesSaying(text: String) = onAllNodesWithText(text).fetchSemanticsNodes().size + + /** + * The list's card for the room with this subject. + * + * `Card(onClick = ...)` merges its descendants, so one node carries both the subject + * and the selected state -- which is also why a selected room is two nodes saying its + * subject and a merely listed one is a single node. The detail pane's app bar says the + * same subject and is not selectable, so this stays unambiguous once a room is open. + */ + private fun ComposeUiTest.card(subject: String) = + onNode(isSelectable() and hasText(subject)) }