From bc120da985dc0f9159a90d557447c7ff406df554 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 15 Jun 2021 09:48:45 +0200 Subject: [PATCH] Code review cleaning --- .../features/spaces/SpaceCreationActivity.kt | 34 +------------------ .../spaces/create/BetaWarningBottomSheet.kt | 18 ++++------ .../create/ChoosePrivateSpaceTypeFragment.kt | 16 ++++----- .../spaces/create/CreateSpaceAction.kt | 1 - .../spaces/create/CreateSpaceEvents.kt | 1 - .../spaces/create/CreateSpaceViewModel.kt | 31 ++++++++--------- 6 files changed, 29 insertions(+), 72 deletions(-) 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 dbcb9fd5bf..6bf31dd5ce 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 @@ -20,7 +20,6 @@ import android.content.Context import android.content.Intent import android.os.Bundle import androidx.fragment.app.Fragment -import androidx.fragment.app.FragmentManager import com.airbnb.mvrx.Loading import com.airbnb.mvrx.viewModel import com.airbnb.mvrx.withState @@ -29,7 +28,6 @@ import im.vector.app.R import im.vector.app.core.di.ScreenComponent import im.vector.app.core.extensions.toMvRxBundle import im.vector.app.core.platform.SimpleFragmentActivity -import im.vector.app.features.spaces.create.BetaWarningBottomSheet import im.vector.app.features.spaces.create.ChoosePrivateSpaceTypeFragment import im.vector.app.features.spaces.create.ChooseSpaceTypeFragment import im.vector.app.features.spaces.create.CreateSpaceAction @@ -42,7 +40,7 @@ import im.vector.app.features.spaces.create.SpaceTopology import im.vector.app.features.spaces.create.SpaceType import javax.inject.Inject -class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Factory, BetaWarningBottomSheet.InteractionListener { +class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Factory { @Inject lateinit var viewModelFactory: CreateSpaceViewModel.Factory @@ -53,29 +51,8 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac val viewModel: CreateSpaceViewModel by viewModel() - private val fragmentLifecycleCallbacks = object : FragmentManager.FragmentLifecycleCallbacks() { - override fun onFragmentAttached(fm: FragmentManager, f: Fragment, context: Context) { - when (f) { - is BetaWarningBottomSheet -> { - f.interactionListener = this@SpaceCreationActivity - } - } - super.onFragmentAttached(fm, f, context) - } - - override fun onFragmentDetached(fm: FragmentManager, f: Fragment) { - when (f) { - is BetaWarningBottomSheet -> { - f.interactionListener = null - } - } - super.onFragmentDetached(fm, f) - } - } - override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - supportFragmentManager.registerFragmentLifecycleCallbacks(fragmentLifecycleCallbacks, false) if (isFirstCreation()) { when (withState(viewModel) { it.step }) { CreateSpaceState.Step.ChooseType -> { @@ -94,11 +71,6 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac } } - override fun onDestroy() { - supportFragmentManager.unregisterFragmentLifecycleCallbacks(fragmentLifecycleCallbacks) - super.onDestroy() - } - override fun initUiAndData() { super.initUiAndData() @@ -206,8 +178,4 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac } override fun create(initialState: CreateSpaceState): CreateSpaceViewModel = viewModelFactory.create(initialState) - - override fun betaWarningOnContinueAnyway() { - viewModel.handle(CreateSpaceAction.ConfirmBetaWarning) - } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/BetaWarningBottomSheet.kt b/vector/src/main/java/im/vector/app/features/spaces/create/BetaWarningBottomSheet.kt index 82e8721b07..fc3d98e6b8 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/BetaWarningBottomSheet.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/BetaWarningBottomSheet.kt @@ -20,33 +20,27 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.fragment.app.setFragmentResult import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment import im.vector.app.databinding.BottomSheetSpaceCreatePrivateWarningBinding class BetaWarningBottomSheet : VectorBaseBottomSheetDialogFragment() { - interface InteractionListener { - fun betaWarningOnContinueAnyway() - } - - var interactionListener: InteractionListener? = null - override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = BottomSheetSpaceCreatePrivateWarningBinding.inflate(inflater, container, false) override val showExpanded = true - override fun onDetach() { - interactionListener = null - super.onDetach() - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) views.continueButton.debouncedClicks { - interactionListener?.betaWarningOnContinueAnyway() + setFragmentResult(REQUEST_KEY, Bundle.EMPTY) dismiss() } } + + companion object { + const val REQUEST_KEY = "BetaWarningBottomSheet" + } } diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/ChoosePrivateSpaceTypeFragment.kt b/vector/src/main/java/im/vector/app/features/spaces/create/ChoosePrivateSpaceTypeFragment.kt index 49fe9a7dec..4031b56c1d 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/ChoosePrivateSpaceTypeFragment.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/ChoosePrivateSpaceTypeFragment.kt @@ -20,6 +20,7 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.fragment.app.setFragmentResultListener import com.airbnb.mvrx.activityViewModel import im.vector.app.R import im.vector.app.core.epoxy.onClick @@ -38,6 +39,13 @@ class ChoosePrivateSpaceTypeFragment @Inject constructor( override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = FragmentSpaceCreateChoosePrivateModelBinding.inflate(layoutInflater, container, false) + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setFragmentResultListener(BetaWarningBottomSheet.REQUEST_KEY) { _, _ -> + sharedViewModel.handle(CreateSpaceAction.SetSpaceTopology(SpaceTopology.MeAndTeammates)) + } + } + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) @@ -52,14 +60,6 @@ class ChoosePrivateSpaceTypeFragment @Inject constructor( sharedViewModel.subscribe { state -> views.accessInfoHelpText.text = stringProvider.getString(R.string.create_spaces_make_sure_access, state.name ?: "") } - - sharedViewModel.observeViewEvents { - when (it) { - CreateSpaceEvents.OnConfirmBetaWarning -> { - sharedViewModel.handle(CreateSpaceAction.SetSpaceTopology(SpaceTopology.MeAndTeammates)) - } - } - } } override fun onBackPressed(toolbarButton: Boolean): Boolean { diff --git a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceAction.kt b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceAction.kt index 291679764f..cd31b40354 100644 --- a/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceAction.kt +++ b/vector/src/main/java/im/vector/app/features/spaces/create/CreateSpaceAction.kt @@ -29,5 +29,4 @@ sealed class CreateSpaceAction : VectorViewModelAction { object NextFromDefaultRooms : CreateSpaceAction() data class DefaultRoomNameChanged(val index: Int, val name: String) : CreateSpaceAction() data class SetSpaceTopology(val topology: SpaceTopology) : CreateSpaceAction() - object ConfirmBetaWarning : CreateSpaceAction() } 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 3e40fc6ad7..c3fa2b2068 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 @@ -27,5 +27,4 @@ sealed class CreateSpaceEvents : VectorViewEvents { data class FinishSuccess(val spaceId: String, val defaultRoomId: String?, val topology: SpaceTopology?) : CreateSpaceEvents() data class ShowModalError(val errorMessage: String) : CreateSpaceEvents() object HideModalLoading : CreateSpaceEvents() - object OnConfirmBetaWarning : 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 3da6a7554f..aff342cea7 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 @@ -70,7 +70,7 @@ class CreateSpaceViewModel @AssistedInject constructor( override fun handle(action: CreateSpaceAction) { when (action) { - is CreateSpaceAction.SetRoomType -> { + is CreateSpaceAction.SetRoomType -> { setState { copy( step = CreateSpaceState.Step.SetDetails, @@ -79,7 +79,7 @@ class CreateSpaceViewModel @AssistedInject constructor( } _viewEvents.post(CreateSpaceEvents.NavigateToDetails) } - is CreateSpaceAction.NameChanged -> { + is CreateSpaceAction.NameChanged -> { setState { copy( nameInlineError = null, @@ -87,20 +87,20 @@ class CreateSpaceViewModel @AssistedInject constructor( ) } } - is CreateSpaceAction.TopicChanged -> { + is CreateSpaceAction.TopicChanged -> { setState { copy( topic = action.topic ) } } - CreateSpaceAction.OnBackPressed -> { + CreateSpaceAction.OnBackPressed -> { handleBackNavigation() } - CreateSpaceAction.NextFromDetails -> { + CreateSpaceAction.NextFromDetails -> { handleNextFromDetails() } - CreateSpaceAction.NextFromDefaultRooms -> { + CreateSpaceAction.NextFromDefaultRooms -> { handleNextFromDefaultRooms() } is CreateSpaceAction.DefaultRoomNameChanged -> { @@ -112,21 +112,18 @@ class CreateSpaceViewModel @AssistedInject constructor( ) } } - is CreateSpaceAction.SetAvatar -> { + is CreateSpaceAction.SetAvatar -> { setState { copy(avatarUri = action.uri) } } - is CreateSpaceAction.SetSpaceTopology -> { + is CreateSpaceAction.SetSpaceTopology -> { handleSetTopology(action) } - CreateSpaceAction.ConfirmBetaWarning -> { - _viewEvents.post(CreateSpaceEvents.OnConfirmBetaWarning) - } }.exhaustive } private fun handleSetTopology(action: CreateSpaceAction.SetSpaceTopology) { when (action.topology) { - SpaceTopology.JustMe -> { + SpaceTopology.JustMe -> { setState { copy( spaceTopology = SpaceTopology.JustMe, @@ -149,10 +146,10 @@ class CreateSpaceViewModel @AssistedInject constructor( private fun handleBackNavigation() = withState { state -> when (state.step) { - CreateSpaceState.Step.ChooseType -> { + CreateSpaceState.Step.ChooseType -> { _viewEvents.post(CreateSpaceEvents.Dismiss) } - CreateSpaceState.Step.SetDetails -> { + CreateSpaceState.Step.SetDetails -> { setState { copy( step = CreateSpaceState.Step.ChooseType, @@ -162,7 +159,7 @@ class CreateSpaceViewModel @AssistedInject constructor( } _viewEvents.post(CreateSpaceEvents.NavigateToChooseType) } - CreateSpaceState.Step.AddRooms -> { + CreateSpaceState.Step.AddRooms -> { if (state.spaceType == SpaceType.Private && state.spaceTopology == SpaceTopology.MeAndTeammates) { setState { copy( @@ -237,7 +234,7 @@ class CreateSpaceViewModel @AssistedInject constructor( ) ) when (result) { - is CreateSpaceTaskResult.Success -> { + is CreateSpaceTaskResult.Success -> { setState { copy(creationResult = Success(result.spaceId)) } @@ -249,7 +246,7 @@ class CreateSpaceViewModel @AssistedInject constructor( ) ) } - is CreateSpaceTaskResult.PartialSuccess -> { + is CreateSpaceTaskResult.PartialSuccess -> { // XXX what can we do here? setState { copy(creationResult = Success(result.spaceId))