From 4c4034032b43dfba2d18ef1182e6a4279cd29771 Mon Sep 17 00:00:00 2001 From: stan <26108664+talanov@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:25:47 -0500 Subject: [PATCH 1/3] Open Start chat in the detail pane next to the conversation list on large screens instead of over both panes (fixes #380) --- .../conversation/entry/NewChatScreenTest.kt | 48 ++++++-- .../navigation/ConversationNavEntriesTest.kt | 108 ++++++++++++++++++ .../ConversationPaneMetadataTest.kt | 48 ++++++++ .../ui/conversation/entry/NewChatScreen.kt | 13 +++ .../navigation/ConversationNavEntries.kt | 6 +- .../navigation/ConversationPaneMetadata.kt | 10 ++ .../component/RecipientSelectionContent.kt | 19 ++- .../navigation/ConversationListNavEntries.kt | 7 +- 8 files changed, 243 insertions(+), 16 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationNavEntriesTest.kt create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadataTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt index 8e5685eb3..afcd10499 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt @@ -1,9 +1,11 @@ package com.android.messaging.ui.conversation.entry import androidx.activity.ComponentActivity +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.ui.semantics.SemanticsActions import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsFocused import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.hasScrollToIndexAction import androidx.compose.ui.test.hasSetTextAction @@ -14,6 +16,9 @@ import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollToIndex import androidx.compose.ui.test.performSemanticsAction import androidx.compose.ui.test.performTextInput +import com.android.messaging.ui.common.components.ListDetailPaneSide +import com.android.messaging.ui.common.components.LocalIsListDetailPane +import com.android.messaging.ui.common.components.LocalListDetailPaneSide import com.android.messaging.ui.conversation.NEW_CHAT_CONTACT_RESOLVING_INDICATOR_TEST_TAG import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_BUTTON_TEST_TAG import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_NEXT_BUTTON_TEST_TAG @@ -253,15 +258,44 @@ class NewChatScreenTest { .assertIsDisplayed() } - private fun setContent(screenModel: NewChatScreenModel) { + @Test + fun opening_inSinglePane_focusesTheRecipientField() { + setContent(screenModel = createScreenModel(initialUiState = NewChatUiState())) + + composeTestRule + .onNode(matcher = hasSetTextAction()) + .assertIsFocused() + } + + @Test + fun opening_inListDetailPane_focusesTheRecipientField() { + setContent( + screenModel = createScreenModel(initialUiState = NewChatUiState()), + isListDetailPane = true, + ) + + composeTestRule + .onNode(matcher = hasSetTextAction()) + .assertIsFocused() + } + + private fun setContent( + screenModel: NewChatScreenModel, + isListDetailPane: Boolean = false, + ) { composeTestRule.setContent { AppTheme { - NewChatScreen( - screenModel = screenModel, - effectHandler = mockk(relaxed = true), - onNavigateBack = {}, - onNavigateToConversation = { _, _ -> }, - ) + CompositionLocalProvider( + LocalIsListDetailPane provides isListDetailPane, + LocalListDetailPaneSide provides ListDetailPaneSide.End, + ) { + NewChatScreen( + screenModel = screenModel, + effectHandler = mockk(relaxed = true), + onNavigateBack = {}, + onNavigateToConversation = { _, _ -> }, + ) + } } } } diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationNavEntriesTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationNavEntriesTest.kt new file mode 100644 index 000000000..3373f564f --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationNavEntriesTest.kt @@ -0,0 +1,108 @@ +@file:OptIn(ExperimentalMaterial3AdaptiveApi::class) + +package com.android.messaging.ui.conversation.navigation + +import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi +import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffoldDefaults +import androidx.compose.material3.adaptive.layout.PaneScaffoldDirective +import androidx.compose.material3.adaptive.navigation.BackNavigationBehavior +import androidx.compose.material3.adaptive.navigation3.ListDetailSceneStrategy +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.entryProvider +import androidx.navigation3.runtime.get +import androidx.navigation3.scene.Scene +import androidx.navigation3.scene.SceneStrategyScope +import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID +import com.android.messaging.ui.common.components.ListDetailPaneSide +import com.android.messaging.ui.conversationlist.navigation.ConversationListNavKey +import com.android.messaging.ui.conversationlist.navigation.conversationListEntries +import com.android.messaging.ui.navigation.ListDetailPaneKey +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +internal class ConversationNavEntriesTest { + + private val entryProvider = entryProvider { + conversationListEntries() + conversationEntries() + } + + @Test + fun newChat_inTwoPaneWindow_opensInTheDetailPaneNextToTheList() { + val entries = navEntries(ConversationListNavKey, NewChatNavKey) + + val scene = listDetailScene( + entries = entries, + maxHorizontalPartitions = TWO_PANES, + ) + + assertEquals(entries, scene?.entries) + } + + @Test + fun newChat_overOpenConversationInTwoPaneWindow_goesBackToThatConversation() { + val entries = navEntries( + ConversationListNavKey, + ConversationNavKey(conversationId = CONVERSATION_ID), + NewChatNavKey, + ) + + val scene = listDetailScene( + entries = entries, + maxHorizontalPartitions = TWO_PANES, + ) + + assertEquals(entries, scene?.entries) + assertEquals(entries.dropLast(n = 1), scene?.previousEntries) + } + + @Test + fun newChat_inSinglePaneWindow_isLeftToFillTheWindow() { + val scene = listDetailScene( + entries = navEntries(ConversationListNavKey, NewChatNavKey), + maxHorizontalPartitions = SINGLE_PANE, + ) + + assertNull(scene) + } + + @Test + fun newChat_inListDetailLayout_takesTheEndSide() { + val entry = entryProvider(NewChatNavKey) + + assertEquals(ListDetailPaneSide.End, entry.metadata[ListDetailPaneKey]) + } + + private fun navEntries(vararg keys: NavKey): List> { + return keys.map(entryProvider) + } + + private fun listDetailScene( + entries: List>, + maxHorizontalPartitions: Int, + ): Scene? { + val sceneStrategy = ListDetailSceneStrategy( + shouldHandleSinglePaneLayout = false, + backNavigationBehavior = BackNavigationBehavior.PopLatest, + directive = PaneScaffoldDirective.Default.copy( + maxHorizontalPartitions = maxHorizontalPartitions, + ), + adaptStrategies = ListDetailPaneScaffoldDefaults.adaptStrategies(), + paneExpansionDragHandle = null, + paneExpansionState = null, + ) + + return with(SceneStrategyScope()) { + with(sceneStrategy) { + calculateScene(entries) + } + } + } + + private companion object { + private const val SINGLE_PANE = 1 + private const val TWO_PANES = 2 + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadataTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadataTest.kt new file mode 100644 index 000000000..5f879c131 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadataTest.kt @@ -0,0 +1,48 @@ +package com.android.messaging.ui.conversation.navigation + +import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID +import com.android.messaging.ui.conversationlist.navigation.ArchivedConversationListNavKey +import com.android.messaging.ui.conversationlist.navigation.ConversationListNavKey +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +internal class ConversationPaneMetadataTest { + + @Test + fun detailPaneConversationId_conversationOnTop_returnsItsId() { + val backStack = listOf( + ConversationListNavKey, + ConversationNavKey(conversationId = CONVERSATION_ID), + ) + + assertEquals(CONVERSATION_ID, detailPaneConversationId(backStack = backStack)) + } + + @Test + fun detailPaneConversationId_newChatOverConversation_returnsNull() { + val backStack = listOf( + ConversationListNavKey, + ConversationNavKey(conversationId = CONVERSATION_ID), + NewChatNavKey, + ) + + assertNull(detailPaneConversationId(backStack = backStack)) + } + + @Test + fun detailPaneConversationId_listPaneOverConversation_returnsTheConversation() { + val backStack = listOf( + ConversationListNavKey, + ConversationNavKey(conversationId = CONVERSATION_ID), + ArchivedConversationListNavKey, + ) + + assertEquals(CONVERSATION_ID, detailPaneConversationId(backStack = backStack)) + } + + @Test + fun detailPaneConversationId_withoutDetailDestination_returnsNull() { + assertNull(detailPaneConversationId(backStack = listOf(ConversationListNavKey))) + } +} diff --git a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt index ab67ce4c5..b8e7c8ab4 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt @@ -21,6 +21,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.PreviewLightDark @@ -28,6 +29,8 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.android.messaging.R import com.android.messaging.data.conversation.model.ConversationId import com.android.messaging.data.conversation.model.ParticipantId +import com.android.messaging.ui.common.components.LocalIsListDetailPane +import com.android.messaging.ui.common.components.contentSurfaceShape import com.android.messaging.ui.conversation.NEW_CHAT_CONTACT_RESOLVING_INDICATOR_TEST_TAG import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_NEXT_BUTTON_TEST_TAG import com.android.messaging.ui.conversation.NEW_CHAT_NAVIGATE_BACK_BUTTON_TEST_TAG @@ -218,9 +221,19 @@ private fun NewChatRecipientSelectionContent( selectedGroupRecipients: ImmutableList, simSelectorUiState: ConversationSimSelectorUiState, ) { + val isListDetailPane = LocalIsListDetailPane.current + RecipientSelectionContent( modifier = Modifier.fillMaxSize(), contentPadding = contentPadding, + containerColor = when { + isListDetailPane -> MaterialTheme.colorScheme.surfaceContainerLow + else -> MaterialTheme.colorScheme.surfaceContainer + }, + containerShape = when { + isListDetailPane -> MaterialTheme.contentSurfaceShape + else -> RectangleShape + }, uiState = newChatRecipientSelectionContentUiState( pickerUiState = pickerUiState, isCreatingGroup = isCreatingGroup, diff --git a/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt b/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt index 221b0fa31..65e429f0a 100644 --- a/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt +++ b/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt @@ -38,7 +38,8 @@ internal fun EntryProviderScope.conversationEntries() { content = conversationScreenRouteContent(), ) entry( - metadata = paneTitleMetadata(R.string.start_new_conversation), + metadata = conversationDetailPaneMetadata() + + paneTitleMetadata(R.string.start_new_conversation), content = newChatRouteContent(), ) entry( @@ -159,6 +160,9 @@ private fun newChatRouteContent(): @Composable (NewChatNavKey) -> Unit { ) navigator.navigateToConversation(conversationId = conversationId) }, + modifier = Modifier + .fillMaxSize() + .consumeOppositePaneInsets(), ) } } diff --git a/src/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadata.kt b/src/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadata.kt index 183f362cb..232bb771b 100644 --- a/src/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadata.kt +++ b/src/com/android/messaging/ui/conversation/navigation/ConversationPaneMetadata.kt @@ -4,6 +4,8 @@ package com.android.messaging.ui.conversation.navigation import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi import androidx.compose.material3.adaptive.navigation3.ListDetailSceneStrategy +import androidx.navigation3.runtime.NavKey +import com.android.messaging.data.conversation.model.ConversationId import com.android.messaging.ui.common.components.ListDetailPaneSide import com.android.messaging.ui.conversation.screen.ConversationDetailPlaceholder import com.android.messaging.ui.navigation.listDetailPaneMetadata @@ -22,3 +24,11 @@ internal fun conversationDetailPaneMetadata(): Map { sceneKey = CONVERSATION_LIST_DETAIL_SCENE_KEY, ) + listDetailPaneMetadata(side = ListDetailPaneSide.End) } + +internal fun detailPaneConversationId(backStack: List): ConversationId? { + val detailPaneDestination = backStack.lastOrNull { navKey -> + navKey is ConversationNavKey || navKey is NewChatNavKey + } + + return (detailPaneDestination as? ConversationNavKey)?.conversationId +} diff --git a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt index 105dcffdf..ca3396d42 100644 --- a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt +++ b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt @@ -25,8 +25,12 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.RectangleShape +import androidx.compose.ui.graphics.Shape import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.tooling.preview.PreviewLightDark import androidx.compose.ui.unit.dp @@ -57,6 +61,8 @@ internal fun RecipientSelectionContent( onRecipientDestinationClick: OnRecipientDestinationAction, modifier: Modifier = Modifier, contentPadding: PaddingValues = PaddingValues(), + containerColor: Color = MaterialTheme.colorScheme.surfaceContainer, + containerShape: Shape = RectangleShape, autoFocusQuery: Boolean = false, onLoadMore: () -> Unit = {}, onPrimaryActionClick: () -> Unit = {}, @@ -81,6 +87,8 @@ internal fun RecipientSelectionContent( RecipientSelectionContentLayout( modifier = modifier, contentPadding = contentPadding, + containerColor = containerColor, + containerShape = containerShape, pinnedTopContent = pinnedTopContent, queryArea = { RecipientSelectionArmedQueryArea( @@ -121,21 +129,23 @@ private fun RecipientSelectionContentLayout( queryArea: @Composable () -> Unit, contactsArea: @Composable () -> Unit, contentPadding: PaddingValues, + containerColor: Color, + containerShape: Shape, modifier: Modifier = Modifier, pinnedTopContent: (@Composable () -> Unit)? = null, ) { val layoutDirection = LocalLayoutDirection.current Surface( - modifier = modifier, - color = MaterialTheme.colorScheme.surfaceContainer, + modifier = modifier.padding(top = contentPadding.calculateTopPadding()), + shape = containerShape, + color = containerColor, ) { Column( modifier = Modifier .fillMaxSize() .padding( start = contentPadding.calculateStartPadding(layoutDirection), - top = contentPadding.calculateTopPadding(), end = contentPadding.calculateEndPadding(layoutDirection), ) .padding(horizontal = 16.dp), @@ -366,6 +376,9 @@ private fun RecipientSelectionAutoFocusEffect( ) { if (autoFocusQuery) { LaunchedEffect(Unit) { + // Focusing before the first layout crashes Compose's lookahead measure when the + // window turns two-pane and the list-detail scaffold is built around this screen + withFrameNanos {} focusRequester.requestFocus() } } diff --git a/src/com/android/messaging/ui/conversationlist/navigation/ConversationListNavEntries.kt b/src/com/android/messaging/ui/conversationlist/navigation/ConversationListNavEntries.kt index cbe5a739a..0360710d2 100644 --- a/src/com/android/messaging/ui/conversationlist/navigation/ConversationListNavEntries.kt +++ b/src/com/android/messaging/ui/conversationlist/navigation/ConversationListNavEntries.kt @@ -14,8 +14,8 @@ import com.android.messaging.ui.blockedparticipants.navigation.BlockedParticipan import com.android.messaging.ui.common.components.LocalIsListDetailPane import com.android.messaging.ui.common.components.consumeOppositePaneInsets import com.android.messaging.ui.contact.navigation.navigateToAddContact -import com.android.messaging.ui.conversation.navigation.ConversationNavKey import com.android.messaging.ui.conversation.navigation.conversationListPaneMetadata +import com.android.messaging.ui.conversation.navigation.detailPaneConversationId import com.android.messaging.ui.conversation.navigation.rememberConversationNavigator import com.android.messaging.ui.conversationlist.archived.ArchivedConversationListScreen import com.android.messaging.ui.conversationlist.archived.ArchivedConversationListViewModel @@ -92,10 +92,7 @@ private fun openedConversationId(navigator: Navigator): ConversationId? { return null } - return navigator.backStack - .filterIsInstance() - .lastOrNull() - ?.conversationId + return detailPaneConversationId(backStack = navigator.backStack) } private fun archivedConversationListRouteContent(): From 7cd66953b430d0076b3f21823e51ad42e4be1e97 Mon Sep 17 00:00:00 2001 From: stan <26108664+talanov@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:09:04 -0500 Subject: [PATCH 2/3] Keep an open conversation scrolled where it was when a large screen rotates between one and two panes --- .../navigation/DisplayCornerDecoratorTest.kt | 61 +++++++++++++++++++ .../ui/navigation/DisplayCornerDecorator.kt | 29 +++++---- 2 files changed, 75 insertions(+), 15 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/ui/navigation/DisplayCornerDecoratorTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/ui/navigation/DisplayCornerDecoratorTest.kt b/app/src/test/kotlin/com/android/messaging/ui/navigation/DisplayCornerDecoratorTest.kt new file mode 100644 index 000000000..ea95d36c1 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/navigation/DisplayCornerDecoratorTest.kt @@ -0,0 +1,61 @@ +package com.android.messaging.ui.navigation + +import androidx.activity.ComponentActivity +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.rememberSaveableStateHolderNavEntryDecorator +import androidx.navigation3.ui.NavDisplay +import com.android.messaging.testutil.assertThat +import com.android.messaging.ui.common.components.ListDetailPaneSide +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class DisplayCornerDecoratorTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + @Test + fun listDetailEntry_whenTheWindowGainsAndLosesASecondPane_keepsItsState() { + var showsTwoPanes by mutableStateOf(false) + val entryStates = mutableSetOf() + composeTestRule.setContent { + NavDisplay( + backStack = listOf(TestKey), + onBack = {}, + entryDecorators = listOf( + rememberSaveableStateHolderNavEntryDecorator(), + rememberListDetailPaneNavEntryDecorator(showsTwoPanes = showsTwoPanes), + rememberDisplayCornerNavEntryDecorator( + topContentKey = NavEntry(TestKey) {}.contentKey, + ), + ), + entryProvider = { key -> + NavEntry( + key = key, + metadata = listDetailPaneMetadata(side = ListDetailPaneSide.End), + ) { + entryStates += remember { Any() } + } + }, + ) + } + + showsTwoPanes = true + composeTestRule.waitForIdle() + showsTwoPanes = false + composeTestRule.waitForIdle() + + assertThat(entryStates.size).isEqualTo(1) + } + + private data object TestKey : NavKey +} diff --git a/src/com/android/messaging/ui/navigation/DisplayCornerDecorator.kt b/src/com/android/messaging/ui/navigation/DisplayCornerDecorator.kt index 3c572db6f..75581b569 100644 --- a/src/com/android/messaging/ui/navigation/DisplayCornerDecorator.kt +++ b/src/com/android/messaging/ui/navigation/DisplayCornerDecorator.kt @@ -41,23 +41,22 @@ private fun DisplayCorneredContent( isTop: Boolean, entry: NavEntry, ) { - if (LocalIsListDetailPane.current) { - entry.Content() - return - } + val pageModifier = when { + LocalIsListDetailPane.current -> Modifier - val transition = LocalNavAnimatedContentScope.current.transition + else -> { + Modifier + .predictiveBackPage( + transition = LocalNavAnimatedContentScope.current.transition, + isOpen = isTop, + ) + .clip( + shape = RoundedCornerShape(size = cornerRadius), + ) + } + } - Box( - modifier = Modifier - .predictiveBackPage( - transition = transition, - isOpen = isTop, - ) - .clip( - shape = RoundedCornerShape(size = cornerRadius), - ), - ) { + Box(modifier = pageModifier) { entry.Content() } } From 696393f06b45a27f4aecc7087a517aa69d334ab7 Mon Sep 17 00:00:00 2001 From: stan <26108664+talanov@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:36:51 -0500 Subject: [PATCH 3/3] Keep Start chat's recipient field focused when a large screen rotates between one and two panes --- .../conversation/entry/NewChatScreenTest.kt | 78 +++++++++++++++++++ .../ui/conversation/entry/NewChatScreen.kt | 6 ++ .../navigation/ConversationNavEntries.kt | 2 + .../component/RecipientSelectionContent.kt | 9 +-- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt index afcd10499..08982940f 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt @@ -1,7 +1,15 @@ package com.android.messaging.ui.conversation.entry import androidx.activity.ComponentActivity +import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi +import androidx.compose.material3.adaptive.layout.PaneScaffoldDirective +import androidx.compose.material3.adaptive.navigation.BackNavigationBehavior +import androidx.compose.material3.adaptive.navigation3.rememberListDetailSceneStrategy import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateListOf +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue import androidx.compose.ui.semantics.SemanticsActions import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertIsDisplayed @@ -16,6 +24,9 @@ import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollToIndex import androidx.compose.ui.test.performSemanticsAction import androidx.compose.ui.test.performTextInput +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.entryProvider +import androidx.navigation3.ui.LocalNavAnimatedContentScope import com.android.messaging.ui.common.components.ListDetailPaneSide import com.android.messaging.ui.common.components.LocalIsListDetailPane import com.android.messaging.ui.common.components.LocalListDetailPaneSide @@ -23,8 +34,14 @@ import com.android.messaging.ui.conversation.NEW_CHAT_CONTACT_RESOLVING_INDICATO import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_BUTTON_TEST_TAG import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_NEXT_BUTTON_TEST_TAG import com.android.messaging.ui.conversation.entry.model.NewChatUiState +import com.android.messaging.ui.conversation.navigation.NewChatNavKey +import com.android.messaging.ui.conversation.navigation.conversationDetailPaneMetadata +import com.android.messaging.ui.conversation.navigation.conversationListPaneMetadata import com.android.messaging.ui.conversation.newChatContactRowTestTag +import com.android.messaging.ui.conversationlist.navigation.ConversationListNavKey import com.android.messaging.ui.core.AppTheme +import com.android.messaging.ui.navigation.AppNavDisplay +import com.android.messaging.ui.navigation.rememberAppSceneStrategies import com.android.messaging.ui.recipientselection.component.row.CONTACT_ID import com.android.messaging.ui.recipientselection.component.row.MOBILE_NORMALIZED_DESTINATION import com.android.messaging.ui.recipientselection.component.row.contactItem @@ -42,6 +59,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config @RunWith(RobolectricTestRunner::class) class NewChatScreenTest { @@ -279,6 +297,60 @@ class NewChatScreenTest { .assertIsFocused() } + @OptIn(ExperimentalMaterial3AdaptiveApi::class) + @Config(qualifiers = TABLET_QUALIFIERS) + @Test + fun paneCountChange_keepsTheRecipientFieldFocused() { + val screenModel = createScreenModel(initialUiState = NewChatUiState()) + val backStack = mutableStateListOf(ConversationListNavKey, NewChatNavKey) + var showsTwoPanes by mutableStateOf(false) + + composeTestRule.setContent { + val maxHorizontalPartitions = when { + showsTwoPanes -> TWO_PANES + else -> SINGLE_PANE + } + + AppTheme { + AppNavDisplay( + backStack = backStack, + entryProvider = entryProvider { + entry(metadata = conversationListPaneMetadata()) {} + entry(metadata = conversationDetailPaneMetadata()) { + NewChatScreen( + screenModel = screenModel, + effectHandler = mockk(relaxed = true), + onNavigateBack = {}, + onNavigateToConversation = { _, _ -> }, + autoFocusQueryKey = LocalNavAnimatedContentScope.current, + ) + } + }, + onBack = {}, + sceneStrategies = rememberAppSceneStrategies( + additionalStrategies = listOf( + rememberListDetailSceneStrategy( + backNavigationBehavior = BackNavigationBehavior.PopLatest, + directive = PaneScaffoldDirective.Default.copy( + maxHorizontalPartitions = maxHorizontalPartitions, + ), + ), + ), + ), + showsTwoPanes = showsTwoPanes, + ) + } + } + + for (twoPanes in listOf(true, false)) { + showsTwoPanes = twoPanes + + composeTestRule + .onNode(matcher = hasSetTextAction()) + .assertIsFocused() + } + } + private fun setContent( screenModel: NewChatScreenModel, isListDetailPane: Boolean = false, @@ -328,4 +400,10 @@ class NewChatScreenTest { every { uiState } returns uiStateFlow } } + + private companion object { + private const val SINGLE_PANE = 1 + private const val TWO_PANES = 2 + private const val TABLET_QUALIFIERS = "w1280dp-h800dp" + } } diff --git a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt index b8e7c8ab4..a52705542 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt @@ -68,6 +68,7 @@ internal fun NewChatScreen( effectHandler: NewChatEffectHandler, onNavigateBack: () -> Unit, onNavigateToConversation: NewChatNavigateToConversation, + autoFocusQueryKey: Any = Unit, modifier: Modifier = Modifier, ) { val uiState by screenModel.uiState.collectAsStateWithLifecycle() @@ -96,6 +97,7 @@ internal fun NewChatScreen( NewChatScreenContent( modifier = modifier, + autoFocusQueryKey = autoFocusQueryKey, isCreatingGroup = uiState.isCreatingGroup, isResolvingConversation = uiState.isResolvingConversation, isResolvingConversationIndicatorVisible = uiState @@ -119,6 +121,7 @@ internal fun NewChatScreen( @Composable private fun NewChatScreenContent( modifier: Modifier = Modifier, + autoFocusQueryKey: Any = Unit, isCreatingGroup: Boolean = false, isResolvingConversation: Boolean = false, isResolvingConversationIndicatorVisible: Boolean = false, @@ -151,6 +154,7 @@ private fun NewChatScreenContent( ) { contentPadding -> NewChatRecipientSelectionContent( contentPadding = contentPadding, + autoFocusQueryKey = autoFocusQueryKey, isCreatingGroup = isCreatingGroup, isResolvingConversation = isResolvingConversation, isResolvingConversationIndicatorVisible = isResolvingConversationIndicatorVisible, @@ -205,6 +209,7 @@ private fun NewChatTopAppBar( @Composable private fun NewChatRecipientSelectionContent( contentPadding: PaddingValues, + autoFocusQueryKey: Any, isCreatingGroup: Boolean, isResolvingConversation: Boolean, isResolvingConversationIndicatorVisible: Boolean, @@ -257,6 +262,7 @@ private fun NewChatRecipientSelectionContent( }, ), autoFocusQuery = true, + autoFocusQueryKey = autoFocusQueryKey, onLoadMore = onLoadMore, onPrimaryActionClick = onCreateGroupConfirmed, onQueryChanged = onQueryChanged, diff --git a/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt b/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt index 65e429f0a..35018ba99 100644 --- a/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt +++ b/src/com/android/messaging/ui/conversation/navigation/ConversationNavEntries.kt @@ -10,6 +10,7 @@ import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation3.runtime.EntryProviderScope import androidx.navigation3.runtime.NavKey +import androidx.navigation3.ui.LocalNavAnimatedContentScope import com.android.messaging.R import com.android.messaging.data.conversation.model.ConversationId import com.android.messaging.ui.common.components.consumeOppositePaneInsets @@ -160,6 +161,7 @@ private fun newChatRouteContent(): @Composable (NewChatNavKey) -> Unit { ) navigator.navigateToConversation(conversationId = conversationId) }, + autoFocusQueryKey = LocalNavAnimatedContentScope.current, modifier = Modifier .fillMaxSize() .consumeOppositePaneInsets(), diff --git a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt index ca3396d42..cf6d4d713 100644 --- a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt +++ b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt @@ -25,7 +25,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.graphics.Color @@ -64,6 +63,7 @@ internal fun RecipientSelectionContent( containerColor: Color = MaterialTheme.colorScheme.surfaceContainer, containerShape: Shape = RectangleShape, autoFocusQuery: Boolean = false, + autoFocusQueryKey: Any = Unit, onLoadMore: () -> Unit = {}, onPrimaryActionClick: () -> Unit = {}, onQueryChanged: (String) -> Unit = {}, @@ -81,6 +81,7 @@ internal fun RecipientSelectionContent( ) RecipientSelectionAutoFocusEffect( autoFocusQuery = autoFocusQuery, + autoFocusQueryKey = autoFocusQueryKey, focusRequester = queryFocusRequester, ) @@ -372,13 +373,11 @@ private fun RecipientSelectionArmedContactsArea( @Composable private fun RecipientSelectionAutoFocusEffect( autoFocusQuery: Boolean, + autoFocusQueryKey: Any, focusRequester: FocusRequester, ) { if (autoFocusQuery) { - LaunchedEffect(Unit) { - // Focusing before the first layout crashes Compose's lookahead measure when the - // window turns two-pane and the list-detail scaffold is built around this screen - withFrameNanos {} + LaunchedEffect(autoFocusQueryKey) { focusRequester.requestFocus() } }