Code review cleaning

This commit is contained in:
Valere 2021-06-15 09:48:45 +02:00
parent 52f0d8a1d2
commit bc120da985
6 changed files with 29 additions and 72 deletions

View File

@ -20,7 +20,6 @@ import android.content.Context
import android.content.Intent import android.content.Intent
import android.os.Bundle import android.os.Bundle
import androidx.fragment.app.Fragment import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import com.airbnb.mvrx.Loading import com.airbnb.mvrx.Loading
import com.airbnb.mvrx.viewModel import com.airbnb.mvrx.viewModel
import com.airbnb.mvrx.withState 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.di.ScreenComponent
import im.vector.app.core.extensions.toMvRxBundle import im.vector.app.core.extensions.toMvRxBundle
import im.vector.app.core.platform.SimpleFragmentActivity 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.ChoosePrivateSpaceTypeFragment
import im.vector.app.features.spaces.create.ChooseSpaceTypeFragment import im.vector.app.features.spaces.create.ChooseSpaceTypeFragment
import im.vector.app.features.spaces.create.CreateSpaceAction 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 im.vector.app.features.spaces.create.SpaceType
import javax.inject.Inject import javax.inject.Inject
class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Factory, BetaWarningBottomSheet.InteractionListener { class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Factory {
@Inject lateinit var viewModelFactory: CreateSpaceViewModel.Factory @Inject lateinit var viewModelFactory: CreateSpaceViewModel.Factory
@ -53,29 +51,8 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac
val viewModel: CreateSpaceViewModel by viewModel() 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?) { override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState) super.onCreate(savedInstanceState)
supportFragmentManager.registerFragmentLifecycleCallbacks(fragmentLifecycleCallbacks, false)
if (isFirstCreation()) { if (isFirstCreation()) {
when (withState(viewModel) { it.step }) { when (withState(viewModel) { it.step }) {
CreateSpaceState.Step.ChooseType -> { CreateSpaceState.Step.ChooseType -> {
@ -94,11 +71,6 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac
} }
} }
override fun onDestroy() {
supportFragmentManager.unregisterFragmentLifecycleCallbacks(fragmentLifecycleCallbacks)
super.onDestroy()
}
override fun initUiAndData() { override fun initUiAndData() {
super.initUiAndData() super.initUiAndData()
@ -206,8 +178,4 @@ class SpaceCreationActivity : SimpleFragmentActivity(), CreateSpaceViewModel.Fac
} }
override fun create(initialState: CreateSpaceState): CreateSpaceViewModel = viewModelFactory.create(initialState) override fun create(initialState: CreateSpaceState): CreateSpaceViewModel = viewModelFactory.create(initialState)
override fun betaWarningOnContinueAnyway() {
viewModel.handle(CreateSpaceAction.ConfirmBetaWarning)
}
} }

View File

@ -20,33 +20,27 @@ import android.os.Bundle
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import androidx.fragment.app.setFragmentResult
import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment import im.vector.app.core.platform.VectorBaseBottomSheetDialogFragment
import im.vector.app.databinding.BottomSheetSpaceCreatePrivateWarningBinding import im.vector.app.databinding.BottomSheetSpaceCreatePrivateWarningBinding
class BetaWarningBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetSpaceCreatePrivateWarningBinding>() { class BetaWarningBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetSpaceCreatePrivateWarningBinding>() {
interface InteractionListener {
fun betaWarningOnContinueAnyway()
}
var interactionListener: InteractionListener? = null
override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) =
BottomSheetSpaceCreatePrivateWarningBinding.inflate(inflater, container, false) BottomSheetSpaceCreatePrivateWarningBinding.inflate(inflater, container, false)
override val showExpanded = true override val showExpanded = true
override fun onDetach() {
interactionListener = null
super.onDetach()
}
override fun onViewCreated(view: View, savedInstanceState: Bundle?) { override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState) super.onViewCreated(view, savedInstanceState)
views.continueButton.debouncedClicks { views.continueButton.debouncedClicks {
interactionListener?.betaWarningOnContinueAnyway() setFragmentResult(REQUEST_KEY, Bundle.EMPTY)
dismiss() dismiss()
} }
} }
companion object {
const val REQUEST_KEY = "BetaWarningBottomSheet"
}
} }

View File

