From 846657e8f3fc81d8434c22523dd9f2226e9e1e55 Mon Sep 17 00:00:00 2001 From: "Kevin .M. Gitonga" Date: Thu, 29 Sep 2022 04:43:57 +0300 Subject: [PATCH] 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 ca5bca5e93a96d6dee5ea7142716eedb00d6bc2d. * Revert "Fix #4186 Add changes to code as suggested on code review." This reverts commit 52953644cf88d75a5a0637b7b128e96bdbb1de42. * Revert "Fix #4186 Uncheck all selection on developer options not working" This reverts commit 0cff83acdf5c6201909461e9194d4f64767a2058. * 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. --- WORKSPACE | 9 ++++++++ app/BUILD.bazel | 1 + app/build.gradle | 2 ++ .../LicenseTextViewerActivityPresenter.kt | 2 +- .../ExplorationActivityPresenter.kt | 2 +- .../app/story/StoryFragmentPresenter.kt | 2 +- .../app/topic/TopicFragmentPresenter.kt | 4 ++-- .../RevisionCardActivityPresenter.kt | 2 +- .../res/layout-sw600dp/story_fragment.xml | 23 +++++++++++++------ .../main/res/layout/exploration_activity.xml | 18 +++++++++++---- .../layout/license_text_viewer_activity.xml | 17 ++++++++++---- .../res/layout/revision_card_activity.xml | 18 +++++++++++---- app/src/main/res/layout/story_fragment.xml | 20 +++++++++++----- app/src/main/res/layout/topic_fragment.xml | 20 +++++++++++----- app/src/main/res/values/styles.xml | 6 ----- .../exploration/ExplorationActivityTest.kt | 16 +++++++++---- .../android/app/story/StoryFragmentTest.kt | 13 ++++++++--- .../android/app/topic/TopicFragmentTest.kt | 12 +++++++--- .../revisioncard/RevisionCardActivityTest.kt | 12 +++++++--- third_party/BUILD.bazel | 8 +++++++ 20 files changed, 147 insertions(+), 60 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 8a7bd724cf7..cb0f252e6d8 100644 --- 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 index 1f8fc6eeacb..7d63d8bd599 100644 --- 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 index 4709cf0f06f..da4755f3401 100644 --- 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/app/src/main/java/org/oppia/android/app/help/thirdparty/LicenseTextViewerActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/help/thirdparty/LicenseTextViewerActivityPresenter.kt index d4c5c34b262..258842054a4 100644 --- a/app/src/main/java/org/oppia/android/app/help/thirdparty/LicenseTextViewerActivityPresenter.kt +++ b/app/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 index fd8126dc548..b8b774c45b3 100755 --- 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 index 24795c0024c..823ac916d1d 100644 --- 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 index e4537020f30..6ff307e8f0f 100644 --- 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 index 514c6d066a0..cebce5a93b4 100644 --- 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 index cdba05b9484..66fb3c73a8c 100644 --- a/app/src/main/res/layout-sw600dp/story_fragment.xml +++ b/app/src/main/res/layout-sw600dp/story_fragment.xml @@ -1,6 +1,7 @@ @@ -36,13 +37,21 @@ app:navigationContentDescription="@string/navigate_up" app:navigationIcon="?attr/homeAsUpIndicator"> - + app:animationDelay="500" + app:animationSpeed="10"> + + + @@ -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 index 7873117ba73..45beb5e5755 100755 --- 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"> - + android:layout_weight="1" + app:animationDelay="500" + app:animationSpeed="10"> + + + - + app:animationDelay="500" + app:animationSpeed="10"> + + + diff --git a/app/src/main/res/layout/revision_card_activity.xml b/app/src/main/res/layout/revision_card_activity.xml index fed831dc978..6ae4b0b8823 100644 --- 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"> - + app:animationDelay="500" + app:animationSpeed="10"> + + + diff --git a/app/src/main/res/layout/story_fragment.xml b/app/src/main/res/layout/story_fragment.xml index e7e38eaccb2..b670ae4ecf0 100644 --- 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"> - + app:animationDelay="500" + app:animationSpeed="10"> + + + diff --git a/app/src/main/res/layout/topic_fragment.xml b/app/src/main/res/layout/topic_fragment.xml index fdcc12484d7..8fe08ef9e74 100644 --- 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"> - + app:animationDelay="500" + app:animationSpeed="10"> + + +