Fix #2581: Marquee auto restart issue (#4392)

* Fix #4186 Uncheck all selection on developer options not working * Fix #4186 Add changes to code as suggested on code review. * Fix #4186 Undo changes on line wrap as advised by code reviewer. * Fix #2581 Marquee restarts automatically in certain scenarios. * Fix #2581 Marquee restarts automatically in certain scenarios. * Fix #2581 Marquee restarts automatically in certain scenarios. * Fix #2581 Marquee restarts automatically in certain scenarios. * Revert "Fix #4186 Undo changes on line wrap as advised by code reviewer." This reverts commit ca5bca5. * Revert "Fix #4186 Add changes to code as suggested on code review." This reverts commit 5295364. * Revert "Fix #4186 Uncheck all selection on developer options not working" This reverts commit 0cff83a. * Insert timer in OnClick listener to enable unselect during subsequent clicks. * Introduce custom translation animation logic to fix issue with the core marquee api. * Introduce custom translation animation logic to fix issue with the core marquee api. * Fix some of the failing tests. * Fix some of the failing tests. * Fix some of the failing tests, exclude MarqueeView from tests. * Revert some of the changes. * Add MarqueeView to bazel list of custom views that import resources. * Remove Marquee tests to fix some failures. * Update exploration_activity.xml to fix failing tests. * Update exploration_activity.xml to fix failing tests. * Fix failing tests. * Fix some of the failing tests. * Fix some of the failing tests. * Fix some of the failing tests. * Fix some of failing test cases. * Update magic value textViewVirtualWidth to use the device width. * Fix Buildifier issue with bazel. * Update to use Library forked to oppia as advised. * Fix custom res import issues. * Add binding adapters for "app:speed" and "app:pause". * Revert to "app" from "marquee". * Revert unnecessary file exemption. * Fix Nit. * Fix failing tests and revert from previous fix, which could not be used in alpha. * Change to pass "app" and "speed" values in code. * Fix import NIT. * Fix failing tests by updating Bazel config files to add Android-MarqueeView as external dependency. * Fix buildifier issues after bazel file changes. * Revert tests for this feature and fix NITs. * Migrate to Oppia's fork of AndroidMarquee-View remove ellipsize assertion on some tests. * Change width to matchParent on exploration_activity.xml. * Update speed and pause to animationSpeed and animationDelay respectively to make it clear. * Change Bazel workspace config to point to Oppia's official fork. * Change to click on MarqueeView as it is parent to the ToolbarTitle on ExplorationActivityTest.kt. * Add indentations to layouts and shallow_since from CI logs. * Revert shallow since. * Update shallow since. * Add class assertion tests. * Fix spacing NIT.
oppia · Sep 29, 2022 · 846657e · 846657e
1 parent 0520a97
commit 846657e
Show file tree

Hide file tree

Showing 20 changed files with 147 additions and 60 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -139,6 +139,15 @@ git_repository(
     shallow_since = "1647554845 -0700",
 )
 
