Sulkta-OSS/element-x-ada
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix media viewer flickering (#6800)

Browse source 
* Fix media viewer flickering

This was caused by the data being loaded triggering an index update that got out of sync with the displayed items, and for an instant, the pager index pointed to the wrong data until it was refreshed

* Reuse the same 'displayer' function for both forwards and backwards pagination

* Make `dataFlow` a property so we don't create a new instance every time we access it

* Remove `pageDataComparator` as it prevented new items from being loaded when a pagination returned no valid items to display but has more items to load

Make sure we modify the current index when loading new data only if it was pointing to the input event id.

* Fix `MediaViewerDataSource` overriding the provided timestamp for `Loading` items

Test emitting different loading items from the data source results in the state displaying the different items, so they will trigger a new pagination attempt

* Add regression test to check loading -> error -> loading states will still trigger 2 separate `LoadMore` events
This commit is contained in:
Jorge Martin Espinosa 2026-05-20 11:32:32 +02:00 • committed by GitHub
parent 52f12031c8
commit 42c141109f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 312 additions and 148 deletions
Download patch file Download diff file Expand all files Collapse all files

12
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt
View file

@ -32,6 +32,7 @@ import timber.log.Timber
import java.util.concurrent.atomic.AtomicBoolean
interface MediaGalleryDataSource {
val isReady: Boolean
fun start(coroutineScope: CoroutineScope)
fun groupedMediaItemsFlow(): Flow<AsyncData<GroupedMediaItems>>
fun getLastData(): AsyncData<GroupedMediaItems>
@ -49,7 +50,7 @@ class TimelineMediaGalleryDataSource(
) : MediaGalleryDataSource {
private var timeline: Timeline? = null
private val groupedMediaItemsFlow = MutableSharedFlow<AsyncData<GroupedMediaItems>>(replay = 1)
private val groupedMediaItemsFlow = MutableSharedFlow<AsyncData<GroupedMediaItems>>(replay = 1, extraBufferCapacity = 10)
override fun groupedMediaItemsFlow(): Flow<AsyncData<GroupedMediaItems>> = groupedMediaItemsFlow
@ -59,9 +60,12 @@ class TimelineMediaGalleryDataSource(
private val isStarted = AtomicBoolean(false)
override val isReady: Boolean get() = isStarted.get() && timeline != null
@OptIn(ExperimentalCoroutinesApi::class)
override fun start(coroutineScope: CoroutineScope) {
if (!isStarted.compareAndSet(false, true)) {
Timber.w("MediaGalleryDataSource for room ${room.roomId} is already started, ignoring subsequent start call")
return
}
flow {
@ -73,10 +77,12 @@ class TimelineMediaGalleryDataSource(
}
mediaTimeline.getTimeline().fold(
{
Timber.d("Timeline media data source flow started for room ${room.roomId}")
timeline = it
emit(it)
},
{
Timber.e(it, "Failed to get media timeline for room ${room.roomId}")
groupedMediaItemsFlow.emit(AsyncData.Failure(it))
},
)
@ -107,13 +113,13 @@ class TimelineMediaGalleryDataSource(
}
override suspend fun loadMore(direction: Timeline.PaginationDirection) {
timeline?.paginate(direction)
timeline?.paginate(direction) ?: Timber.w("Timeline is not ready yet, cannot load more media items for room ${room.roomId}")
}
override suspend fun deleteItem(eventId: EventId) {
timeline?.redactEvent(
eventOrTransactionId = eventId.toEventOrTransactionId(),
reason = null,
)
) ?: Timber.w("Timeline is not ready yet, cannot delete media item with eventId $eventId for room ${room.roomId}")
}
}

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

@ -94,7 +94,9 @@ class MediaGalleryPresenter(
mode = event.mode
}
is MediaGalleryEvent.LoadMore -> coroutineScope.launch {
mediaGalleryDataSource.loadMore(event.direction)
if (mediaGalleryDataSource.isReady) {
mediaGalleryDataSource.loadMore(event.direction)
}
}
is MediaGalleryEvent.Delete -> coroutineScope.launch {
mediaGalleryDataSource.deleteItem(event.eventId)

86
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt
View file

@ -11,12 +11,13 @@ package io.element.android.libraries.mediaviewer.impl.viewer
import androidx.annotation.VisibleForTesting
import androidx.compose.runtime.Composable
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.ProduceStateScope
import androidx.compose.runtime.State
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.produceState
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.core.extensions.mapCatchingExceptions
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.media.MediaSource
@ -37,14 +38,17 @@ import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.withContext
import timber.log.Timber
import java.util.concurrent.ConcurrentHashMap
class MediaViewerDataSource(
mode: MediaViewerMode,
coroutineScope: CoroutineScope,
private val dispatcher: CoroutineDispatcher,
private val galleryDataSource: MediaGalleryDataSource,
private val mediaLoader: MatrixMediaLoader,
@ -76,40 +80,58 @@ class MediaViewerDataSource(
localMediaStates.clear()
}
/**
* Helper function to translate the [dataFlow] result to a Compose [State] that can be observed in the UI.
*/
@Composable
fun collectAsState(): State<ImmutableList<MediaViewerPageData>> {
return remember { dataFlow() }.collectAsState(initialData())
fun produceState(
producer: suspend ProduceStateScope<ImmutableList<MediaViewerPageData>>.(StateFlow<ImmutableList<MediaViewerPageData>>) -> Unit
): State<ImmutableList<MediaViewerPageData>> {
return produceState(initialValue = initialData()) {
producer(dataFlow)
}
}
/**
* Find the index of the page corresponding to the given eventId, or null if not found.
*/
fun findEventIndex(eventId: EventId?): Int? {
if (eventId == null) return null
return dataFlow.value.indexOfFirst { (it as? MediaViewerPageData.MediaViewerData)?.eventId == eventId }.takeIf { it >= 0 }
}
@VisibleForTesting
internal fun dataFlow(): Flow<ImmutableList<MediaViewerPageData>> {
return galleryDataSource.groupedMediaItemsFlow()
.map { groupedItems ->
when (groupedItems) {
AsyncData.Uninitialized,
is AsyncData.Loading -> {
persistentListOf(
MediaViewerPageData.Loading(
direction = Timeline.PaginationDirection.BACKWARDS,
timestamp = systemClock.epochMillis(),
pagerKey = Long.MIN_VALUE,
)
internal val dataFlow: StateFlow<ImmutableList<MediaViewerPageData>> = galleryDataSource.groupedMediaItemsFlow()
.map { groupedItems ->
when (groupedItems) {
AsyncData.Uninitialized,
is AsyncData.Loading -> {
persistentListOf(
MediaViewerPageData.Loading(
direction = Timeline.PaginationDirection.BACKWARDS,
timestamp = systemClock.epochMillis(),
pagerKey = Long.MIN_VALUE,
)
}
is AsyncData.Failure -> {
persistentListOf(
MediaViewerPageData.Failure(groupedItems.error),
)
}
is AsyncData.Success -> {
withContext(dispatcher) {
val mediaItems = groupedItems.data.getItems(galleryMode)
buildMediaViewerPageList(mediaItems)
}
)
}
is AsyncData.Failure -> {
persistentListOf(
MediaViewerPageData.Failure(groupedItems.error),
)
}
is AsyncData.Success -> {
withContext(dispatcher) {
val mediaItems = groupedItems.data.getItems(galleryMode)
buildMediaViewerPageList(mediaItems)
}
}
}
}
}
.stateIn(
scope = CoroutineScope(coroutineScope.coroutineContext + dispatcher),
started = SharingStarted.Lazily,
initialValue = initialData(),
)
private fun initialData(): ImmutableList<MediaViewerPageData> {
val initialMediaItems =
@ -148,7 +170,7 @@ class MediaViewerDataSource(
is MediaItem.LoadingIndicator -> add(
MediaViewerPageData.Loading(
direction = mediaItem.direction,
timestamp = systemClock.epochMillis(),
timestamp = mediaItem.timestamp,
pagerKey = pagerKeysHandler.getKey(mediaItem),
)
)
@ -161,7 +183,9 @@ class MediaViewerDataSource(
}
suspend fun loadMore(direction: Timeline.PaginationDirection) {
galleryDataSource.loadMore(direction)
if (galleryDataSource.isReady) {
galleryDataSource.loadMore(direction)
}
}
suspend fun loadMedia(data: MediaViewerPageData.MediaViewerData) {

2
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerNode.kt
View file

@ -13,6 +13,7 @@ import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.lifecycle.lifecycleScope
import com.bumble.appyx.core.modality.BuildContext
import com.bumble.appyx.core.node.Node
import com.bumble.appyx.core.plugin.Plugin
@ -118,6 +119,7 @@ class MediaViewerNode(
navigator = this,
dataSource = MediaViewerDataSource(
mode = inputs.mode,
coroutineScope = lifecycleScope,
dispatcher = coroutineDispatchers.computation,
galleryDataSource = mediaGallerySource,
mediaLoader = mediaLoader,

104
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt
View file

@ -14,14 +14,12 @@ 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
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 dev.zacsweers.metro.Assisted
import dev.zacsweers.metro.AssistedFactory
import dev.zacsweers.metro.AssistedInject
@ -45,10 +43,7 @@ import io.element.android.libraries.mediaviewer.impl.model.mediaPermissions
import io.element.android.libraries.ui.strings.CommonStrings
import kotlinx.collections.immutable.ImmutableList
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.flow.collectLatest
import kotlinx.coroutines.launch
import io.element.android.libraries.androidutils.R as UtilsR
@ -75,12 +70,27 @@ class MediaViewerPresenter(
@Composable
override fun present(): MediaViewerState {
val coroutineScope = rememberCoroutineScope()
val data = dataSource.collectAsState()
val currentIndex = remember { mutableIntStateOf(searchIndex(data.value, inputs.eventId)) }
val currentIndex = remember { mutableIntStateOf(dataSource.findEventIndex(inputs.eventId) ?: 0) }
val data = dataSource.produceState { flow ->
flow.collectLatest { new ->
val existingItem = value.getOrNull(currentIndex.intValue)
val newItem = new.getOrNull(currentIndex.intValue)
if (existingItem is MediaViewerPageData.MediaViewerData && existingItem.eventId == inputs.eventId && newItem != existingItem) {
currentIndex.intValue = dataSource.findEventIndex(inputs.eventId) ?: 0
} else if (currentIndex.intValue > 0 && value.firstOrNull() is MediaViewerPageData.Loading &&
new.firstOrNull() !is MediaViewerPageData.Loading) {
// Restore index based on the eventId after the initial items have been loaded
currentIndex.intValue = dataSource.findEventIndex(inputs.eventId) ?: 0
}
value = new
}
}
val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState()
NoMoreItemsBackwardSnackBarDisplayer(currentIndex, data)
NoMoreItemsForwardSnackBarDisplayer(currentIndex, data)
// Add both forward and backward pagination state checks to display a snackbar when there is no more items to load in either direction
NoMoreItemsSnackBarDisplayer(currentIndex, data, Timeline.PaginationDirection.FORWARDS)
NoMoreItemsSnackBarDisplayer(currentIndex, data, Timeline.PaginationDirection.BACKWARDS)
val permissions by room.permissionsAsState(MediaPermissions.DEFAULT) { perms ->
perms.mediaPermissions()
@ -176,52 +186,39 @@ class MediaViewerPresenter(
}
@Composable
private fun NoMoreItemsBackwardSnackBarDisplayer(
private fun NoMoreItemsSnackBarDisplayer(
currentIndex: IntState,
data: State<ImmutableList<MediaViewerPageData>>,
direction: Timeline.PaginationDirection,
) {
// With newest-first ordering, backward loading indicator is at the last index
val isRenderingLoadingBackward by remember {
derivedStateOf {
currentIndex.intValue == data.value.lastIndex &&
data.value.size > 1 &&
data.value.lastOrNull() is MediaViewerPageData.Loading
var previousIndex by remember { mutableIntStateOf(currentIndex.intValue) }
var previousDataSize by remember { mutableIntStateOf(data.value.size) }
var wasLoading: Boolean? by remember { mutableStateOf(null) }
LaunchedEffect(currentIndex.intValue, data.value) {
fun isLoading(index: Int, data: List<MediaViewerPageData>, direction: Timeline.PaginationDirection): Boolean {
return when (direction) {
Timeline.PaginationDirection.BACKWARDS -> index == data.lastIndex && data.lastOrNull() is MediaViewerPageData.Loading
Timeline.PaginationDirection.FORWARDS -> index == 0 && data.firstOrNull() 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)
// Reset the effect when the user navigate to another item so we only take into account index changes caused by data changes
if (previousIndex != currentIndex.intValue) {
wasLoading = null
previousIndex = currentIndex.intValue
}
// If we were navigating backwards and the data size grew, we can discard the previous value: it means we received new items
if (direction == Timeline.PaginationDirection.BACKWARDS && previousDataSize < data.value.size) {
wasLoading = null
}
}
}
@Composable
private fun NoMoreItemsForwardSnackBarDisplayer(
currentIndex: IntState,
data: State<ImmutableList<MediaViewerPageData>>,
) {
// With newest-first ordering, forward loading indicator is at the first index
val isRenderingLoadingForward by remember {
derivedStateOf {
currentIndex.intValue == 0 &&
data.value.size > 1 &&
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)
val isLoading = isLoading(currentIndex.intValue, data.value, direction)
if (wasLoading == true && !isLoading) {
showNoMoreItemsSnackbar()
}
previousDataSize = data.value.size
wasLoading = isLoading
}
}
@ -293,13 +290,4 @@ class MediaViewerPresenter(
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
}.coerceAtLeast(0)
}
}

2
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerState.kt
View file

@ -8,6 +8,7 @@
package io.element.android.libraries.mediaviewer.impl.viewer
import androidx.compose.runtime.Immutable
import androidx.compose.runtime.State
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
@ -29,6 +30,7 @@ data class MediaViewerState(
val eventSink: (MediaViewerEvent) -> Unit,
)
@Immutable
sealed interface MediaViewerPageData {
val pagerKey: Long

9
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerView.kt
View file

@ -44,7 +44,6 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.alpha
@ -179,9 +178,11 @@ fun MediaViewerView(
val pagerState = rememberPagerState(state.currentIndex, 0f) {
state.listData.size
}
LaunchedEffect(pagerState) {
snapshotFlow { pagerState.currentPage }.collect { page ->
state.eventSink(MediaViewerEvent.OnNavigateTo(page))
LaunchedEffect(pagerState.targetPage, state.currentIndex) {
// Only emit an index navigation change when it's triggered by the user scrolling
if (pagerState.targetPage != state.currentIndex && pagerState.isScrollInProgress) {
state.eventSink(MediaViewerEvent.OnNavigateTo(pagerState.targetPage))
}
}
HorizontalPager(

2
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt
View file

@ -30,6 +30,8 @@ class SingleMediaGalleryDataSource(
override fun groupedMediaItemsFlow() = flowOf(AsyncData.Success(data))
override fun getLastData(): AsyncData<GroupedMediaItems> = AsyncData.Success(data)
override val isReady: Boolean = true
override suspend fun loadMore(direction: Timeline.PaginationDirection) = Unit
override suspend fun deleteItem(eventId: EventId) = Unit

6
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/DefaultMediaViewerEntryPointTest.kt
View file

@ -33,16 +33,21 @@ import io.element.android.tests.testutils.lambda.lambdaError
import io.element.android.tests.testutils.node.TestParentNode
import io.element.android.tests.testutils.testCoroutineDispatchers
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import org.junit.Rule
import org.junit.Test
@OptIn(ExperimentalCoroutinesApi::class)
class DefaultMediaViewerEntryPointTest {
@get:Rule
val instantTaskExecutorRule = InstantTaskExecutorRule()
@Test
fun `test node builder`() = runTest {
Dispatchers.setMain(testCoroutineDispatchers().main)
val entryPoint = DefaultMediaViewerEntryPoint()
val mockMediaUri: Uri = mockk("localMediaUri")
val localMediaFactory = FakeLocalMediaFactory(mockMediaUri)
@ -89,6 +94,7 @@ class DefaultMediaViewerEntryPointTest {
@Test
fun `test node builder avatar`() = runTest {
Dispatchers.setMain(testCoroutineDispatchers().main)
val entryPoint = DefaultMediaViewerEntryPoint()
val mockMediaUri: Uri = mockk("localMediaUri")
val localMediaFactory = FakeLocalMediaFactory(mockMediaUri)

12
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt
View file

@ -15,18 +15,20 @@ import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems
import io.element.android.tests.testutils.lambda.lambdaError
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
class FakeMediaGalleryDataSource(
initialData: AsyncData<GroupedMediaItems> = AsyncData.Uninitialized,
private val isReadyResult: () -> Boolean = { true },
private val startLambda: () -> Unit = { lambdaError() },
private val loadMoreLambda: (Timeline.PaginationDirection) -> Unit = { lambdaError() },
private val deleteItemLambda: (EventId) -> Unit = { lambdaError() },
) : MediaGalleryDataSource {
) : MediaGalleryDataSource {
override fun start(coroutineScope: CoroutineScope) = startLambda()
private val groupedMediaItemsFlow = MutableSharedFlow<AsyncData<GroupedMediaItems>>(
replay = 1
)
private val groupedMediaItemsFlow = MutableStateFlow(initialData)
override val isReady: Boolean get() = isReadyResult()
override fun groupedMediaItemsFlow(): Flow<AsyncData<GroupedMediaItems>> {
return groupedMediaItemsFlow

32
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt
View file

@ -62,15 +62,20 @@ class MediaViewerDataSourceTest {
@Test
fun `test dataFlow uninitialized, loading and error`() = runTest {
val galleryDataSource = FakeMediaGalleryDataSource()
val galleryDataSource = FakeMediaGalleryDataSource(
initialData = AsyncData.Uninitialized,
)
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
// The flow starts with an empty result
assertThat(awaitItem()).isEmpty()
galleryDataSource.emitGroupedMediaItems(AsyncData.Uninitialized)
assertThat(awaitItem().first()).isInstanceOf(MediaViewerPageData.Loading::class.java)
galleryDataSource.emitGroupedMediaItems(AsyncData.Loading())
assertThat(awaitItem().first()).isInstanceOf(MediaViewerPageData.Loading::class.java)
// No items emitted, we were already loading data
ensureAllEventsConsumed()
galleryDataSource.emitGroupedMediaItems(AsyncData.Failure(AN_EXCEPTION))
assertThat(awaitItem().first()).isEqualTo(MediaViewerPageData.Failure(AN_EXCEPTION))
}
@ -82,7 +87,7 @@ class MediaViewerDataSourceTest {
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -102,7 +107,8 @@ class MediaViewerDataSourceTest {
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -141,7 +147,8 @@ class MediaViewerDataSourceTest {
mode = MediaViewerMode.TimelineImagesAndVideos(timelineMode = Timeline.Mode.Media),
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -163,7 +170,8 @@ class MediaViewerDataSourceTest {
mode = MediaViewerMode.TimelineFilesAndAudios(timelineMode = Timeline.Mode.Media),
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -184,7 +192,8 @@ class MediaViewerDataSourceTest {
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -217,7 +226,8 @@ class MediaViewerDataSourceTest {
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -241,7 +251,8 @@ class MediaViewerDataSourceTest {
galleryDataSource = galleryDataSource,
mediaLoader = mediaLoader,
)
sut.dataFlow().test {
sut.dataFlow.test {
skipItems(1)
galleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
aGroupedMediaItems(
@ -271,6 +282,7 @@ class MediaViewerDataSourceTest {
mediaLoader: MatrixMediaLoader = FakeMatrixMediaLoader(),
localMediaFactory: LocalMediaFactory = FakeLocalMediaFactory(mockMediaUrl),
) = MediaViewerDataSource(
coroutineScope = backgroundScope,
mode = mode,
dispatcher = testCoroutineDispatchers().computation,
galleryDataSource = galleryDataSource,

120
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenterTest.kt
View file

@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.matrix.api.timeline.item.event.EventOrTransactionId
import io.element.android.libraries.matrix.api.timeline.item.event.toEventOrTransactionId
import io.element.android.libraries.matrix.test.AN_EVENT_ID
import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
import io.element.android.libraries.matrix.test.A_SESSION_ID_2
import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.media.FakeMatrixMediaLoader
@ -51,6 +52,8 @@ import io.mockk.mockk
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test
@ -97,6 +100,8 @@ class MediaViewerPresenterTest {
assertThat(initialState.snackbarMessage).isNull()
assertThat(initialState.canShowInfo).isTrue()
assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
cancelAndIgnoreRemainingEvents()
}
}
@ -120,6 +125,8 @@ class MediaViewerPresenterTest {
assertThat(initialState.snackbarMessage).isNull()
assertThat(initialState.canShowInfo).isFalse()
assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
cancelAndIgnoreRemainingEvents()
}
}
@ -143,6 +150,8 @@ class MediaViewerPresenterTest {
assertThat(initialState.snackbarMessage).isNull()
assertThat(initialState.canShowInfo).isTrue()
assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
cancelAndIgnoreRemainingEvents()
}
}
@ -167,6 +176,8 @@ class MediaViewerPresenterTest {
assertThat(initialState.snackbarMessage).isNull()
assertThat(initialState.canShowInfo).isTrue()
assertThat(initialState.mediaBottomSheetState).isEqualTo(MediaBottomSheetState.Hidden)
cancelAndIgnoreRemainingEvents()
}
}
@ -555,6 +566,8 @@ class MediaViewerPresenterTest {
)
val finalState = awaitItem()
assertThat(finalState.currentIndex).isEqualTo(1)
cancelAndIgnoreRemainingEvents()
}
}
@ -578,32 +591,34 @@ class MediaViewerPresenterTest {
mode: MediaViewerEntryPoint.MediaViewerMode,
expectedSnackbarResId: Int,
) = runTest {
val image = anImage.copy(eventId = AN_EVENT_ID)
val mediaGalleryDataSource = FakeMediaGalleryDataSource(
initialData = AsyncData.Success(
if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(),
fileItems = persistentListOf(aForwardLoadingIndicator, image, aBackwardLoadingIndicator),
)
} else {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, image, aBackwardLoadingIndicator),
fileItems = persistentListOf(),
)
}
),
startLambda = { },
)
val presenter = createMediaViewerPresenter(
eventId = AN_EVENT_ID,
localMediaFactory = localMediaFactory,
mode = mode,
mediaGalleryDataSource = mediaGalleryDataSource,
)
presenter.test {
awaitFirstItem()
mediaGalleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(),
fileItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator),
)
} else {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator),
fileItems = persistentListOf(),
)
}
)
)
val updatedState = awaitItem()
val updatedState = awaitFirstItem()
advanceUntilIdle()
runCurrent()
// User navigate to the first item (forward loading indicator)
updatedState.eventSink(
MediaViewerEvent.OnNavigateTo(0)
@ -614,17 +629,17 @@ class MediaViewerPresenterTest {
if (mode is MediaViewerEntryPoint.MediaViewerMode.TimelineFilesAndAudios) {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(),
fileItems = persistentListOf(anImage, aBackwardLoadingIndicator),
fileItems = persistentListOf(image, aBackwardLoadingIndicator),
)
} else {
GroupedMediaItems(
imageAndVideoItems = persistentListOf(anImage, aBackwardLoadingIndicator),
imageAndVideoItems = persistentListOf(image, aBackwardLoadingIndicator),
fileItems = persistentListOf(),
)
}
)
)
skipItems(1)
skipItems(2)
val stateWithSnackbar = awaitItem()
assertThat(stateWithSnackbar.snackbarMessage!!.messageResId).isEqualTo(expectedSnackbarResId)
}
@ -707,21 +722,19 @@ class MediaViewerPresenterTest {
fun `present - no snackbar displayed when there is no more items but not displaying a loading item`() = runTest {
val mediaGalleryDataSource = FakeMediaGalleryDataSource(
startLambda = { },
initialData = AsyncData.Success(
GroupedMediaItems(
imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, anImage.copy(eventId = AN_EVENT_ID_2), aBackwardLoadingIndicator),
fileItems = persistentListOf(),
)
)
)
val presenter = createMediaViewerPresenter(
eventId = AN_EVENT_ID,
localMediaFactory = localMediaFactory,
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(
@ -774,6 +787,51 @@ class MediaViewerPresenterTest {
}
}
@Test
fun `present - receiving loading items with different timestamps emits different items too`() = runTest {
val loadMoreLambda = lambdaRecorder<Timeline.PaginationDirection, Unit> { }
val mediaGalleryDataSource = FakeMediaGalleryDataSource(
startLambda = { },
loadMoreLambda = loadMoreLambda,
)
val presenter = createMediaViewerPresenter(
localMediaFactory = localMediaFactory,
mediaGalleryDataSource = mediaGalleryDataSource,
)
val anImage = aMediaItemImage(
mediaSourceUrl = aUrl,
)
presenter.test {
awaitFirstItem()
mediaGalleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
GroupedMediaItems(
imageAndVideoItems = persistentListOf(aForwardLoadingIndicator, anImage, aBackwardLoadingIndicator),
fileItems = persistentListOf(),
)
)
)
val updatedState = awaitItem()
// Get the exact same items, but with new timestamps for the loading indicators
mediaGalleryDataSource.emitGroupedMediaItems(
AsyncData.Success(
GroupedMediaItems(
imageAndVideoItems = persistentListOf(
aForwardLoadingIndicator.copy(timestamp = 1234L),
anImage,
aBackwardLoadingIndicator.copy(timestamp = 1234L),
),
fileItems = persistentListOf(),
)
)
)
// We should get a new list of items, which should not be equal to the previous one
assertThat(updatedState.listData).isNotEqualTo(awaitItem().listData)
}
}
@Test
fun `present - view in timeline hides the bottom sheet and invokes the navigator`() = runTest {
val onViewInTimelineClickLambda = lambdaRecorder<EventId, Unit> { }
@ -794,6 +852,7 @@ class MediaViewerPresenterTest {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(MediaViewerEvent.OpenInfo(aMediaViewerPageData()))
skipItems(1)
val withBottomSheetState = awaitItem()
assertThat(withBottomSheetState.mediaBottomSheetState).isInstanceOf(MediaBottomSheetState.Details::class.java)
initialState.eventSink(MediaViewerEvent.ViewInTimeline(AN_EVENT_ID))
@ -823,6 +882,7 @@ class MediaViewerPresenterTest {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(MediaViewerEvent.OpenInfo(aMediaViewerPageData()))
skipItems(1)
val withBottomSheetState = awaitItem()
assertThat(withBottomSheetState.mediaBottomSheetState).isInstanceOf(MediaBottomSheetState.Details::class.java)
initialState.eventSink(MediaViewerEvent.Forward(AN_EVENT_ID))
@ -854,6 +914,7 @@ class MediaViewerPresenterTest {
presenter.test {
val initialState = awaitItem()
initialState.eventSink(MediaViewerEvent.OpenInfo(aMediaViewerPageData()))
skipItems(1)
val withBottomSheetState = awaitItem()
assertThat(withBottomSheetState.mediaBottomSheetState).isInstanceOf(MediaBottomSheetState.Details::class.java)
initialState.eventSink(MediaViewerEvent.Forward(AN_EVENT_ID))
@ -892,6 +953,7 @@ internal fun TestScope.createMediaViewerPresenter(
),
navigator = mediaViewerNavigator,
dataSource = MediaViewerDataSource(
coroutineScope = backgroundScope,
mode = mode,
dispatcher = testCoroutineDispatchers().computation,
galleryDataSource = mediaGalleryDataSource,

69
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerViewTest.kt
View file

@ -13,6 +13,11 @@ package io.element.android.libraries.mediaviewer.impl.viewer
import android.net.Uri
import androidx.activity.ComponentActivity
import androidx.annotation.StringRes
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.test.AndroidComposeUiTest
import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.test.assertHasClickAction
@ -23,6 +28,7 @@ import androidx.compose.ui.test.swipeDown
import androidx.compose.ui.test.v2.runAndroidComposeUiTest
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.mediaviewer.impl.details.aMediaBottomSheetStateDetails
import io.element.android.libraries.mediaviewer.test.viewer.aLocalMedia
import io.element.android.libraries.ui.strings.CommonStrings
@ -33,9 +39,11 @@ import io.element.android.tests.testutils.ensureCalledOnce
import io.element.android.tests.testutils.pressBack
import io.element.android.tests.testutils.setSafeContent
import io.mockk.mockk
import kotlinx.coroutines.delay
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.annotation.Config
import kotlin.time.Duration.Companion.milliseconds
@RunWith(AndroidJUnit4::class)
class MediaViewerViewTest {
@ -60,7 +68,6 @@ class MediaViewerViewTest {
}
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(state.listData.first() as MediaViewerPageData.MediaViewerData),
)
)
@ -122,7 +129,6 @@ class MediaViewerViewTest {
onNodeWithContentDescription(contentDescription).performClick()
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(data),
expectedEvent,
)
@ -178,7 +184,6 @@ class MediaViewerViewTest {
clickOn(textRes)
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(data),
expectedEvent,
)
@ -208,7 +213,6 @@ class MediaViewerViewTest {
.assertDoesNotExist()
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(state.listData.first() as MediaViewerPageData.MediaViewerData),
)
)
@ -231,7 +235,6 @@ class MediaViewerViewTest {
}
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(state.listData.first() as MediaViewerPageData.MediaViewerData),
)
)
@ -256,7 +259,6 @@ class MediaViewerViewTest {
clickOn(CommonStrings.action_retry)
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(data),
MediaViewerEvent.LoadMedia(data),
)
@ -282,12 +284,65 @@ class MediaViewerViewTest {
clickOn(CommonStrings.action_cancel)
eventsRecorder.assertList(
listOf(
MediaViewerEvent.OnNavigateTo(0),
MediaViewerEvent.LoadMedia(data),
MediaViewerEvent.ClearLoadingError(data)
)
)
}
@Test
fun `loading event after an error triggers load more Event`() = runAndroidComposeUiTest {
val eventsRecorder = EventsRecorder<MediaViewerEvent>()
val states = listOf(
aMediaViewerState(
listData = listOf(aMediaViewerPageDataLoading(timestamp = 0L)),
eventSink = eventsRecorder,
),
aMediaViewerState(
listData = listOf(MediaViewerPageData.Failure(IllegalStateException("error"))),
eventSink = eventsRecorder,
),
aMediaViewerState(
listData = listOf(aMediaViewerPageDataLoading(timestamp = 0L)),
eventSink = eventsRecorder,
),
// This one should be ignored since it has the same timestamp as the last one, it should not trigger a recomposition
aMediaViewerState(
listData = listOf(aMediaViewerPageDataLoading(timestamp = 0L)),
eventSink = eventsRecorder,
),
)
setSafeContent {
// Iterate over the states with a delay to give the view some time to trigger the `LoadMore` Event
var state by remember { mutableStateOf(states.first()) }
LaunchedEffect(Unit) {
val iterator = states.iterator()
while (iterator.hasNext()) {
delay(200.milliseconds)
state = iterator.next()
}
}
MediaViewerView(
state = state,
textFileViewer = { _, _ -> },
onBackClick = EnsureNeverCalled(),
audioFocus = null,
)
}
// Advance time to let the states update
mainClock.advanceTimeBy(3_000)
// `LoadMore` should be called twice, once for the first loading state, and once for the second one even though they have the same timestamp because
// of the intermediate error state.
// The third one will be ignored since it has the same timestamp as the second one and it'll be discarded by the Compose's equality diffing.
eventsRecorder.assertList(
listOf(
MediaViewerEvent.LoadMore(direction = Timeline.PaginationDirection.BACKWARDS),
MediaViewerEvent.LoadMore(direction = Timeline.PaginationDirection.BACKWARDS),
)
)
}
}
private fun AndroidComposeUiTest<ComponentActivity>.setMediaViewerView(