From e28163a116ca46db86b67fca1f95def4e8d81413 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 10 Dec 2024 12:58:24 +0100 Subject: [PATCH] Introduce GroupedMediaItems for code clarity --- .../impl/gallery/MediaGalleryPresenter.kt | 15 +-- .../impl/gallery/MediaGalleryState.kt | 8 +- .../impl/gallery/MediaGalleryStateProvider.kt | 100 ++++++++++-------- .../impl/gallery/MediaGalleryView.kt | 94 ++++++++-------- .../impl/gallery/MediaItemsPostProcessor.kt | 64 ++++++----- .../gallery/FakeMediaItemsPostProcessor.kt | 10 +- .../impl/gallery/MediaGalleryPresenterTest.kt | 21 ++-- 7 files changed, 171 insertions(+), 141 deletions(-) diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt index f5737a27b3..68f2aa98f0 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt +++ b/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.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, - fileItems = fileItems, + groupedMediaItems = groupedMediaItems, mediaBottomSheetState = mediaBottomSheetState, snackbarMessage = snackbarMessage, eventSink = ::handleEvents diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryState.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryState.kt index bcec4a37ca..51ae794175 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryState.kt +++ b/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>, - val fileItems: AsyncData>, + val groupedMediaItems: AsyncData, val mediaBottomSheetState: MediaBottomSheetState, val snackbarMessage: SnackbarMessage?, val eventSink: (MediaGalleryEvents) -> Unit, ) +data class GroupedMediaItems( + val imageAndVideoItems: ImmutableList, + val fileItems: ImmutableList, +) + enum class MediaGalleryMode(val stringResource: Int) { Images(R.string.screen_media_browser_list_mode_media), Files(R.string.screen_media_browser_list_mode_files), diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryStateProvider.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryStateProvider.kt index 7e551b02f4..55fffa2a9c 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryStateProvider.kt +++ b/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 { override val values: Sequence get() = sequenceOf( aMediaGalleryState(), - aMediaGalleryState(imageAndVideoItems = AsyncData.Loading()), - aMediaGalleryState(imageAndVideoItems = AsyncData.Success(emptyList().toPersistentList())), + aMediaGalleryState(groupedMediaItems = AsyncData.Loading()), + aMediaGalleryState(groupedMediaItems = AsyncData.Success(aGroupedMediaItems())), aMediaGalleryState( - imageAndVideoItems = AsyncData.Success( - listOf( - aDate(id = UniqueId("0")), - anImage(id = UniqueId("1")), - aDate( - id = UniqueId("2"), - formattedDate = "September 2004", - ), - anImage(id = UniqueId("3")), - aVideo(id = UniqueId("4")), - anImage(id = UniqueId("5")), - anImage(id = UniqueId("6")), - anImage(id = UniqueId("7")), - anImage(id = UniqueId("8")), - anImage(id = UniqueId("9")), - ).toImmutableList() - ) + groupedMediaItems = AsyncData.Success( + aGroupedMediaItems( + imageAndVideoItems = listOf( + aDate(id = UniqueId("0")), + anImage(id = UniqueId("1")), + aDate( + id = UniqueId("2"), + formattedDate = "September 2004", + ), + anImage(id = UniqueId("3")), + aVideo(id = UniqueId("4")), + anImage(id = UniqueId("5")), + anImage(id = UniqueId("6")), + anImage(id = UniqueId("7")), + anImage(id = UniqueId("8")), + anImage(id = UniqueId("9")), + ).toImmutableList() + ) + ), ), aMediaGalleryState(mode = MediaGalleryMode.Files), - aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Loading()), - aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Success(emptyList().toPersistentList())), - aMediaGalleryState(mode = MediaGalleryMode.Files, fileItems = AsyncData.Success(emptyList().toPersistentList())), + aMediaGalleryState(mode = MediaGalleryMode.Files, groupedMediaItems = AsyncData.Loading()), + aMediaGalleryState(mode = MediaGalleryMode.Files, groupedMediaItems = AsyncData.Success(aGroupedMediaItems())), aMediaGalleryState( mode = MediaGalleryMode.Files, - fileItems = AsyncData.Success( - listOf( - aDate(id = UniqueId("0")), - aFile(id = UniqueId("1")), - aDate( - id = UniqueId("2"), - formattedDate = "September 2004", - ), - aFile(id = UniqueId("3")), - aFile(id = UniqueId("4")), - aFile(id = UniqueId("5")), - aFile(id = UniqueId("6")), - ).toImmutableList() - ) + groupedMediaItems = AsyncData.Success( + aGroupedMediaItems( + fileItems = listOf( + aDate(id = UniqueId("0")), + aFile(id = UniqueId("1")), + aDate( + id = UniqueId("2"), + formattedDate = "September 2004", + ), + aFile(id = UniqueId("3")), + aFile(id = UniqueId("4")), + aFile(id = UniqueId("5")), + 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> = AsyncData.Uninitialized, - fileItems: AsyncData> = AsyncData.Uninitialized, + groupedMediaItems: AsyncData = AsyncData.Uninitialized, mediaBottomSheetState: MediaBottomSheetState = MediaBottomSheetState.Hidden, ) = MediaGalleryState( roomName = roomName, mode = mode, - imageAndVideoItems = imageAndVideoItems, - fileItems = fileItems, + groupedMediaItems = groupedMediaItems, mediaBottomSheetState = mediaBottomSheetState, snackbarMessage = null, eventSink = {} ) + +private fun aGroupedMediaItems( + imageAndVideoItems: List = emptyList(), + fileItems: List = emptyList(), +) = GroupedMediaItems( + imageAndVideoItems = imageAndVideoItems.toImmutableList(), + fileItems = fileItems.toImmutableList(), +) diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryView.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryView.kt index 88b0c5a769..9e3d89d44e 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryView.kt +++ b/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) { - MediaGalleryMode.Images -> MediaGalleryImages( - imagesAndVideos = state.imageAndVideoItems, - eventSink = state.eventSink, - onItemClick = onItemClick, - ) - MediaGalleryMode.Files -> MediaGalleryFiles( - files = state.fileItems, - eventSink = state.eventSink, - onItemClick = onItemClick, - ) - } + MediaGalleryPage( + mode = mode, + state = state, + onItemClick = onItemClick, + ) } } } @@ -179,62 +172,69 @@ fun MediaGalleryView( } @Composable -private fun MediaGalleryImages( - imagesAndVideos: AsyncData>, - eventSink: (MediaGalleryEvents) -> Unit, +private fun MediaGalleryPage( + mode: MediaGalleryMode, + 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()) { - EmptyContent() - } else { - MediaGalleryImageGrid( - imagesAndVideos = imagesAndVideos.data, - eventSink = eventSink, + when (mode) { + MediaGalleryMode.Images -> MediaGalleryImages( + imagesAndVideos = groupedMediaItems.data.imageAndVideoItems, + eventSink = state.eventSink, + onItemClick = onItemClick, + ) + 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( - files: AsyncData>, +private fun MediaGalleryImages( + imagesAndVideos: ImmutableList, eventSink: (MediaGalleryEvents) -> Unit, onItemClick: (MediaItem.Event) -> Unit, ) { - when (files) { - AsyncData.Uninitialized, - is AsyncData.Loading -> { - LoadingContent(MediaGalleryMode.Files) - } - is AsyncData.Success -> { - if (files.data.isEmpty()) { - EmptyContent() - } else { - MediaGalleryFilesList( - files = files.data, - eventSink = eventSink, - onItemClick = onItemClick, - ) - } - } - is AsyncData.Failure -> { - ErrorContent( - error = files.error, - ) - } + if (imagesAndVideos.isEmpty()) { + EmptyContent() + } else { + MediaGalleryImageGrid( + imagesAndVideos = imagesAndVideos, + eventSink = eventSink, + onItemClick = onItemClick, + ) + } +} + +@Composable +private fun MediaGalleryFiles( + files: ImmutableList, + eventSink: (MediaGalleryEvents) -> Unit, + onItemClick: (MediaItem.Event) -> Unit, +) { + if (files.isEmpty()) { + EmptyContent() + } else { + MediaGalleryFilesList( + files = files, + eventSink = eventSink, + onItemClick = onItemClick, + ) } } diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaItemsPostProcessor.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaItemsPostProcessor.kt index 85e972eff2..ff983ac97f 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaItemsPostProcessor.kt +++ b/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>, - predicate: (MediaItem.Event) -> Boolean, - ): AsyncData> + ): AsyncData } @ContributesBinding(RoomScope::class) class DefaultMediaItemsPostProcessor @Inject constructor() : MediaItemsPostProcessor { override fun process( mediaItems: AsyncData>, - predicate: (MediaItem.Event) -> Boolean, - ): AsyncData> { + ): AsyncData { return when (mediaItems) { - is AsyncData.Uninitialized -> mediaItems - is AsyncData.Loading -> mediaItems - is AsyncData.Failure -> mediaItems + is AsyncData.Uninitialized -> AsyncData.Uninitialized + is AsyncData.Loading -> AsyncData.Loading() + is AsyncData.Failure -> AsyncData.Failure(mediaItems.error) is AsyncData.Success -> AsyncData.Success( - process( - mediaItems = mediaItems.data, - predicate = predicate, - ) + mediaItems.data.process() ) } } - private fun process( - mediaItems: List, - predicate: (MediaItem.Event) -> Boolean, - ) = buildList { - val eventList = mutableListOf() - for (item in mediaItems) { + private fun List.process(): GroupedMediaItems { + val imageAndVideoItems = mutableListOf() + val fileItems = mutableListOf() + + val imageAndVideoItemsSubList = mutableListOf() + val fileItemsSublist = mutableListOf() + 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) - eventList.clear() + imageAndVideoItems.addAll(imageAndVideoItemsSubList) + imageAndVideoItemsSubList.clear() + } + if (fileItemsSublist.isNotEmpty()) { + // Date separator first + fileItems.add(item) + // Then events + fileItems.addAll(fileItemsSublist) + fileItemsSublist.clear() } } is MediaItem.Event -> { - if (predicate(item)) { - eventList.add(item) + when (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(), + ) + } } diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/FakeMediaItemsPostProcessor.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/FakeMediaItemsPostProcessor.kt index 637c60d57d..c94fbbbb2c 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/FakeMediaItemsPostProcessor.kt +++ b/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>, predicate: (MediaItem.Event) -> Boolean): AsyncData> { - return mediaItems + override fun process(mediaItems: AsyncData>): AsyncData { + return AsyncData.Success( + GroupedMediaItems( + imageAndVideoItems = persistentListOf(), + fileItems = persistentListOf() + ) + ) } } diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenterTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenterTest.kt index 3acc293f35..8867b8ebc3 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenterTest.kt +++ b/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.fileItems.dataOrNull()).isEmpty() + assertThat(initialState.groupedMediaItems.dataOrNull()).isEqualTo( + 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))