Introduce GroupedMediaItems for code clarity

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt

@@ -77,23 +77,13 @@ class MediaGalleryPresenter @AssistedInject constructor(
         var mediaItems by remember {
             mutableStateOf<AsyncData<ImmutableList<MediaItem>>>(AsyncData.Uninitialized)
         }
-        val imageAndVideoItems by remember {
+        val groupedMediaItems by remember {
             derivedStateOf {
                 mediaItemsPostProcessor.process(
                     mediaItems = mediaItems,
-                    predicate = { it is MediaItem.Image || it is MediaItem.Video },
                 )
             }
         }
-        val fileItems by remember {
-            derivedStateOf {
-                mediaItemsPostProcessor.process(
-                    mediaItems = mediaItems,
-                    predicate = { it is MediaItem.File },
-                )
-            }
-        }
-
         val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState()
         localMediaActions.Configure()
 
@@ -165,8 +155,7 @@ class MediaGalleryPresenter @AssistedInject constructor(
         return MediaGalleryState(
             roomName = roomInfo?.name ?: room.displayName,
             mode = mode,
-            imageAndVideoItems = imageAndVideoItems,
+            groupedMediaItems = groupedMediaItems,
-            fileItems = fileItems,
             mediaBottomSheetState = mediaBottomSheetState,
             snackbarMessage = snackbarMessage,
             eventSink = ::handleEvents

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryState.kt

@@ -16,13 +16,17 @@ import kotlinx.collections.immutable.ImmutableList
 data class MediaGalleryState(
     val roomName: String,
     val mode: MediaGalleryMode,
-    val imageAndVideoItems: AsyncData<ImmutableList<MediaItem>>,
+    val groupedMediaItems: AsyncData<GroupedMediaItems>,
-    val fileItems: AsyncData<ImmutableList<MediaItem>>,
     val mediaBottomSheetState: MediaBottomSheetState,
     val snackbarMessage: SnackbarMessage?,
     val eventSink: (MediaGalleryEvents) -> Unit,
 )
 
+data class GroupedMediaItems(
+    val imageAndVideoItems: ImmutableList<MediaItem>,
+    val fileItems: ImmutableList<MediaItem>,
+)
+
 enum class MediaGalleryMode(val stringResource: Int) {
     Images(R.string.screen_media_browser_list_mode_media),
     Files(R.string.screen_media_browser_list_mode_files),

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryStateProvider.kt

@@ -11,79 +11,91 @@ import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.matrix.api.core.UniqueId
 import io.element.android.libraries.mediaviewer.impl.details.MediaBottomSheetState
-import io.element.android.libraries.mediaviewer.impl.details.aMediaDeleteConfirmationState
 import io.element.android.libraries.mediaviewer.impl.details.aMediaDetailsBottomSheetState
 import io.element.android.libraries.mediaviewer.impl.gallery.ui.aDate
 import io.element.android.libraries.mediaviewer.impl.gallery.ui.aFile
 import io.element.android.libraries.mediaviewer.impl.gallery.ui.aVideo
 import io.element.android.libraries.mediaviewer.impl.gallery.ui.anImage
-import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.toImmutableList
-import kotlinx.collections.immutable.toPersistentList
 
 open class MediaGalleryStateProvider : PreviewParameterProvider<MediaGalleryState> {
     override val values: Sequence<MediaGalleryState>
         get() = sequenceOf(
             aMediaGalleryState(),
-            aMediaGalleryState(imageAndVideoItems = AsyncData.Loading()),
+            aMediaGalleryState(groupedMediaItems = AsyncData.Loading()),
-            aMediaGalleryState(imageAndVideoItems = AsyncData.Success(emptyList<MediaItem.Image>().toPersistentList())),
+            aMediaGalleryState(groupedMediaItems = AsyncData.Success(aGroupedMediaItems())),
             aMediaGalleryState(
-                imageAndVideoItems = AsyncData.Success(
+                groupedMediaItems = AsyncData.Success(
-                    listOf(
+                    aGroupedMediaItems(
-                        aDate(id = UniqueId("0")),
+                        imageAndVideoItems = listOf(
-                        anImage(id = UniqueId("1")),
+                            aDate(id = UniqueId("0")),
-                        aDate(
+                            anImage(id = UniqueId("1")),
-                            id = UniqueId("2"),
+                            aDate(
-                            formattedDate = "September 2004",
+                                id = UniqueId("2"),
-                        ),
+                                formattedDate = "September 2004",
-                        anImage(id = UniqueId("3")),
+                            ),
-                        aVideo(id = UniqueId("4")),
+                            anImage(id = UniqueId("3")),
-                        anImage(id = UniqueId("5")),
+                            aVideo(id = UniqueId("4")),
-                        anImage(id = UniqueId("6")),
+                            anImage(id = UniqueId("5")),
-                        anImage(id = UniqueId("7")),
+                            anImage(id = UniqueId("6")),
-                        anImage(id = UniqueId("8")),
+                            anImage(id = UniqueId("7")),
-                        anImage(id = UniqueId("9")),
+                            anImage(id = UniqueId("8")),
-                    ).toImmutableList()
+                            anImage(id = UniqueId("9")),
-                )
+                        ).toImmutableList()
+                    )
+                ),
             ),
             aMediaGalleryState(mode = MediaGalleryMode.Files),
-            aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Loading()),
+            aMediaGalleryState(mode = MediaGalleryMode.Files, groupedMediaItems = AsyncData.Loading()),
-            aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Success(emptyList<MediaItem.File>().toPersistentList())),
+            aMediaGalleryState(mode = MediaGalleryMode.Files, groupedMediaItems = AsyncData.Success(aGroupedMediaItems())),
-            aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Success(emptyList<MediaItem.File>().toPersistentList())),
             aMediaGalleryState(
                 mode = MediaGalleryMode.Files,
-                fileItems = AsyncData.Success(
+                groupedMediaItems = AsyncData.Success(
-                    listOf(
+                    aGroupedMediaItems(
-                        aDate(id = UniqueId("0")),
+                        fileItems = listOf(
-                        aFile(id = UniqueId("1")),
+                            aDate(id = UniqueId("0")),
-                        aDate(
+                            aFile(id = UniqueId("1")),
-                            id = UniqueId("2"),
+                            aDate(
-                            formattedDate = "September 2004",
+                                id = UniqueId("2"),
-                        ),
+                                formattedDate = "September 2004",
-                        aFile(id = UniqueId("3")),
+                            ),
-                        aFile(id = UniqueId("4")),
+                            aFile(id = UniqueId("3")),
-                        aFile(id = UniqueId("5")),
+                            aFile(id = UniqueId("4")),
-                        aFile(id = UniqueId("6")),
+                            aFile(id = UniqueId("5")),
-                    ).toImmutableList()
+                            aFile(id = UniqueId("6")),
-                )
+                        ).toImmutableList()
+                    )
+                ),
             ),
             aMediaGalleryState(mediaBottomSheetState = aMediaDetailsBottomSheetState()),
-            aMediaGalleryState(mediaBottomSheetState = aMediaDeleteConfirmationState()),
+            aMediaGalleryState(
+                groupedMediaItems = AsyncData.Failure(Exception("Failed to load media")),
+            ),
+            aMediaGalleryState(
+                mode = MediaGalleryMode.Files,
+                groupedMediaItems = AsyncData.Failure(Exception("Failed to load media")),
+            ),
         )
 }
 
 private fun aMediaGalleryState(
     roomName: String = "Room name",
     mode: MediaGalleryMode = MediaGalleryMode.Images,
-    imageAndVideoItems: AsyncData<ImmutableList<MediaItem>> = AsyncData.Uninitialized,
+    groupedMediaItems: AsyncData<GroupedMediaItems> = AsyncData.Uninitialized,
-    fileItems: AsyncData<ImmutableList<MediaItem>> = AsyncData.Uninitialized,
     mediaBottomSheetState: MediaBottomSheetState = MediaBottomSheetState.Hidden,
 ) = MediaGalleryState(
     roomName = roomName,
     mode = mode,
-    imageAndVideoItems = imageAndVideoItems,
+    groupedMediaItems = groupedMediaItems,
-    fileItems = fileItems,
     mediaBottomSheetState = mediaBottomSheetState,
     snackbarMessage = null,
     eventSink = {}
 )
+
+private fun aGroupedMediaItems(
+    imageAndVideoItems: List<MediaItem> = emptyList(),
+    fileItems: List<MediaItem> = emptyList(),
+) = GroupedMediaItems(
+    imageAndVideoItems = imageAndVideoItems.toImmutableList(),
+    fileItems = fileItems.toImmutableList(),
+)

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryView.kt

@@ -127,18 +127,11 @@ fun MediaGalleryView(
                 modifier = Modifier,
             ) { page ->
                 val mode = MediaGalleryMode.entries[page]
-                when (mode) {
+                MediaGalleryPage(
-                    MediaGalleryMode.Images -> MediaGalleryImages(
+                    mode = mode,
-                        imagesAndVideos = state.imageAndVideoItems,
+                    state = state,
-                        eventSink = state.eventSink,
+                    onItemClick = onItemClick,
-                        onItemClick = onItemClick,
+                )
-                    )
-                    MediaGalleryMode.Files -> MediaGalleryFiles(
-                        files = state.fileItems,
-                        eventSink = state.eventSink,
-                        onItemClick = onItemClick,
-                    )
-                }
             }
         }
     }
