Skip to content

Commit 704697b

Browse files
committed
fix(mediaviewer): Fix wrong image shown after opening from a group
Opening the first image of a group (or any image within the first two positions of the currently loaded media) triggered an older-history prefetch that prepends older items to the pager's item list and shifts the pager to compensate. That shift correction was emitted as a one-shot SharedFlow event, collected in a long-lived LaunchedEffect whose closure captured the item count from the composition active when the effect first launched. By the time the event fired, the item list had already grown, but the effect's clamp (`items.size` / later `pagerState.pageCount`) still reflected the stale, smaller count, so the pager settled on the wrong, small index - showing an unrelated, often much older image a moment after the tap. Moved the shift into UiState itself (MediaViewerViewModel.PendingShift) so the Compose side always applies it from the very same state snapshot as the already-updated item list, removing the possibility of a stale item count at the point the correction is applied. Assisted-by: Claude Code:claude-sonnet-5 Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
1 parent ca2e285 commit 704697b

2 files changed

Lines changed: 17 additions & 19 deletions

File tree

app/src/main/java/com/nextcloud/talk/mediaviewer/activities/MediaViewerScreen.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,13 @@ fun MediaViewerScreen(
120120
}
121121
val coroutineScope = rememberCoroutineScope()
122122

123-
// See MediaViewerViewModel.indexShiftEvents: prepending older groups shifts every existing
124-
// index, so the pager must silently jump to keep the same item on screen.
125-
LaunchedEffect(viewModel) {
126-
viewModel.indexShiftEvents.collect { shift ->
127-
if (shift != 0) {
128-
pagerState.scrollToPage((pagerState.currentPage + shift).coerceIn(0, items.size - 1))
123+
LaunchedEffect(uiState.pendingShift?.id) {
124+
val shift = uiState.pendingShift
125+
if (shift != null) {
126+
if (shift.amount != 0) {
127+
pagerState.scrollToPage((pagerState.currentPage + shift.amount).coerceIn(0, items.size - 1))
129128
}
129+
viewModel.consumePendingShift(shift.id)
130130
}
131131
}
132132

app/src/main/java/com/nextcloud/talk/mediaviewer/viewmodels/MediaViewerViewModel.kt

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import com.nextcloud.talk.shareditems.repositories.SharedItemsRepository
3030
import com.nextcloud.talk.utils.CapabilitiesUtil
3131
import com.nextcloud.talk.utils.FileUtils
3232
import io.reactivex.Observable
33-
import kotlinx.coroutines.flow.MutableSharedFlow
3433
import kotlinx.coroutines.flow.MutableStateFlow
3534
import kotlinx.coroutines.flow.StateFlow
3635
import kotlinx.coroutines.flow.collect
@@ -52,13 +51,16 @@ import kotlin.coroutines.resumeWithException
5251
class MediaViewerViewModel @Inject constructor(private val sharedItemsRepository: SharedItemsRepository) :
5352
ViewModel() {
5453

54+
data class PendingShift(val id: Long, val amount: Int)
55+
5556
data class UiState(
5657
val groups: List<MediaViewerGroup> = emptyList(),
5758
val currentGlobalIndex: Int = 0,
5859
val loadingOlder: Boolean = false,
5960
val canLoadOlder: Boolean = true,
6061
val cachedFilePaths: Map<Long, String> = emptyMap(),
61-
val downloadingMessageIds: Set<Long> = emptySet()
62+
val downloadingMessageIds: Set<Long> = emptySet(),
63+
val pendingShift: PendingShift? = null
6264
) {
6365
val flattenedItems: List<MediaViewerItem> get() = groups.flatMap { it.items }
6466
val currentItem: MediaViewerItem? get() = flattenedItems.getOrNull(currentGlobalIndex)
@@ -71,14 +73,7 @@ class MediaViewerViewModel @Inject constructor(private val sharedItemsRepository
7173

7274
private val _uiState = MutableStateFlow(UiState())
7375
val uiState: StateFlow<UiState> = _uiState
74-
75-
// HorizontalPager indexes into the flattened item list; prepending older groups shifts every
76-
// existing index by the number of items prepended. This is a one-shot event (not part of
77-
// UiState) so the pager can silently jump to the shifted position - via
78-
// pagerState.scrollToPage(pagerState.currentPage + shift) - the moment it fires, keeping the
79-
// same item on screen instead of visually jumping to whatever is now at the old index.
80-
private val _indexShiftEvents = MutableSharedFlow<Int>(extraBufferCapacity = 1)
81-
val indexShiftEvents = _indexShiftEvents
76+
private var nextShiftId = 0L
8277

8378
private lateinit var user: User
8479
private lateinit var repositoryParameters: SharedItemsRepository.Parameters
@@ -112,6 +107,10 @@ class MediaViewerViewModel @Inject constructor(private val sharedItemsRepository
112107
onPageSettled(globalIndex)
113108
}
114109

110+
fun consumePendingShift(id: Long) {
111+
_uiState.update { if (it.pendingShift?.id == id) it.copy(pendingShift = null) else it }
112+
}
113+
115114
private fun ensureCachedAround(globalIndex: Int) {
116115
val items = _uiState.value.flattenedItems
117116
for (index in (globalIndex - PREFETCH_RADIUS)..(globalIndex + PREFETCH_RADIUS)) {
@@ -215,17 +214,16 @@ class MediaViewerViewModel @Inject constructor(private val sharedItemsRepository
215214
}
216215

217216
oldestKnownMessageId = olderItems.first().messageId
218-
val previousCount = _uiState.value.flattenedItems.size
219217
_uiState.update { current ->
220218
val combined = (olderItems + current.flattenedItems).toMediaViewerGroups()
221219
current.copy(
222220
groups = combined,
223221
currentGlobalIndex = current.currentGlobalIndex + olderItems.size,
224222
loadingOlder = false,
225-
canLoadOlder = sharedItems?.moreItemsExisting == true
223+
canLoadOlder = sharedItems?.moreItemsExisting == true,
224+
pendingShift = PendingShift(id = nextShiftId++, amount = olderItems.size)
226225
)
227226
}
228-
_indexShiftEvents.tryEmit(_uiState.value.flattenedItems.size - previousCount)
229227
}
230228
}
231229

0 commit comments

Comments
 (0)