Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class LayoutsItemViewHolder(
RecyclerView.HORIZONTAL,
false
).apply { initialPrefetchItemCount = prefetchItemCount }
setRecycledViewPool(RecyclerView.RecycledViewPool())
adapter = LayoutsAdapter(parent.context, dimensionProvider)

addOnScrollListener(object : OnScrollListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import android.view.MenuItem
import androidx.activity.viewModels
import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import kotlinx.coroutines.cancel
import dagger.hilt.android.AndroidEntryPoint
import org.wordpress.android.R
import org.wordpress.android.ui.ActivityLauncher
Expand Down Expand Up @@ -69,6 +70,7 @@ class SiteCreationActivity : LocaleAwareActivity(),
setContentView(R.layout.site_creation_activity)
val siteCreationSource = intent.extras?.getString(ARG_CREATE_SITE_SOURCE)
mainViewModel.start(savedInstanceState, SiteCreationSource.fromString(siteCreationSource))
mainViewModel.preloadThumbnails(this)

observeVMState()
}
Expand Down Expand Up @@ -174,7 +176,15 @@ class SiteCreationActivity : LocaleAwareActivity(),
val fragment = when (target.wizardStep) {
INTENTS -> SiteCreationIntentsFragment()
SITE_NAME -> SiteCreationSiteNameFragment.newInstance(target.wizardState.siteIntent)
SITE_DESIGNS -> HomePagePickerFragment.newInstance(target.wizardState.siteIntent)
SITE_DESIGNS -> {
// Cancel preload job before displaying the theme picker.
if (target.wizardStep == SITE_DESIGNS) {
mainViewModel.preloadingJob?.cancel(
"Preload did not complete before theme picker was shown."
)
}
HomePagePickerFragment.newInstance(target.wizardState.siteIntent)
}
DOMAINS -> SiteCreationDomainsFragment.newInstance(
screenTitle
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package org.wordpress.android.ui.sitecreation

import android.annotation.SuppressLint
import android.content.Context
import android.os.Bundle
import android.os.Parcelable
import androidx.annotation.StringRes
import androidx.lifecycle.LiveData
import androidx.lifecycle.Transformations
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.bumptech.glide.Glide
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.parcelize.Parcelize
import org.wordpress.android.R
import org.wordpress.android.fluxc.Dispatcher
import org.wordpress.android.ui.sitecreation.SiteCreationMainVM.SiteCreationScreenTitle.ScreenTitleEmpty
import org.wordpress.android.ui.sitecreation.SiteCreationMainVM.SiteCreationScreenTitle.ScreenTitleGeneral
import org.wordpress.android.ui.sitecreation.SiteCreationMainVM.SiteCreationScreenTitle.ScreenTitleStepCount
Expand All @@ -19,7 +26,9 @@ import org.wordpress.android.ui.sitecreation.SiteCreationStep.SITE_PREVIEW
import org.wordpress.android.ui.sitecreation.misc.SiteCreationSource
import org.wordpress.android.ui.sitecreation.misc.SiteCreationTracker
import org.wordpress.android.ui.sitecreation.previews.SitePreviewViewModel.CreateSiteState
import org.wordpress.android.ui.sitecreation.usecases.FetchHomePageLayoutsUseCase
import org.wordpress.android.ui.utils.UiString.UiStringRes
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.util.experiments.SiteNameABExperiment
import org.wordpress.android.util.wizard.WizardManager
import org.wordpress.android.util.wizard.WizardNavigationTarget
Expand Down Expand Up @@ -50,13 +59,28 @@ typealias NavigationTarget = WizardNavigationTarget<SiteCreationStep, SiteCreati
class SiteCreationMainVM @Inject constructor(
private val tracker: SiteCreationTracker,
private val wizardManager: WizardManager<SiteCreationStep>,
private val siteNameABExperiment: SiteNameABExperiment
private val siteNameABExperiment: SiteNameABExperiment,
private val networkUtils: NetworkUtilsWrapper,
private val dispatcher: Dispatcher,
private val fetchHomePageLayoutsUseCase: FetchHomePageLayoutsUseCase
) : ViewModel() {
init {
// TODO: Remove the duplicate {,un}registration in the picker view model
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the cancellation (interruption) flow, we need to have this in both places. The only thing left to remove is this comment, which I forgot to remove before merging 🤦‍♂️ .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed here 5351d73.

Part of PR: #16619

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ovi!

dispatcher.register(fetchHomePageLayoutsUseCase)
}

override fun onCleared() {
super.onCleared()
dispatcher.unregister(fetchHomePageLayoutsUseCase)
}

private var isStarted = false
private var siteCreationCompleted = false

private lateinit var siteCreationState: SiteCreationState

internal var preloadingJob: Job? = null

val navigationTargetObservable: SingleEventObservable<NavigationTarget> by lazy {
SingleEventObservable(
Transformations.map(wizardManager.navigatorLiveData) {
Expand Down Expand Up @@ -97,6 +121,24 @@ class SiteCreationMainVM @Inject constructor(
}
}

fun preloadThumbnails(context: Context) {
if (preloadingJob == null) {
preloadingJob = viewModelScope.launch(Dispatchers.IO) {
if (networkUtils.isNetworkAvailable()) {
val response = fetchHomePageLayoutsUseCase.fetchStarterDesigns()
for (design in response.designs) {
Glide.with(context)
.downloadOnly()
.load(design.previewMobile)
.submit()
.get() // This makes each call blocking, so subsequent calls can be cancelled if needed.
}
}
preloadingJob = null
}
}
}

fun writeToBundle(outState: Bundle) {
outState.putBoolean(KEY_SITE_CREATION_COMPLETED, siteCreationCompleted)
outState.putInt(KEY_CURRENT_STEP, wizardManager.currentStep)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.mockito.ArgumentCaptor
import org.mockito.Mock
import org.mockito.junit.MockitoJUnitRunner
import org.wordpress.android.R
import org.wordpress.android.fluxc.Dispatcher
import org.wordpress.android.fluxc.model.experiments.Variation
import org.wordpress.android.fluxc.model.experiments.Variation.Control
import org.wordpress.android.ui.sitecreation.SiteCreationMainVM.SiteCreationScreenTitle.ScreenTitleEmpty
Expand All @@ -30,6 +31,8 @@ import org.wordpress.android.ui.sitecreation.misc.SiteCreationSource
import org.wordpress.android.ui.sitecreation.misc.SiteCreationTracker
import org.wordpress.android.ui.sitecreation.previews.SitePreviewViewModel.CreateSiteState
import org.wordpress.android.ui.sitecreation.previews.SitePreviewViewModel.CreateSiteState.SiteCreationCompleted
import org.wordpress.android.ui.sitecreation.usecases.FetchHomePageLayoutsUseCase
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.util.experiments.SiteNameABExperiment
import org.wordpress.android.util.wizard.WizardManager
import org.wordpress.android.viewmodel.SingleLiveEvent
Expand Down Expand Up @@ -58,6 +61,9 @@ class SiteCreationMainVMTest {
@Mock lateinit var wizardManager: WizardManager<SiteCreationStep>
@Mock lateinit var siteCreationStep: SiteCreationStep
@Mock lateinit var siteNameABExperiment: SiteNameABExperiment
@Mock lateinit var networkUtils: NetworkUtilsWrapper
@Mock lateinit var dispatcher: Dispatcher
@Mock lateinit var fetchHomePageLayoutsUseCase: FetchHomePageLayoutsUseCase
private val wizardManagerNavigatorLiveData = SingleLiveEvent<SiteCreationStep>()

private lateinit var viewModel: SiteCreationMainVM
Expand All @@ -69,7 +75,7 @@ class SiteCreationMainVMTest {
wizardManagerNavigatorLiveData.value = siteCreationStep
Unit
}
viewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
viewModel = getNewViewModel()
viewModel.start(null, SiteCreationSource.UNSPECIFIED)
viewModel.navigationTargetObservable.observeForever(navigationTargetObserver)
viewModel.wizardFinishedObservable.observeForever(wizardFinishedObserver)
Expand All @@ -84,16 +90,16 @@ class SiteCreationMainVMTest {
@Test
fun siteCreationSiteNameExperimentControlTracked() {
whenever(siteNameABExperiment.getVariation()).thenReturn(Control)
viewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
viewModel.start(null, SiteCreationSource.UNSPECIFIED)
val newViewModel = getNewViewModel()
newViewModel.start(null, SiteCreationSource.UNSPECIFIED)
verify(tracker).trackSiteNameExperimentVariation(Control)
}

@Test
fun siteCreationSiteNameExperimentTreatmentTracked() {
whenever(siteNameABExperiment.getVariation()).thenReturn(Variation.fromName("wpandroid_site_name_v1"))
viewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
viewModel.start(null, SiteCreationSource.UNSPECIFIED)
val newViewModel = getNewViewModel()
newViewModel.start(null, SiteCreationSource.UNSPECIFIED)
verify(tracker).trackSiteNameExperimentVariation(Variation.fromName("wpandroid_site_name_v1"))
}

Expand Down Expand Up @@ -220,7 +226,7 @@ class SiteCreationMainVMTest {
.thenReturn(expectedState)

// we need to create a new instance of the VM as the `viewModel` has already been started in setUp()
val newViewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
val newViewModel = getNewViewModel()
newViewModel.start(savedInstanceState, SiteCreationSource.UNSPECIFIED)

/* we need to simulate navigation to the next step (Domain selection, see comment above) as
Expand All @@ -241,15 +247,15 @@ class SiteCreationMainVMTest {
.thenReturn(SiteCreationState())

// we need to create a new instance of the VM as the `viewModel` has already been started in setUp()
val newViewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
val newViewModel = getNewViewModel()
newViewModel.start(savedInstanceState, SiteCreationSource.UNSPECIFIED)

verify(wizardManager).setCurrentStepIndex(index)
}

@Test
fun `given null instance state, when start, then site creation accessed including source is tracked`() {
val newViewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
val newViewModel = getNewViewModel()
newViewModel.start(null, SiteCreationSource.UNSPECIFIED)

// Because setup is run before every test, we expect this to be tracked twice
Expand All @@ -264,7 +270,7 @@ class SiteCreationMainVMTest {
whenever(savedInstanceState.getParcelable<SiteCreationState>(KEY_SITE_CREATION_STATE))
.thenReturn(expectedState)

val newViewModel = SiteCreationMainVM(tracker, wizardManager, siteNameABExperiment)
val newViewModel = getNewViewModel()
newViewModel.start(savedInstanceState, SiteCreationSource.UNSPECIFIED)

// Because setup is run before every test, we expect this to be tracked on that first instance only
Expand All @@ -273,4 +279,13 @@ class SiteCreationMainVMTest {

private fun currentWizardState(vm: SiteCreationMainVM) =
vm.navigationTargetObservable.lastEvent!!.wizardState

private fun getNewViewModel() = SiteCreationMainVM(
tracker,
wizardManager,
siteNameABExperiment,
networkUtils,
dispatcher,
fetchHomePageLayoutsUseCase
)
}