From 9a4d8f87f66f2955d4279925968553c69b52368d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 5 Dec 2019 22:38:49 +0100 Subject: [PATCH] Breadcrumbs: auto-review --- docs/signup.md | 2 +- .../user/accountdata/AccountDataModule.kt | 6 +++--- .../user/accountdata/UpdateBreadcrumbsTask.kt | 4 ++-- .../room/breadcrumbs/BreadcrumbsController.kt | 14 +++++++------- .../room/breadcrumbs/BreadcrumbsFragment.kt | 5 ++--- .../room/breadcrumbs/BreadcrumbsViewModel.kt | 4 ++-- .../room/breadcrumbs/BreadcrumbsViewState.kt | 2 +- .../home/room/detail/RoomDetailActivity.kt | 18 ++++++++++-------- .../home/room/detail/RoomDetailSharedAction.kt | 2 +- .../home/room/list/RoomSummaryController.kt | 2 +- .../main/res/layout/fragment_breadcrumbs.xml | 2 +- 11 files changed, 31 insertions(+), 30 deletions(-) diff --git a/docs/signup.md b/docs/signup.md index 7372ad2204..995f5d50a6 100644 --- a/docs/signup.md +++ b/docs/signup.md @@ -17,7 +17,7 @@ Client request the sign-up flows, once the homeserver is chosen by the user and } ``` -We get the flows with a 401, which also means the the registration is possible on this homeserver. +We get the flows with a 401, which also means that the registration is possible on this homeserver. ```json { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/AccountDataModule.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/AccountDataModule.kt index 37b63dcd07..1fd4162d0a 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/AccountDataModule.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/AccountDataModule.kt @@ -35,11 +35,11 @@ internal abstract class AccountDataModule { } @Binds - abstract fun bindUpdateUserAccountDataTask(updateUserAccountDataTask: DefaultUpdateUserAccountDataTask): UpdateUserAccountDataTask + abstract fun bindUpdateUserAccountDataTask(task: DefaultUpdateUserAccountDataTask): UpdateUserAccountDataTask @Binds - abstract fun bindSaveBreadcrumbsTask(saveBreadcrumbsTask: DefaultSaveBreadcrumbsTask): SaveBreadcrumbsTask + abstract fun bindSaveBreadcrumbsTask(task: DefaultSaveBreadcrumbsTask): SaveBreadcrumbsTask @Binds - abstract fun bindUpdateBreadcrumsTask(saveBreadcrumbsTask: DefaultUpdateBreadcrumbsTask): UpdateBreadcrumbsTask + abstract fun bindUpdateBreadcrumsTask(task: DefaultUpdateBreadcrumbsTask): UpdateBreadcrumbsTask } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/UpdateBreadcrumbsTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/UpdateBreadcrumbsTask.kt index bde4d138af..b11072a0bd 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/UpdateBreadcrumbsTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/user/accountdata/UpdateBreadcrumbsTask.kt @@ -46,9 +46,9 @@ internal class DefaultUpdateBreadcrumbsTask @Inject constructor( ?.recentRoomIds ?.apply { // Modify the list to add the newTopRoomId first - // Ensure the roomId is not already in the list + // Ensure the newTopRoomId is not already in the list remove(params.newTopRoomId) - // Add the room at first position + // Add the newTopRoomId at first position add(0, params.newTopRoomId) } ?.take(MAX_BREADCRUMBS_ROOMS_NUMBER) diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsController.kt b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsController.kt index ea359ee2f7..3e400b37ea 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsController.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsController.kt @@ -18,7 +18,6 @@ package im.vector.riotx.features.home.room.breadcrumbs import android.view.View import com.airbnb.epoxy.EpoxyController -import im.vector.matrix.android.api.session.room.model.RoomSummary import im.vector.riotx.core.utils.DebouncedClickListener import im.vector.riotx.features.home.AvatarRenderer import javax.inject.Inject @@ -33,7 +32,7 @@ class BreadcrumbsController @Inject constructor( init { // We are requesting a model build directly as the first build of epoxy is on the main thread. - // It avoids to build the the whole list of rooms on the main thread. + // It avoids to build the whole list of breadcrumbs on the main thread. requestModelBuild() } @@ -43,11 +42,12 @@ class BreadcrumbsController @Inject constructor( } override fun buildModels() { - val nonNullViewState = viewState ?: return + val safeViewState = viewState ?: return - // TODO Display a loading, or an empty state + // An empty breadcrumbs list can only be temporary because when entering in a room, + // this one is added to the breadcrumbs - nonNullViewState.asyncRooms.invoke() + safeViewState.asyncBreadcrumbs.invoke() ?.forEach { breadcrumbsItem { id(it.roomId) @@ -61,7 +61,7 @@ class BreadcrumbsController @Inject constructor( hasDraft(it.userDrafts.isNotEmpty()) itemClickListener( DebouncedClickListener(View.OnClickListener { _ -> - listener?.onRoomClicked(it) + listener?.onBreadcrumbClicked(it.roomId) }) ) } @@ -69,6 +69,6 @@ class BreadcrumbsController @Inject constructor( } interface Listener { - fun onRoomClicked(room: RoomSummary) + fun onBreadcrumbClicked(roomId: String) } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsFragment.kt index 036208ec6a..e57a2ddac4 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsFragment.kt @@ -20,7 +20,6 @@ import android.os.Bundle import android.view.View import androidx.recyclerview.widget.LinearLayoutManager import com.airbnb.mvrx.fragmentViewModel -import im.vector.matrix.android.api.session.room.model.RoomSummary import im.vector.riotx.R import im.vector.riotx.core.platform.VectorBaseFragment import im.vector.riotx.features.home.room.detail.RoomDetailSharedAction @@ -65,7 +64,7 @@ class BreadcrumbsFragment @Inject constructor( // BreadcrumbsController.Listener ************************************************************** - override fun onRoomClicked(room: RoomSummary) { - sharedActionViewModel.post(RoomDetailSharedAction.OpenRoom(room.roomId)) + override fun onBreadcrumbClicked(roomId: String) { + sharedActionViewModel.post(RoomDetailSharedAction.SwitchToRoom(roomId)) } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewModel.kt index 4b462a05a6..83e9e0fb3f 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewModel.kt @@ -59,8 +59,8 @@ class BreadcrumbsViewModel @AssistedInject constructor(@Assisted initialState: B session.rx() .liveBreadcrumbs() .observeOn(Schedulers.computation()) - .execute { asyncRooms -> - copy(asyncRooms = asyncRooms) + .execute { asyncBreadcrumbs -> + copy(asyncBreadcrumbs = asyncBreadcrumbs) } } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewState.kt b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewState.kt index cb00db4c9f..7cc634c8b0 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewState.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/breadcrumbs/BreadcrumbsViewState.kt @@ -22,5 +22,5 @@ import com.airbnb.mvrx.Uninitialized import im.vector.matrix.android.api.session.room.model.RoomSummary data class BreadcrumbsViewState( - val asyncRooms: Async> = Uninitialized + val asyncBreadcrumbs: Async> = Uninitialized ) : MvRxState diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailActivity.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailActivity.kt index bdc3b59eff..431c9e6395 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailActivity.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailActivity.kt @@ -57,14 +57,7 @@ class RoomDetailActivity : VectorBaseActivity(), ToolbarConfigurable { .observe() .subscribe { sharedAction -> when (sharedAction) { - is RoomDetailSharedAction.OpenRoom -> { - drawerLayout.closeDrawer(GravityCompat.START) - // Do not replace the Fragment if it's the same roomId - if (currentRoomId != sharedAction.roomId) { - currentRoomId = sharedAction.roomId - replaceFragment(R.id.roomDetailContainer, RoomDetailFragment::class.java, RoomDetailArgs(sharedAction.roomId)) - } - } + is RoomDetailSharedAction.SwitchToRoom -> switchToRoom(sharedAction) } } .disposeOnDestroy() @@ -72,6 +65,15 @@ class RoomDetailActivity : VectorBaseActivity(), ToolbarConfigurable { drawerLayout.addDrawerListener(drawerListener) } + private fun switchToRoom(switchToRoom: RoomDetailSharedAction.SwitchToRoom) { + drawerLayout.closeDrawer(GravityCompat.START) + // Do not replace the Fragment if it's the same roomId + if (currentRoomId != switchToRoom.roomId) { + currentRoomId = switchToRoom.roomId + replaceFragment(R.id.roomDetailContainer, RoomDetailFragment::class.java, RoomDetailArgs(switchToRoom.roomId)) + } + } + override fun onDestroy() { drawerLayout.removeDrawerListener(drawerListener) super.onDestroy() diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailSharedAction.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailSharedAction.kt index 6a88bb1f13..95dd34ebb8 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailSharedAction.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailSharedAction.kt @@ -22,5 +22,5 @@ import im.vector.riotx.core.platform.VectorSharedAction * Supported navigation actions for [RoomDetailActivity] */ sealed class RoomDetailSharedAction : VectorSharedAction { - data class OpenRoom(val roomId: String) : RoomDetailSharedAction() + data class SwitchToRoom(val roomId: String) : RoomDetailSharedAction() } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomSummaryController.kt b/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomSummaryController.kt index 4107bf01b2..6ffe37cb15 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomSummaryController.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomSummaryController.kt @@ -42,7 +42,7 @@ class RoomSummaryController @Inject constructor(private val stringProvider: Stri init { // We are requesting a model build directly as the first build of epoxy is on the main thread. - // It avoids to build the the whole list of rooms on the main thread. + // It avoids to build the whole list of rooms on the main thread. requestModelBuild() } diff --git a/vector/src/main/res/layout/fragment_breadcrumbs.xml b/vector/src/main/res/layout/fragment_breadcrumbs.xml index c17b35fdee..22cceadc03 100644 --- a/vector/src/main/res/layout/fragment_breadcrumbs.xml +++ b/vector/src/main/res/layout/fragment_breadcrumbs.xml @@ -5,4 +5,4 @@ android:layout_width="wrap_content" android:layout_height="match_parent" android:background="?riotx_background" - tools:listitem="@layout/item_breadcrumbs" /> \ No newline at end of file + tools:listitem="@layout/item_breadcrumbs" />