@ -20,6 +20,7 @@ import android.os.Bundle
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import androidx.fragment.app.setFragmentResultListener
import com.airbnb.mvrx.activityViewModel import com.airbnb.mvrx.activityViewModel
import im.vector.app.R import im.vector.app.R
import im.vector.app.core.epoxy.onClick import im.vector.app.core.epoxy.onClick
@ -38,6 +39,13 @@ class ChoosePrivateSpaceTypeFragment @Inject constructor(
override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) = override fun getBinding(inflater: LayoutInflater, container: ViewGroup?) =
FragmentSpaceCreateChoosePrivateModelBinding.inflate(layoutInflater, container, false) 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?) { override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState) super.onViewCreated(view, savedInstanceState)
@ -52,14 +60,6 @@ class ChoosePrivateSpaceTypeFragment @Inject constructor(
sharedViewModel.subscribe { state -> sharedViewModel.subscribe { state ->
views.accessInfoHelpText.text = stringProvider.getString(R.string.create_spaces_make_sure_access, state.name ?: "") 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 { override fun onBackPressed(toolbarButton: Boolean): Boolean {

View File

@ -29,5 +29,4 @@ sealed class CreateSpaceAction : VectorViewModelAction {
object NextFromDefaultRooms : CreateSpaceAction() object NextFromDefaultRooms : CreateSpaceAction()
data class DefaultRoomNameChanged(val index: Int, val name: String) : CreateSpaceAction() data class DefaultRoomNameChanged(val index: Int, val name: String) : CreateSpaceAction()
data class SetSpaceTopology(val topology: SpaceTopology) : CreateSpaceAction() data class SetSpaceTopology(val topology: SpaceTopology) : CreateSpaceAction()
object ConfirmBetaWarning : CreateSpaceAction()
} }

View File

@ -27,5 +27,4 @@ sealed class CreateSpaceEvents : VectorViewEvents {
data class FinishSuccess(val spaceId: 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() data class ShowModalError(val errorMessage: String) : CreateSpaceEvents()
object HideModalLoading : CreateSpaceEvents() object HideModalLoading : CreateSpaceEvents()
object OnConfirmBetaWarning : CreateSpaceEvents()
} }

View File

@ -70,7 +70,7 @@ class CreateSpaceViewModel @AssistedInject constructor(
override fun handle(action: CreateSpaceAction) { override fun handle(action: CreateSpaceAction) {
when (action) { when (action) {
is CreateSpaceAction.SetRoomType -> { is CreateSpaceAction.SetRoomType -> {
setState { setState {
copy( copy(
step = CreateSpaceState.Step.SetDetails, step = CreateSpaceState.Step.SetDetails,
@ -79,7 +79,7 @@ class CreateSpaceViewModel @AssistedInject constructor(
} }
_viewEvents.post(CreateSpaceEvents.NavigateToDetails) _viewEvents.post(CreateSpaceEvents.NavigateToDetails)
} }
is CreateSpaceAction.NameChanged -> { is CreateSpaceAction.NameChanged -> {
setState { setState {
copy( copy(
nameInlineError = null, nameInlineError = null,
@ -87,20 +87,20 @@ class CreateSpaceViewModel @AssistedInject constructor(
) )
} }
} }
is CreateSpaceAction.TopicChanged -> { is CreateSpaceAction.TopicChanged -> {
setState { setState {
copy( copy(
topic = action.topic topic = action.topic
) )
} }
} }
CreateSpaceAction.OnBackPressed -> { CreateSpaceAction.OnBackPressed -> {
handleBackNavigation() handleBackNavigation()
} }
CreateSpaceAction.NextFromDetails -> { CreateSpaceAction.NextFromDetails -> {
handleNextFromDetails() handleNextFromDetails()
} }
CreateSpaceAction.NextFromDefaultRooms -> { CreateSpaceAction.NextFromDefaultRooms -> {
handleNextFromDefaultRooms() handleNextFromDefaultRooms()
} }
is CreateSpaceAction.DefaultRoomNameChanged -> { is CreateSpaceAction.DefaultRoomNameChanged -> {
@ -112,21 +112,18 @@ class CreateSpaceViewModel @AssistedInject constructor(
) )
} }
} }
is CreateSpaceAction.SetAvatar -> { is CreateSpaceAction.SetAvatar -> {
setState { copy(avatarUri = action.uri) } setState { copy(avatarUri = action.uri) }
} }
is CreateSpaceAction.SetSpaceTopology -> { is CreateSpaceAction.SetSpaceTopology -> {
handleSetTopology(action) handleSetTopology(action)
} }
CreateSpaceAction.ConfirmBetaWarning -> {
_viewEvents.post(CreateSpaceEvents.OnConfirmBetaWarning)
}
}.exhaustive }.exhaustive
} }
private fun handleSetTopology(action: CreateSpaceAction.SetSpaceTopology) { private fun handleSetTopology(action: CreateSpaceAction.SetSpaceTopology) {
when (action.topology) { when (action.topology) {
SpaceTopology.JustMe -> { SpaceTopology.JustMe -> {
setState { setState {
copy( copy(
spaceTopology = SpaceTopology.JustMe, spaceTopology = SpaceTopology.JustMe,
@ -149,10 +146,10 @@ class CreateSpaceViewModel @AssistedInject constructor(
private fun handleBackNavigation() = withState { state -> private fun handleBackNavigation() = withState { state ->
when (state.step) { when (state.step) {
CreateSpaceState.Step.ChooseType -> { CreateSpaceState.Step.ChooseType -> {
_viewEvents.post(CreateSpaceEvents.Dismiss) _viewEvents.post(CreateSpaceEvents.Dismiss)
} }
CreateSpaceState.Step.SetDetails -> { CreateSpaceState.Step.SetDetails -> {
setState { setState {
copy( copy(
step = CreateSpaceState.Step.ChooseType, step = CreateSpaceState.Step.ChooseType,
@ -162,7 +159,7 @@ class CreateSpaceViewModel @AssistedInject constructor(
} }
_viewEvents.post(CreateSpaceEvents.NavigateToChooseType) _viewEvents.post(CreateSpaceEvents.NavigateToChooseType)
} }
CreateSpaceState.Step.AddRooms -> { CreateSpaceState.Step.AddRooms -> {
if (state.spaceType == SpaceType.Private && state.spaceTopology == SpaceTopology.MeAndTeammates) { if (state.spaceType == SpaceType.Private && state.spaceTopology == SpaceTopology.MeAndTeammates) {
setState { setState {
copy( copy(
@ -237,7 +234,7 @@ class CreateSpaceViewModel @AssistedInject constructor(
) )
) )
when (result) { when (result) {
is CreateSpaceTaskResult.Success -> { is CreateSpaceTaskResult.Success -> {
setState { setState {
copy(creationResult = Success(result.spaceId)) 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? // XXX what can we do here?
setState { setState {
copy(creationResult = Success(result.spaceId)) copy(creationResult = Success(result.spaceId))