From 6a4fed2baf4c072e6156901db287e1db593cb1b5 Mon Sep 17 00:00:00 2001 From: bxdxnn <267911624+bxdxnn@users.noreply.github.com> Date: Fri, 17 Apr 2026 12:30:21 +0300 Subject: [PATCH] Natural media viewer swiping order (#6431) --- .../impl/viewer/MediaViewerDataSource.kt | 23 ++++++++-- .../impl/viewer/MediaViewerPresenter.kt | 20 ++++++--- .../impl/viewer/MediaViewerPresenterTest.kt | 43 ++++++++++--------- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt index 928e5d9ca8..a9fb5d645c 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt +++ b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt @@ -120,11 +120,25 @@ class MediaViewerDataSource( */ private fun buildMediaViewerPageList(groupedItems: List) = buildList { // Filter out DateSeparator items, we do not need them for the media viewer - val groupedItemsNoDateSeparator = groupedItems.filterNot { it is MediaItem.DateSeparator } - pagerKeysHandler.accept(groupedItemsNoDateSeparator) - groupedItemsNoDateSeparator.forEach { mediaItem -> + val itemsNoDateSeparator = groupedItems.filterNot { it is MediaItem.DateSeparator } + // Separate loading indicators and media events + val loadingIndicators = itemsNoDateSeparator.filterIsInstance() + val mediaEvents = itemsNoDateSeparator.filterIsInstance() + // Determine backward and forward loading indicators + val backwardLoading = loadingIndicators.find { it.direction == Timeline.PaginationDirection.BACKWARDS } + val forwardLoading = loadingIndicators.find { it.direction == Timeline.PaginationDirection.FORWARDS } + // Build ordered list: backward loading, media events (oldest first), forward loading + // Media events are currently newest first, reverse to get oldest first + val orderedEvents = mediaEvents.reversed() + // Create new list of MediaItem in order: backwardLoading, orderedEvents, forwardLoading + val orderedItems = buildList { + backwardLoading?.let { add(it) } + addAll(orderedEvents) + forwardLoading?.let { add(it) } + } + pagerKeysHandler.accept(orderedItems) + orderedItems.forEach { mediaItem -> when (mediaItem) { - is MediaItem.DateSeparator -> Unit is MediaItem.Event -> { val sourceUrl = mediaItem.mediaSource().safeUrl val localMedia = localMediaStates.getOrPut(sourceUrl) { @@ -148,6 +162,7 @@ class MediaViewerDataSource( pagerKey = pagerKeysHandler.getKey(mediaItem), ) ) + is MediaItem.DateSeparator -> Unit // already filtered out } } }.toImmutableList() 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 dc0feb70cf..ae581fa8d4 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 @@ -179,15 +179,19 @@ class MediaViewerPresenter( ) { val isRenderingLoadingBackward by remember { derivedStateOf { - currentIndex.intValue == data.value.lastIndex && + currentIndex.intValue == 0 && data.value.size > 1 && - data.value.lastOrNull() is MediaViewerPageData.Loading + data.value.firstOrNull() is MediaViewerPageData.Loading && + (data.value.firstOrNull() as? MediaViewerPageData.Loading)?.direction == Timeline.PaginationDirection.BACKWARDS } } if (isRenderingLoadingBackward) { LaunchedEffect(Unit) { // Observe the loading data vanishing - snapshotFlow { data.value.lastOrNull() is MediaViewerPageData.Loading } + snapshotFlow { + val first = data.value.firstOrNull() + first is MediaViewerPageData.Loading && first.direction == Timeline.PaginationDirection.BACKWARDS + } .distinctUntilChanged() .filter { !it } .onEach { showNoMoreItemsSnackbar() } @@ -203,15 +207,19 @@ class MediaViewerPresenter( ) { val isRenderingLoadingForward by remember { derivedStateOf { - currentIndex.intValue == 0 && + currentIndex.intValue == data.value.lastIndex && data.value.size > 1 && - data.value.firstOrNull() is MediaViewerPageData.Loading + data.value.lastOrNull() is MediaViewerPageData.Loading && + (data.value.lastOrNull() as? MediaViewerPageData.Loading)?.direction == Timeline.PaginationDirection.FORWARDS } } if (isRenderingLoadingForward) { LaunchedEffect(Unit) { // Observe the loading data vanishing - snapshotFlow { data.value.firstOrNull() is MediaViewerPageData.Loading } + snapshotFlow { + val last = data.value.lastOrNull() + last is MediaViewerPageData.Loading && last.direction == Timeline.PaginationDirection.FORWARDS + } .distinctUntilChanged() .filter { !it } .onEach { showNoMoreItemsSnackbar() } 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 a9d1704bdc..c217ea3306 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 @@ -593,20 +593,20 @@ class MediaViewerPresenterTest { if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { GroupedMediaItems( imageAndVideoItems = persistentListOf(), - fileItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(aBackwardLoadingIndicator, anImage, aForwardLoadingIndicator), ) } else { GroupedMediaItems( - imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + imageAndVideoItems = persistentListOf(aBackwardLoadingIndicator, anImage, aForwardLoadingIndicator), fileItems = persistentListOf(), ) } ) ) val updatedState = awaitItem() - // User navigate to the first item (forward loading indicator) + // User navigate to the last item (forward loading indicator) updatedState.eventSink( - MediaViewerEvents.OnNavigateTo(0) + MediaViewerEvents.OnNavigateTo(2) ) // data source claims that there is no more items to load forward mediaGalleryDataSource.emitGroupedMediaItems( @@ -614,19 +614,21 @@ class MediaViewerPresenterTest { if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { GroupedMediaItems( imageAndVideoItems = persistentListOf(), - fileItems = persistentListOf(anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(aBackwardLoadingIndicator, anImage), ) } else { GroupedMediaItems( - imageAndVideoItems = persistentListOf(anImage, aBackwardLoadingIndicator), + imageAndVideoItems = persistentListOf(aBackwardLoadingIndicator, anImage), fileItems = persistentListOf(), ) } ) ) - skipItems(1) - val stateWithSnackbar = awaitItem() - assertThat(stateWithSnackbar.snackbarMessage!!.messageResId).isEqualTo(expectedSnackbarResId) + var stateWithSnackbar = awaitItem() + while (stateWithSnackbar.snackbarMessage == null) { + stateWithSnackbar = awaitItem() + } + assertThat(stateWithSnackbar.snackbarMessage.messageResId).isEqualTo(expectedSnackbarResId) } } @@ -665,41 +667,42 @@ class MediaViewerPresenterTest { if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { GroupedMediaItems( imageAndVideoItems = persistentListOf(), - fileItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + fileItems = persistentListOf(aBackwardLoadingIndicator, anImage, aForwardLoadingIndicator), ) } else { GroupedMediaItems( - imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + imageAndVideoItems = persistentListOf(aBackwardLoadingIndicator, anImage, aForwardLoadingIndicator), fileItems = persistentListOf(), ) } ) ) val updatedState = awaitItem() - // User navigate to the last item (backward loading indicator) + // User navigate to the first item (backward loading indicator) updatedState.eventSink( - MediaViewerEvents.OnNavigateTo(2) + MediaViewerEvents.OnNavigateTo(0) ) - skipItems(1) // data source claims that there is no more items to load backward mediaGalleryDataSource.emitGroupedMediaItems( AsyncData.Success( if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) { GroupedMediaItems( imageAndVideoItems = persistentListOf(), - fileItems = persistentListOf(aForwardLoadingIndicator, anImage), + fileItems = persistentListOf(anImage, aForwardLoadingIndicator), ) } else { GroupedMediaItems( - imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage), + imageAndVideoItems = persistentListOf(anImage, aForwardLoadingIndicator), fileItems = persistentListOf(), ) } ) ) - skipItems(1) - val stateWithSnackbar = awaitItem() - assertThat(stateWithSnackbar.snackbarMessage!!.messageResId).isEqualTo(expectedSnackbarResId) + var stateWithSnackbar = awaitItem() + while (stateWithSnackbar.snackbarMessage == null) { + stateWithSnackbar = awaitItem() + } + assertThat(stateWithSnackbar.snackbarMessage.messageResId).isEqualTo(expectedSnackbarResId) } } @@ -717,7 +720,7 @@ class MediaViewerPresenterTest { mediaGalleryDataSource.emitGroupedMediaItems( AsyncData.Success( GroupedMediaItems( - imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator), + imageAndVideoItems = persistentListOf(aBackwardLoadingIndicator, anImage, aForwardLoadingIndicator), fileItems = persistentListOf(), ) )