diff --git a/composeApp/src/commonMain/composeResources/values/strings.xml b/composeApp/src/commonMain/composeResources/values/strings.xml index 0a1dc303..d8c5e8b0 100644 --- a/composeApp/src/commonMain/composeResources/values/strings.xml +++ b/composeApp/src/commonMain/composeResources/values/strings.xml @@ -160,6 +160,7 @@ Private to you Messages Profile + Pick a conversation to read it here. Profile is ready Profile keys Profiles 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 50c0772a..eb81bbff 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 @@ -1,13 +1,19 @@ package press.mantra.compose.ui.composable 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.Spacer +import androidx.compose.foundation.layout.fillMaxHeight +import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.foundation.pager.HorizontalPager import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add +import androidx.compose.material3.Icon import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.ExtendedFloatingActionButton @@ -26,8 +32,10 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp @@ -57,7 +65,14 @@ import mantra.composeapp.generated.resources.new_chat import press.mantra.compose.ui.composable.widgets.ErrorState import androidx.compose.material3.SnackbarHost import press.mantra.compose.ui.composable.widgets.LocalSnackbarHostState +import androidx.compose.ui.Alignment +import press.mantra.compose.repository.FrostSigningRepository +import press.mantra.compose.ui.composable.navigation.routes.ChatRoomMessagingRoute +import press.mantra.compose.ui.composable.widgets.EmptyState +import press.mantra.compose.ui.theme.breakpoint +import press.mantra.compose.ui.theme.listPaneWidthFor import press.mantra.compose.ui.theme.readableContent +import mantra.composeapp.generated.resources.pick_a_conversation_to_read_it_here @OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) @Composable @@ -68,13 +83,39 @@ fun HomeScreen( onNavigateToDirectMessageDetail: (Route) -> Unit, onNavigateToChatRoomCreation: () -> Unit, nostrRepository: NostrRepository, - chatRepository: ChatRepository + chatRepository: ChatRepository, + // Only the detail pane uses this, and only from the expanded breakpoint up. It is a + // required parameter rather than a nullable one because a home screen that silently + // loses its detail pane on a desktop is a worse failure than a compile error. + frostSigningRepository: FrostSigningRepository, ) { val sheetState = rememberModalBottomSheetState() val scope = rememberCoroutineScope() var showBottomSheet by remember { mutableStateOf(false) } val openNpubDialog = remember { mutableStateOf(false) } + // Null below the expanded breakpoint, which is where the room list is the whole screen + // and tapping a room navigates, exactly as it did before panes existed. + val listPaneWidth = listPaneWidthFor(MaterialTheme.breakpoint) + + // The room the detail pane is showing. Two saveable strings rather than the route + // object, because that is all the route carries and both survive a process death that + // a `@Serializable` route would need a Saver to survive. + var selectedChatRoomId by rememberSaveable { mutableStateOf(null) } + var selectedRelayHint by rememberSaveable { mutableStateOf(null) } + + // Tapping a room means two different things at two widths, and the list does not need + // to know which: it hands over a `ChatRoomMessagingRoute` either way, and this decides + // whether that is a destination to push or a selection to make. + val onOpenChatRoom: (Route) -> Unit = { route -> + if (listPaneWidth != null && route is ChatRoomMessagingRoute) { + selectedChatRoomId = route.chatRoomId + selectedRelayHint = route.relayHint + } else { + onNavigateToDirectMessageDetail(route) + } + } + val homeScreenViewModel: HomeViewModel = viewModel( factory = HomeViewModel.factory( activeUserPublicKey = activeUserPublicKey, @@ -126,24 +167,16 @@ fun HomeScreen( }, floatingActionButton = { - ExtendedFloatingActionButton( - onClick = { - showBottomSheet = true - }, - icon = { - Icon( - Icons.Default.Add, - contentDescription = "New chat" - ) - }, - text = { - Text(stringResource(Res.string.new_chat)) - } - ) + // Only while there is one pane. With two, the scaffold's FAB slot is + // the bottom-right of the *window*, which is on top of the + // transcript's send button; M3 puts a list-detail layout's primary + // action in the list pane, and so does the branch below. + if (listPaneWidth == null) NewChatButton { showBottomSheet = true } } ) { innerPadding -> - Column( - modifier = Modifier.padding(innerPadding).readableContent().fillMaxSize() + @Composable + fun ChatRoomListPane(modifier: Modifier) = Column( + modifier = modifier ) { PrimaryScrollableTabRow( modifier = Modifier.padding(MaterialTheme.spacing.space125), @@ -189,7 +222,7 @@ fun HomeScreen( key(true) { chatRoomListViewModel.RenderFeed( - onNavigateToDirectMessageDetail = onNavigateToDirectMessageDetail + onNavigateToDirectMessageDetail = onOpenChatRoom ) } @@ -202,6 +235,72 @@ fun HomeScreen( } } + if (listPaneWidth == null) { + // Compact and medium: the list is the screen, and a room is a route. + // Byte for byte the layout this screen has always had. + ChatRoomListPane( + Modifier.padding(innerPadding).readableContent().fillMaxSize() + ) + } else { + // Expanded and up: the list keeps a fixed width and the room fills the + // rest. No `readableContent()` on the pair -- capping the two panes + // together to one column's measure is the opposite of what a second + // pane is for. Each pane holds its own content to a measure instead, + // and `ChatRoomMessagingScreen` already does. + Row(modifier = Modifier.padding(innerPadding).fillMaxSize()) { + Box( + modifier = Modifier + .width(listPaneWidth) + .fillMaxHeight() + // The pane's width is the one thing about this layout that + // is a number rather than a rule, and the only way to check + // a number is to measure it in a composition. + .testTag(ChatListPaneTag) + ) { + ChatRoomListPane(Modifier.fillMaxSize()) + + Box( + modifier = Modifier + .align(Alignment.BottomEnd) + .padding(MaterialTheme.spacing.containerPadding) + ) { + NewChatButton { showBottomSheet = true } + } + } + + Spacer(modifier = Modifier.width(MaterialTheme.spacing.paneGap)) + + Box(modifier = Modifier.weight(1f).fillMaxHeight()) { + val chatRoomId = selectedChatRoomId + if (chatRoomId == null) { + EmptyState( + message = stringResource(Res.string.pick_a_conversation_to_read_it_here) + ) + } else { + // Keyed on the room, so switching rooms rebuilds the + // screen's view models rather than feeding a new id to + // ones already subscribed to another room's relay. + key(chatRoomId) { + ChatRoomMessagingScreen( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId, + relayHint = selectedRelayHint, + nostrRepository = nostrRepository, + chatRepository = chatRepository, + frostSigningRepository = frostSigningRepository, + // "Replace what is on screen" is a route push on a + // phone and a change of selection here. It fires when + // a conversation that did not exist yet has just been + // created and has an id at last. + onNavigateToRouteAndPopUpInclusive = onOpenChatRoom, + onNavigateToRoute = onNavigateToRoute, + ) + } + } + } + } + } + if (showBottomSheet) { NewChatBottomSheetDialog( scope = scope, @@ -276,8 +375,35 @@ It has survived not only five centuries, but also the leap into electronic types onNavigateToChatRoomCreation = {}, onNavigateToDirectMessageDetail = {}, nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY, - chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY + chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, ) } } -} \ No newline at end of file +} + +/** + * The one action the chat list offers, in whichever slot the layout has for it. + * + * Extracted only so that the two branches above cannot drift: on one pane it is the + * `Scaffold`'s floating action button, on two it sits in the bottom corner of the list + * pane, and a "new chat" that reads differently depending on window width would be a + * strange thing to discover. + */ +@Composable +private fun NewChatButton(onClick: () -> Unit) { + ExtendedFloatingActionButton( + onClick = onClick, + icon = { + Icon( + Icons.Default.Add, + // Decorative: the button's own text says "New chat" beside it. + contentDescription = null, + ) + }, + text = { Text(stringResource(Res.string.new_chat)) }, + ) +} + +/** Addresses the chat list pane from a layout test. See `ChatPaneLayoutJvmTest`. */ +const val ChatListPaneTag = "chat-list-pane" diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt index 4bed071e..f86a23e6 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt @@ -699,7 +699,8 @@ fun MantraNavHost( ) }, nostrRepository = databaseNostrRepository, - chatRepository = databaseChatRepository + chatRepository = databaseChatRepository, + frostSigningRepository = databaseFrostSigningRepository, ) } composable { diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Panes.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Panes.kt new file mode 100644 index 00000000..e32278c4 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Panes.kt @@ -0,0 +1,30 @@ +package press.mantra.compose.ui.theme + +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp + +/** + * How wide a list pane should be at [breakpoint], or `null` where M3 asks for one pane. + * + * **`null` below expanded is the spec, not caution.** The breakpoints page says not to put + * two dense panes in a medium window, and `calculatePaneScaffoldDirective` in + * `material3-adaptive` says the same thing in code: `maxHorizontalPartitions = 1` for both + * compact and medium. A chat transcript is exactly the dense content that rule is about. + * + * The two widths are that same function's, so a hand-built pair of panes measures the same + * as a `ListDetailPaneScaffold` would: `DefaultPreferredWidth` at expanded, and + * `DefaultPreferredWidthXL` from large upward, where the directive also allows a third + * partition this app has no content for. + * + * **Why not the scaffold itself.** `ListDetailPaneScaffold` earns its API surface by + * owning the single-pane case too -- showing the detail *instead of* the list on a phone, + * and animating between them. This app cannot hand it that: the chat room is a navigation + * destination reached from eleven places, so on a compact window the detail has to stay a + * pushed route. A scaffold permanently in its two-pane state would be a `Row` with more + * words, and a navigator whose history nothing reads. + */ +fun listPaneWidthFor(breakpoint: Breakpoint): Dp? = when (breakpoint) { + Breakpoint.Compact, Breakpoint.Medium -> null + Breakpoint.Expanded -> 360.dp + Breakpoint.Large, Breakpoint.ExtraLarge -> 412.dp +} diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Spacing.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Spacing.kt index 99c6b6c8..f333eeda 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Spacing.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/theme/Spacing.kt @@ -98,6 +98,12 @@ data class Spacing( /** Between adjacent touch targets, which M3 asks to be at least 8dp apart. */ val targetGap: Dp = space100, + + /** + * Between two panes. M3's own `PaneScaffoldDirective` uses 24dp at every breakpoint + * that has a second pane, which is why this does not vary with the window either. + */ + val paneGap: Dp = space300, ) /** diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/PanesTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/PanesTest.kt new file mode 100644 index 00000000..c1824a40 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/ui/theme/PanesTest.kt @@ -0,0 +1,75 @@ +package press.mantra.compose.ui.theme + +import androidx.compose.ui.unit.dp +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * When a second pane appears, and how wide the first one is. + * + * The numbers are `material3-adaptive`'s own `calculatePaneScaffoldDirective`, transcribed + * so that a hand-built pair of panes measures the same as a `ListDetailPaneScaffold` would. + * A transcription error here is the kind that looks fine on the one desktop anybody opens. + */ +class PanesTest { + + @Test + fun `no second pane below expanded`() { + // M3: "don't use two panes in medium layouts with high information density", and + // `calculatePaneScaffoldDirective` agrees in code -- `maxHorizontalPartitions = 1` + // for compact and medium alike. A chat transcript is that dense content. + assertNull(listPaneWidthFor(Breakpoint.Compact)) + assertNull(listPaneWidthFor(Breakpoint.Medium)) + } + + @Test + fun `the list pane takes the directive's two widths`() { + assertEquals(360.dp, listPaneWidthFor(Breakpoint.Expanded)) + assertEquals(412.dp, listPaneWidthFor(Breakpoint.Large)) + assertEquals(412.dp, listPaneWidthFor(Breakpoint.ExtraLarge)) + } + + @Test + fun `the list pane never widens past the window that first allowed it`() { + // The failure a fixed width invites: a pane wider than the breakpoint that opens + // it leaves the detail pane with nothing, or negative space. 360dp of list plus + // 24dp of gap inside the 840dp window that first allows two panes leaves 456dp for + // the transcript, which is above the 40-character floor. + Breakpoint.entries.forEach { breakpoint -> + val width = listPaneWidthFor(breakpoint) ?: return@forEach + val gap = Spacing().paneGap + val detail = breakpoint.minWidth - width - gap + + assertTrue( + detail >= readableWidthFor(16.dp, charactersPerLine = 40), + "at $breakpoint the detail pane starts at $detail, under a 40-character line", + ) + } + } + + @Test + fun `the gap between panes is the directive's 24dp at every width that has one`() { + // `PaneScaffoldDirective` uses 24dp for `horizontalPartitionSpacerSize` at every + // breakpoint with a second pane, which is why `paneGap` does not vary either. + assertEquals(24.dp, Spacing().paneGap) + assertEquals( + spacingFor(Breakpoint.Compact).paneGap, + spacingFor(Breakpoint.ExtraLarge).paneGap, + "the pane gap moved with the breakpoint, which the directive does not do", + ) + } + + @Test + fun `two panes begin exactly where the breakpoint says`() { + // The pairing that has to hold: the first breakpoint with a list pane is the first + // one M3 recommends two panes in. If a breakpoint were inserted, or `Expanded` + // renumbered, this is what notices. + assertNotNull(listPaneWidthFor(Breakpoint.Expanded)) + assertEquals(840.dp, Breakpoint.Expanded.minWidth) + assertNull(listPaneWidthFor(Breakpoint.ofWidth(839.dp))) + assertNotNull(listPaneWidthFor(Breakpoint.ofWidth(840.dp))) + } +} 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 new file mode 100644 index 00000000..fa4a4c38 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/ChatPaneLayoutJvmTest.kt @@ -0,0 +1,134 @@ +package press.mantra.compose.ui.composable + +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertWidthIsEqualTo +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.runDesktopComposeUiTest +import androidx.compose.ui.unit.dp +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import press.mantra.compose.database.model.NostrEvent +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalProfileWithFollowing +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.FrostSigningRepository +import press.mantra.compose.repository.NostrRepository +import press.mantra.compose.ui.composable.widgets.ProvideSnackbarHost +import press.mantra.compose.ui.theme.TorchTheme +import press.mantra.compose.ui.view.state.HomeScreenUIState +import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent +import kotlin.test.Test + +/** + * The chat list-detail split, measured in windows of the widths the phase names. + * + * Three 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 + * places, and on a phone it stays one; + * - at expanded and above the list pane is exactly the width + * `calculatePaneScaffoldDirective` gives it, so a hand-built pair measures the same as + * 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. + * + * 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 + * reimplementation of two large interfaces, and it keeps the test about layout: the screen + * asks for a profile and a room list, and what matters here is where they are drawn. + */ +@OptIn(ExperimentalTestApi::class) +class ChatPaneLayoutJvmTest { + + private val publicKey = "de1a1e64d1c4e0d6bd97b0e73d4dfd0e1ec1cdd1e6d8d0e2b9a7f3c5d0e1a2b3" + private val emptyDetail = "Pick a conversation to read it here." + + @Test + fun `a phone window is one pane, as it has always been`() = runDesktopComposeUiTest(400, 900) { + setContent { Home() } + + onNodeWithText(emptyDetail).assertDoesNotExist() + } + + @Test + fun `a medium window is still one pane, because a transcript is dense`() = + runDesktopComposeUiTest(700, 900) { + setContent { Home() } + + // M3: no two panes in a medium window with high information density. + onNodeWithText(emptyDetail).assertDoesNotExist() + } + + @Test + fun `an expanded window splits, with the directive's 360dp list pane`() = + runDesktopComposeUiTest(1000, 900) { + setContent { Home() } + + onNodeWithText(emptyDetail).assertIsDisplayed() + onNodeWithTag(ChatListPaneTag).assertWidthIsEqualTo(360.dp) + } + + @Test + fun `a large window widens the list pane to 412dp`() = runDesktopComposeUiTest(1400, 900) { + setContent { Home() } + + onNodeWithText(emptyDetail).assertIsDisplayed() + onNodeWithTag(ChatListPaneTag).assertWidthIsEqualTo(412.dp) + } + + @androidx.compose.runtime.Composable + private fun Home() { + TorchTheme { + // Required, and deliberately so: `LocalSnackbarHostState` throws rather than + // defaulting, because a default would make every `notify` on a screen that + // forgot the host a silent no-op. + ProvideSnackbarHost { + HomeScreen( + activeUserPublicKey = publicKey, + initialHomeScreenUIState = HomeScreenUIState.Loaded(profileWithFollowing = profile), + onNavigateToRoute = {}, + onNavigateToDirectMessageDetail = {}, + onNavigateToChatRoomCreation = {}, + nostrRepository = FixedProfile, + chatRepository = NoRooms, + frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, + ) + } + } + } + + private val profile = LocalProfileWithFollowing( + nostrEvent = NostrEvent( + id = "aa11bb22cc33dd44ee55ff6600778899aabbccddeeff00112233445566778899", + pubKey = publicKey, + content = "", + tags = emptyArray(), + sig = "", + kind = TextNoteEvent.KIND, + ), + profile = Profile( + publicKey = publicKey, + displayName = "Reader", + nostrEventId = "aa11bb22cc33dd44ee55ff6600778899aabbccddeeff00112233445566778899", + ), + following = emptyList(), + ) + + /** The no-op repository, plus the one read the home screen makes of it. */ + private val FixedProfile = object : NostrRepository by NostrRepository.NO_OP_NOSTR_REPOSITORY { + override suspend fun observeProfileWithFollowing( + publicKey: String, + ): Flow = flowOf(profile) + } + + /** The no-op repository, plus the one read the room list makes of it. */ + private val NoRooms = object : ChatRepository by ChatRepository.NO_OP_CHAT_REPOSITORY { + override suspend fun observeChatRoomListByPublicKey( + publicKey: String, + ): Flow> = flowOf(emptyList()) + } +}