feat(chat): the room list marks the room the pane is answering with
The commit before this one made a second tap change the transcript. It did not make the list say which room the transcript belongs to, and on a tablet those are two halves of one question: a reader choosing their next room is looking at the list, not at the app bar on the other side of the window that repeats the title. With nothing marked, the only evidence of a selection was in the half of the screen they had looked away from -- which is its own reason the pane read as unresponsive even once it worked. **`secondaryContainer`, which is M3's container for a selected list item**, set through `CardDefaults.cardColors` so the content colour comes with it: `cardColors`' `contentColor` defaults to `contentColorFor(containerColor)`, so the row's text moves to `onSecondaryContainer` without a second call site to keep in step. A row's title keeps its `ProfileColor`, which is a fixed-lightness HSL colour and already scheme-blind, so the swap does not change what that line is contrasted against in either scheme -- a pre-existing question this commit neither fixes nor worsens. **`isSelected` is a predicate, and nullable, because the two layouts disagree about whether selection exists at all** -- not merely about its value. Where the list is the whole screen, tapping a room navigates to it and the list is about to be gone: no room is "the selected one", and both a highlight left behind on the way out and a reader told "not selected" about every room would be describing a pane that is not on the screen. `null` says that. A predicate answering false for every room does not, because that is also exactly what a two-pane layout looks like before the first tap, and those two states are different things for the list to be saying. **The call site reads the layout rather than the selection**, `listPaneWidth == null` rather than `selectedChatRoomId == null`. The selection is `rememberSaveable` and outlives a window that narrows, so a tablet turned from landscape to portrait keeps it -- and deriving the mark from the selection would leave a highlighted row on a one-pane screen with no pane beside it. **The semantics property is set only where it means something.** A row in a selection-bearing list carries `selected`, true or false, so a screen reader can say which of them the pane is showing; a row in a list that navigates carries nothing, rather than announcing "not selected" about a pane that does not exist. `Modifier.then` of an empty `Modifier` is how the row opts out, which keeps the branch at the one place that knows. **Tests: 2 cases, both checked against the feature removed.** The first taps two rooms in a 1000dp window and asserts the mark lands on the right card, moves rather than accumulating, and is absent before the first tap while the rows are already selectable -- "asked nothing yet" and "not the one being shown" being the two states the nullable predicate exists to tell apart. The second asserts a 700dp list has no selectable row at all, in two halves on purpose: a list that never rendered also has no selectable rows, so it checks the room is on screen before checking it is not selectable. Both first drafts counted `isSelectable()` across the whole window and failed by exactly one, because `PrimaryScrollableTabRow`'s `Tab` is selectable too. The assertions are scoped to the rows through a matcher that requires both selectability and the room's subject -- which works because `Card(onClick = ...)` merges its descendants, so one node carries the subject and the selected state together. That merge is also why a selected room is two nodes saying its subject and a merely listed one is a single node, which is what the previous commit's case counts. **Not done.** Still nobody has looked at this on a tablet. The tests prove the mark follows the selection and that a one-pane list stays silent; they say nothing about whether the colour reads as a selection at arm's length on a real screen. 976 jvm tests, 454 common tests, `m3Audit` meeting every budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b95d723edd
commit
fbf376fe20
3 files changed
+115
-6
No files matched your search
@@ -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 }
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+40
-2
@@ -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(
|
||||
|
||||
Reference in new issue
Block a user