@@ -179,62 +172,69 @@ fun MediaGalleryView(
 }
 
 @Composable
-private fun MediaGalleryImages(
+private fun MediaGalleryPage(
-    imagesAndVideos: AsyncData<ImmutableList<MediaItem>>,
+    mode: MediaGalleryMode,
-    eventSink: (MediaGalleryEvents) -> Unit,
+    state: MediaGalleryState,
     onItemClick: (MediaItem.Event) -> Unit,
 ) {
-    when (imagesAndVideos) {
+    when (val groupedMediaItems = state.groupedMediaItems) {
         AsyncData.Uninitialized,
         is AsyncData.Loading -> {
-            LoadingContent(MediaGalleryMode.Images)
+            LoadingContent(mode)
         }
         is AsyncData.Success -> {
-            if (imagesAndVideos.data.isEmpty()) {
+            when (mode) {
-                EmptyContent()
+                MediaGalleryMode.Images -> MediaGalleryImages(
-            } else {
+                    imagesAndVideos = groupedMediaItems.data.imageAndVideoItems,
-                MediaGalleryImageGrid(
+                    eventSink = state.eventSink,
-                    imagesAndVideos = imagesAndVideos.data,
+                    onItemClick = onItemClick,
-                    eventSink = eventSink,
+                )
+                MediaGalleryMode.Files -> MediaGalleryFiles(
+                    files = groupedMediaItems.data.fileItems,
+                    eventSink = state.eventSink,
                     onItemClick = onItemClick,
                 )
             }
         }
         is AsyncData.Failure -> {
             ErrorContent(
-                error = imagesAndVideos.error,
+                error = groupedMediaItems.error,
             )
         }
     }
 }
 
 @Composable
-private fun MediaGalleryFiles(
+private fun MediaGalleryImages(
-    files: AsyncData<ImmutableList<MediaItem>>,
+    imagesAndVideos: ImmutableList<MediaItem>,
     eventSink: (MediaGalleryEvents) -> Unit,
     onItemClick: (MediaItem.Event) -> Unit,
 ) {
-    when (files) {
+    if (imagesAndVideos.isEmpty()) {
-        AsyncData.Uninitialized,
+        EmptyContent()
-        is AsyncData.Loading -> {
+    } else {
-            LoadingContent(MediaGalleryMode.Files)
+        MediaGalleryImageGrid(
-        }
+            imagesAndVideos = imagesAndVideos,
-        is AsyncData.Success -> {
+            eventSink = eventSink,
-            if (files.data.isEmpty()) {
+            onItemClick = onItemClick,
-                EmptyContent()
+        )
-            } else {
+    }
-                MediaGalleryFilesList(
+}
-                    files = files.data,
+
-                    eventSink = eventSink,
+@Composable
-                    onItemClick = onItemClick,
+private fun MediaGalleryFiles(
-                )
+    files: ImmutableList<MediaItem>,
-            }
+    eventSink: (MediaGalleryEvents) -> Unit,
-        }
+    onItemClick: (MediaItem.Event) -> Unit,
-        is AsyncData.Failure -> {
+) {
-            ErrorContent(
+    if (files.isEmpty()) {
-                error = files.error,
+        EmptyContent()
-            )
+    } else {
-        }
+        MediaGalleryFilesList(
+            files = files,
+            eventSink = eventSink,
+            onItemClick = onItemClick,
+        )
     }
 }
 

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaItemsPostProcessor.kt

@@ -17,54 +17,68 @@ import javax.inject.Inject
 interface MediaItemsPostProcessor {
     fun process(
         mediaItems: AsyncData<ImmutableList<MediaItem>>,
-        predicate: (MediaItem.Event) -> Boolean,
+    ): AsyncData<GroupedMediaItems>
-    ): AsyncData<ImmutableList<MediaItem>>
 }
 
 @ContributesBinding(RoomScope::class)
 class DefaultMediaItemsPostProcessor @Inject constructor() : MediaItemsPostProcessor {
     override fun process(
         mediaItems: AsyncData<ImmutableList<MediaItem>>,
-        predicate: (MediaItem.Event) -> Boolean,
+    ): AsyncData<GroupedMediaItems> {
-    ): AsyncData<ImmutableList<MediaItem>> {
         return when (mediaItems) {
-            is AsyncData.Uninitialized -> mediaItems
+            is AsyncData.Uninitialized -> AsyncData.Uninitialized
-            is AsyncData.Loading -> mediaItems
+            is AsyncData.Loading -> AsyncData.Loading()
-            is AsyncData.Failure -> mediaItems
+            is AsyncData.Failure -> AsyncData.Failure(mediaItems.error)
             is AsyncData.Success -> AsyncData.Success(
-                process(
+                mediaItems.data.process()
-                    mediaItems = mediaItems.data,
-                    predicate = predicate,
-                )
             )
         }
     }
 
-    private fun process(
+    private fun List<MediaItem>.process(): GroupedMediaItems {
-        mediaItems: List<MediaItem>,
+        val imageAndVideoItems = mutableListOf<MediaItem>()
-        predicate: (MediaItem.Event) -> Boolean,
+        val fileItems = mutableListOf<MediaItem>()
-    ) = buildList {
+
-        val eventList = mutableListOf<MediaItem.Event>()
+        val imageAndVideoItemsSubList = mutableListOf<MediaItem.Event>()
-        for (item in mediaItems) {
+        val fileItemsSublist = mutableListOf<MediaItem.Event>()
+        forEach { item ->
             when (item) {
                 is MediaItem.DateSeparator -> {
-                    if (eventList.isNotEmpty()) {
+                    if (imageAndVideoItemsSubList.isNotEmpty()) {
                         // Date separator first
-                        add(item)
+                        imageAndVideoItems.add(item)
                         // Then events
-                        addAll(eventList)
+                        imageAndVideoItems.addAll(imageAndVideoItemsSubList)
-                        eventList.clear()
+                        imageAndVideoItemsSubList.clear()
+                    }
+                    if (fileItemsSublist.isNotEmpty()) {
+                        // Date separator first
+                        fileItems.add(item)
+                        // Then events
+                        fileItems.addAll(fileItemsSublist)
+                        fileItemsSublist.clear()
                     }
                 }
                 is MediaItem.Event -> {
-                    if (predicate(item)) {
+                    when (item) {
-                        eventList.add(item)
+                        is MediaItem.Image,
+                        is MediaItem.Video -> {
+                            imageAndVideoItemsSubList.add(item)
+                        }
+                        is MediaItem.File -> {
+                            fileItemsSublist.add(item)
+                        }
                     }
                 }
                 is MediaItem.LoadingIndicator -> {
-                    add(item)
+                    imageAndVideoItems.add(item)
+                    fileItems.add(item)
                 }
             }
         }
-    }.toImmutableList()
+        return GroupedMediaItems(
+            imageAndVideoItems = imageAndVideoItems.toImmutableList(),
+            fileItems = fileItems.toImmutableList(),
+        )
+    }
 }

libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/FakeMediaItemsPostProcessor.kt

@@ -9,9 +9,15 @@ package io.element.android.libraries.mediaviewer.impl.gallery
 
 import io.element.android.libraries.architecture.AsyncData
 import kotlinx.collections.immutable.ImmutableList
+import kotlinx.collections.immutable.persistentListOf
 
 class FakeMediaItemsPostProcessor : MediaItemsPostProcessor {
-    override fun process(mediaItems: AsyncData<ImmutableList<MediaItem>>, predicate: (MediaItem.Event) -> Boolean): AsyncData<ImmutableList<MediaItem>> {
+    override fun process(mediaItems: AsyncData<ImmutableList<MediaItem>>): AsyncData<GroupedMediaItems> {
-        return mediaItems
+        return AsyncData.Success(
+            GroupedMediaItems(
+                imageAndVideoItems = persistentListOf(),
+                fileItems = persistentListOf()
+            )
+        )
     }
 }

libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenterTest.kt

@@ -29,6 +29,7 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.lambda.value
 import io.element.android.tests.testutils.test
 import io.mockk.mockk
+import kotlinx.collections.immutable.persistentListOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
@@ -54,13 +55,17 @@ class MediaGalleryPresenterTest {
             )
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.mode).isEqualTo(MediaGalleryMode.Images)
             assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
             assertThat(initialState.roomName).isEqualTo(A_ROOM_NAME)
-            assertThat(initialState.imageAndVideoItems.dataOrNull()).isEmpty()
+            assertThat(initialState.groupedMediaItems.dataOrNull()).isEqualTo(
-            assertThat(initialState.fileItems.dataOrNull()).isEmpty()
+                GroupedMediaItems(
+                    imageAndVideoItems = persistentListOf(),
+                    fileItems = persistentListOf(),
+                )
+            )
             assertThat(initialState.snackbarMessage).isNull()
         }
     }
@@ -79,7 +84,7 @@ class MediaGalleryPresenterTest {
             )
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.mode).isEqualTo(MediaGalleryMode.Images)
             initialState.eventSink(MediaGalleryEvents.ChangeMode(MediaGalleryMode.Files))
@@ -111,7 +116,7 @@ class MediaGalleryPresenterTest {
             )
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
             val item = anImage(
@@ -155,7 +160,7 @@ class MediaGalleryPresenterTest {
             )
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
             val item = anImage(
@@ -188,7 +193,7 @@ class MediaGalleryPresenterTest {
             )
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             // Delete bottom sheet
             val item = anImage()
@@ -221,7 +226,7 @@ class MediaGalleryPresenterTest {
             navigator = navigator,
         )
         presenter.test {
-            skipItems(2)
+            skipItems(1)
             val initialState = awaitItem()
             initialState.eventSink(MediaGalleryEvents.ViewInTimeline(AN_EVENT_ID))
             onViewInTimelineClickLambda.assertions().isCalledOnce().with(value(AN_EVENT_ID))