From c571a05753360d48fe28b2c356b68e74ffe18f04 Mon Sep 17 00:00:00 2001 From: Abhay Garg <54636525+prayutsu@users.noreply.github.com> Date: Wed, 3 Feb 2021 16:06:29 +0530 Subject: [PATCH] Fix #1873: Fix OnboardingFragmentTest test cases on Robolectric (#2482) * fixed one test * fix 4 remaining tests * try fixing tests bu introducing scrollview * tried adding scrollview * fix AndroidManifest.xml * fix nit * fix basic set of tests * fixed 3 failing tests * removed extra changes * removed extra changes * try fixing tests with testCoroutineDispatchers * try advancedUntilIdle() * tried migrating to viewpager2 * tried implementing new methods * Replace ViewPager with ViewPager2 * Add ignore annotations * fix lint * add dependencies to BUILD.bazel * remove unnecessary ignore annotations * fix lint * fix lint * fix lint * fix lint error * remove redundant lines * Fix Viewpager2 regressions * Fix lint errors. * Remove comments * Fix all test cases on Robolectric * Try fixing flakiness in espresso * Revert "Try fixing flakiness in espresso" This reverts commit 2151c455 * Replace isDisplayed() with isCompletelyDisplayed() * Fix suggested changes * Try fixing test cases by using isADescendantOf() * Revert changes * Try stabilizing ttion est cases by changing configuration * Revert last commit to remove unwanted configuration changes * Fix ktlint error * Try adding custom ViewMatcher * Add custom ViewMatcher * Add custom ViewAction for scrolling ViewPager pages * Disable smooth scrolling to fix test cases * Remove holder.setIsRecycleable(false) and optimise perform() * Remove unnecessary id of constraintLayout in onboarding_slide.xml * Replace advancedUntilIdle bu runCurrent wherever applicable * Remove unnecessary delay * Add argument comment for smoothScroll * Add a space after argument comment * Fix suggested changes. --- .../onboarding/OnboardingFragmentPresenter.kt | 22 +--- .../app/onboarding/OnboardingPagerAdapter.kt | 94 +++++++++------ .../res/layout-land/onboarding_fragment.xml | 4 +- .../onboarding_fragment.xml | 4 +- .../layout-sw600dp-land/onboarding_slide.xml | 2 +- .../onboarding_fragment.xml | 2 +- .../main/res/layout/onboarding_fragment.xml | 2 +- .../app/onboarding/OnboardingFragmentTest.kt | 111 ++++++++++++------ 8 files changed, 142 insertions(+), 99 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt index 2c3feda298e..6471f2f0900 100644 --- a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingFragmentPresenter.kt @@ -6,18 +6,14 @@ import android.view.ViewGroup import android.widget.ImageView import android.widget.LinearLayout import androidx.appcompat.app.AppCompatActivity -import androidx.core.text.TextUtilsCompat -import androidx.core.view.ViewCompat import androidx.fragment.app.Fragment -import androidx.viewpager.widget.ViewPager +import androidx.viewpager2.widget.ViewPager2 import org.oppia.android.R import org.oppia.android.app.fragment.FragmentScope import org.oppia.android.app.viewmodel.ViewModelProvider import org.oppia.android.databinding.OnboardingFragmentBinding import org.oppia.android.util.statusbar.StatusBarColor -import java.util.Locale import javax.inject.Inject -import kotlin.collections.ArrayList /** The presenter for [OnboardingFragment]. */ @FragmentScope @@ -28,7 +24,6 @@ class OnboardingFragmentPresenter @Inject constructor( private val viewModelProviderFinalSlide: ViewModelProvider ) : OnboardingNavigationListener { private val dotsList = ArrayList() - private lateinit var onboardingPagerAdapter: OnboardingPagerAdapter private lateinit var binding: OnboardingFragmentBinding fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { @@ -50,18 +45,11 @@ class OnboardingFragmentPresenter @Inject constructor( } private fun setUpViewPager() { - onboardingPagerAdapter = + val adapter = OnboardingPagerAdapter(fragment.requireContext(), getOnboardingSlideFinalViewModel()) - binding.onboardingSlideViewPager.adapter = onboardingPagerAdapter - // TODO (#2194 ): Remove this once the implementation is updated to ViewPager2 - val isRTL = TextUtilsCompat - .getLayoutDirectionFromLocale(Locale.getDefault()) == ViewCompat.LAYOUT_DIRECTION_RTL - if (isRTL) { - binding.onboardingSlideViewPager.rotationY = 180f - } - binding.onboardingSlideViewPager.addOnPageChangeListener( - object : - ViewPager.OnPageChangeListener { + binding.onboardingSlideViewPager.adapter = adapter + binding.onboardingSlideViewPager.registerOnPageChangeCallback( + object : ViewPager2.OnPageChangeCallback() { override fun onPageScrollStateChanged(state: Int) { } diff --git a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt index 10b5900f031..1c589c18342 100644 --- a/app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt +++ b/app/src/main/java/org/oppia/android/app/onboarding/OnboardingPagerAdapter.kt @@ -2,65 +2,81 @@ package org.oppia.android.app.onboarding import android.content.Context import android.view.LayoutInflater -import android.view.View import android.view.ViewGroup -import android.widget.ScrollView -import androidx.core.text.TextUtilsCompat -import androidx.core.view.ViewCompat -import androidx.viewpager.widget.PagerAdapter +import androidx.recyclerview.widget.RecyclerView import org.oppia.android.databinding.OnboardingSlideBinding import org.oppia.android.databinding.OnboardingSlideFinalBinding -import java.util.Locale -/** Adapter to control the slide details in onboarding flow. */ +private const val VIEW_TYPE_TITLE_TEXT = 1 +private const val VIEW_TYPE_STORY_ITEM = 2 + class OnboardingPagerAdapter( val context: Context, val onboardingSlideFinalViewModel: OnboardingSlideFinalViewModel -) : PagerAdapter() { - // TODO (#2194 ): Remove this once the implementation is updated to ViewPager2 - val isRTL = TextUtilsCompat - .getLayoutDirectionFromLocale(Locale.getDefault()) == ViewCompat.LAYOUT_DIRECTION_RTL - override fun instantiateItem(container: ViewGroup, position: Int): Any { - if (position == TOTAL_NUMBER_OF_SLIDES - 1) { - val binding = - OnboardingSlideFinalBinding.inflate( +) : + RecyclerView.Adapter() { + + override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder { + return when (viewType) { + VIEW_TYPE_TITLE_TEXT -> { + val binding = OnboardingSlideBinding.inflate( LayoutInflater.from(context), - container, + parent, false ) - binding.viewModel = onboardingSlideFinalViewModel - if (isRTL) { - binding.finalLayout!!.rotationY = 180f + OnboardingSlideViewHolder(binding) } - - container.addView(binding.root) - return binding.root + VIEW_TYPE_STORY_ITEM -> { + val binding = + OnboardingSlideFinalBinding.inflate( + LayoutInflater.from(context), + parent, + false + ) + OnboardingSlideFinalViewHolder(binding) + } + else -> throw IllegalArgumentException("Invalid view type: $viewType") } + } - val binding = OnboardingSlideBinding.inflate( - LayoutInflater.from(context), - container, - false - ) - if (isRTL) { - binding.root.rotationY = 180f + override fun getItemViewType(position: Int): Int { + return if (position == TOTAL_NUMBER_OF_SLIDES - 1) { + VIEW_TYPE_STORY_ITEM + } else { + VIEW_TYPE_TITLE_TEXT } - val onboardingSlideViewModel = - OnboardingSlideViewModel(context, ViewPagerSlide.getSlideForPosition(position)) - binding.viewModel = onboardingSlideViewModel - container.addView(binding.root) - return binding.root } - override fun getCount(): Int { + override fun getItemCount(): Int { return TOTAL_NUMBER_OF_SLIDES } - override fun isViewFromObject(view: View, `object`: Any): Boolean { - return view == `object` + override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) { + when (holder.itemViewType) { + VIEW_TYPE_TITLE_TEXT -> { + val onboardingSlideViewModel = + OnboardingSlideViewModel(context, ViewPagerSlide.getSlideForPosition(position)) + (holder as OnboardingSlideViewHolder).bind(onboardingSlideViewModel) + } + VIEW_TYPE_STORY_ITEM -> { + (holder as OnboardingSlideFinalViewHolder).bind(onboardingSlideFinalViewModel) + } + } + } + + inner class OnboardingSlideViewHolder( + private val binding: OnboardingSlideBinding + ) : RecyclerView.ViewHolder(binding.root) { + fun bind(onboardingSlideViewModel: OnboardingSlideViewModel) { + binding.viewModel = onboardingSlideViewModel + } } - override fun destroyItem(container: ViewGroup, position: Int, `object`: Any) { - container.removeView(`object` as ScrollView) + inner class OnboardingSlideFinalViewHolder( + private val binding: OnboardingSlideFinalBinding + ) : RecyclerView.ViewHolder(binding.root) { + fun bind(onboardingSlideFinalViewModel: OnboardingSlideFinalViewModel) { + binding.viewModel = onboardingSlideFinalViewModel + } } } diff --git a/app/src/main/res/layout-land/onboarding_fragment.xml b/app/src/main/res/layout-land/onboarding_fragment.xml index bc0a25dcba0..dd48715d574 100644 --- a/app/src/main/res/layout-land/onboarding_fragment.xml +++ b/app/src/main/res/layout-land/onboarding_fragment.xml @@ -20,10 +20,10 @@ android:layout_height="match_parent" android:background="@color/oppiaBackgroundYellowIvory"> - - - - { + return isAssignableFrom(ViewPager2::class.java) + } + + override fun perform(uiController: UiController?, view: View?) { + (view as ViewPager2).setCurrentItem(position, /* smoothScroll= */ false) + } + } + } + // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. // TODO(#1675): Add NetworkModule once data module is migrated off of Moshi. @Singleton