From d8115a79a4c57172db6757700472eda8ee72eede Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 12 Aug 2022 11:17:13 +0200 Subject: [PATCH 01/22] Adds persisted backstack --- .../im/vector/app/SpaceStateHandlerImpl.kt | 19 +++++++++++++++-- .../features/settings/VectorPreferences.kt | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index c6a4b2c5f0..a8739bfd0f 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -23,6 +23,7 @@ import im.vector.app.core.utils.BehaviorDataSource import im.vector.app.features.analytics.AnalyticsTracker import im.vector.app.features.analytics.plan.UserProperties import im.vector.app.features.session.coroutineScope +import im.vector.app.features.settings.VectorPreferences import im.vector.app.features.ui.UiStateRepository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -52,7 +53,8 @@ class SpaceStateHandlerImpl @Inject constructor( private val sessionDataSource: ActiveSessionDataSource, private val uiStateRepository: UiStateRepository, private val activeSessionHolder: ActiveSessionHolder, - private val analyticsTracker: AnalyticsTracker + private val analyticsTracker: AnalyticsTracker, + private val vectorPreferences: VectorPreferences, ) : SpaceStateHandler { private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main) @@ -82,7 +84,7 @@ class SpaceStateHandlerImpl @Inject constructor( } if (isForwardNavigation) { - spaceBackstack.addLast(currentSpace?.roomId) + addToBackstacks(spaceSummary) } if (persistNow) { @@ -104,6 +106,15 @@ class SpaceStateHandlerImpl @Inject constructor( } } + private fun addToBackstacks(space: RoomSummary?) { + val spaceId = space?.roomId ?: ROOT_SPACE_ID + spaceBackstack.addLast(spaceId) + + val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() + currentPersistedBackstack.add(spaceId) + vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) + } + private fun observeActiveSession() { sessionDataSource.stream() .distinctUntilChanged() @@ -144,4 +155,8 @@ class SpaceStateHandlerImpl @Inject constructor( val session = activeSessionHolder.getSafeActiveSession() ?: return uiStateRepository.storeSelectedSpace(selectedSpaceDataSource.currentValue?.orNull()?.roomId, session.sessionId) } + + companion object { + private const val ROOT_SPACE_ID = "ROOT" + } } diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index ac14bfc3c7..0a99cffe47 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -25,6 +25,7 @@ import androidx.core.content.edit import com.squareup.seismic.ShakeDetector import im.vector.app.R import im.vector.app.core.di.DefaultSharedPreferences +import im.vector.app.core.extensions.join import im.vector.app.core.resources.BuildMeta import im.vector.app.core.time.Clock import im.vector.app.features.disclaimer.SHARED_PREF_KEY @@ -77,6 +78,7 @@ class VectorPreferences @Inject constructor( const val SETTINGS_ALLOW_INTEGRATIONS_KEY = "SETTINGS_ALLOW_INTEGRATIONS_KEY" const val SETTINGS_INTEGRATION_MANAGER_UI_URL_KEY = "SETTINGS_INTEGRATION_MANAGER_UI_URL_KEY" const val SETTINGS_SECURE_MESSAGE_RECOVERY_PREFERENCE_KEY = "SETTINGS_SECURE_MESSAGE_RECOVERY_PREFERENCE_KEY" + const val SETTINGS_PERSISTED_SPACE_BACKSTACK = "SETTINGS_PERSISTED_SPACE_BACKSTACK" const val SETTINGS_CRYPTOGRAPHY_HS_ADMIN_DISABLED_E2E_DEFAULT = "SETTINGS_CRYPTOGRAPHY_HS_ADMIN_DISABLED_E2E_DEFAULT" // const val SETTINGS_SECURE_BACKUP_RESET_PREFERENCE_KEY = "SETTINGS_SECURE_BACKUP_RESET_PREFERENCE_KEY" @@ -1113,6 +1115,25 @@ class VectorPreferences @Inject constructor( .apply() } + /** + * Sets the space backstack that is used for up navigation + * This needs to be persisted because navigating up through spaces should work across sessions + * + * Only the IDs of the spaces are stored + */ + fun setPersistedSpaceBackstack(spaceBackstack: List) { + val spaceIdsJoined = spaceBackstack.joinToString(",") + defaultPrefs.edit().putString(SETTINGS_PERSISTED_SPACE_BACKSTACK, spaceIdsJoined).apply() + } + + /** + * Gets the space backstack used for up navigation + */ + fun getPersistedSpaceBackstack(): List { + val spaceIdsJoined = defaultPrefs.getString(SETTINGS_PERSISTED_SPACE_BACKSTACK, null) + return spaceIdsJoined?.split(",").orEmpty() + } + fun showLiveSenderInfo(): Boolean { return defaultPrefs.getBoolean(SETTINGS_TIMELINE_SHOW_LIVE_SENDER_INFO, getDefault(R.bool.settings_timeline_show_live_sender_info_default)) } From 7ee58ccc88184daa88d70204bdb337711b452442 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 12 Aug 2022 13:18:26 +0200 Subject: [PATCH 02/22] Fixes back navigation --- .../im/vector/app/SpaceStateHandlerImpl.kt | 10 +++------- .../app/features/home/NewHomeDetailFragment.kt | 18 ++++++++++-------- .../app/features/settings/VectorPreferences.kt | 5 ++--- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index a8739bfd0f..800b3f9589 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -77,14 +77,14 @@ class SpaceStateHandlerImpl @Inject constructor( val activeSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return val currentSpace = selectedSpaceDataSource.currentValue?.orNull() val spaceSummary = spaceId?.let { activeSession.getRoomSummary(spaceId) } - val sameSpaceSelected = currentSpace != null && spaceId == currentSpace.roomId + val sameSpaceSelected = spaceId == currentSpace?.roomId if (sameSpaceSelected) { return } if (isForwardNavigation) { - addToBackstacks(spaceSummary) + addToBackstacks(currentSpace) } if (persistNow) { @@ -107,7 +107,7 @@ class SpaceStateHandlerImpl @Inject constructor( } private fun addToBackstacks(space: RoomSummary?) { - val spaceId = space?.roomId ?: ROOT_SPACE_ID + val spaceId = space?.roomId spaceBackstack.addLast(spaceId) val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() @@ -155,8 +155,4 @@ class SpaceStateHandlerImpl @Inject constructor( val session = activeSessionHolder.getSafeActiveSession() ?: return uiStateRepository.storeSelectedSpace(selectedSpaceDataSource.currentValue?.orNull()?.roomId, session.sessionId) } - - companion object { - private const val ROOT_SPACE_ID = "ROOT" - } } diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 4766cd5006..16600abea5 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -178,14 +178,18 @@ class NewHomeDetailFragment @Inject constructor( } private fun navigateBack() { - val previousSpaceId = spaceStateHandler.getSpaceBackstack().removeLastOrNull() - val parentSpaceId = spaceStateHandler.getCurrentSpace()?.flattenParentIds?.lastOrNull() - setCurrentSpace(previousSpaceId ?: parentSpaceId) + val spaceBackstack = spaceStateHandler.getSpaceBackstack() + + try { + val previousSpaceId = spaceBackstack.removeLast() + setCurrentSpace(previousSpaceId) + } catch (e: NoSuchElementException) { + requireActivity().finish() + } } private fun setCurrentSpace(spaceId: String?) { spaceStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false) - sharedActionViewModel.post(HomeActivitySharedAction.OnCloseSpace) } private fun handleCallStarted() { @@ -452,10 +456,8 @@ class NewHomeDetailFragment @Inject constructor( return this } - override fun onBackPressed(toolbarButton: Boolean) = if (spaceStateHandler.getCurrentSpace() != null) { + override fun onBackPressed(toolbarButton: Boolean): Boolean { navigateBack() - true - } else { - false + return true } } diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index 0a99cffe47..6e2fdd1d6c 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -25,7 +25,6 @@ import androidx.core.content.edit import com.squareup.seismic.ShakeDetector import im.vector.app.R import im.vector.app.core.di.DefaultSharedPreferences -import im.vector.app.core.extensions.join import im.vector.app.core.resources.BuildMeta import im.vector.app.core.time.Clock import im.vector.app.features.disclaimer.SHARED_PREF_KEY @@ -1121,7 +1120,7 @@ class VectorPreferences @Inject constructor( * * Only the IDs of the spaces are stored */ - fun setPersistedSpaceBackstack(spaceBackstack: List) { + fun setPersistedSpaceBackstack(spaceBackstack: List) { val spaceIdsJoined = spaceBackstack.joinToString(",") defaultPrefs.edit().putString(SETTINGS_PERSISTED_SPACE_BACKSTACK, spaceIdsJoined).apply() } @@ -1129,7 +1128,7 @@ class VectorPreferences @Inject constructor( /** * Gets the space backstack used for up navigation */ - fun getPersistedSpaceBackstack(): List { + fun getPersistedSpaceBackstack(): List { val spaceIdsJoined = defaultPrefs.getString(SETTINGS_PERSISTED_SPACE_BACKSTACK, null) return spaceIdsJoined?.split(",").orEmpty() } From 5012f37e6f0453ee0dd6dee6aa1427757b1938b9 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 12 Aug 2022 13:28:01 +0200 Subject: [PATCH 03/22] Refactors space backstack handling --- vector/src/main/java/im/vector/app/SpaceStateHandler.kt | 6 +++--- .../src/main/java/im/vector/app/SpaceStateHandlerImpl.kt | 9 ++++++++- .../im/vector/app/features/home/HomeDetailFragment.kt | 2 +- .../im/vector/app/features/home/NewHomeDetailFragment.kt | 4 +--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index d9f002be37..dcd4eec230 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -51,11 +51,11 @@ interface SpaceStateHandler : DefaultLifecycleObserver { ) /** - * Gets the current backstack of spaces (via their id). + * Gets the Space ID of the space on top of the backstack * - * null may be an entry in the ArrayDeque to indicate the root space (All Chats) + * May return null to indicate the All Chats space */ - fun getSpaceBackstack(): ArrayDeque + fun popSpaceBackstack(): String? /** * Gets a flow of the selected space for clients to react immediately to space changes. diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index 800b3f9589..af050df995 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -138,7 +138,14 @@ class SpaceStateHandlerImpl @Inject constructor( }.launchIn(session.coroutineScope) } - override fun getSpaceBackstack() = spaceBackstack + override fun popSpaceBackstack(): String? { + val poppedSpaceId = spaceBackstack.removeLast() + vectorPreferences.getPersistedSpaceBackstack().toMutableList().apply { + removeLast() + vectorPreferences.setPersistedSpaceBackstack(this) + } + return poppedSpaceId + } override fun getSelectedSpaceFlow() = selectedSpaceFlow 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 d4c89c1bca..e4a572e2a0 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 @@ -183,7 +183,7 @@ class HomeDetailFragment @Inject constructor( } private fun navigateBack() { - val previousSpaceId = spaceStateHandler.getSpaceBackstack().removeLastOrNull() + val previousSpaceId = spaceStateHandler.popSpaceBackstack() val parentSpaceId = spaceStateHandler.getCurrentSpace()?.flattenParentIds?.lastOrNull() setCurrentSpace(previousSpaceId ?: parentSpaceId) } diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 16600abea5..09617a8dd8 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -178,10 +178,8 @@ class NewHomeDetailFragment @Inject constructor( } private fun navigateBack() { - val spaceBackstack = spaceStateHandler.getSpaceBackstack() - try { - val previousSpaceId = spaceBackstack.removeLast() + val previousSpaceId = spaceStateHandler.popSpaceBackstack() setCurrentSpace(previousSpaceId) } catch (e: NoSuchElementException) { requireActivity().finish() From 894d4f700ea056327f49a8e92e6960bbaed259af Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Mon, 15 Aug 2022 20:41:29 +0200 Subject: [PATCH 04/22] Makes space bottom sheet header reflect backstack --- .../java/im/vector/app/SpaceStateHandler.kt | 2 + .../im/vector/app/SpaceStateHandlerImpl.kt | 41 ++++++++++++++----- .../features/settings/VectorPreferences.kt | 4 +- .../features/spaces/NewSpaceListHeaderItem.kt | 28 ++++++++++++- .../spaces/NewSpaceSummaryController.kt | 9 +++- .../app/features/spaces/SpaceListViewModel.kt | 7 +++- .../app/features/spaces/SpaceListViewState.kt | 1 + .../res/layout/item_new_space_list_header.xml | 1 + 8 files changed, 76 insertions(+), 17 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index dcd4eec230..b8f90471e8 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -57,6 +57,8 @@ interface SpaceStateHandler : DefaultLifecycleObserver { */ fun popSpaceBackstack(): String? + fun getPersistedSpaceBackstack(): List + /** * Gets a flow of the selected space for clients to react immediately to space changes. */ diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index af050df995..53e2ebd1b5 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -75,26 +75,28 @@ class SpaceStateHandlerImpl @Inject constructor( isForwardNavigation: Boolean, ) { val activeSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return - val currentSpace = selectedSpaceDataSource.currentValue?.orNull() - val spaceSummary = spaceId?.let { activeSession.getRoomSummary(spaceId) } - val sameSpaceSelected = spaceId == currentSpace?.roomId + val spaceToLeave = selectedSpaceDataSource.currentValue?.orNull() + val spaceToSet = spaceId?.let { activeSession.getRoomSummary(spaceId) } + val sameSpaceSelected = spaceId == spaceToLeave?.roomId if (sameSpaceSelected) { return } if (isForwardNavigation) { - addToBackstacks(currentSpace) + addToBackstacks(spaceToLeave, spaceToSet) + } else { + popBackstackUntil(spaceToSet) } if (persistNow) { - uiStateRepository.storeSelectedSpace(spaceSummary?.roomId, activeSession.sessionId) + uiStateRepository.storeSelectedSpace(spaceToSet?.roomId, activeSession.sessionId) } - if (spaceSummary == null) { + if (spaceToSet == null) { selectedSpaceDataSource.post(Option.empty()) } else { - selectedSpaceDataSource.post(Option.just(spaceSummary)) + selectedSpaceDataSource.post(Option.just(spaceToSet)) } if (spaceId != null) { @@ -106,12 +108,29 @@ class SpaceStateHandlerImpl @Inject constructor( } } - private fun addToBackstacks(space: RoomSummary?) { + private fun addToBackstacks(spaceToLeave: RoomSummary?, spaceToSet: RoomSummary?) { + spaceBackstack.addLast(spaceToLeave?.roomId) + + // Only add to the persisted backstack if the space to set is not All Chats, else reset the persisted stack + if (spaceToSet != null && spaceToLeave != null) { + val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() + currentPersistedBackstack.add(spaceToLeave.roomId) + vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) + } else if (spaceToSet == null) { + vectorPreferences.setPersistedSpaceBackstack(emptyList()) + } + } + + private fun popBackstackUntil(space: RoomSummary?) { val spaceId = space?.roomId - spaceBackstack.addLast(spaceId) + while (spaceBackstack.last() != spaceId) { + spaceBackstack.removeLast() + } val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() - currentPersistedBackstack.add(spaceId) + while (currentPersistedBackstack.last() != spaceId) { + currentPersistedBackstack.removeLast() + } vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) } @@ -147,6 +166,8 @@ class SpaceStateHandlerImpl @Inject constructor( return poppedSpaceId } + override fun getPersistedSpaceBackstack() = vectorPreferences.getPersistedSpaceBackstack() + override fun getSelectedSpaceFlow() = selectedSpaceFlow override fun getSafeActiveSpaceId(): String? { diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index 6e2fdd1d6c..cb571da9d3 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -1121,7 +1121,7 @@ class VectorPreferences @Inject constructor( * Only the IDs of the spaces are stored */ fun setPersistedSpaceBackstack(spaceBackstack: List) { - val spaceIdsJoined = spaceBackstack.joinToString(",") + val spaceIdsJoined = spaceBackstack.takeIf { it.isNotEmpty() }?.joinToString(",") defaultPrefs.edit().putString(SETTINGS_PERSISTED_SPACE_BACKSTACK, spaceIdsJoined).apply() } @@ -1130,7 +1130,7 @@ class VectorPreferences @Inject constructor( */ fun getPersistedSpaceBackstack(): List { val spaceIdsJoined = defaultPrefs.getString(SETTINGS_PERSISTED_SPACE_BACKSTACK, null) - return spaceIdsJoined?.split(",").orEmpty() + return spaceIdsJoined?.takeIf { it.isNotEmpty() }?.split(",").orEmpty() } fun showLiveSenderInfo(): Boolean { diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt index 8fc53f07d4..647b31084e 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt @@ -16,6 +16,9 @@ package im.vector.app.features.spaces +import android.content.Context +import android.widget.TextView +import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyModelClass import im.vector.app.R import im.vector.app.core.epoxy.VectorEpoxyHolder @@ -23,5 +26,28 @@ import im.vector.app.core.epoxy.VectorEpoxyModel @EpoxyModelClass abstract class NewSpaceListHeaderItem : VectorEpoxyModel(R.layout.item_new_space_list_header) { - class Holder : VectorEpoxyHolder() + + @EpoxyAttribute var currentSpace: String? = null + @EpoxyAttribute var spaceHistory: List> = emptyList() + + override fun bind(holder: Holder) { + super.bind(holder) + holder.spaceHeader.text = buildSpaceHeaderText(holder.spaceHeader.context) + } + + private fun buildSpaceHeaderText(context: Context): String { + val allChats = context.getString(R.string.all_chats) + var spaceHeaderText = allChats + if (spaceHistory.isNotEmpty()) { + spaceHeaderText += " > ${spaceHistory.joinToString(" > ") { it.second }}" + } + if (currentSpace != null) { + spaceHeaderText += " > $currentSpace" + } + return spaceHeaderText + } + + class Holder : VectorEpoxyHolder() { + val spaceHeader by bind(R.id.space_header) + } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt index 7c4435bf59..47b0f23f44 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt @@ -50,7 +50,8 @@ class NewSpaceSummaryController @Inject constructor( nonNullViewState.spaces, nonNullViewState.selectedSpace, nonNullViewState.rootSpacesOrdered, - nonNullViewState.homeAggregateCount + nonNullViewState.homeAggregateCount, + nonNullViewState.spaceHistory, ) } @@ -58,11 +59,15 @@ class NewSpaceSummaryController @Inject constructor( spaceSummaries: List?, selectedSpace: RoomSummary?, rootSpaces: List?, - homeCount: RoomAggregateNotificationCount + homeCount: RoomAggregateNotificationCount, + spaceHistory: List>, ) { val host = this + newSpaceListHeaderItem { id("space_list_header") + currentSpace(selectedSpace?.displayName) + spaceHistory(spaceHistory) } if (selectedSpace != null) { diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt index 9048026771..fdec36add0 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt @@ -65,7 +65,7 @@ class SpaceListViewModel @AssistedInject constructor( private val session: Session, private val vectorPreferences: VectorPreferences, private val autoAcceptInvites: AutoAcceptInvites, - private val analyticsTracker: AnalyticsTracker + private val analyticsTracker: AnalyticsTracker, ) : VectorViewModel(initialState) { @AssistedFactory @@ -85,11 +85,14 @@ class SpaceListViewModel @AssistedInject constructor( } observeSpaceSummaries() + val spaceHistory = spaceStateHandler.getPersistedSpaceBackstack() + .map { it to it?.let { session.roomService().getRoomSummary(it)?.displayName }.orEmpty() } spaceStateHandler.getSelectedSpaceFlow() .distinctUntilChanged() .setOnEach { selectedSpaceOption -> copy( - selectedSpace = selectedSpaceOption.orNull() + selectedSpace = selectedSpaceOption.orNull(), + spaceHistory = spaceHistory, ) } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt index f75c336b5d..ebdc9e72ac 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt @@ -32,5 +32,6 @@ data class SpaceListViewState( val spaceOrderInfo: Map? = null, val spaceOrderLocalEchos: Map? = null, val expandedStates: Map = emptyMap(), + val spaceHistory: List> = emptyList(), // List of space id to display name val homeAggregateCount: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0) ) : MavericksState diff --git a/vector/src/main/res/layout/item_new_space_list_header.xml b/vector/src/main/res/layout/item_new_space_list_header.xml index 2c52304249..bb05f4cb53 100644 --- a/vector/src/main/res/layout/item_new_space_list_header.xml +++ b/vector/src/main/res/layout/item_new_space_list_header.xml @@ -1,6 +1,7 @@ Date: Tue, 16 Aug 2022 10:59:32 +0200 Subject: [PATCH 05/22] Adds working back navigation --- .../im/vector/app/SpaceStateHandlerImpl.kt | 17 +------------- .../features/home/NewHomeDetailFragment.kt | 23 +++++-------------- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index 53e2ebd1b5..6c70c3266a 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -85,8 +85,6 @@ class SpaceStateHandlerImpl @Inject constructor( if (isForwardNavigation) { addToBackstacks(spaceToLeave, spaceToSet) - } else { - popBackstackUntil(spaceToSet) } if (persistNow) { @@ -121,19 +119,6 @@ class SpaceStateHandlerImpl @Inject constructor( } } - private fun popBackstackUntil(space: RoomSummary?) { - val spaceId = space?.roomId - while (spaceBackstack.last() != spaceId) { - spaceBackstack.removeLast() - } - - val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() - while (currentPersistedBackstack.last() != spaceId) { - currentPersistedBackstack.removeLast() - } - vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) - } - private fun observeActiveSession() { sessionDataSource.stream() .distinctUntilChanged() @@ -160,7 +145,7 @@ class SpaceStateHandlerImpl @Inject constructor( override fun popSpaceBackstack(): String? { val poppedSpaceId = spaceBackstack.removeLast() vectorPreferences.getPersistedSpaceBackstack().toMutableList().apply { - removeLast() + removeLastOrNull() vectorPreferences.setPersistedSpaceBackstack(this) } return poppedSpaceId diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 09617a8dd8..4e429c4bf9 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -79,7 +79,6 @@ class NewHomeDetailFragment @Inject constructor( private val viewModel: HomeDetailViewModel by fragmentViewModel() private val unknownDeviceDetectorSharedViewModel: UnknownDeviceDetectorSharedViewModel by activityViewModel() - private val unreadMessagesSharedViewModel: UnreadMessagesSharedViewModel by activityViewModel() private val serverBackupStatusViewModel: ServerBackupStatusViewModel by activityViewModel() private lateinit var sharedActionViewModel: HomeSharedActionViewModel @@ -177,19 +176,6 @@ class NewHomeDetailFragment @Inject constructor( } } - private fun navigateBack() { - try { - val previousSpaceId = spaceStateHandler.popSpaceBackstack() - setCurrentSpace(previousSpaceId) - } catch (e: NoSuchElementException) { - requireActivity().finish() - } - } - - private fun setCurrentSpace(spaceId: String?) { - spaceStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false) - } - private fun handleCallStarted() { dismissLoadingDialog() val fragmentTag = HomeTab.DialPad.toFragmentTag() @@ -454,8 +440,11 @@ class NewHomeDetailFragment @Inject constructor( return this } - override fun onBackPressed(toolbarButton: Boolean): Boolean { - navigateBack() - return true + override fun onBackPressed(toolbarButton: Boolean) = try { + val lastSpace = spaceStateHandler.popSpaceBackstack() + spaceStateHandler.setCurrentSpace(lastSpace, isForwardNavigation = false) + true + } catch (e: NoSuchElementException) { + false } } From fc301c8a2e956c63d3e3b4c84b53fa50d8d42fdc Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 16 Aug 2022 11:50:15 +0200 Subject: [PATCH 06/22] Unifies back and persisted navigation --- .../im/vector/app/SpaceStateHandlerImpl.kt | 18 +++++++----------- .../features/spaces/NewSpaceListHeaderItem.kt | 7 +++++-- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index 6c70c3266a..c36b6de17b 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -60,7 +60,6 @@ class SpaceStateHandlerImpl @Inject constructor( private val coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main) private val selectedSpaceDataSource = BehaviorDataSource>(Option.empty()) private val selectedSpaceFlow = selectedSpaceDataSource.stream() - private val spaceBackstack = ArrayDeque() override fun getCurrentSpace(): RoomSummary? { return selectedSpaceDataSource.currentValue?.orNull()?.let { spaceSummary -> @@ -84,7 +83,7 @@ class SpaceStateHandlerImpl @Inject constructor( } if (isForwardNavigation) { - addToBackstacks(spaceToLeave, spaceToSet) + addToBackstack(spaceToLeave, spaceToSet) } if (persistNow) { @@ -106,15 +105,13 @@ class SpaceStateHandlerImpl @Inject constructor( } } - private fun addToBackstacks(spaceToLeave: RoomSummary?, spaceToSet: RoomSummary?) { - spaceBackstack.addLast(spaceToLeave?.roomId) - + private fun addToBackstack(spaceToLeave: RoomSummary?, spaceToSet: RoomSummary?) { // Only add to the persisted backstack if the space to set is not All Chats, else reset the persisted stack - if (spaceToSet != null && spaceToLeave != null) { + if (spaceToSet != null) { val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() - currentPersistedBackstack.add(spaceToLeave.roomId) + currentPersistedBackstack.add(spaceToLeave?.roomId) vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) - } else if (spaceToSet == null) { + } else { vectorPreferences.setPersistedSpaceBackstack(emptyList()) } } @@ -143,12 +140,11 @@ class SpaceStateHandlerImpl @Inject constructor( } override fun popSpaceBackstack(): String? { - val poppedSpaceId = spaceBackstack.removeLast() vectorPreferences.getPersistedSpaceBackstack().toMutableList().apply { - removeLastOrNull() + val poppedSpaceId = removeLast() vectorPreferences.setPersistedSpaceBackstack(this) + return poppedSpaceId } - return poppedSpaceId } override fun getPersistedSpaceBackstack() = vectorPreferences.getPersistedSpaceBackstack() diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt index 647b31084e..29538be16e 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt @@ -38,8 +38,11 @@ abstract class NewSpaceListHeaderItem : VectorEpoxyModel ") { it.second }}" + + val nonRootSpaceHistory = spaceHistory.filter { it.second.isNotEmpty() } + + if (nonRootSpaceHistory.isNotEmpty()) { + spaceHeaderText += " > ${nonRootSpaceHistory.joinToString(" > ") { it.second }}" } if (currentSpace != null) { spaceHeaderText += " > $currentSpace" From a59011ffd241a0c08f37a72f03ee15297f54f752 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 17 Aug 2022 23:33:31 +0200 Subject: [PATCH 07/22] Adds debug button to new app layout --- .../app/features/home/NewHomeDetailFragment.kt | 16 ++++++++++++++++ .../main/res/layout/fragment_new_home_detail.xml | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 4e429c4bf9..1acbeccc28 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -22,6 +22,7 @@ import android.view.Menu import android.view.MenuItem import android.view.View import android.view.ViewGroup +import androidx.core.view.isVisible import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import com.airbnb.mvrx.activityViewModel @@ -36,6 +37,7 @@ import im.vector.app.core.platform.OnBackPressed import im.vector.app.core.platform.VectorBaseActivity import im.vector.app.core.platform.VectorBaseFragment import im.vector.app.core.platform.VectorMenuProvider +import im.vector.app.core.resources.BuildMeta import im.vector.app.core.resources.ColorProvider import im.vector.app.core.ui.views.CurrentCallsView import im.vector.app.core.ui.views.CurrentCallsViewPresenter @@ -71,6 +73,7 @@ class NewHomeDetailFragment @Inject constructor( private val vectorPreferences: VectorPreferences, private val spaceStateHandler: SpaceStateHandler, private val session: Session, + private val buildMeta: BuildMeta, ) : VectorBaseFragment(), KeysBackupBanner.Delegate, CurrentCallsView.Callback, @@ -125,6 +128,7 @@ class NewHomeDetailFragment @Inject constructor( setupToolbar() setupKeysBackupBanner() setupActiveCallView() + setupDebugButton() withState(viewModel) { // Update the navigation view if needed (for when we restore the tabs) @@ -192,6 +196,7 @@ class NewHomeDetailFragment @Inject constructor( updateTabVisibilitySafely(R.id.bottom_action_notification, vectorPreferences.labAddNotificationTab()) callManager.checkForProtocolsSupportIfNeeded() refreshSpaceState() + refreshDebugButtonState() } private fun refreshSpaceState() { @@ -367,6 +372,17 @@ class NewHomeDetailFragment @Inject constructor( } } + private fun setupDebugButton() { + views.debugButton.debouncedClicks { + sharedActionViewModel.post(HomeActivitySharedAction.CloseDrawer) + navigator.openDebug(requireActivity()) + } + } + + private fun refreshDebugButtonState() { + views.debugButton.isVisible = buildMeta.isDebug && vectorPreferences.developerMode() + } + /* ========================================================================================== * KeysBackupBanner Listener * ========================================================================================== */ diff --git a/vector/src/main/res/layout/fragment_new_home_detail.xml b/vector/src/main/res/layout/fragment_new_home_detail.xml index 8066ab1bd9..eae8a7ba30 100644 --- a/vector/src/main/res/layout/fragment_new_home_detail.xml +++ b/vector/src/main/res/layout/fragment_new_home_detail.xml @@ -74,6 +74,19 @@ + + From fc76d08186bdf02c80d627f4de602b3d35e1afeb Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 09:22:37 +0200 Subject: [PATCH 08/22] Makes home activity restart if app layout flag change is detected --- .../java/im/vector/app/features/home/HomeActivity.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt index 12cdaecdf9..16ee0c1958 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt @@ -41,6 +41,7 @@ import im.vector.app.core.di.ActiveSessionHolder import im.vector.app.core.extensions.hideKeyboard import im.vector.app.core.extensions.registerStartForActivityResult import im.vector.app.core.extensions.replaceFragment +import im.vector.app.core.extensions.restart import im.vector.app.core.extensions.validateBackPressed import im.vector.app.core.platform.VectorBaseActivity import im.vector.app.core.platform.VectorMenuProvider @@ -136,6 +137,8 @@ class HomeActivity : @Inject lateinit var fcmHelper: FcmHelper @Inject lateinit var nightlyProxy: NightlyProxy + private var isNewAppLayoutEnabled: Boolean = false // delete once old app layout is removed + private val createSpaceResultLauncher = registerStartForActivityResult { activityResult -> if (activityResult.resultCode == Activity.RESULT_OK) { val spaceId = SpaceCreationActivity.getCreatedSpaceId(activityResult.data) @@ -193,6 +196,7 @@ class HomeActivity : override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + isNewAppLayoutEnabled = vectorFeatures.isNewAppLayoutEnabled() analyticsScreenName = MobileScreen.ScreenName.Home supportFragmentManager.registerFragmentLifecycleCallbacks(fragmentLifecycleCallbacks, false) unifiedPushHelper.register(this) { @@ -556,6 +560,14 @@ class HomeActivity : // Check nightly nightlyProxy.onHomeResumed() + + checkNewAppLayoutFlagChange() + } + + private fun checkNewAppLayoutFlagChange() { + if (buildMeta.isDebug && vectorFeatures.isNewAppLayoutEnabled() != isNewAppLayoutEnabled) { + restart() + } } override fun getMenuRes() = if (vectorFeatures.isNewAppLayoutEnabled()) R.menu.menu_new_home else R.menu.menu_home From 0b8c68739e2148dd744295a02b12c018fa469fe9 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 09:43:29 +0200 Subject: [PATCH 09/22] Makes debug settings refresh state onResume in old app layout --- .../java/im/vector/app/features/home/HomeDrawerFragment.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/HomeDrawerFragment.kt b/vector/src/main/java/im/vector/app/features/home/HomeDrawerFragment.kt index 535f38e68e..c10027cc3f 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeDrawerFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeDrawerFragment.kt @@ -113,10 +113,14 @@ class HomeDrawerFragment @Inject constructor( } // Debug menu - views.homeDrawerHeaderDebugView.isVisible = buildMeta.isDebug && vectorPreferences.developerMode() views.homeDrawerHeaderDebugView.debouncedClicks { sharedActionViewModel.post(HomeActivitySharedAction.CloseDrawer) navigator.openDebug(requireActivity()) } } + + override fun onResume() { + super.onResume() + views.homeDrawerHeaderDebugView.isVisible = buildMeta.isDebug && vectorPreferences.developerMode() + } } From 2e323e6f2e52401176d086b702fae3d188697933 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 10:09:32 +0200 Subject: [PATCH 10/22] Makes debug icon hide when collapsing toolbar --- .../im/vector/app/features/home/NewHomeDetailFragment.kt | 5 +++++ vector/src/main/res/layout/fragment_new_home_detail.xml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 1acbeccc28..760b6d8a4f 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -28,6 +28,7 @@ import androidx.lifecycle.lifecycleScope import com.airbnb.mvrx.activityViewModel import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState +import com.google.android.material.appbar.AppBarLayout import com.google.android.material.badge.BadgeDrawable import im.vector.app.R import im.vector.app.SpaceStateHandler @@ -377,6 +378,10 @@ class NewHomeDetailFragment @Inject constructor( sharedActionViewModel.post(HomeActivitySharedAction.CloseDrawer) navigator.openDebug(requireActivity()) } + + views.appBarLayout.addOnOffsetChangedListener(AppBarLayout.OnOffsetChangedListener { _, verticalOffset -> + views.debugButton.isVisible = verticalOffset == 0 + }) } private fun refreshDebugButtonState() { diff --git a/vector/src/main/res/layout/fragment_new_home_detail.xml b/vector/src/main/res/layout/fragment_new_home_detail.xml index eae8a7ba30..d54d799a62 100644 --- a/vector/src/main/res/layout/fragment_new_home_detail.xml +++ b/vector/src/main/res/layout/fragment_new_home_detail.xml @@ -68,8 +68,8 @@ android:id="@+id/avatar" android:layout_width="36dp" android:layout_height="36dp" - android:padding="6dp" android:contentDescription="@string/a11y_open_settings" + android:padding="6dp" tools:src="@sample/user_round_avatars" /> From 136ca4bafbdf24bb4fb6693b14f6c73e51a799fe Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 10:16:54 +0200 Subject: [PATCH 11/22] Adds changelog file --- changelog.d/6871.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6871.feature diff --git a/changelog.d/6871.feature b/changelog.d/6871.feature new file mode 100644 index 0000000000..313be1a602 --- /dev/null +++ b/changelog.d/6871.feature @@ -0,0 +1 @@ +Improves Developer Mode Debug Button UX and adds it to New App Layout From 2fb794dd59f7cf02ca1fcc0b74bff0a658298fbc Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 14:09:22 +0200 Subject: [PATCH 12/22] Fixes space header showing "Empty Space" after switching to newly created space --- vector/src/main/java/im/vector/app/SpaceStateHandler.kt | 2 ++ .../src/main/java/im/vector/app/SpaceStateHandlerImpl.kt | 5 ++++- .../java/im/vector/app/features/home/HomeActivity.kt | 9 ++++++++- .../vector/app/features/navigation/DefaultNavigator.kt | 9 +++++++-- .../java/im/vector/app/features/navigation/Navigator.kt | 7 ++++++- .../vector/app/features/spaces/SpaceCreationActivity.kt | 6 ++++++ .../app/features/spaces/create/CreateSpaceEvents.kt | 2 +- .../app/features/spaces/create/CreateSpaceViewModel.kt | 2 ++ 8 files changed, 36 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index b8f90471e8..2651d62f1a 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -42,12 +42,14 @@ interface SpaceStateHandler : DefaultLifecycleObserver { * @param session the current active session * @param persistNow if true, the current space will immediately be persisted in shared prefs * @param isForwardNavigation whether this navigation is a forward action to properly handle backstack + * @param overriddenSpaceName overrides the display name of the space being set */ fun setCurrentSpace( spaceId: String?, session: Session? = null, persistNow: Boolean = false, isForwardNavigation: Boolean = true, + overriddenSpaceName: String? = null, ) /** diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index c36b6de17b..7bd0ac70a6 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -72,10 +72,13 @@ class SpaceStateHandlerImpl @Inject constructor( session: Session?, persistNow: Boolean, isForwardNavigation: Boolean, + overriddenSpaceName: String?, ) { val activeSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return val spaceToLeave = selectedSpaceDataSource.currentValue?.orNull() - val spaceToSet = spaceId?.let { activeSession.getRoomSummary(spaceId) } + val spaceToSet = spaceId?.let { activeSession.getRoomSummary(spaceId) }?.let { + if (overriddenSpaceName != null) it.copy(displayName = overriddenSpaceName) else it + } val sameSpaceSelected = spaceId == spaceToLeave?.roomId if (sameSpaceSelected) { diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt index 12cdaecdf9..4b86c739a0 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt @@ -55,6 +55,8 @@ import im.vector.app.features.analytics.plan.MobileScreen import im.vector.app.features.analytics.plan.ViewRoom import im.vector.app.features.crypto.recover.SetupMode import im.vector.app.features.disclaimer.showDisclaimerDialog +import im.vector.app.features.home.room.list.actions.RoomListSharedAction +import im.vector.app.features.home.room.list.actions.RoomListSharedActionViewModel import im.vector.app.features.matrixto.MatrixToBottomSheet import im.vector.app.features.matrixto.OriginOfMatrixTo import im.vector.app.features.navigation.Navigator @@ -110,6 +112,7 @@ class HomeActivity : VectorMenuProvider { private lateinit var sharedActionViewModel: HomeSharedActionViewModel + private lateinit var roomListSharedActionViewModel: RoomListSharedActionViewModel private val homeActivityViewModel: HomeActivityViewModel by viewModel() @@ -139,6 +142,7 @@ class HomeActivity : private val createSpaceResultLauncher = registerStartForActivityResult { activityResult -> if (activityResult.resultCode == Activity.RESULT_OK) { val spaceId = SpaceCreationActivity.getCreatedSpaceId(activityResult.data) + val spaceName = SpaceCreationActivity.getCreatedSpaceName(activityResult.data) val defaultRoomId = SpaceCreationActivity.getDefaultRoomId(activityResult.data) val isJustMe = SpaceCreationActivity.isJustMeSpace(activityResult.data) views.drawerLayout.closeDrawer(GravityCompat.START) @@ -155,8 +159,10 @@ class HomeActivity : navigator.switchToSpace( context = this, spaceId = spaceId, - postSwitchOption + postSwitchOption, + overriddenSpaceName = spaceName, ) + roomListSharedActionViewModel.post(RoomListSharedAction.CloseBottomSheet) } } } @@ -205,6 +211,7 @@ class HomeActivity : } } sharedActionViewModel = viewModelProvider[HomeSharedActionViewModel::class.java] + roomListSharedActionViewModel = viewModelProvider[RoomListSharedActionViewModel::class.java] views.drawerLayout.addDrawerListener(drawerListener) if (isFirstCreation()) { if (vectorFeatures.isNewAppLayoutEnabled()) { diff --git a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt index 38db642287..c661a66767 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt @@ -177,12 +177,17 @@ class DefaultNavigator @Inject constructor( startActivity(context, intent, buildTask) } - override fun switchToSpace(context: Context, spaceId: String, postSwitchSpaceAction: Navigator.PostSwitchSpaceAction) { + override fun switchToSpace( + context: Context, + spaceId: String, + postSwitchSpaceAction: Navigator.PostSwitchSpaceAction, + overriddenSpaceName: String?, + ) { if (sessionHolder.getSafeActiveSession()?.getRoomSummary(spaceId) == null) { fatalError("Trying to open an unknown space $spaceId", vectorPreferences.failFast()) return } - spaceStateHandler.setCurrentSpace(spaceId) + spaceStateHandler.setCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) when (postSwitchSpaceAction) { Navigator.PostSwitchSpaceAction.None -> { // go back to home if we are showing room details? diff --git a/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt b/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt index 8e01b3ed50..bb59c84555 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt @@ -68,7 +68,12 @@ interface Navigator { data class OpenDefaultRoom(val roomId: String, val showShareSheet: Boolean) : PostSwitchSpaceAction() } - fun switchToSpace(context: Context, spaceId: String, postSwitchSpaceAction: PostSwitchSpaceAction) + fun switchToSpace( + context: Context, + spaceId: String, + postSwitchSpaceAction: PostSwitchSpaceAction, + overriddenSpaceName: String? = null, + ) fun openSpacePreview(context: Context, spaceId: String) diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt index 9fa4a53efc..aeca7021eb 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt @@ -104,6 +104,7 @@ class SpaceCreationActivity : SimpleFragmentActivity() { is CreateSpaceEvents.FinishSuccess -> { setResult(RESULT_OK, Intent().apply { putExtra(RESULT_DATA_CREATED_SPACE_ID, it.spaceId) + putExtra(RESULT_DATA_CREATED_SPACE_NAME, it.spaceName) putExtra(RESULT_DATA_DEFAULT_ROOM_ID, it.defaultRoomId) putExtra(RESULT_DATA_CREATED_SPACE_IS_JUST_ME, it.topology == SpaceTopology.JustMe) }) @@ -159,6 +160,7 @@ class SpaceCreationActivity : SimpleFragmentActivity() { companion object { private const val RESULT_DATA_CREATED_SPACE_ID = "RESULT_DATA_CREATED_SPACE_ID" + private const val RESULT_DATA_CREATED_SPACE_NAME = "RESULT_DATA_CREATED_SPACE_NAME" private const val RESULT_DATA_DEFAULT_ROOM_ID = "RESULT_DATA_DEFAULT_ROOM_ID" private const val RESULT_DATA_CREATED_SPACE_IS_JUST_ME = "RESULT_DATA_CREATED_SPACE_IS_JUST_ME" @@ -172,6 +174,10 @@ class SpaceCreationActivity : SimpleFragmentActivity() { return data?.extras?.getString(RESULT_DATA_CREATED_SPACE_ID) } + fun getCreatedSpaceName(data: Intent?): String? { + return data?.extras?.getString(RESULT_DATA_CREATED_SPACE_NAME) + } + fun getDefaultRoomId(data: Intent?): String? { return data?.extras?.getString(RESULT_DATA_DEFAULT_ROOM_ID) } diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt index eeb2ca30ff..45d1700352 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt @@ -25,7 +25,7 @@ sealed class CreateSpaceEvents : VectorViewEvents { object NavigateToAdd3Pid : CreateSpaceEvents() object NavigateToChoosePrivateType : CreateSpaceEvents() object Dismiss : CreateSpaceEvents() - data class FinishSuccess(val spaceId: String, val defaultRoomId: String?, val topology: SpaceTopology?) : CreateSpaceEvents() + data class FinishSuccess(val spaceId: String, val spaceName: String, val defaultRoomId: String?, val topology: SpaceTopology?) : CreateSpaceEvents() data class ShowModalError(val errorMessage: String) : CreateSpaceEvents() object HideModalLoading : CreateSpaceEvents() data class ShowModalLoading(val message: String?) : CreateSpaceEvents() diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt index b680f77df2..761f0f791b 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt @@ -380,6 +380,7 @@ class CreateSpaceViewModel @AssistedInject constructor( _viewEvents.post( CreateSpaceEvents.FinishSuccess( result.spaceId, + spaceName, result.childIds.firstOrNull(), state.spaceTopology ) @@ -393,6 +394,7 @@ class CreateSpaceViewModel @AssistedInject constructor( _viewEvents.post( CreateSpaceEvents.FinishSuccess( result.spaceId, + spaceName, result.childIds.firstOrNull(), state.spaceTopology ) From 789dffe4df41b75a75ab1b3c93ace568e12d03cf Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 16:51:54 +0200 Subject: [PATCH 13/22] Adds tests related to backstack --- .../java/im/vector/app/SpaceStateHandler.kt | 2 +- .../im/vector/app/SpaceStateHandlerImpl.kt | 14 ++++---- .../features/settings/VectorPreferences.kt | 4 +-- .../app/features/spaces/SpaceListViewModel.kt | 2 +- .../vector/app/SpaceStateHandlerImplTest.kt | 33 ++++++++++++------- .../app/test/fakes/FakeVectorPreferences.kt | 11 ++++++- 6 files changed, 42 insertions(+), 24 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index 2651d62f1a..acc2279d1a 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -59,7 +59,7 @@ interface SpaceStateHandler : DefaultLifecycleObserver { */ fun popSpaceBackstack(): String? - fun getPersistedSpaceBackstack(): List + fun getSpaceBackstack(): List /** * Gets a flow of the selected space for clients to react immediately to space changes. diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index 7bd0ac70a6..d2b3abba59 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -109,13 +109,13 @@ class SpaceStateHandlerImpl @Inject constructor( } private fun addToBackstack(spaceToLeave: RoomSummary?, spaceToSet: RoomSummary?) { - // Only add to the persisted backstack if the space to set is not All Chats, else reset the persisted stack + // Only add to the backstack if the space to set is not All Chats, else clear the backstack if (spaceToSet != null) { - val currentPersistedBackstack = vectorPreferences.getPersistedSpaceBackstack().toMutableList() + val currentPersistedBackstack = vectorPreferences.getSpaceBackstack().toMutableList() currentPersistedBackstack.add(spaceToLeave?.roomId) - vectorPreferences.setPersistedSpaceBackstack(currentPersistedBackstack) + vectorPreferences.setSpaceBackstack(currentPersistedBackstack) } else { - vectorPreferences.setPersistedSpaceBackstack(emptyList()) + vectorPreferences.setSpaceBackstack(emptyList()) } } @@ -143,14 +143,14 @@ class SpaceStateHandlerImpl @Inject constructor( } override fun popSpaceBackstack(): String? { - vectorPreferences.getPersistedSpaceBackstack().toMutableList().apply { + vectorPreferences.getSpaceBackstack().toMutableList().apply { val poppedSpaceId = removeLast() - vectorPreferences.setPersistedSpaceBackstack(this) + vectorPreferences.setSpaceBackstack(this) return poppedSpaceId } } - override fun getPersistedSpaceBackstack() = vectorPreferences.getPersistedSpaceBackstack() + override fun getSpaceBackstack() = vectorPreferences.getSpaceBackstack() override fun getSelectedSpaceFlow() = selectedSpaceFlow diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index cb571da9d3..d973609009 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -1120,7 +1120,7 @@ class VectorPreferences @Inject constructor( * * Only the IDs of the spaces are stored */ - fun setPersistedSpaceBackstack(spaceBackstack: List) { + fun setSpaceBackstack(spaceBackstack: List) { val spaceIdsJoined = spaceBackstack.takeIf { it.isNotEmpty() }?.joinToString(",") defaultPrefs.edit().putString(SETTINGS_PERSISTED_SPACE_BACKSTACK, spaceIdsJoined).apply() } @@ -1128,7 +1128,7 @@ class VectorPreferences @Inject constructor( /** * Gets the space backstack used for up navigation */ - fun getPersistedSpaceBackstack(): List { + fun getSpaceBackstack(): List { val spaceIdsJoined = defaultPrefs.getString(SETTINGS_PERSISTED_SPACE_BACKSTACK, null) return spaceIdsJoined?.takeIf { it.isNotEmpty() }?.split(",").orEmpty() } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt index fdec36add0..d8e5d49ddf 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt @@ -85,7 +85,7 @@ class SpaceListViewModel @AssistedInject constructor( } observeSpaceSummaries() - val spaceHistory = spaceStateHandler.getPersistedSpaceBackstack() + val spaceHistory = spaceStateHandler.getSpaceBackstack() .map { it to it?.let { session.roomService().getRoomSummary(it)?.displayName }.orEmpty() } spaceStateHandler.getSelectedSpaceFlow() .distinctUntilChanged() diff --git a/vector/src/test/java/im/vector/app/SpaceStateHandlerImplTest.kt b/vector/src/test/java/im/vector/app/SpaceStateHandlerImplTest.kt index 36d372cfac..3a01054db2 100644 --- a/vector/src/test/java/im/vector/app/SpaceStateHandlerImplTest.kt +++ b/vector/src/test/java/im/vector/app/SpaceStateHandlerImplTest.kt @@ -21,11 +21,13 @@ import im.vector.app.test.fakes.FakeActiveSessionHolder import im.vector.app.test.fakes.FakeAnalyticsTracker import im.vector.app.test.fakes.FakeSession import im.vector.app.test.fakes.FakeUiStateRepository +import im.vector.app.test.fakes.FakeVectorPreferences import im.vector.app.test.fixtures.RoomSummaryFixture.aRoomSummary import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.amshove.kluent.shouldBe import org.amshove.kluent.shouldBeEqualTo +import org.junit.Before import org.junit.Test internal class SpaceStateHandlerImplTest { @@ -38,14 +40,21 @@ internal class SpaceStateHandlerImplTest { private val uiStateRepository = FakeUiStateRepository() private val activeSessionHolder = FakeActiveSessionHolder(session) private val analyticsTracker = FakeAnalyticsTracker() + private val vectorPreferences = FakeVectorPreferences() private val spaceStateHandler = SpaceStateHandlerImpl( sessionDataSource.instance, uiStateRepository, activeSessionHolder.instance, analyticsTracker, + vectorPreferences.instance, ) + @Before + fun setup() { + vectorPreferences.givenSpaceBackstack(emptyList()) + } + @Test fun `given selected space doesn't exist, when getCurrentSpace, then return null`() { val currentSpace = spaceStateHandler.getCurrentSpace() @@ -77,33 +86,33 @@ internal class SpaceStateHandlerImplTest { } @Test - fun `given is forward navigation and no current space, when setCurrentSpace, then null added to backstack`() { + fun `given not in space and is forward navigation, when setCurrentSpace, then null added to backstack`() { spaceStateHandler.setCurrentSpace(spaceId, session, isForwardNavigation = true) - val backstack = spaceStateHandler.getSpaceBackstack() - - backstack.size shouldBe 1 - backstack.first() shouldBe null + vectorPreferences.verifySetSpaceBackstack(listOf(null)) } @Test - fun `given is forward navigation and is in space, when setCurrentSpace, then previous space added to backstack`() { + fun `given in space and is forward navigation, when setCurrentSpace, then previous space added to backstack`() { spaceStateHandler.setCurrentSpace(spaceId, session, isForwardNavigation = true) spaceStateHandler.setCurrentSpace("secondSpaceId", session, isForwardNavigation = true) - val backstack = spaceStateHandler.getSpaceBackstack() - - backstack.size shouldBe 2 - backstack shouldBeEqualTo listOf(null, spaceId) + vectorPreferences.verifySetSpaceBackstack(listOf(spaceId)) } @Test fun `given is not forward navigation, when setCurrentSpace, then previous space not added to backstack`() { spaceStateHandler.setCurrentSpace(spaceId, session, isForwardNavigation = false) - val backstack = spaceStateHandler.getSpaceBackstack() + vectorPreferences.verifySetSpaceBackstack(listOf(spaceId), inverse = true) + } - backstack.size shouldBe 0 + @Test + fun `given navigating to all chats, when setCurrentSpace, then backstack cleared`() { + spaceStateHandler.setCurrentSpace(spaceId, session) + spaceStateHandler.setCurrentSpace(null, session) + + vectorPreferences.verifySetSpaceBackstack(emptyList()) } @Test diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt index eb8f9ac413..bc761d9016 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt @@ -19,12 +19,21 @@ package im.vector.app.test.fakes import im.vector.app.features.settings.VectorPreferences import io.mockk.every import io.mockk.mockk +import io.mockk.verify class FakeVectorPreferences { - val instance = mockk() + val instance = mockk(relaxUnitFun = true) fun givenUseCompleteNotificationFormat(value: Boolean) { every { instance.useCompleteNotificationFormat() } returns value } + + fun givenSpaceBackstack(value: List) { + every { instance.getSpaceBackstack() } returns value + } + + fun verifySetSpaceBackstack(value: List, inverse: Boolean = false) { + verify(inverse = inverse) { instance.setSpaceBackstack(value) } + } } From 58e1fe4c01c02e8b9e418f5070be40a291c1d1fc Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 17:15:12 +0200 Subject: [PATCH 14/22] Adds DefaultNavigatorTest --- .../features/navigation/DefaultNavigator.kt | 14 +++- .../navigation/DefaultNavigatorTest.kt | 77 +++++++++++++++++++ .../vector/app/test/fakes/FakeRoomService.kt | 6 ++ .../im/vector/app/test/fakes/FakeSession.kt | 2 +- .../app/test/fakes/FakeSpaceStateHandler.kt | 28 +++++++ ...akeSupportedVerificationMethodsProvider.kt | 25 ++++++ .../app/test/fakes/FakeWidgetArgsBuilder.kt | 25 ++++++ 7 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeSupportedVerificationMethodsProvider.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeWidgetArgsBuilder.kt diff --git a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt index c661a66767..0282556900 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt @@ -188,7 +188,15 @@ class DefaultNavigator @Inject constructor( return } spaceStateHandler.setCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) - when (postSwitchSpaceAction) { + handlePostSwitchAction(context, spaceId, postSwitchSpaceAction) + } + + private fun handlePostSwitchAction( + context: Context, + spaceId: String, + action: Navigator.PostSwitchSpaceAction, + ) { + when (action) { Navigator.PostSwitchSpaceAction.None -> { // go back to home if we are showing room details? // This is a bit ugly, but the navigator is supposed to know about the activity stack @@ -204,9 +212,9 @@ class DefaultNavigator @Inject constructor( } is Navigator.PostSwitchSpaceAction.OpenDefaultRoom -> { val args = TimelineArgs( - postSwitchSpaceAction.roomId, + action.roomId, eventId = null, - openShareSpaceForId = spaceId.takeIf { postSwitchSpaceAction.showShareSheet } + openShareSpaceForId = spaceId.takeIf { action.showShareSheet } ) val intent = RoomDetailActivity.newIntent(context, args, false) startActivity(context, intent, false) diff --git a/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt new file mode 100644 index 0000000000..79c514083c --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.navigation + +import im.vector.app.test.fakes.FakeActiveSessionHolder +import im.vector.app.test.fakes.FakeAnalyticsTracker +import im.vector.app.test.fakes.FakeContext +import im.vector.app.test.fakes.FakeSpaceStateHandler +import im.vector.app.test.fakes.FakeSupportedVerificationMethodsProvider +import im.vector.app.test.fakes.FakeVectorFeatures +import im.vector.app.test.fakes.FakeVectorPreferences +import im.vector.app.test.fakes.FakeWidgetArgsBuilder +import im.vector.app.test.fixtures.RoomSummaryFixture.aRoomSummary +import org.junit.Test + +internal class DefaultNavigatorTest { + + private val sessionHolder = FakeActiveSessionHolder() + private val vectorPreferences = FakeVectorPreferences() + private val widgetArgsBuilder = FakeWidgetArgsBuilder() + private val spaceStateHandler = FakeSpaceStateHandler() + private val supportedVerificationMethodsProvider = FakeSupportedVerificationMethodsProvider() + private val features = FakeVectorFeatures() + private val analyticsTracker = FakeAnalyticsTracker() + + private val navigator = DefaultNavigator( + sessionHolder.instance, + vectorPreferences.instance, + widgetArgsBuilder.instance, + spaceStateHandler, + supportedVerificationMethodsProvider.instance, + features, + analyticsTracker, + ) + + /** + * The below tests are by no means all that we want to test in [DefaultNavigator] + * Please add relevant tests as you make changes to or related to other functions in the class + */ + + @Test + fun `when switchToSpace, then current space set`() { + val spaceId = "space-id" + val spaceSummary = aRoomSummary(spaceId) + sessionHolder.fakeSession.fakeRoomService.getRoomSummaryReturns(spaceSummary) + + navigator.switchToSpace(FakeContext().instance, spaceId, Navigator.PostSwitchSpaceAction.None) + + spaceStateHandler.verifySetCurrentSpace(spaceId) + } + + @Test + fun `given non-null overriddenSpaceName, when switchToSpace, then current space set`() { + val spaceId = "space-id" + val spaceSummary = aRoomSummary(spaceId) + sessionHolder.fakeSession.fakeRoomService.getRoomSummaryReturns(spaceSummary) + val overriddenSpaceName = "new-space-name" + + navigator.switchToSpace(FakeContext().instance, spaceId, Navigator.PostSwitchSpaceAction.None, overriddenSpaceName) + + spaceStateHandler.verifySetCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeRoomService.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeRoomService.kt index b09256f747..506e96ba11 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeRoomService.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeRoomService.kt @@ -16,12 +16,18 @@ package im.vector.app.test.fakes +import io.mockk.every import io.mockk.mockk import org.matrix.android.sdk.api.session.room.RoomService +import org.matrix.android.sdk.api.session.room.model.RoomSummary class FakeRoomService( private val fakeRoom: FakeRoom = FakeRoom() ) : RoomService by mockk() { override fun getRoom(roomId: String) = fakeRoom + + fun getRoomSummaryReturns(roomSummary: RoomSummary?) { + every { getRoomSummary(any()) } returns roomSummary + } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSession.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSession.kt index 18af88ba0f..ee016ecae3 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSession.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSession.kt @@ -37,7 +37,7 @@ class FakeSession( val fakeProfileService: FakeProfileService = FakeProfileService(), val fakeHomeServerCapabilitiesService: FakeHomeServerCapabilitiesService = FakeHomeServerCapabilitiesService(), val fakeSharedSecretStorageService: FakeSharedSecretStorageService = FakeSharedSecretStorageService(), - private val fakeRoomService: FakeRoomService = FakeRoomService(), + val fakeRoomService: FakeRoomService = FakeRoomService(), private val fakeEventService: FakeEventService = FakeEventService(), ) : Session by mockk(relaxed = true) { diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt new file mode 100644 index 0000000000..04358d6a1f --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.SpaceStateHandler +import io.mockk.mockk +import io.mockk.verify + +class FakeSpaceStateHandler : SpaceStateHandler by mockk(relaxUnitFun = true) { + + fun verifySetCurrentSpace(spaceId: String, overriddenSpaceName: String? = null) { + verify { setCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSupportedVerificationMethodsProvider.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSupportedVerificationMethodsProvider.kt new file mode 100644 index 0000000000..ecb7c3289b --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSupportedVerificationMethodsProvider.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.features.crypto.verification.SupportedVerificationMethodsProvider +import io.mockk.mockk + +class FakeSupportedVerificationMethodsProvider { + + val instance = mockk() +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeWidgetArgsBuilder.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeWidgetArgsBuilder.kt new file mode 100644 index 0000000000..3a810b2c23 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeWidgetArgsBuilder.kt @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.features.widgets.WidgetArgsBuilder +import io.mockk.mockk + +class FakeWidgetArgsBuilder { + + val instance = mockk() +} From e17e3fe00a130e5dee869d001949c64c1ef439c4 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 18 Aug 2022 17:22:19 +0200 Subject: [PATCH 15/22] Adds changelog file --- changelog.d/6877.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6877.wip diff --git a/changelog.d/6877.wip b/changelog.d/6877.wip new file mode 100644 index 0000000000..415749a297 --- /dev/null +++ b/changelog.d/6877.wip @@ -0,0 +1 @@ +[New Layout] Adds space backstack for back navigation and space sheet header From 7fafd07e2c07e2cb6ddf4dea395bd0a9c79ddaaa Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 19 Aug 2022 13:32:31 +0200 Subject: [PATCH 16/22] Changes changelog file --- changelog.d/6877.wip | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/6877.wip b/changelog.d/6877.wip index 415749a297..d1e1c7c82d 100644 --- a/changelog.d/6877.wip +++ b/changelog.d/6877.wip @@ -1 +1 @@ -[New Layout] Adds space backstack for back navigation and space sheet header +[New Layout] Adds back navigation through spaces From 4162eb8e556756d292166d9148151ef3860b79e0 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 19 Aug 2022 14:23:08 +0200 Subject: [PATCH 17/22] Removes space header --- .../java/im/vector/app/SpaceStateHandler.kt | 6 +- .../im/vector/app/SpaceStateHandlerImpl.kt | 5 +- .../vector/app/features/home/HomeActivity.kt | 2 - .../features/navigation/DefaultNavigator.kt | 3 +- .../app/features/navigation/Navigator.kt | 1 - .../features/spaces/NewSpaceListHeaderItem.kt | 56 ------------------- .../spaces/NewSpaceSummaryController.kt | 8 --- .../features/spaces/SpaceCreationActivity.kt | 6 -- .../app/features/spaces/SpaceListViewModel.kt | 7 +-- .../app/features/spaces/SpaceListViewState.kt | 1 - .../spaces/create/CreateSpaceEvents.kt | 2 +- .../spaces/create/CreateSpaceViewModel.kt | 2 - .../res/layout/item_new_space_list_header.xml | 17 ------ .../navigation/DefaultNavigatorTest.kt | 19 ++----- .../app/test/fakes/FakeDebugNavigator.kt | 22 ++++++++ .../app/test/fakes/FakeSpaceStateHandler.kt | 4 +- 16 files changed, 35 insertions(+), 126 deletions(-) delete mode 100644 vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt delete mode 100644 vector/src/main/res/layout/item_new_space_list_header.xml create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeDebugNavigator.kt diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index acc2279d1a..64b6fc4e8b 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -42,20 +42,18 @@ interface SpaceStateHandler : DefaultLifecycleObserver { * @param session the current active session * @param persistNow if true, the current space will immediately be persisted in shared prefs * @param isForwardNavigation whether this navigation is a forward action to properly handle backstack - * @param overriddenSpaceName overrides the display name of the space being set */ fun setCurrentSpace( spaceId: String?, session: Session? = null, persistNow: Boolean = false, isForwardNavigation: Boolean = true, - overriddenSpaceName: String? = null, ) /** - * Gets the Space ID of the space on top of the backstack + * Gets the Space ID of the space on top of the backstack. * - * May return null to indicate the All Chats space + * May return null to indicate the All Chats space. */ fun popSpaceBackstack(): String? diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt index d2b3abba59..a665b619c0 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandlerImpl.kt @@ -72,13 +72,10 @@ class SpaceStateHandlerImpl @Inject constructor( session: Session?, persistNow: Boolean, isForwardNavigation: Boolean, - overriddenSpaceName: String?, ) { val activeSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return val spaceToLeave = selectedSpaceDataSource.currentValue?.orNull() - val spaceToSet = spaceId?.let { activeSession.getRoomSummary(spaceId) }?.let { - if (overriddenSpaceName != null) it.copy(displayName = overriddenSpaceName) else it - } + val spaceToSet = spaceId?.let { activeSession.getRoomSummary(spaceId) } val sameSpaceSelected = spaceId == spaceToLeave?.roomId if (sameSpaceSelected) { diff --git a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt index a95ca19900..4c84a8e905 100644 --- a/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt +++ b/vector/src/main/java/im/vector/app/features/home/HomeActivity.kt @@ -141,7 +141,6 @@ class HomeActivity : private val createSpaceResultLauncher = registerStartForActivityResult { activityResult -> if (activityResult.resultCode == Activity.RESULT_OK) { val spaceId = SpaceCreationActivity.getCreatedSpaceId(activityResult.data) - val spaceName = SpaceCreationActivity.getCreatedSpaceName(activityResult.data) val defaultRoomId = SpaceCreationActivity.getDefaultRoomId(activityResult.data) val isJustMe = SpaceCreationActivity.isJustMeSpace(activityResult.data) views.drawerLayout.closeDrawer(GravityCompat.START) @@ -159,7 +158,6 @@ class HomeActivity : context = this, spaceId = spaceId, postSwitchOption, - overriddenSpaceName = spaceName, ) roomListSharedActionViewModel.post(RoomListSharedAction.CloseBottomSheet) } diff --git a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt index f30386a477..3b56867a20 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/DefaultNavigator.kt @@ -180,13 +180,12 @@ class DefaultNavigator @Inject constructor( context: Context, spaceId: String, postSwitchSpaceAction: Navigator.PostSwitchSpaceAction, - overriddenSpaceName: String?, ) { if (sessionHolder.getSafeActiveSession()?.getRoomSummary(spaceId) == null) { fatalError("Trying to open an unknown space $spaceId", vectorPreferences.failFast()) return } - spaceStateHandler.setCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) + spaceStateHandler.setCurrentSpace(spaceId) handlePostSwitchAction(context, spaceId, postSwitchSpaceAction) } diff --git a/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt b/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt index bb59c84555..5d86456ba5 100644 --- a/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt +++ b/vector/src/main/java/im/vector/app/features/navigation/Navigator.kt @@ -72,7 +72,6 @@ interface Navigator { context: Context, spaceId: String, postSwitchSpaceAction: PostSwitchSpaceAction, - overriddenSpaceName: String? = null, ) fun openSpacePreview(context: Context, spaceId: String) diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt deleted file mode 100644 index 29538be16e..0000000000 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceListHeaderItem.kt +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (c) 2021 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package im.vector.app.features.spaces - -import android.content.Context -import android.widget.TextView -import com.airbnb.epoxy.EpoxyAttribute -import com.airbnb.epoxy.EpoxyModelClass -import im.vector.app.R -import im.vector.app.core.epoxy.VectorEpoxyHolder -import im.vector.app.core.epoxy.VectorEpoxyModel - -@EpoxyModelClass -abstract class NewSpaceListHeaderItem : VectorEpoxyModel(R.layout.item_new_space_list_header) { - - @EpoxyAttribute var currentSpace: String? = null - @EpoxyAttribute var spaceHistory: List> = emptyList() - - override fun bind(holder: Holder) { - super.bind(holder) - holder.spaceHeader.text = buildSpaceHeaderText(holder.spaceHeader.context) - } - - private fun buildSpaceHeaderText(context: Context): String { - val allChats = context.getString(R.string.all_chats) - var spaceHeaderText = allChats - - val nonRootSpaceHistory = spaceHistory.filter { it.second.isNotEmpty() } - - if (nonRootSpaceHistory.isNotEmpty()) { - spaceHeaderText += " > ${nonRootSpaceHistory.joinToString(" > ") { it.second }}" - } - if (currentSpace != null) { - spaceHeaderText += " > $currentSpace" - } - return spaceHeaderText - } - - class Holder : VectorEpoxyHolder() { - val spaceHeader by bind(R.id.space_header) - } -} diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt index 47b0f23f44..524c7d82af 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt @@ -51,7 +51,6 @@ class NewSpaceSummaryController @Inject constructor( nonNullViewState.selectedSpace, nonNullViewState.rootSpacesOrdered, nonNullViewState.homeAggregateCount, - nonNullViewState.spaceHistory, ) } @@ -60,16 +59,9 @@ class NewSpaceSummaryController @Inject constructor( selectedSpace: RoomSummary?, rootSpaces: List?, homeCount: RoomAggregateNotificationCount, - spaceHistory: List>, ) { val host = this - newSpaceListHeaderItem { - id("space_list_header") - currentSpace(selectedSpace?.displayName) - spaceHistory(spaceHistory) - } - if (selectedSpace != null) { addSubSpaces(selectedSpace, spaceSummaries, homeCount) } else { diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt index aeca7021eb..9fa4a53efc 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceCreationActivity.kt @@ -104,7 +104,6 @@ class SpaceCreationActivity : SimpleFragmentActivity() { is CreateSpaceEvents.FinishSuccess -> { setResult(RESULT_OK, Intent().apply { putExtra(RESULT_DATA_CREATED_SPACE_ID, it.spaceId) - putExtra(RESULT_DATA_CREATED_SPACE_NAME, it.spaceName) putExtra(RESULT_DATA_DEFAULT_ROOM_ID, it.defaultRoomId) putExtra(RESULT_DATA_CREATED_SPACE_IS_JUST_ME, it.topology == SpaceTopology.JustMe) }) @@ -160,7 +159,6 @@ class SpaceCreationActivity : SimpleFragmentActivity() { companion object { private const val RESULT_DATA_CREATED_SPACE_ID = "RESULT_DATA_CREATED_SPACE_ID" - private const val RESULT_DATA_CREATED_SPACE_NAME = "RESULT_DATA_CREATED_SPACE_NAME" private const val RESULT_DATA_DEFAULT_ROOM_ID = "RESULT_DATA_DEFAULT_ROOM_ID" private const val RESULT_DATA_CREATED_SPACE_IS_JUST_ME = "RESULT_DATA_CREATED_SPACE_IS_JUST_ME" @@ -174,10 +172,6 @@ class SpaceCreationActivity : SimpleFragmentActivity() { return data?.extras?.getString(RESULT_DATA_CREATED_SPACE_ID) } - fun getCreatedSpaceName(data: Intent?): String? { - return data?.extras?.getString(RESULT_DATA_CREATED_SPACE_NAME) - } - fun getDefaultRoomId(data: Intent?): String? { return data?.extras?.getString(RESULT_DATA_DEFAULT_ROOM_ID) } diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt index d8e5d49ddf..c7f8e0d411 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewModel.kt @@ -85,15 +85,10 @@ class SpaceListViewModel @AssistedInject constructor( } observeSpaceSummaries() - val spaceHistory = spaceStateHandler.getSpaceBackstack() - .map { it to it?.let { session.roomService().getRoomSummary(it)?.displayName }.orEmpty() } spaceStateHandler.getSelectedSpaceFlow() .distinctUntilChanged() .setOnEach { selectedSpaceOption -> - copy( - selectedSpace = selectedSpaceOption.orNull(), - spaceHistory = spaceHistory, - ) + copy(selectedSpace = selectedSpaceOption.orNull()) } // XXX there should be a way to refactor this and share it diff --git a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt index ebdc9e72ac..f75c336b5d 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/SpaceListViewState.kt @@ -32,6 +32,5 @@ data class SpaceListViewState( val spaceOrderInfo: Map? = null, val spaceOrderLocalEchos: Map? = null, val expandedStates: Map = emptyMap(), - val spaceHistory: List> = emptyList(), // List of space id to display name val homeAggregateCount: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0) ) : MavericksState diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt index 45d1700352..eeb2ca30ff 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceEvents.kt @@ -25,7 +25,7 @@ sealed class CreateSpaceEvents : VectorViewEvents { object NavigateToAdd3Pid : CreateSpaceEvents() object NavigateToChoosePrivateType : CreateSpaceEvents() object Dismiss : CreateSpaceEvents() - data class FinishSuccess(val spaceId: String, val spaceName: String, val defaultRoomId: String?, val topology: SpaceTopology?) : CreateSpaceEvents() + data class FinishSuccess(val spaceId: String, val defaultRoomId: String?, val topology: SpaceTopology?) : CreateSpaceEvents() data class ShowModalError(val errorMessage: String) : CreateSpaceEvents() object HideModalLoading : CreateSpaceEvents() data class ShowModalLoading(val message: String?) : CreateSpaceEvents() diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt index 761f0f791b..b680f77df2 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceViewModel.kt @@ -380,7 +380,6 @@ class CreateSpaceViewModel @AssistedInject constructor( _viewEvents.post( CreateSpaceEvents.FinishSuccess( result.spaceId, - spaceName, result.childIds.firstOrNull(), state.spaceTopology ) @@ -394,7 +393,6 @@ class CreateSpaceViewModel @AssistedInject constructor( _viewEvents.post( CreateSpaceEvents.FinishSuccess( result.spaceId, - spaceName, result.childIds.firstOrNull(), state.spaceTopology ) diff --git a/vector/src/main/res/layout/item_new_space_list_header.xml b/vector/src/main/res/layout/item_new_space_list_header.xml deleted file mode 100644 index bb05f4cb53..0000000000 --- a/vector/src/main/res/layout/item_new_space_list_header.xml +++ /dev/null @@ -1,17 +0,0 @@ - - diff --git a/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt index 79c514083c..bdf1dbd4e6 100644 --- a/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt +++ b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt @@ -19,6 +19,7 @@ package im.vector.app.features.navigation import im.vector.app.test.fakes.FakeActiveSessionHolder import im.vector.app.test.fakes.FakeAnalyticsTracker import im.vector.app.test.fakes.FakeContext +import im.vector.app.test.fakes.FakeDebugNavigator import im.vector.app.test.fakes.FakeSpaceStateHandler import im.vector.app.test.fakes.FakeSupportedVerificationMethodsProvider import im.vector.app.test.fakes.FakeVectorFeatures @@ -36,6 +37,7 @@ internal class DefaultNavigatorTest { private val supportedVerificationMethodsProvider = FakeSupportedVerificationMethodsProvider() private val features = FakeVectorFeatures() private val analyticsTracker = FakeAnalyticsTracker() + private val debugNavigator = FakeDebugNavigator() private val navigator = DefaultNavigator( sessionHolder.instance, @@ -45,11 +47,12 @@ internal class DefaultNavigatorTest { supportedVerificationMethodsProvider.instance, features, analyticsTracker, + debugNavigator, ) /** - * The below tests are by no means all that we want to test in [DefaultNavigator] - * Please add relevant tests as you make changes to or related to other functions in the class + * The below test is by no means all that we want to test in [DefaultNavigator]. + * Please add relevant tests as you make changes to or related to other functions in the class. */ @Test @@ -62,16 +65,4 @@ internal class DefaultNavigatorTest { spaceStateHandler.verifySetCurrentSpace(spaceId) } - - @Test - fun `given non-null overriddenSpaceName, when switchToSpace, then current space set`() { - val spaceId = "space-id" - val spaceSummary = aRoomSummary(spaceId) - sessionHolder.fakeSession.fakeRoomService.getRoomSummaryReturns(spaceSummary) - val overriddenSpaceName = "new-space-name" - - navigator.switchToSpace(FakeContext().instance, spaceId, Navigator.PostSwitchSpaceAction.None, overriddenSpaceName) - - spaceStateHandler.verifySetCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) - } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeDebugNavigator.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeDebugNavigator.kt new file mode 100644 index 0000000000..42074093b3 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeDebugNavigator.kt @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.test.fakes + +import im.vector.app.core.debug.DebugNavigator +import io.mockk.mockk + +class FakeDebugNavigator : DebugNavigator by mockk() diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt index 04358d6a1f..f95b968c91 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSpaceStateHandler.kt @@ -22,7 +22,7 @@ import io.mockk.verify class FakeSpaceStateHandler : SpaceStateHandler by mockk(relaxUnitFun = true) { - fun verifySetCurrentSpace(spaceId: String, overriddenSpaceName: String? = null) { - verify { setCurrentSpace(spaceId, overriddenSpaceName = overriddenSpaceName) } + fun verifySetCurrentSpace(spaceId: String) { + verify { setCurrentSpace(spaceId) } } } From 1bd46e902bacd053b8999276644606c1fbf32cec Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 19 Aug 2022 14:33:10 +0200 Subject: [PATCH 18/22] Fixes documentation lint errors --- vector/src/main/java/im/vector/app/SpaceStateHandler.kt | 2 +- .../im/vector/app/features/settings/VectorPreferences.kt | 8 ++++---- .../app/features/navigation/DefaultNavigatorTest.kt | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt index acc2279d1a..39ed9bf4f6 100644 --- a/vector/src/main/java/im/vector/app/SpaceStateHandler.kt +++ b/vector/src/main/java/im/vector/app/SpaceStateHandler.kt @@ -53,7 +53,7 @@ interface SpaceStateHandler : DefaultLifecycleObserver { ) /** - * Gets the Space ID of the space on top of the backstack + * Gets the Space ID of the space on top of the backstack. * * May return null to indicate the All Chats space */ diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index 32638f5efc..cefbe64d9d 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -1128,10 +1128,10 @@ class VectorPreferences @Inject constructor( } /** - * Sets the space backstack that is used for up navigation - * This needs to be persisted because navigating up through spaces should work across sessions + * Sets the space backstack that is used for up navigation. + * This needs to be persisted because navigating up through spaces should work across sessions. * - * Only the IDs of the spaces are stored + * Only the IDs of the spaces are stored. */ fun setSpaceBackstack(spaceBackstack: List) { val spaceIdsJoined = spaceBackstack.takeIf { it.isNotEmpty() }?.joinToString(",") @@ -1139,7 +1139,7 @@ class VectorPreferences @Inject constructor( } /** - * Gets the space backstack used for up navigation + * Gets the space backstack used for up navigation. */ fun getSpaceBackstack(): List { val spaceIdsJoined = defaultPrefs.getString(SETTINGS_PERSISTED_SPACE_BACKSTACK, null) diff --git a/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt index 79c514083c..6301fe1549 100644 --- a/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt +++ b/vector/src/test/java/im/vector/app/features/navigation/DefaultNavigatorTest.kt @@ -48,8 +48,8 @@ internal class DefaultNavigatorTest { ) /** - * The below tests are by no means all that we want to test in [DefaultNavigator] - * Please add relevant tests as you make changes to or related to other functions in the class + * The below tests are by no means all that we want to test in [DefaultNavigator]. + * Please add relevant tests as you make changes to or related to other functions in the class. */ @Test From d7daca915093a1ef04aa17ead24a92ee304934e1 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 23 Aug 2022 10:36:52 +0200 Subject: [PATCH 19/22] Replaces try catch back handling with check for backstack root --- .../features/home/NewHomeDetailFragment.kt | 34 ++++--------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 99193af7d4..f94cb7e932 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -24,7 +24,6 @@ import android.view.MenuItem import android.view.View import android.view.ViewGroup import androidx.core.view.isVisible -import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import com.airbnb.mvrx.activityViewModel import com.airbnb.mvrx.fragmentViewModel @@ -180,23 +179,11 @@ class NewHomeDetailFragment @Inject constructor( } } - private fun navigateBack() { - val previousSpaceId = spaceStateHandler.getSpaceBackstack().removeLastOrNull() - val parentSpaceId = spaceStateHandler.getCurrentSpace()?.flattenParentIds?.lastOrNull() - setCurrentSpace(previousSpaceId ?: parentSpaceId) - } - private fun setCurrentSpace(spaceId: String?) { spaceStateHandler.setCurrentSpace(spaceId, isForwardNavigation = false) sharedActionViewModel.post(HomeActivitySharedAction.OnCloseSpace) } - private fun handleCallStarted() { - dismissLoadingDialog() - val fragmentTag = HomeTab.DialPad.toFragmentTag() - (childFragmentManager.findFragmentByTag(fragmentTag) as? DialPadFragment)?.clear() - } - override fun onDestroyView() { currentCallsViewPresenter.unBind() super.onDestroyView() @@ -454,20 +441,13 @@ class NewHomeDetailFragment @Inject constructor( } } - private fun DialPadFragment.applyCallback(): DialPadFragment { - callback = object : DialPadFragment.Callback { - override fun onOkClicked(formatted: String?, raw: String?) { - if (raw.isNullOrEmpty()) return - viewModel.handle(HomeDetailAction.StartCallWithPhoneNumber(raw)) - } - } - return this + override fun onBackPressed(toolbarButton: Boolean) = if (spaceStateHandler.isRoot()) { + false + } else { + val lastSpace = spaceStateHandler.popSpaceBackstack() + spaceStateHandler.setCurrentSpace(lastSpace, isForwardNavigation = false) + true } - override fun onBackPressed(toolbarButton: Boolean) = if (spaceStateHandler.getCurrentSpace() != null) { - navigateBack() - true - } catch (e: NoSuchElementException) { - false - } + private fun SpaceStateHandler.isRoot() = getSpaceBackstack().isEmpty() } From 51ccd43724c15540b16ae1228295ea62045aeea7 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 25 Aug 2022 14:16:20 +0200 Subject: [PATCH 20/22] Removes change space from strings xml --- vector/src/main/res/values/strings.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/vector/src/main/res/values/strings.xml b/vector/src/main/res/values/strings.xml index 980524dee8..a43eac3664 100644 --- a/vector/src/main/res/values/strings.xml +++ b/vector/src/main/res/values/strings.xml @@ -139,7 +139,6 @@ All Chats Start Chat Create Room - Change Space Explore Rooms From 457157995af4d6c7b3bae585d30930ea865262c8 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 25 Aug 2022 17:19:39 +0200 Subject: [PATCH 21/22] Removes unused import --- .../java/im/vector/app/features/home/NewHomeDetailFragment.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt index 9e0750c530..e3aebe8594 100644 --- a/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/NewHomeDetailFragment.kt @@ -29,7 +29,6 @@ import com.airbnb.mvrx.activityViewModel import com.airbnb.mvrx.fragmentViewModel import com.airbnb.mvrx.withState import com.google.android.material.appbar.AppBarLayout -import com.google.android.material.badge.BadgeDrawable import dagger.hilt.android.AndroidEntryPoint import im.vector.app.R import im.vector.app.SpaceStateHandler From f62271354e56fb5748e5a4e60e03b2e9fad6353e Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Fri, 26 Aug 2022 15:08:25 +0200 Subject: [PATCH 22/22] Removes unused variable --- .../im/vector/app/features/spaces/NewSpaceSummaryController.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt index 6d8f6b7b36..80421659dd 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/NewSpaceSummaryController.kt @@ -63,8 +63,6 @@ class NewSpaceSummaryController @Inject constructor( homeCount: RoomAggregateNotificationCount, expandedStates: Map, ) { - val host = this - addHomeItem(selectedSpace == null, homeCount) addSpaces(spaceSummaries, selectedSpace, rootSpaces, expandedStates) addCreateItem()