diff --git a/vector/src/main/java/im/vector/app/AppStateHandler.kt b/vector/src/main/java/im/vector/app/AppStateHandler.kt index 42169eb182b..864fa136efd 100644 --- a/vector/src/main/java/im/vector/app/AppStateHandler.kt +++ b/vector/src/main/java/im/vector/app/AppStateHandler.kt @@ -21,6 +21,7 @@ import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.asFlow import arrow.core.Option import arrow.core.getOrElse +import de.spiritcroc.matrixsdk.util.DbgUtil import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.utils.BehaviorDataSource import im.vector.app.features.analytics.AnalyticsTracker @@ -45,6 +46,7 @@ import org.matrix.android.sdk.api.session.getRoomSummary import org.matrix.android.sdk.api.session.group.model.GroupSummary import org.matrix.android.sdk.api.session.initsync.SyncStatusService import org.matrix.android.sdk.api.session.room.model.RoomSummary +import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -53,6 +55,17 @@ sealed class RoomGroupingMethod { data class BySpace(val spaceSummary: RoomSummary?) : RoomGroupingMethod() } +enum class SelectSpaceFrom { + // Initialized / uiStateRepository + INIT, + // Swiped in home pager + SWIPE, + // Persisted after swipe in home pager + PERSIST_SWIPE, + // Selected from non-pager UI + SELECT, +} + fun RoomGroupingMethod.space() = (this as? RoomGroupingMethod.BySpace)?.spaceSummary fun RoomGroupingMethod.group() = (this as? RoomGroupingMethod.ByLegacyGroup)?.groupSummary @@ -74,10 +87,11 @@ class AppStateHandler @Inject constructor( // SchildiChat: the boolean means pendingSwipe and defaults to false. Set to true for swiping spaces, so you want to ignore updates which have pendingSwipe = true. // Call it different then the upstream one so we don't forget adding `first` to upstream's logic. - private val selectedSpaceDataSourceSc = BehaviorDataSource>>(Option.empty()) + private val selectedSpaceDataSourceSc = BehaviorDataSource>>(Option.empty()) val selectedRoomGroupingFlow = selectedSpaceDataSourceSc.stream().map { it.map { it.first } } - val selectedRoomGroupingFlowIgnoreSwipe = selectedSpaceDataSourceSc.stream().filter { it.getOrElse{ null }?.second != true }.map { it.map { it.first } } + val selectedRoomGroupingFlowIgnoreSwipe = selectedSpaceDataSourceSc.stream() + .filter { it.getOrElse{ null }?.second != SelectSpaceFrom.SWIPE } fun getCurrentRoomGroupingMethod(): RoomGroupingMethod? { // XXX we should somehow make it live :/ just a work around @@ -94,7 +108,12 @@ class AppStateHandler @Inject constructor( } } - fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, pendingSwipe: Boolean = false) { + fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, from: SelectSpaceFrom = SelectSpaceFrom.SELECT) { + if (DbgUtil.isDbgEnabled(DbgUtil.DBG_VIEW_PAGER) && from == SelectSpaceFrom.SELECT) { + // We want a stack trace + Timber.w(Exception("Home pager: setCurrentSpace/SELECT")) + } + val uSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return if (selectedSpaceDataSourceSc.currentValue?.orNull()?.first is RoomGroupingMethod.BySpace && spaceId == selectedSpaceDataSourceSc.currentValue?.orNull()?.first?.space()?.roomId) return @@ -105,7 +124,7 @@ class AppStateHandler @Inject constructor( uiStateRepository.storeSelectedSpace(spaceSum?.roomId, uSession.sessionId) } - selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.BySpace(spaceSum), pendingSwipe))) + selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.BySpace(spaceSum), from))) if (spaceId != null) { uSession.coroutineScope.launch(Dispatchers.IO) { tryOrNull { @@ -120,7 +139,7 @@ class AppStateHandler @Inject constructor( if (selectedSpaceDataSourceSc.currentValue?.orNull()?.first is RoomGroupingMethod.ByLegacyGroup && groupId == selectedSpaceDataSourceSc.currentValue?.orNull()?.first?.group()?.groupId) return val activeGroup = groupId?.let { uSession.groupService().getGroupSummary(groupId) } - selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.ByLegacyGroup(activeGroup), false))) + selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.ByLegacyGroup(activeGroup), SelectSpaceFrom.SELECT))) if (groupId != null) { uSession.coroutineScope.launch { tryOrNull { @@ -137,7 +156,7 @@ class AppStateHandler @Inject constructor( // sessionDataSource could already return a session while activeSession holder still returns null it.orNull()?.let { session -> if (uiStateRepository.isGroupingMethodSpace(session.sessionId)) { - setCurrentSpace(uiStateRepository.getSelectedSpace(session.sessionId), session) + setCurrentSpace(uiStateRepository.getSelectedSpace(session.sessionId), session, from = SelectSpaceFrom.INIT) } else { setCurrentGroup(uiStateRepository.getSelectedGroup(session.sessionId), session) } @@ -193,8 +212,8 @@ class AppStateHandler @Inject constructor( val uSession = activeSessionHolder.getSafeActiveSession() ?: return // We want to persist it, so we also want to remove the pendingSwipe status - if (currentValue.second) { - selectedSpaceDataSourceSc.post(Option.just(Pair(currentMethod, false))) + if (currentValue.second == SelectSpaceFrom.SWIPE) { + selectedSpaceDataSourceSc.post(Option.just(Pair(currentMethod, SelectSpaceFrom.PERSIST_SWIPE))) } // Persist it across app restarts diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt index 6b041ff1b73..1b6aea97d26 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt @@ -34,10 +34,12 @@ import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState import com.google.android.material.badge.BadgeDrawable import de.spiritcroc.matrixsdk.util.DbgUtil +import de.spiritcroc.matrixsdk.util.Dimber import de.spiritcroc.viewpager.reduceDragSensitivity import im.vector.app.AppStateHandler import im.vector.app.R import im.vector.app.RoomGroupingMethod +import im.vector.app.SelectSpaceFrom import im.vector.app.core.extensions.restart import im.vector.app.core.extensions.toMvRxBundle import im.vector.app.core.platform.StateView @@ -83,6 +85,7 @@ class HomeDetailFragment @Inject constructor( CurrentCallsView.Callback { private val DEBUG_VIEW_PAGER = DbgUtil.isDbgEnabled(DbgUtil.DBG_VIEW_PAGER) + private val viewPagerDimber = Dimber("Home pager", DbgUtil.DBG_VIEW_PAGER) private val viewModel: HomeDetailViewModel by fragmentViewModel() private val unknownDeviceDetectorSharedViewModel: UnknownDeviceDetectorSharedViewModel by activityViewModel() @@ -153,7 +156,7 @@ class HomeDetailFragment @Inject constructor( // space pager: update appStateHandler's current page to update rest of the UI accordingly views.roomListContainerPager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() { override fun onPageSelected(position: Int) { - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: selected page $position $initialPageSelected") + viewPagerDimber.i{"Home pager: selected page $position $initialPageSelected"} super.onPageSelected(position) if (!initialPageSelected) { // Do not apply before we have restored the previous value @@ -164,13 +167,7 @@ class HomeDetailFragment @Inject constructor( initialPageSelected = true } } - val selectedId = getSpaceIdForPageIndex(position) - appStateHandler.setCurrentSpace(selectedId, pendingSwipe = true) - if (pagerPagingEnabled) { - onSpaceChange( - selectedId?.let { viewModel.getRoom(it)?.roomSummary() } - ) - } + selectSpaceFromSwipe(position) } }) @@ -255,6 +252,16 @@ class HomeDetailFragment @Inject constructor( } } + private fun selectSpaceFromSwipe(position: Int) { + val selectedId = getSpaceIdForPageIndex(position) + appStateHandler.setCurrentSpace(selectedId, from = SelectSpaceFrom.SWIPE) + if (pagerPagingEnabled) { + onSpaceChange( + selectedId?.let { viewModel.getRoom(it)?.roomSummary() } + ) + } + } + private fun handleCallStarted() { dismissLoadingDialog() val fragmentTag = HomeTab.DialPad.toFragmentTag() @@ -542,17 +549,18 @@ class HomeDetailFragment @Inject constructor( } */ - private fun setupViewPager(roomGroupingMethod: RoomGroupingMethod, spaces: List?, tab: HomeTab) { + private fun setupViewPager(roomGroupingMethodPair: Pair, spaces: List?, tab: HomeTab) { + val roomGroupingMethod = roomGroupingMethodPair.first val oldAdapter = views.roomListContainerPager.adapter as? FragmentStateAdapter val pagingAllowed = vectorPreferences.enableSpacePager() && tab is HomeTab.RoomList if (pagingAllowed && spaces == null) { - Timber.i("Home pager: Skip initial setup, root spaces not known yet") + viewPagerDimber.i{"Home pager: Skip initial setup, root spaces not known yet"} views.roomListContainerStateView.state = StateView.State.Loading return } else { views.roomListContainerStateView.state = StateView.State.Content } - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: setup, old adapter: $oldAdapter") + viewPagerDimber.i{"Home pager: setup, old adapter: $oldAdapter"} val unsafeSpaces = spaces?.map { it.roomId } ?: listOf() val selectedSpaceId = (roomGroupingMethod as? RoomGroupingMethod.BySpace)?.spaceSummary?.roomId val selectedIndex = getPageIndexForSpaceId(selectedSpaceId, unsafeSpaces) @@ -561,10 +569,20 @@ class HomeDetailFragment @Inject constructor( // Check if we need to recreate the adapter for a new tab if (oldAdapter != null) { val changed = pagerTab != tab || pagerSpaces != safeSpaces || pagerPagingEnabled != pagingEnabled - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: has changed: $changed (${pagerTab != tab} ${pagerSpaces != safeSpaces} ${pagerPagingEnabled != pagingEnabled} $selectedIndex ${views.roomListContainerPager.currentItem}) | $safeSpaces") + viewPagerDimber.i{ "has changed: $changed (${pagerTab != tab} ${pagerSpaces != safeSpaces} ${pagerPagingEnabled != pagingEnabled} $selectedIndex ${roomGroupingMethodPair.second} ${views.roomListContainerPager.currentItem}) | $safeSpaces" } if (!changed) { + // No need to re-setup pager, just check for selected page if (pagingEnabled) { - // No need to re-setup pager, just check for selected page + // Prioritize the actually displayed space for all space changes except for SELECT ones, to avoid + // unexpected page changes onResume for old updates + if (roomGroupingMethodPair.second != SelectSpaceFrom.SELECT) { + // Tell the rest of the UI that we want to keep displaying the current space + viewPagerDimber.i { "Discard space change from ${roomGroupingMethodPair.second} (${selectedSpaceId}/$selectedIndex), persist own (${views.roomListContainerPager.currentItem})" } + if (views.roomListContainerPager.currentItem != selectedIndex) { + selectSpaceFromSwipe(views.roomListContainerPager.currentItem) + } + return + } if (selectedIndex != null) { if (selectedIndex != views.roomListContainerPager.currentItem) { // post() mitigates a case where we could end up in an endless loop circling around the same few spaces @@ -613,16 +631,16 @@ class HomeDetailFragment @Inject constructor( } override fun createFragment(position: Int): Fragment { - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: create fragment for position $position") + viewPagerDimber.i{"Home pager: create fragment for position $position"} return when (tab) { is HomeTab.DialPad -> createDialPadFragment() is HomeTab.RoomList -> { val params = if (pagingEnabled) { val spaceId = getSpaceIdForPageIndex(position) - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: position $position -> space $spaceId") + viewPagerDimber.i{"Home pager: position $position -> space $spaceId"} RoomListParams(tab.displayMode, spaceId).toMvRxBundle() } else { - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: paging disabled; position $position -> follow") + viewPagerDimber.i{"Home pager: paging disabled; position $position -> follow"} RoomListParams(tab.displayMode, SPACE_ID_FOLLOW_APP).toMvRxBundle() } childFragmentManager.fragmentFactory.instantiate(activity!!.classLoader, RoomListFragment::class.java.name).apply { @@ -645,7 +663,7 @@ class HomeDetailFragment @Inject constructor( return } try { - if (DEBUG_VIEW_PAGER) Timber.i("Home pager: set initial page $selectedIndex") + viewPagerDimber.i{"Home pager: set initial page $selectedIndex"} views.roomListContainerPager.setCurrentItem(selectedIndex ?: 0, false) initialPageSelected = true } catch (e: Exception) { diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt index 24ad2e6d74c..50096488b17 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt @@ -225,7 +225,7 @@ class HomeDetailViewModel @AssistedInject constructor( appStateHandler.selectedRoomGroupingFlowIgnoreSwipe .setOnEach { copy( - roomGroupingMethodIgnoreSwipe = it.orNull() ?: RoomGroupingMethod.BySpace(null) + roomGroupingMethodIgnoreSwipe = it.orNull() ) } } diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewState.kt b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewState.kt index 0d4cac12d54..ab4caeb8e26 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDetailViewState.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDetailViewState.kt @@ -22,6 +22,7 @@ import com.airbnb.mvrx.MavericksState import com.airbnb.mvrx.Uninitialized import im.vector.app.R import im.vector.app.RoomGroupingMethod +import im.vector.app.SelectSpaceFrom import org.matrix.android.sdk.api.session.initsync.SyncStatusService import org.matrix.android.sdk.api.session.room.model.RoomSummary import org.matrix.android.sdk.api.session.sync.SyncState @@ -29,7 +30,7 @@ import org.matrix.android.sdk.api.util.MatrixItem data class HomeDetailViewState( val roomGroupingMethod: RoomGroupingMethod = RoomGroupingMethod.BySpace(null), - val roomGroupingMethodIgnoreSwipe: RoomGroupingMethod? = null, + val roomGroupingMethodIgnoreSwipe: Pair? = null, val myMatrixItem: MatrixItem? = null, val asyncRooms: Async> = Uninitialized, val currentTab: HomeTab = HomeTab.RoomList(RoomListDisplayMode.ALL),