Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media navigation with swipe gesture #4161

Merged
merged 38 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
35cc5df
Create MediaGalleryDataSource and extract logic from MediaGalleryPres…
bmarty Jan 15, 2025
cbf7bf0
Let MediaGalleryDataSource be an interface
bmarty Jan 16, 2025
a06607f
Remove RetryLoading (use LoadMedia)
bmarty Jan 17, 2025
32db42a
Suppress Detekt false positive (?)
bmarty Jan 17, 2025
7df65b0
Add support for files navigation (when coming from the gallery)
bmarty Jan 17, 2025
ca5c06f
Open in SingleMedia mode when coming from the timeline
bmarty Jan 17, 2025
8d6550b
If not displayed, make sure to pause the audio / video
bmarty Jan 17, 2025
98a786b
media viewer : create MediaViewerDataSource
ganfra Jan 20, 2025
d26414f
Provide duration
bmarty Jan 17, 2025
bad4566
Remove unused import
bmarty Jan 20, 2025
3248041
media viewer : use collectAsState in the DataSource
ganfra Jan 21, 2025
c0542c8
Fix and write tests
bmarty Jan 21, 2025
d6ebec8
Improve loading state and add preview.
bmarty Jan 21, 2025
6c904a4
Add exception for Konsist
bmarty Jan 21, 2025
aa68a35
sync strings
bmarty Jan 21, 2025
a4cf1d7
Restore caption rendering
bmarty Jan 22, 2025
f286f6d
Small cleanup
bmarty Jan 22, 2025
428b81c
Add test on SingleMediaGalleryDataSource
bmarty Jan 22, 2025
18c3b5b
Add test on MediaViewerDataSource
bmarty Jan 22, 2025
621558a
MediaViewer: add error case in the UI.
bmarty Jan 22, 2025
e496bc3
Introduce MediaViewerFlickToDismiss and extract to its own file
bmarty Jan 22, 2025
dfda13a
Restore overlay when user cancel the dragging
bmarty Jan 22, 2025
bac69c4
Add timestamp to trigger back pagination.
bmarty Jan 22, 2025
b72f4bb
Fix tests.
bmarty Jan 22, 2025
74be5a5
Update screenshots
ElementBot Jan 22, 2025
1580c73
Ensure gallery is paginating to get new items.
bmarty Jan 23, 2025
77887f9
Fix formatting.
bmarty Jan 23, 2025
dff9005
Remove useless parameter
bmarty Jan 23, 2025
54182b0
Simplify with code `coerceAtLeast(0)``
bmarty Jan 23, 2025
2fde015
Simplify
bmarty Jan 23, 2025
afd8161
Add documentation on buildMediaViewerPageList.
bmarty Jan 23, 2025
a5515b0
Add name to argument for clarity.
bmarty Jan 23, 2025
7dd797b
Use Black for code clarity.
bmarty Jan 23, 2025
beb835c
Fix color for media viewer according to Figma.
bmarty Jan 23, 2025
05660fb
Cleanup
bmarty Jan 23, 2025
4d2edfa
Improve code clarity
bmarty Jan 23, 2025
ba0502c
Update screenshots
ElementBot Jan 23, 2025
da22758
Fix pagination restart issue and cover by unit test.
bmarty Jan 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@

package io.element.android.libraries.mediaviewer.impl.viewer

import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.State
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.media.MatrixMediaLoader
import io.element.android.libraries.matrix.api.media.MediaFile
import io.element.android.libraries.matrix.api.timeline.Timeline
Expand Down Expand Up @@ -56,33 +59,27 @@ class MediaViewerDataSource(
localMediaStates.clear()
}

fun initialPageIndex(eventId: EventId?): Int {
if (eventId == null) {
return 0
}
val mediaItems =
galleryDataSource.getLastData().dataOrNull()?.getItems(galleryMode).orEmpty()
val pageList = buildMediaViewerPageList(mediaItems)
return pageList.indexOfFirst { data ->
when (data) {
is MediaViewerPageData.MediaViewerData -> data.eventId == eventId
else -> false
}
}
.takeIf { it != -1 }
?: 0
@Composable
fun collectAsState(): State<PersistentList<MediaViewerPageData>> {
return remember { dataFlow() }.collectAsState(initialData())
}

fun dataFlow(): Flow<PersistentList<MediaViewerPageData>> {
private fun dataFlow(): Flow<PersistentList<MediaViewerPageData>> {
return galleryDataSource.groupedMediaItemsFlow()
.map {
val groupedItems = it.dataOrNull()?.getItems(galleryMode).orEmpty()
.map { groupedItems ->
val mediaItems = groupedItems.dataOrNull()?.getItems(galleryMode).orEmpty()
withContext(dispatcher) {
buildMediaViewerPageList(groupedItems)
buildMediaViewerPageList(mediaItems)
}
}
}

private fun initialData(): PersistentList<MediaViewerPageData> {
val initialMediaItems =
galleryDataSource.getLastData().dataOrNull()?.getItems(galleryMode).orEmpty()
return buildMediaViewerPageList(initialMediaItems)
}

private fun buildMediaViewerPageList(groupedItems: List<MediaItem>) = buildList {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some comments to this function? It wasn't easy to understand why passing an empty list was still necessary and the side effects this method has with localMediaStates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, done

groupedItems.forEach { mediaItem ->
when (mediaItem) {
Expand Down Expand Up @@ -112,6 +109,14 @@ class MediaViewerDataSource(
}
}.toPersistentList()

fun clearLoadingError(data: MediaViewerPageData.MediaViewerData) {
localMediaStates[data.mediaSource.url]?.value = AsyncData.Uninitialized
}

suspend fun loadMore(direction: Timeline.PaginationDirection) {
galleryDataSource.loadMore(direction)
}

suspend fun loadMedia(data: MediaViewerPageData.MediaViewerData) {
Timber.d("loadMedia for ${data.eventId}")
val localMediaState = localMediaStates.getOrPut(data.mediaSource.url) {
Expand Down Expand Up @@ -141,11 +146,5 @@ class MediaViewerDataSource(
}
}

fun clearLoadingError(data: MediaViewerPageData.MediaViewerData) {
localMediaStates[data.mediaSource.url]?.value = AsyncData.Uninitialized
}

suspend fun loadMore(direction: Timeline.PaginationDirection) {
galleryDataSource.loadMore(direction)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf
Expand All @@ -35,8 +34,6 @@ import io.element.android.libraries.mediaviewer.api.local.LocalMedia
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.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import io.element.android.libraries.androidutils.R as UtilsR
Expand All @@ -61,8 +58,8 @@ class MediaViewerPresenter @AssistedInject constructor(
@Composable
override fun present(): MediaViewerState {
val coroutineScope = rememberCoroutineScope()
val data: ImmutableList<MediaViewerPageData> by dataSource.dataFlow().collectAsState(persistentListOf())
var currentIndex by remember { mutableIntStateOf(dataSource.initialPageIndex(inputs.eventId)) }
val data by dataSource.collectAsState()
var currentIndex by remember { mutableIntStateOf(searchIndex(data, inputs.eventId)) }
val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState()

var mediaBottomSheetState by remember { mutableStateOf<MediaBottomSheetState>(MediaBottomSheetState.Hidden) }
Expand Down Expand Up @@ -202,4 +199,15 @@ class MediaViewerPresenter @AssistedInject constructor(
CommonStrings.error_unknown
}
}

private fun searchIndex(data: List<MediaViewerPageData>, eventId: EventId?): Int {
if (eventId == null) {
return 0
}
return data.indexOfFirst {
(it as? MediaViewerPageData.MediaViewerData)?.eventId == eventId
}
.takeIf { it != -1 }
?: 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep writing the coerceAtLeast(0) suggestion and the function keeps moving 😅 .

}
}