From d4dffabc415cfdd4120c4e7478cf11e9b165ddb1 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 5 Aug 2026 15:07:54 +0000 Subject: [PATCH] Scroll the media-send selected-media rail to the newest selection. --- .../mediasend/MediaSendFlowViewModel.kt | 10 ++ .../signal/mediasend/MediaSendNavigation.kt | 12 +- .../screens/select/MediaSelectScreen.kt | 75 +++++++-- .../screens/select/MediaSelectViewModel.kt | 8 +- .../org/signal/mediasend/test/TestTags.kt | 3 + .../select/MediaSelectScreenRailTest.kt | 158 ++++++++++++++++++ 6 files changed, 242 insertions(+), 24 deletions(-) create mode 100644 feature/media-send/src/test/java/org/signal/mediasend/screens/select/MediaSelectScreenRailTest.kt diff --git a/feature/media-send/src/main/java/org/signal/mediasend/MediaSendFlowViewModel.kt b/feature/media-send/src/main/java/org/signal/mediasend/MediaSendFlowViewModel.kt index 1e908cd356..794cc2e948 100644 --- a/feature/media-send/src/main/java/org/signal/mediasend/MediaSendFlowViewModel.kt +++ b/feature/media-send/src/main/java/org/signal/mediasend/MediaSendFlowViewModel.kt @@ -24,6 +24,7 @@ import com.bumptech.glide.load.engine.GlideException import com.bumptech.glide.request.RequestListener import com.bumptech.glide.request.target.Target import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -125,6 +126,13 @@ class MediaSendFlowViewModel( private val internalToastEvents: Channel = Channel(Channel.BUFFERED) internal val toastEvents: Flow = internalToastEvents.receiveAsFlow() + /** + * The media that has most recently landed in the selection, for the screens that follow the selection as it grows. + * Only this knows which that is: what the user picked is not necessarily what survived validation. + */ + private val internalSelectionAdditions: Channel = Channel(capacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) + internal val selectionAdditions: Flow = internalSelectionAdditions.receiveAsFlow() + internal val usernameScannedDialog = DialogController() internal val linkedDeviceScannedDialog = DialogController() internal val saveToStorageDialog = DialogController() @@ -587,6 +595,8 @@ class MediaSendFlowViewModel( ) } + updatedMedia.lastOrNull { item -> media.any { it.uri == item.uri } }?.let { internalSelectionAdditions.trySend(it) } + if (initializedEditorStates.values.any { it is EditorState.VideoTrim && it.videoTrimData.isDurationEdited }) { internalSnackbarEvents.trySend(SnackbarEvent(message = R.string.MediaSendViewModel__video_trimmed_to_fit)) } diff --git a/feature/media-send/src/main/java/org/signal/mediasend/MediaSendNavigation.kt b/feature/media-send/src/main/java/org/signal/mediasend/MediaSendNavigation.kt index e976f2dcc7..d8a106b07d 100644 --- a/feature/media-send/src/main/java/org/signal/mediasend/MediaSendNavigation.kt +++ b/feature/media-send/src/main/java/org/signal/mediasend/MediaSendNavigation.kt @@ -82,7 +82,8 @@ internal fun MediaSendNavigation( factory = MediaSelectViewModel.Factory( parentState = viewModel.state, parentEventEmitter = viewModel::onEvent, - mediaFolder = null + mediaFolder = null, + selectionAdditions = viewModel.selectionAdditions ) ) val state by selectViewModel.state.collectAsStateWithLifecycle() @@ -91,7 +92,8 @@ internal fun MediaSendNavigation( MediaSelectScreen( state = state, - onEvent = selectViewModel::onEvent + onEvent = selectViewModel::onEvent, + selectionAdditions = selectViewModel.selectionAdditions ) } @@ -100,7 +102,8 @@ internal fun MediaSendNavigation( factory = MediaSelectViewModel.Factory( parentState = viewModel.state, parentEventEmitter = viewModel::onEvent, - mediaFolder = key.folder + mediaFolder = key.folder, + selectionAdditions = viewModel.selectionAdditions ) ) val state by selectViewModel.state.collectAsStateWithLifecycle() @@ -109,7 +112,8 @@ internal fun MediaSendNavigation( MediaSelectScreen( state = state, - onEvent = selectViewModel::onEvent + onEvent = selectViewModel::onEvent, + selectionAdditions = selectViewModel.selectionAdditions ) } diff --git a/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectScreen.kt b/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectScreen.kt index f1d6a42d4f..4548392528 100644 --- a/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectScreen.kt +++ b/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectScreen.kt @@ -55,6 +55,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -76,6 +77,12 @@ import androidx.compose.ui.unit.dp import androidx.core.net.toUri import androidx.lifecycle.compose.LifecycleResumeEffect import androidx.window.core.layout.WindowSizeClass +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.first import org.signal.core.models.media.Media import org.signal.core.models.media.MediaFolder import org.signal.core.ui.compose.AllDevicePreviews @@ -116,7 +123,8 @@ private const val PLACEHOLDER_COUNT = 100 @Composable internal fun MediaSelectScreen( state: MediaSelectState, - onEvent: (MediaSelectScreenEvents) -> Unit + onEvent: (MediaSelectScreenEvents) -> Unit, + selectionAdditions: Flow = emptyFlow() ) { // Without read access there is nothing to browse, and with selected-photos access and nothing selected there is // nothing yet. Both show the placeholder grid behind a call to action, so both use the denser file grid. @@ -251,6 +259,7 @@ internal fun MediaSelectScreen( ) { SelectedMediaRow( selectedMedia = state.selectedMedia, + selectionAdditions = selectionAdditions, alignment = gridConfiguration.bottomBarAlignment, onEvent = onEvent, modifier = Modifier @@ -706,6 +715,7 @@ private fun NextButton(mediaSelectionCount: Int, recipientChatColor: Color? = nu @Composable private fun SelectedMediaRow( selectedMedia: List, + selectionAdditions: Flow, alignment: Alignment.Horizontal, onEvent: (MediaSelectScreenEvents) -> Unit, modifier: Modifier = Modifier @@ -722,6 +732,26 @@ private fun SelectedMediaRow( onEvent = reorderBuffer::onReorderListEvent ) + LaunchedEffect(listState, selectionAdditions) { + selectionAdditions.collect { addition -> + // Located in the order the rail is rendering rather than in the selection, which a drag in progress is holding + // back. The addition is also announced with the state that carries it, ahead of the layout that measures it, and a + // list asked for an item it has not measured yet believes it is already at its end and gives up without moving. + val index = snapshotFlow { + val index = reorderBuffer.items.indexOfFirst { it.uri == addition.uri } + if (index < listState.layoutInfo.totalItemsCount) index else -1 + }.first { it >= 0 } + + try { + listState.animateScrollToItem(index) + } catch (e: CancellationException) { + // A touch on the rail preempts the scroll and cancels it. That is the user taking the rail over, and it ends + // this scroll rather than our interest in the next addition. + currentCoroutineContext().ensureActive() + } + } + } + LazyRow( state = listState, modifier = modifier.reorderableList(reorderableListState), @@ -729,7 +759,10 @@ private fun SelectedMediaRow( ) { itemsIndexed(reorderBuffer.items, key = { _, media -> media.uri }) { index, media -> ReorderableItem(reorderableListState, index) { - MediaThumbnail(media) { + MediaThumbnail( + media = media, + modifier = Modifier.testTag(TestTags.selectedMediaThumbnail(media.uri.toString())) + ) { onEvent(MediaSelectScreenEvents.SetFocusedMedia(media)) onEvent(MediaSelectScreenEvents.NavigateToEdit) } @@ -744,22 +777,28 @@ private fun MediaThumbnail( modifier: Modifier = Modifier, onClick: () -> Unit ) { - if (LocalInspectionMode.current) { - Box( - modifier = modifier - .size(MediaSendMetrics.SelectedMediaPreviewSize) - .background(color = Previews.rememberRandomColor(), shape = RoundedCornerShape(8.dp)) - .clickable(onClick = onClick, onClickLabel = media.fileName, role = Role.Button) - ) - } else { - GlideImage( - model = media.uri, - imageSize = MediaSendMetrics.SelectedMediaPreviewSize, - modifier = modifier - .size(MediaSendMetrics.SelectedMediaPreviewSize) - .clip(RoundedCornerShape(8.dp)) - .clickable(onClick = onClick, onClickLabel = media.fileName, role = Role.Button) - ) + // Sized by the box rather than by what ends up in it. The thumbnail emits nothing at all until it has loaded, and an + // item with no width until then is one the rail cannot lay out, scroll to, or show. + Box( + modifier = modifier + .size(MediaSendMetrics.SelectedMediaPreviewSize) + .clip(RoundedCornerShape(8.dp)) + .background(color = MaterialTheme.colorScheme.surfaceVariant) + .clickable(onClick = onClick, onClickLabel = media.fileName, role = Role.Button) + ) { + if (LocalInspectionMode.current) { + Box( + modifier = Modifier + .fillMaxSize() + .background(color = Previews.rememberRandomColor()) + ) + } else { + GlideImage( + model = media.uri, + imageSize = MediaSendMetrics.SelectedMediaPreviewSize, + modifier = Modifier.fillMaxSize() + ) + } } } diff --git a/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectViewModel.kt b/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectViewModel.kt index 0a10db103a..4ab5d004a4 100644 --- a/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectViewModel.kt +++ b/feature/media-send/src/main/java/org/signal/mediasend/screens/select/MediaSelectViewModel.kt @@ -8,6 +8,7 @@ package org.signal.mediasend.screens.select import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ -42,6 +43,8 @@ internal class MediaSelectViewModel( private val parentState: StateFlow, private val parentEventEmitter: (MediaSendFlowEvent) -> Unit, mediaFolder: MediaFolder?, + /** Passed straight through: the rail follows it, and nothing about it is this screen's to decide. */ + val selectionAdditions: Flow, private val repository: MediaSendRepository = MediaSendDependencies.mediaSendRepository ) : EventDrivenViewModel(TAG) { @@ -156,11 +159,12 @@ internal class MediaSelectViewModel( class Factory( private val parentState: StateFlow, private val parentEventEmitter: (MediaSendFlowEvent) -> Unit, - private val mediaFolder: MediaFolder? + private val mediaFolder: MediaFolder?, + private val selectionAdditions: Flow ) : ViewModelProvider.Factory { @Suppress("UNCHECKED_CAST") override fun create(modelClass: Class): T { - return MediaSelectViewModel(parentState, parentEventEmitter, mediaFolder) as T + return MediaSelectViewModel(parentState, parentEventEmitter, mediaFolder, selectionAdditions) as T } } } diff --git a/feature/media-send/src/main/java/org/signal/mediasend/test/TestTags.kt b/feature/media-send/src/main/java/org/signal/mediasend/test/TestTags.kt index 67b643d8af..042667a3ef 100644 --- a/feature/media-send/src/main/java/org/signal/mediasend/test/TestTags.kt +++ b/feature/media-send/src/main/java/org/signal/mediasend/test/TestTags.kt @@ -22,6 +22,9 @@ object TestTags { // Media Select Screen const val MEDIA_SELECT_GRID = "media_select_grid" + /** Tag for the selected media rail's thumbnail of the media at [uri]. */ + fun selectedMediaThumbnail(uri: String): String = "selected_media_thumbnail_$uri" + // Schedule Send Menu const val SCHEDULE_SEND_PICK_TIME_OPTION = "schedule_send_pick_time_option" diff --git a/feature/media-send/src/test/java/org/signal/mediasend/screens/select/MediaSelectScreenRailTest.kt b/feature/media-send/src/test/java/org/signal/mediasend/screens/select/MediaSelectScreenRailTest.kt new file mode 100644 index 0000000000..b50de716b2 --- /dev/null +++ b/feature/media-send/src/test/java/org/signal/mediasend/screens/select/MediaSelectScreenRailTest.kt @@ -0,0 +1,158 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.signal.mediasend.screens.select + +import android.app.Application +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.runtime.toMutableStateList +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.unit.dp +import androidx.core.net.toUri +import androidx.test.core.app.ApplicationProvider +import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.receiveAsFlow +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.signal.core.models.media.Media +import org.signal.core.models.media.MediaFolder +import org.signal.core.ui.CoreUiDependenciesRule +import org.signal.core.ui.compose.theme.SignalTheme +import org.signal.mediasend.MediaSendDependenciesRule +import org.signal.mediasend.test.TestTags + +/** + * Covers the selected media rail following the newest selection. The rail is narrower than the selection it holds well + * before the selection is large, so whether the newest thumbnail is on screen is not something the state can be asked. + */ +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class, qualifiers = "w400dp-h800dp") +class MediaSelectScreenRailTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @get:Rule + val coreUiDependenciesRule = CoreUiDependenciesRule(ApplicationProvider.getApplicationContext()) + + @get:Rule + val mediaSendDependenciesRule = MediaSendDependenciesRule(ApplicationProvider.getApplicationContext()) + + private val selectionAdditions = Channel(capacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) + private val selectedMedia = MEDIA.take(OVERFLOWING_SELECTION).toMutableStateList() + + @Test + fun `Given a rail that overflows, when an addition is announced, then the newest thumbnail is on screen`() { + setContent() + assertNotShown(MEDIA.last()) + + select(MEDIA.last()) + + composeTestRule.onNodeWithTag(tagFor(MEDIA.last())).assertIsDisplayed() + } + + @Test + fun `Given a rail that overflows, when the selection grows unannounced, then the rail stays where it was`() { + setContent() + + selectedMedia += MEDIA.last() + composeTestRule.waitForIdle() + + assertNotShown(MEDIA.last()) + composeTestRule.onNodeWithTag(tagFor(MEDIA.first())).assertIsDisplayed() + } + + @Test + fun `Given a rail scrolled to the newest, when another addition is announced, then it follows on`() { + setContent() + select(MEDIA[OVERFLOWING_SELECTION]) + composeTestRule.onNodeWithTag(tagFor(MEDIA[OVERFLOWING_SELECTION])).assertIsDisplayed() + + select(MEDIA.last()) + + composeTestRule.onNodeWithTag(tagFor(MEDIA.last())).assertIsDisplayed() + } + + /** Adds [media] to the selection the way the flow does: the state grows, and the addition is announced. */ + private fun select(media: Media) { + selectedMedia += media + selectionAdditions.trySend(media) + composeTestRule.waitForIdle() + } + + /** Off-viewport rail items are never composed, so being absent and being off screen both count as not shown. */ + private fun assertNotShown(media: Media) { + val tag = tagFor(media) + if (composeTestRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()) { + composeTestRule.onNodeWithTag(tag).assertIsNotDisplayed() + } + } + + private fun tagFor(media: Media): String = TestTags.selectedMediaThumbnail(media.uri.toString()) + + private fun setContent() { + composeTestRule.setContent { + SignalTheme { + Box(modifier = Modifier.size(RAIL_WIDTH.dp, RAIL_HEIGHT.dp)) { + MediaSelectScreen( + state = MediaSelectState.Files( + selectedMediaFolder = FOLDER, + selectedMediaFolderItems = MEDIA, + selectedMedia = selectedMedia + ), + onEvent = {}, + selectionAdditions = selectionAdditions.receiveAsFlow() + ) + } + } + } + + composeTestRule.waitForIdle() + } + + private companion object { + private const val RAIL_WIDTH = 400f + private const val RAIL_HEIGHT = 800f + + /** Enough thumbnails at 44dp plus 12dp of spacing to run past the end of a rail this wide several times over. */ + private const val OVERFLOWING_SELECTION = 12 + + private val FOLDER = MediaFolder( + thumbnailUri = "content://folder".toUri(), + title = "Camera", + itemCount = 16, + bucketId = "bucket", + folderType = MediaFolder.FolderType.CAMERA + ) + + private val MEDIA: List = (0 until 16).map { index -> + Media( + uri = "content://media/$index".toUri(), + contentType = "image/jpeg", + date = index.toLong(), + width = 100, + height = 100, + size = 1024, + duration = 0, + isBorderless = false, + isVideoGif = false, + bucketId = "bucket", + caption = null, + transformProperties = null, + fileName = "media_$index.jpg" + ) + } + } +}