From d2af7e3f9121844dc03c3d5fea3076adfd3b57c6 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 17 Sep 2021 14:26:25 +0200 Subject: [PATCH] Code review --- changelog.d/3752.feature | 2 +- .../src/main/java/im/vector/app/AppStateHandler.kt | 5 ++--- .../vector/app/core/extensions/ViewExtensions.kt | 8 ++++++++ .../app/core/ui/views/BottomSheetActionButton.kt | 4 ++-- .../roomdirectory/createroom/CreateRoomActivity.kt | 2 +- .../roomdirectory/createroom/CreateRoomFragment.kt | 10 +++++----- .../createroom/CreateRoomViewState.kt | 2 +- .../createroom/CreateSubSpaceController.kt | 12 +++++------- .../manage/SpaceAddRoomSpaceChooserBottomSheet.kt | 14 +------------- .../features/spaces/manage/SpaceManageActivity.kt | 2 +- .../bottom_sheet_add_rooms_or_spaces_to_space.xml | 2 +- 11 files changed, 28 insertions(+), 35 deletions(-) diff --git a/changelog.d/3752.feature b/changelog.d/3752.feature index c7b22c72c3..742c015778 100644 --- a/changelog.d/3752.feature +++ b/changelog.d/3752.feature @@ -1 +1 @@ -Better expose adding spaces as Subspaces #3752 \ No newline at end of file +Better expose adding spaces as Subspaces \ No newline at end of file diff --git a/vector/src/main/java/im/vector/app/AppStateHandler.kt b/vector/src/main/java/im/vector/app/AppStateHandler.kt index e0a26b9183..e6bc1b08a2 100644 --- a/vector/src/main/java/im/vector/app/AppStateHandler.kt +++ b/vector/src/main/java/im/vector/app/AppStateHandler.kt @@ -67,11 +67,10 @@ class AppStateHandler @Inject constructor( return selectedSpaceDataSource.currentValue?.orNull()?.let { if (it is RoomGroupingMethod.BySpace) { // try to refresh sum? - return it.spaceSummary?.roomId?.let { activeSessionHolder.getSafeActiveSession()?.getRoomSummary(it) }?.let { + it.spaceSummary?.roomId?.let { activeSessionHolder.getSafeActiveSession()?.getRoomSummary(it) }?.let { RoomGroupingMethod.BySpace(it) } ?: it - } - return it + } else it } } diff --git a/vector/src/main/java/im/vector/app/core/extensions/ViewExtensions.kt b/vector/src/main/java/im/vector/app/core/extensions/ViewExtensions.kt index 92dc76670f..b28f33816b 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/ViewExtensions.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/ViewExtensions.kt @@ -16,11 +16,14 @@ package im.vector.app.core.extensions +import android.graphics.drawable.Drawable import android.text.InputType import android.view.View import android.view.ViewGroup import android.widget.EditText +import android.widget.ImageView import androidx.appcompat.widget.SearchView +import androidx.core.view.isVisible import im.vector.app.R /** @@ -50,3 +53,8 @@ fun View.getMeasurements(): Pair { val height = measuredHeight return width to height } + +fun ImageView.setDrawableOrHide(drawableRes: Drawable?) { + setImageDrawable(drawableRes) + isVisible = drawableRes != null +} diff --git a/vector/src/main/java/im/vector/app/core/ui/views/BottomSheetActionButton.kt b/vector/src/main/java/im/vector/app/core/ui/views/BottomSheetActionButton.kt index 959767dd75..a3e8b3780c 100644 --- a/vector/src/main/java/im/vector/app/core/ui/views/BottomSheetActionButton.kt +++ b/vector/src/main/java/im/vector/app/core/ui/views/BottomSheetActionButton.kt @@ -26,6 +26,7 @@ import androidx.core.view.isGone import androidx.core.view.isInvisible import androidx.core.view.isVisible import im.vector.app.R +import im.vector.app.core.extensions.setDrawableOrHide import im.vector.app.core.extensions.setTextOrHide import im.vector.app.databinding.ViewBottomSheetActionButtonBinding import im.vector.app.features.themes.ThemeUtils @@ -80,8 +81,7 @@ class BottomSheetActionButton @JvmOverloads constructor( var rightIcon: Drawable? = null set(value) { field = value - views.bottomSheetActionIcon.setImageDrawable(value) - views.bottomSheetActionIcon.isVisible = field != null + views.bottomSheetActionIcon.setDrawableOrHide(value) } var tint: Int? = null diff --git a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomActivity.kt b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomActivity.kt index 08659187ad..4afda8a0e9 100644 --- a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomActivity.kt +++ b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomActivity.kt @@ -55,7 +55,7 @@ class CreateRoomActivity : VectorBaseActivity(), ToolbarC CreateRoomFragment::class.java, CreateRoomArgs( intent?.getStringExtra(INITIAL_NAME) ?: "", - isSubSpace = intent?.getBooleanExtra(IS_SPACE, false) ?: false + isSpace = intent?.getBooleanExtra(IS_SPACE, false) ?: false ) ) } diff --git a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomFragment.kt b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomFragment.kt index 316de6729b..70f041bd69 100644 --- a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomFragment.kt +++ b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomFragment.kt @@ -53,7 +53,7 @@ import javax.inject.Inject data class CreateRoomArgs( val initialName: String, val parentSpaceId: String? = null, - val isSubSpace: Boolean = false + val isSpace: Boolean = false ) : Parcelable class CreateRoomFragment @Inject constructor( @@ -98,7 +98,7 @@ class CreateRoomFragment @Inject constructor( override fun onResume() { super.onResume() - views.createRoomTitle.text = getString(if (args.isSubSpace) R.string.create_new_space else R.string.create_new_room) + views.createRoomTitle.text = getString(if (args.isSpace) R.string.create_new_space else R.string.create_new_room) } private fun setupRoomJoinRuleSharedActionViewModel() { @@ -121,7 +121,7 @@ class CreateRoomFragment @Inject constructor( private fun setupWaitingView() { views.waitingView.waitingStatusText.isVisible = true views.waitingView.waitingStatusText.setText( - if (args.isSubSpace) R.string.create_space_in_progress else R.string.create_room_in_progress + if (args.isSpace) R.string.create_space_in_progress else R.string.create_room_in_progress ) } @@ -133,7 +133,7 @@ class CreateRoomFragment @Inject constructor( } private fun setupRecyclerView() { - if (args.isSubSpace) { + if (args.isSpace) { views.createRoomForm.configureWith(createSpaceController) createSpaceController.listener = this } else { @@ -236,7 +236,7 @@ class CreateRoomFragment @Inject constructor( sharedActionViewModel.post(RoomDirectorySharedAction.Close) } else { // Populate list with Epoxy - if (args.isSubSpace) { + if (args.isSpace) { createSpaceController.setData(state) } else { createRoomController.setData(state) diff --git a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewState.kt b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewState.kt index 3d813c3ee3..db56a19904 100644 --- a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewState.kt +++ b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateRoomViewState.kt @@ -44,7 +44,7 @@ data class CreateRoomViewState( constructor(args: CreateRoomArgs) : this( roomName = args.initialName, parentSpaceId = args.parentSpaceId, - isSubSpace = args.isSubSpace + isSubSpace = args.isSpace ) /** diff --git a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateSubSpaceController.kt b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateSubSpaceController.kt index 2dffd2bff8..6d292c85da 100644 --- a/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateSubSpaceController.kt +++ b/vector/src/main/java/im/vector/app/features/roomdirectory/createroom/CreateSubSpaceController.kt @@ -47,13 +47,11 @@ class CreateSubSpaceController @Inject constructor( private fun buildForm(data: CreateRoomViewState, enableFormElement: Boolean) { val host = this - if (data.isSubSpace) { - genericPillItem { - id("beta") - imageRes(R.drawable.ic_beta_pill) - tintIcon(false) - text(host.stringProvider.getString(R.string.space_add_space_to_any_space_you_manage)) - } + genericPillItem { + id("beta") + imageRes(R.drawable.ic_beta_pill) + tintIcon(false) + text(host.stringProvider.getString(R.string.space_add_space_to_any_space_you_manage)) } formEditableSquareAvatarItem { diff --git a/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceAddRoomSpaceChooserBottomSheet.kt b/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceAddRoomSpaceChooserBottomSheet.kt index c4d4b84a18..cf52e77cc5 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceAddRoomSpaceChooserBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceAddRoomSpaceChooserBottomSheet.kt @@ -17,27 +17,17 @@ package im.vector.app.features.spaces.manage import android.os.Bundle -import android.os.Parcelable import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.fragment.app.setFragmentResult -import com.airbnb.mvrx.args import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment import im.vector.app.databinding.BottomSheetAddRoomsOrSpacesToSpaceBinding -import kotlinx.parcelize.Parcelize class SpaceAddRoomSpaceChooserBottomSheet : VectorBaseBottomSheetDialogFragment() { - @Parcelize - data class Args( - val spaceId: String - ) : Parcelable - override val showExpanded = true - private val addSubRoomsArgs: Args by args() - override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = BottomSheetAddRoomsOrSpacesToSpaceBinding.inflate(inflater, container, false) @@ -68,9 +58,7 @@ class SpaceAddRoomSpaceChooserBottomSheet : VectorBaseBottomSheetDialogFragment< fun newInstance(spaceId: String) : SpaceAddRoomSpaceChooserBottomSheet { - return SpaceAddRoomSpaceChooserBottomSheet().apply { - setArguments(Args(spaceId)) - } + return SpaceAddRoomSpaceChooserBottomSheet() } } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageActivity.kt b/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageActivity.kt index 8185347aaa..16a1e18da2 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageActivity.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/manage/SpaceManageActivity.kt @@ -153,7 +153,7 @@ class SpaceManageActivity : VectorBaseActivity(), addFragmentToBackstack( R.id.simpleFragmentContainer, CreateRoomFragment::class.java, - CreateRoomArgs("", parentSpaceId = args?.spaceId, isSubSpace = true) + CreateRoomArgs("", parentSpaceId = args?.spaceId, isSpace = true) ) } SpaceManagedSharedViewEvents.NavigateToManageRooms -> { diff --git a/vector/src/main/res/layout/bottom_sheet_add_rooms_or_spaces_to_space.xml b/vector/src/main/res/layout/bottom_sheet_add_rooms_or_spaces_to_space.xml index e86fb6e58d..e80a55bc16 100644 --- a/vector/src/main/res/layout/bottom_sheet_add_rooms_or_spaces_to_space.xml +++ b/vector/src/main/res/layout/bottom_sheet_add_rooms_or_spaces_to_space.xml @@ -4,7 +4,7 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:background="?colorSurface" + android:background="?android:colorBackground" android:orientation="vertical">