From bfa9191102bcaa0d375405349771d5638b535902 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 27 Jan 2025 11:14:43 +0100 Subject: [PATCH 1/4] Media Viewer: show snackbar when reaching end of timeline. --- .../impl/viewer/MediaViewerPresenter.kt | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt index b039576e0a..9ad458efe6 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt @@ -10,12 +10,15 @@ package io.element.android.libraries.mediaviewer.impl.viewer import android.content.ActivityNotFoundException import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject @@ -31,10 +34,15 @@ import io.element.android.libraries.matrix.api.room.powerlevels.canRedactOwn import io.element.android.libraries.matrix.api.timeline.item.event.toEventOrTransactionId import io.element.android.libraries.mediaviewer.api.MediaViewerEntryPoint import io.element.android.libraries.mediaviewer.api.local.LocalMedia +import io.element.android.libraries.mediaviewer.impl.R import io.element.android.libraries.mediaviewer.impl.details.MediaBottomSheetState import io.element.android.libraries.mediaviewer.impl.local.LocalMediaActions import io.element.android.libraries.ui.strings.CommonStrings import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import io.element.android.libraries.androidutils.R as UtilsR @@ -64,6 +72,37 @@ class MediaViewerPresenter @AssistedInject constructor( var currentIndex by remember { mutableIntStateOf(searchIndex(data, inputs.eventId)) } val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() + val isRenderingLoadingBackward by remember { + derivedStateOf { + currentIndex == data.lastIndex && data.lastOrNull() is MediaViewerPageData.Loading + } + } + if (isRenderingLoadingBackward) { + LaunchedEffect(Unit) { + // Observe the loading data vanishing + snapshotFlow { data.lastOrNull() is MediaViewerPageData.Loading } + .distinctUntilChanged() + .filter { !it } + .onEach { showNoMoreItemsSnackbar() } + .launchIn(this) + } + } + val isRenderingLoadingForward by remember { + derivedStateOf { + currentIndex == 0 && data.firstOrNull() is MediaViewerPageData.Loading + } + } + if (isRenderingLoadingForward) { + LaunchedEffect(Unit) { + // Observe the loading data vanishing + snapshotFlow { data.firstOrNull() is MediaViewerPageData.Loading } + .distinctUntilChanged() + .filter { !it } + .onEach { showNoMoreItemsSnackbar() } + .launchIn(this) + } + } + var mediaBottomSheetState by remember { mutableStateOf(MediaBottomSheetState.Hidden) } DisposableEffect(Unit) { @@ -143,6 +182,16 @@ class MediaViewerPresenter @AssistedInject constructor( ) } + private fun showNoMoreItemsSnackbar() { + val messageResId = when (inputs.mode) { + MediaViewerEntryPoint.MediaViewerMode.SingleMedia, + MediaViewerEntryPoint.MediaViewerMode.TimelineImagesAndVideos -> R.string.screen_media_details_no_more_media_to_show + MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios -> R.string.screen_media_details_no_more_files_to_show + } + val message = SnackbarMessage(messageResId) + snackbarDispatcher.post(message) + } + private fun CoroutineScope.downloadMedia( data: MediaViewerPageData.MediaViewerData, ) = launch { From fd38d8ea9acbdc219a0acd4a691225f1b894ffac Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 27 Jan 2025 12:11:01 +0100 Subject: [PATCH 2/4] Media Viewer: Add test on snackbar when reaching end of timeline --- .../impl/viewer/MediaViewerPresenterTest.kt | 202 +++++++++++++++++- 1 file changed, 200 insertions(+), 2 deletions(-) diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt index 47e48ca9e1..b5cd060a50 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt @@ -28,12 +28,14 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.mediaviewer.api.MediaViewerEntryPoint import io.element.android.libraries.mediaviewer.api.anApkMediaInfo +import io.element.android.libraries.mediaviewer.impl.R import io.element.android.libraries.mediaviewer.impl.details.MediaBottomSheetState import io.element.android.libraries.mediaviewer.impl.gallery.FakeMediaGalleryDataSource import io.element.android.libraries.mediaviewer.impl.gallery.GroupedMediaItems import io.element.android.libraries.mediaviewer.impl.gallery.MediaGalleryDataSource import io.element.android.libraries.mediaviewer.impl.gallery.MediaGalleryMode import io.element.android.libraries.mediaviewer.impl.gallery.ui.aMediaItemImage +import io.element.android.libraries.mediaviewer.impl.gallery.ui.aMediaItemLoadingIndicator import io.element.android.libraries.mediaviewer.test.FakeLocalMediaActions import io.element.android.libraries.mediaviewer.test.FakeLocalMediaFactory import io.element.android.services.toolbox.test.systemclock.FakeSystemClock @@ -62,6 +64,16 @@ class MediaViewerPresenterTest { private val localMediaFactory = FakeLocalMediaFactory(mockMediaUri) private val aUrl = "aUrl" + private val anImage = aMediaItemImage( + mediaSourceUrl = aUrl, + ) + private val aBackwardLoadingIndicator = aMediaItemLoadingIndicator( + direction = Timeline.PaginationDirection.BACKWARDS + ) + private val aForwardLoadingIndicator = aMediaItemLoadingIndicator( + direction = Timeline.PaginationDirection.FORWARDS + ) + @Test fun `present - initial state null Event`() = runTest { val presenter = createMediaViewerPresenter( @@ -504,6 +516,187 @@ class MediaViewerPresenterTest { } } + @Test + fun `present - snackbar displayed when there is no more items forward images and videos`() { + `present - snackbar displayed when there is no more items forward`( + mode = MediaViewerEntryPoint.MediaViewerMode.TimelineImagesAndVideos, + expectedSnackbarResId = R.string.screen_media_details_no_more_media_to_show, + ) + } + + @Test + fun `present - snackbar displayed when there is no more items forward files and audio`() { + `present - snackbar displayed when there is no more items forward`( + mode = MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios, + expectedSnackbarResId = R.string.screen_media_details_no_more_files_to_show, + ) + } + + private fun `present - snackbar displayed when there is no more items forward`( + mode: MediaViewerEntryPoint.MediaViewerMode, + expectedSnackbarResId: Int, + ) = runTest { + val mediaGalleryDataSource = FakeMediaGalleryDataSource( + startLambda = { }, + ) + val presenter = createMediaViewerPresenter( + mode = mode, + mediaGalleryDataSource = mediaGalleryDataSource, + ) + presenter.test { + awaitFirstItem() + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + if (mode == MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(), + fileItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + ) + } else { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(), + ) + } + ) + ) + val updatedState = awaitItem() + // User navigate to the first item (forward loading indicator) + updatedState.eventSink( + MediaViewerEvents.OnNavigateTo(0) + ) + // data source claims that there is no more items to load forward + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + if (mode == MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(), + fileItems = persistentListOf(anImage, aBackwardLoadingIndicator), + ) + } else { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(), + ) + } + ) + ) + skipItems(1) + val stateWithSnackbar = awaitItem() + assertThat(stateWithSnackbar.snackbarMessage!!.messageResId).isEqualTo(expectedSnackbarResId) + } + } + + @Test + fun `present - snackbar displayed when there is no more items backward images and videos`() { + `present - snackbar displayed when there is no more items backward`( + mode = MediaViewerEntryPoint.MediaViewerMode.TimelineImagesAndVideos, + expectedSnackbarResId = R.string.screen_media_details_no_more_media_to_show, + ) + } + + @Test + fun `present - snackbar displayed when there is no more items backward files and audio`() { + `present - snackbar displayed when there is no more items backward`( + mode = MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios, + expectedSnackbarResId = R.string.screen_media_details_no_more_files_to_show, + ) + } + + private fun `present - snackbar displayed when there is no more items backward`( + mode: MediaViewerEntryPoint.MediaViewerMode, + expectedSnackbarResId: Int, + ) = runTest { + val mediaGalleryDataSource = FakeMediaGalleryDataSource( + startLambda = { }, + ) + val presenter = createMediaViewerPresenter( + mode = mode, + mediaGalleryDataSource = mediaGalleryDataSource, + ) + presenter.test { + awaitFirstItem() + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + if (mode == MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(), + fileItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + ) + } else { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(), + ) + } + ) + ) + val updatedState = awaitItem() + // User navigate to the last item (backward loading indicator) + updatedState.eventSink( + MediaViewerEvents.OnNavigateTo(2) + ) + skipItems(1) + // data source claims that there is no more items to load backward + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + if (mode == MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(), + fileItems = persistentListOf(aForwardLoadingIndicator, anImage), + ) + } else { + GroupedMediaItems( + imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage), + fileItems = persistentListOf(), + ) + } + ) + ) + skipItems(1) + val stateWithSnackbar = awaitItem() + assertThat(stateWithSnackbar.snackbarMessage!!.messageResId).isEqualTo(expectedSnackbarResId) + } + } + + @Test + fun `present - no snackbar displayed when there is no more items but not displaying a loading item`() = runTest { + val mediaGalleryDataSource = FakeMediaGalleryDataSource( + startLambda = { }, + ) + val presenter = createMediaViewerPresenter( + mediaGalleryDataSource = mediaGalleryDataSource, + ) + presenter.test { + awaitFirstItem() + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + GroupedMediaItems( + imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(), + ) + ) + ) + val updatedState = awaitItem() + // User navigate to the media + updatedState.eventSink( + MediaViewerEvents.OnNavigateTo(1) + ) + skipItems(1) + // data source claims that there is no more items to load at all + mediaGalleryDataSource.emitGroupedMediaItems( + AsyncData.Success( + GroupedMediaItems( + imageAndVideoItems = persistentListOf(anImage), + fileItems = persistentListOf(), + ) + ) + ) + val finalState = awaitItem() + assertThat(finalState.snackbarMessage).isNull() + } + } + @Test fun `present - load more`() = runTest { val loadMoreLambda = lambdaRecorder { } @@ -565,6 +758,7 @@ class MediaViewerPresenterTest { private fun TestScope.createMediaViewerPresenter( eventId: EventId? = null, + mode: MediaViewerEntryPoint.MediaViewerMode = MediaViewerEntryPoint.MediaViewerMode.SingleMedia, matrixMediaLoader: FakeMatrixMediaLoader = FakeMatrixMediaLoader(), localMediaActions: FakeLocalMediaActions = FakeLocalMediaActions(), mediaGalleryDataSource: MediaGalleryDataSource = FakeMediaGalleryDataSource( @@ -578,7 +772,7 @@ class MediaViewerPresenterTest { ): MediaViewerPresenter { return MediaViewerPresenter( inputs = MediaViewerEntryPoint.Params( - mode = MediaViewerEntryPoint.MediaViewerMode.SingleMedia, + mode = mode, eventId = eventId, mediaInfo = TESTED_MEDIA_INFO, mediaSource = aMediaSource(), @@ -587,7 +781,11 @@ class MediaViewerPresenterTest { ), navigator = mediaViewerNavigator, dataSource = MediaViewerDataSource( - galleryMode = MediaGalleryMode.Images, + galleryMode = when (mode) { + MediaViewerEntryPoint.MediaViewerMode.SingleMedia -> MediaGalleryMode.Images + MediaViewerEntryPoint.MediaViewerMode.TimelineImagesAndVideos -> MediaGalleryMode.Images + MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios -> MediaGalleryMode.Files + }, dispatcher = testCoroutineDispatchers().computation, galleryDataSource = mediaGalleryDataSource, mediaLoader = matrixMediaLoader, From 5eda0b28b47b9e95f34dcf621ffb111ba7e5abc0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 27 Jan 2025 14:29:31 +0100 Subject: [PATCH 3/4] Suppress large test class. --- .../mediaviewer/impl/viewer/MediaViewerPresenterTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt index b5cd060a50..8a2d536a78 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt +++ b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt @@ -56,6 +56,7 @@ private val TESTED_MEDIA_INFO = anApkMediaInfo( senderId = A_USER_ID, ) +@Suppress("LargeClass") class MediaViewerPresenterTest { @get:Rule val warmUpRule = WarmUpRule() From 792c350a1bf9cf46bb054f6e7b9417d2e11681c8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 27 Jan 2025 15:07:17 +0100 Subject: [PATCH 4/4] Extract snackbar displayer to its own methods. --- .../impl/viewer/MediaViewerPresenter.kt | 89 +++++++++++-------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt index 9ad458efe6..b926291461 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt @@ -10,7 +10,9 @@ package io.element.android.libraries.mediaviewer.impl.viewer import android.content.ActivityNotFoundException import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.IntState import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.State import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -38,6 +40,7 @@ import io.element.android.libraries.mediaviewer.impl.R import io.element.android.libraries.mediaviewer.impl.details.MediaBottomSheetState import io.element.android.libraries.mediaviewer.impl.local.LocalMediaActions import io.element.android.libraries.ui.strings.CommonStrings +import kotlinx.collections.immutable.PersistentList import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter @@ -68,40 +71,12 @@ class MediaViewerPresenter @AssistedInject constructor( @Composable override fun present(): MediaViewerState { val coroutineScope = rememberCoroutineScope() - val data by dataSource.collectAsState() - var currentIndex by remember { mutableIntStateOf(searchIndex(data, inputs.eventId)) } + val data = dataSource.collectAsState() + val currentIndex = remember { mutableIntStateOf(searchIndex(data.value, inputs.eventId)) } val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() - val isRenderingLoadingBackward by remember { - derivedStateOf { - currentIndex == data.lastIndex && data.lastOrNull() is MediaViewerPageData.Loading - } - } - if (isRenderingLoadingBackward) { - LaunchedEffect(Unit) { - // Observe the loading data vanishing - snapshotFlow { data.lastOrNull() is MediaViewerPageData.Loading } - .distinctUntilChanged() - .filter { !it } - .onEach { showNoMoreItemsSnackbar() } - .launchIn(this) - } - } - val isRenderingLoadingForward by remember { - derivedStateOf { - currentIndex == 0 && data.firstOrNull() is MediaViewerPageData.Loading - } - } - if (isRenderingLoadingForward) { - LaunchedEffect(Unit) { - // Observe the loading data vanishing - snapshotFlow { data.firstOrNull() is MediaViewerPageData.Loading } - .distinctUntilChanged() - .filter { !it } - .onEach { showNoMoreItemsSnackbar() } - .launchIn(this) - } - } + NoMoreItemsBackwardSnackBarDisplayer(currentIndex, data) + NoMoreItemsForwardSnackBarDisplayer(currentIndex, data) var mediaBottomSheetState by remember { mutableStateOf(MediaBottomSheetState.Hidden) } @@ -164,7 +139,7 @@ class MediaViewerPresenter @AssistedInject constructor( mediaBottomSheetState = MediaBottomSheetState.Hidden } is MediaViewerEvents.OnNavigateTo -> { - currentIndex = event.index + currentIndex.intValue = event.index } is MediaViewerEvents.LoadMore -> coroutineScope.launch { dataSource.loadMore(event.direction) @@ -173,8 +148,8 @@ class MediaViewerPresenter @AssistedInject constructor( } return MediaViewerState( - listData = data, - currentIndex = currentIndex, + listData = data.value, + currentIndex = currentIndex.intValue, snackbarMessage = snackbarMessage, canShowInfo = inputs.canShowInfo, mediaBottomSheetState = mediaBottomSheetState, @@ -182,6 +157,50 @@ class MediaViewerPresenter @AssistedInject constructor( ) } + @Composable + private fun NoMoreItemsBackwardSnackBarDisplayer( + currentIndex: IntState, + data: State>, + ) { + val isRenderingLoadingBackward by remember { + derivedStateOf { + currentIndex.intValue == data.value.lastIndex && data.value.lastOrNull() is MediaViewerPageData.Loading + } + } + if (isRenderingLoadingBackward) { + LaunchedEffect(Unit) { + // Observe the loading data vanishing + snapshotFlow { data.value.lastOrNull() is MediaViewerPageData.Loading } + .distinctUntilChanged() + .filter { !it } + .onEach { showNoMoreItemsSnackbar() } + .launchIn(this) + } + } + } + + @Composable + private fun NoMoreItemsForwardSnackBarDisplayer( + currentIndex: IntState, + data: State>, + ) { + val isRenderingLoadingForward by remember { + derivedStateOf { + currentIndex.intValue == 0 && data.value.firstOrNull() is MediaViewerPageData.Loading + } + } + if (isRenderingLoadingForward) { + LaunchedEffect(Unit) { + // Observe the loading data vanishing + snapshotFlow { data.value.firstOrNull() is MediaViewerPageData.Loading } + .distinctUntilChanged() + .filter { !it } + .onEach { showNoMoreItemsSnackbar() } + .launchIn(this) + } + } + } + private fun showNoMoreItemsSnackbar() { val messageResId = when (inputs.mode) { MediaViewerEntryPoint.MediaViewerMode.SingleMedia,