+# A custom fork of Android-MarqueeView that is updated with latest dependencies compatible with Oppia and also
+# min target SDK version set to be compatible with Oppia.
+git_repository(
+    name = "marqueeview",
+    commit = "a935a78c88a01958716396e6f2cb4abf4559eccc",
+    remote = "https://github.com/oppia/Android-MarqueeView",
+    shallow_since = "1663393399 -0400",
+)
+
 bind(
     name = "databinding_annotation_processor",
     actual = "//tools/android:compiler_annotation_processor",

diff --git a/app/BUILD.bazel b/app/BUILD.bazel
@@ -590,6 +590,7 @@ android_library(
         "//third_party:androidx_recyclerview_recyclerview",
         "//third_party:androidx_viewpager2_viewpager2",
         "//third_party:androidx_viewpager_viewpager",
+        "//third_party:asia_ivity_android_marqueeview",
         "//third_party:circularimageview_circular_image_view",
         "//third_party:com_google_android_flexbox_flexbox",
         "//third_party:com_google_android_material_material",

diff --git a/app/build.gradle b/app/build.gradle
@@ -171,6 +171,7 @@ dependencies {
       'com.google.firebase:firebase-crashlytics:17.0.0',
       'com.google.guava:guava:28.1-android',
       'com.google.protobuf:protobuf-javalite:3.17.3',
+      'com.github.oppia:Android-MarqueeView:a935a78c88',
       'com.github.oppia:CircularImageview:35d08ba88a',
       'de.hdodenhof:circleimageview:3.0.1',
       'nl.dionsegijn:konfetti:1.2.5',
@@ -209,6 +210,7 @@ dependencies {
       'androidx.test.espresso:espresso-intents:3.1.0',
       'androidx.test.ext:junit:1.1.1',
       'com.github.bumptech.glide:mocks:4.11.0',
+      'com.github.oppia:Android-MarqueeView:a935a78c88',
       'com.google.truth:truth:1.1.3',
       'androidx.work:work-testing:2.4.0',
       'com.google.truth.extensions:truth-liteproto-extension:1.1.3',

diff --git a/...src/main/java/org/oppia/android/app/help/thirdparty/LicenseTextViewerActivityPresenter.kt b/...src/main/java/org/oppia/android/app/help/thirdparty/LicenseTextViewerActivityPresenter.kt
@@ -45,7 +45,7 @@ class LicenseTextViewerActivityPresenter @Inject constructor(
     }
 
     binding.licenseTextViewerActivityToolbarTitle.setOnClickListener {
-      binding.licenseTextViewerActivityToolbarTitle.isSelected = true
+      binding.licenseTextViewerActivityMarqueeView.startMarquee()
     }
 
     if (getLicenseTextViewerFragment() == null) {

diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt
@@ -90,7 +90,7 @@ class ExplorationActivityPresenter @Inject constructor(
     activity.setSupportActionBar(explorationToolbar)
 
     binding.explorationToolbarTitle.setOnClickListener {
-      binding.explorationToolbarTitle.isSelected = true
+      binding.explorationMarqueeView.startMarquee()
     }
 
     binding.explorationToolbar.setNavigationOnClickListener {

diff --git a/app/src/main/java/org/oppia/android/app/story/StoryFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/story/StoryFragmentPresenter.kt
@@ -89,7 +89,7 @@ class StoryFragmentPresenter @Inject constructor(
     }
 
     binding.storyToolbarTitle.setOnClickListener {
-      binding.storyToolbarTitle.isSelected = true
+      binding.storyMarqueeView?.startMarquee()
     }
 
     linearLayoutManager = LinearLayoutManager(activity.applicationContext)

diff --git a/app/src/main/java/org/oppia/android/app/topic/TopicFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/TopicFragmentPresenter.kt
@@ -58,8 +58,8 @@ class TopicFragmentPresenter @Inject constructor(
       (activity as TopicActivity).finish()
     }
 
-    binding.topicToolbar.setOnClickListener {
-      binding.topicToolbarTitle.isSelected = true
+    binding.topicToolbarTitle.setOnClickListener {
+      binding.topicMarqueeView.startMarquee()
     }
 
     viewModel.setInternalProfileId(internalProfileId)

diff --git a/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt
@@ -60,7 +60,7 @@ class RevisionCardActivityPresenter @Inject constructor(
       (activity as RevisionCardActivity).finish()
     }
     binding.revisionCardToolbarTitle.setOnClickListener {
-      binding.revisionCardToolbarTitle.isSelected = true
+      binding.revisionCardMarqueeView.startMarquee()
     }
     subscribeToSubtopicTitle()
 

diff --git a/app/src/main/res/layout-sw600dp/story_fragment.xml b/app/src/main/res/layout-sw600dp/story_fragment.xml
@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <layout xmlns:android="http://schemas.android.com/apk/res/android"
   xmlns:app="http://schemas.android.com/apk/res-auto"
+  xmlns:marquee="http://schemas.android.com/apk/res-auto"
   xmlns:tools="http://schemas.android.com/tools">
 
   <data>
@@ -36,13 +37,21 @@
         app:navigationContentDescription="@string/navigate_up"
         app:navigationIcon="?attr/homeAsUpIndicator">
 
-        <TextView
-          android:id="@+id/story_toolbar_title"
-          style="@style/MarqueeToolbarTitleTextView"
-          android:layout_width="wrap_content"
+        <asia.ivity.android.marqueeview.MarqueeView
+          android:id="@+id/story_marquee_view"
+          android:layout_width="match_parent"
           android:layout_height="wrap_content"
-          android:layout_marginEnd="20dp"
-          android:text="@{viewModel.storyNameLiveData}" />
+          app:animationDelay="500"
+          app:animationSpeed="10">
+
+          <TextView
+            android:id="@+id/story_toolbar_title"
+            style="@style/MarqueeToolbarTitleTextView"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginEnd="20dp"
+            android:text="@{viewModel.storyNameLiveData}" />
+        </asia.ivity.android.marqueeview.MarqueeView>
       </androidx.appcompat.widget.Toolbar>
     </com.google.android.material.appbar.AppBarLayout>
 
@@ -62,9 +71,9 @@
         android:background="@color/component_color_story_activity_background_color"
         android:clipToPadding="false"
         android:overScrollMode="never"
-        android:paddingBottom="@dimen/story_fragment_padding_bottom"
         android:paddingStart="64dp"
         android:paddingEnd="64dp"
+        android:paddingBottom="@dimen/story_fragment_padding_bottom"
         android:scrollbars="none"
         app:data="@{viewModel.storyChapterLiveData}"
         tools:listitem="@layout/story_chapter_view" />

diff --git a/app/src/main/res/layout/exploration_activity.xml b/app/src/main/res/layout/exploration_activity.xml
@@ -38,13 +38,21 @@
           android:layout_height="wrap_content"
           android:orientation="horizontal">
 
-          <TextView
-            android:id="@+id/exploration_toolbar_title"
-            style="@style/MarqueeToolbarTitleTextView"
-            android:layout_width="0dp"
+          <asia.ivity.android.marqueeview.MarqueeView
+            android:id="@+id/exploration_marquee_view"
+            android:layout_width="match_parent"
             android:layout_height="wrap_content"
             android:layout_marginEnd="20dp"
-            android:layout_weight="1" />
+            android:layout_weight="1"
+            app:animationDelay="500"
+            app:animationSpeed="10">
+
+            <TextView
+              android:id="@+id/exploration_toolbar_title"
+              style="@style/MarqueeToolbarTitleTextView"
+              android:layout_width="match_parent"
+              android:layout_height="wrap_content" />
+          </asia.ivity.android.marqueeview.MarqueeView>
 
           <ImageView
             android:id="@+id/action_audio_player"

diff --git a/app/src/main/res/layout/license_text_viewer_activity.xml b/app/src/main/res/layout/license_text_viewer_activity.xml
@@ -24,13 +24,20 @@
         app:navigationContentDescription="@string/navigate_up"
         app:navigationIcon="?attr/homeAsUpIndicator">
 
-        <TextView
-          android:id="@+id/license_text_viewer_activity_toolbar_title"
-          style="@style/MarqueeToolbarTitleTextView"
+        <asia.ivity.android.marqueeview.MarqueeView
+          android:id="@+id/license_text_viewer_activity_marquee_view"
           android:layout_width="match_parent"
           android:layout_height="wrap_content"
-          android:layout_marginEnd="20dp"
-          android:minHeight="48dp" />
+          app:animationDelay="500"
+          app:animationSpeed="10">
+
+          <TextView
+            android:id="@+id/license_text_viewer_activity_toolbar_title"
+            style="@style/MarqueeToolbarTitleTextView"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:layout_marginEnd="20dp" />
+        </asia.ivity.android.marqueeview.MarqueeView>
       </androidx.appcompat.widget.Toolbar>
     </com.google.android.material.appbar.AppBarLayout>
 

diff --git a/app/src/main/res/layout/revision_card_activity.xml b/app/src/main/res/layout/revision_card_activity.xml
@@ -29,12 +29,20 @@
         app:navigationContentDescription="@string/navigate_up"
         app:navigationIcon="?attr/homeAsUpIndicator">
 
-        <TextView
-          android:id="@+id/revision_card_toolbar_title"
-          style="@style/MarqueeToolbarTitleTextView"
-          android:layout_width="wrap_content"
+        <asia.ivity.android.marqueeview.MarqueeView
+          android:id="@+id/revision_card_marquee_view"
+          android:layout_width="match_parent"
           android:layout_height="wrap_content"
-          android:layout_marginEnd="20dp"/>
+          app:animationDelay="500"
+          app:animationSpeed="10">
+
+          <TextView
+            android:id="@+id/revision_card_toolbar_title"
+            style="@style/MarqueeToolbarTitleTextView"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginEnd="20dp" />
+        </asia.ivity.android.marqueeview.MarqueeView>
       </androidx.appcompat.widget.Toolbar>
     </com.google.android.material.appbar.AppBarLayout>
 

diff --git a/app/src/main/res/layout/story_fragment.xml b/app/src/main/res/layout/story_fragment.xml
@@ -36,13 +36,21 @@
         app:navigationContentDescription="@string/navigate_up"
         app:navigationIcon="?attr/homeAsUpIndicator">
 
-        <TextView
-          android:id="@+id/story_toolbar_title"
-          style="@style/MarqueeToolbarTitleTextView"
-          android:layout_width="wrap_content"
+        <asia.ivity.android.marqueeview.MarqueeView
+          android:id="@+id/story_marquee_view"
+          android:layout_width="match_parent"
           android:layout_height="wrap_content"
-          android:layout_marginEnd="20dp"
-          android:text="@{viewModel.storyNameLiveData}"/>
+          app:animationDelay="500"
+          app:animationSpeed="10">
+
+          <TextView
+            android:id="@+id/story_toolbar_title"
+            style="@style/MarqueeToolbarTitleTextView"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:layout_marginEnd="20dp"
+            android:text="@{viewModel.storyNameLiveData}" />
+        </asia.ivity.android.marqueeview.MarqueeView>
       </androidx.appcompat.widget.Toolbar>
     </com.google.android.material.appbar.AppBarLayout>
 

diff --git a/app/src/main/res/layout/topic_fragment.xml b/app/src/main/res/layout/topic_fragment.xml
@@ -34,13 +34,21 @@
         app:navigationContentDescription="@string/navigate_up"
         app:navigationIcon="?attr/homeAsUpIndicator">
 
-        <TextView
-          android:id="@+id/topic_toolbar_title"
-          style="@style/MarqueeToolbarTitleTextView"
-          android:layout_width="wrap_content"
+        <asia.ivity.android.marqueeview.MarqueeView
+          android:id="@+id/topic_marquee_view"
+          android:layout_width="match_parent"
           android:layout_height="wrap_content"
-          android:layout_marginEnd="20dp"
-          android:text="@{viewModel.topicToolbarTitleLiveData}" />
+          app:animationDelay="500"
+          app:animationSpeed="10">
+
+          <TextView
+            android:id="@+id/topic_toolbar_title"
+            style="@style/MarqueeToolbarTitleTextView"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:layout_marginEnd="20dp"
+            android:text="@{viewModel.topicToolbarTitleLiveData}" />
+        </asia.ivity.android.marqueeview.MarqueeView>
       </androidx.appcompat.widget.Toolbar>
 
       <com.google.android.material.tabs.TabLayout

diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml
@@ -450,19 +450,13 @@
   <style name="BorderlessMaterialButton" parent="Widget.AppCompat.Button.Borderless" />
 
   <style name="MarqueeToolbarTitleTextView" parent="TextView.Common1">
-    <item name="android:ellipsize">marquee</item>
-    <item name="android:fadingEdge">horizontal</item>
-    <item name="android:fadingEdgeLength">20dp</item>
     <item name="android:focusable">false</item>
     <item name="android:focusableInTouchMode">true</item>
     <item name="android:fontFamily">sans-serif</item>
-    <item name="android:marqueeRepeatLimit">1</item>
     <item name="android:minHeight">48dp</item>
     <item name="android:textAlignment">viewStart</item>
     <item name="android:textColor">@color/color_def_white</item>
     <item name="android:textSize">20sp</item>
-    <item name="android:requiresFadingEdge">horizontal</item>
-    <item name="android:scrollHorizontally">true</item>
     <item name="android:singleLine">true</item>
     <item name="fontFamily">sans-serif</item>
     <item name="textAllCaps">false</item>

diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt
@@ -3,7 +3,6 @@ package org.oppia.android.app.player.exploration
 import android.app.Application
 import android.content.Context
 import android.content.Intent
-import android.text.TextUtils
 import android.view.View
 import android.widget.TextView
 import androidx.appcompat.app.AppCompatActivity
@@ -30,6 +29,7 @@ import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent
 import androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra
 import androidx.test.espresso.matcher.RootMatchers.isDialog
 import androidx.test.espresso.matcher.ViewMatchers.Visibility
+import androidx.test.espresso.matcher.ViewMatchers.assertThat
 import androidx.test.espresso.matcher.ViewMatchers.isChecked
 import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
 import androidx.test.espresso.matcher.ViewMatchers.isRoot
@@ -41,6 +41,7 @@ import androidx.test.espresso.util.HumanReadables
 import androidx.test.espresso.util.TreeIterables
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.rule.ActivityTestRule
+import asia.ivity.android.marqueeview.MarqueeView
 import com.google.common.truth.Truth.assertThat
 import dagger.Component
 import dagger.Module
@@ -50,6 +51,7 @@ import org.hamcrest.CoreMatchers.allOf
 import org.hamcrest.CoreMatchers.containsString
 import org.hamcrest.Description
 import org.hamcrest.Matcher
+import org.hamcrest.Matchers.instanceOf
 import org.hamcrest.Matchers.not
 import org.junit.After
 import org.junit.Before
@@ -321,10 +323,12 @@ class ExplorationActivityTest {
     val explorationToolbarTitle: TextView =
       explorationActivityTestRule.activity.findViewById(R.id.exploration_toolbar_title)
     ViewCompat.setLayoutDirection(explorationToolbarTitle, ViewCompat.LAYOUT_DIRECTION_RTL)
+    val explorationMarqueeView: MarqueeView =
+      explorationActivityTestRule.activity.findViewById(R.id.exploration_marquee_view)
 
-    onView(withId(R.id.exploration_toolbar_title)).perform(click())
-    assertThat(explorationToolbarTitle.ellipsize).isEqualTo(TextUtils.TruncateAt.MARQUEE)
+    onView(withId(R.id.exploration_marquee_view)).perform(click())
     assertThat(explorationToolbarTitle.textAlignment).isEqualTo(View.TEXT_ALIGNMENT_VIEW_START)
+    assertThat(explorationMarqueeView, instanceOf(MarqueeView::class.java))
   }
 
   @Test
@@ -340,11 +344,13 @@ class ExplorationActivityTest {
     )
     val explorationToolbarTitle: TextView =
       explorationActivityTestRule.activity.findViewById(R.id.exploration_toolbar_title)
+    val explorationMarqueeView: MarqueeView =
+      explorationActivityTestRule.activity.findViewById(R.id.exploration_marquee_view)
     ViewCompat.setLayoutDirection(explorationToolbarTitle, ViewCompat.LAYOUT_DIRECTION_LTR)
 
-    onView(withId(R.id.exploration_toolbar_title)).perform(click())
-    assertThat(explorationToolbarTitle.ellipsize).isEqualTo(TextUtils.TruncateAt.MARQUEE)
+    onView(withId(R.id.exploration_marquee_view)).perform(click())
     assertThat(explorationToolbarTitle.textAlignment).isEqualTo(View.TEXT_ALIGNMENT_VIEW_START)
+    assertThat(explorationMarqueeView, instanceOf(MarqueeView::class.java))
   }
 
   @Test