Fix #1845: Replace Language code with icon in AudioBar (#2499)

* replace icon * removed comment * removed wild card import * fixed ids of audio_fragment * fixed failing tests * lint fix * fixed failing build * added TODO for ignored robolectric test
oppia · Jan 30, 2021 · 46870e8 · 46870e8
1 parent 9fd6d7a
commit 46870e8
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 57 deletions.
diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt
@@ -78,27 +78,28 @@ class AudioFragmentPresenter @Inject constructor(
       )
 
     val binding = AudioFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
-    binding.sbAudioProgress.setOnSeekBarChangeListener(object : SeekBar.OnSeekBarChangeListener {
-      override fun onProgressChanged(seekBar: SeekBar?, progress: Int, fromUser: Boolean) {
-        if (fromUser) {
-          userProgress = progress
+    binding.audioProgressSeekBar.setOnSeekBarChangeListener(
+      object : SeekBar.OnSeekBarChangeListener {
+        override fun onProgressChanged(seekBar: SeekBar?, progress: Int, fromUser: Boolean) {
+          if (fromUser) {
+            userProgress = progress
+          }
         }
-      }
 
-      override fun onStartTrackingTouch(seekBar: SeekBar?) {
-        userIsSeeking = true
-      }
+        override fun onStartTrackingTouch(seekBar: SeekBar?) {
+          userIsSeeking = true
+        }
 
-      override fun onStopTrackingTouch(seekBar: SeekBar?) {
-        viewModel.handleSeekTo(userProgress)
-        userIsSeeking = false
-      }
-    })
+        override fun onStopTrackingTouch(seekBar: SeekBar?) {
+          viewModel.handleSeekTo(userProgress)
+          userIsSeeking = false
+        }
+      })
     viewModel.playStatusLiveData.observe(
       fragment,
       Observer {
         prepared = it != UiAudioPlayStatus.LOADING && it != UiAudioPlayStatus.FAILED
-        binding.sbAudioProgress.isEnabled = prepared
+        binding.audioProgressSeekBar.isEnabled = prepared
       }
     )
 

diff --git a/app/src/main/res/drawable/ic_audio_lang_24px.xml b/app/src/main/res/drawable/ic_audio_lang_24px.xml
@@ -0,0 +1,12 @@
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+  android:width="24dp"
+  android:height="24dp"
+  android:viewportWidth="24"
+  android:viewportHeight="24">
+  <path
+    android:fillColor="#fff"
+    android:pathData="M21.04,5.93C19.04,1.92 15.45,0.21 11.38,0.05A12.21,12.21 0,0 0,5.04 1.63,9.77 9.77,0 0,0 0.04,9.45a8.84,8.84 0,0 0,2 6.44,1.75 1.75,0 0,1 0.3,2.06c-0.47,0.89 -1,1.74 -1.56,2.59 -0.21,0.33 -0.41,0.64 -0.21,1a0.9,0.9 0,0 0,1 0.51,12.16 12.16,0 0,0 2.53,-0.7c1,-0.44 1.87,-1.09 2.81,-1.62a1.06,1.06 0,0 1,0.72 -0.09,11.52 11.52,0 0,0 9.43,-1.17A9.5,9.5 0,0 0,21.04 5.93ZM9.25,12.05a0.62,0.62 0,0 1,-0.51 -0.25,6.73 6.73,0 0,1 -0.43,-1.19c-0.11,-0.39 -0.29,-0.6 -0.74,-0.54a3.43,3.43 0,0 1,-1.06 0c-0.59,-0.1 -0.83,0.19 -1,0.7 -0.07,0.29 -0.2,0.56 -0.28,0.85s-0.29,0.56 -0.65,0.43 -0.34,-0.44 -0.23,-0.76c0.65,-1.87 1.29,-3.74 2,-5.6 0.12,-0.33 0.1,-0.83 0.59,-0.84s0.53,0.5 0.65,0.84c0.64,1.84 1.28,3.67 1.91,5.51 0.05,0.13 0.08,0.27 0.16,0.5C9.53,11.79 9.4,12.05 9.25,12.05ZM17.5,10.6a0.54,0.54 0,0 0,-0.4 0.23,7.94 7.94,0 0,1 -1.86,3.08l1.6,1c0.28,0.16 0.57,0.32 0.38,0.69s-0.51,0.33 -0.82,0.15c-0.62,-0.37 -1.23,-0.76 -1.8,-1.11l-2.31,1.17c-0.28,0.14 -0.61,0.2 -0.77,-0.18s0.1,-0.55 0.43,-0.7c0.65,-0.29 1.29,-0.63 1.92,-0.94l-1.2,-1.57c-0.2,-0.28 -0.26,-0.59 0.07,-0.78s0.54,0 0.74,0.28c0.32,0.47 0.68,0.9 1.06,1.4a6.39,6.39 0,0 0,1.66 -2.7L12.04,10.62c-0.32,0 -0.56,-0.11 -0.56,-0.48s0.24,-0.48 0.57,-0.48h2.32a3.16,3.16 0,0 1,0 -0.88c0.06,-0.21 0.27,-0.5 0.45,-0.53s0.49,0.2 0.49,0.54 0,0.53 0,0.87L17.44,9.66c0.36,0 0.77,0 0.78,0.48S17.86,10.56 17.5,10.58Z" />
+  <path
+    android:fillColor="#fff"
+    android:pathData="M6.14,9.11L7.79,9.11L6.96,6.73C6.65,7.61 6.4,8.35 6.14,9.11Z" />
+</vector>
diff --git a/app/src/main/res/layout/audio_fragment.xml b/app/src/main/res/layout/audio_fragment.xml
@@ -29,7 +29,7 @@
     android:padding="4dp">
 
     <ImageView
-      android:id="@+id/ivPlayPauseAudio"
+      android:id="@+id/play_pause_audio_icon"
       android:layout_width="48dp"
       android:layout_height="48dp"
       android:layout_marginStart="8dp"
@@ -43,7 +43,7 @@
       app:layout_constraintTop_toTopOf="parent" />
 
     <SeekBar
-      android:id="@+id/sbAudioProgress"
+      android:id="@+id/audio_progress_seek_bar"
       style="@style/AudioSeekBar"
       android:layout_width="0dp"
       android:layout_height="26dp"
@@ -58,15 +58,15 @@
       android:thumb="@drawable/seekbar_thumb"
       android:thumbOffset="16dp"
       app:layout_constraintBottom_toBottomOf="parent"
-      app:layout_constraintEnd_toStartOf="@+id/tvAudioLanguage"
-      app:layout_constraintStart_toEndOf="@+id/ivPlayPauseAudio"
+      app:layout_constraintEnd_toStartOf="@+id/audio_language_icon"
+      app:layout_constraintStart_toEndOf="@+id/play_pause_audio_icon"
       app:layout_constraintTop_toTopOf="parent" />
 
-    <TextView
-      android:id="@+id/tvAudioLanguage"
+    <ImageView
+      android:id="@+id/audio_language_icon"
       android:layout_width="wrap_content"
       android:layout_height="wrap_content"
-      android:fontFamily="sans-serif-medium"
+      android:contentDescription="@{viewModel.currentLanguageCode}"
       android:gravity="center"
       android:minWidth="48dp"
       android:minHeight="48dp"
@@ -75,10 +75,7 @@
       android:paddingTop="16dp"
       android:paddingEnd="16dp"
       android:paddingBottom="16dp"
-      android:text="@{viewModel.currentLanguageCode}"
-      android:textAllCaps="true"
-      android:textColor="@color/white"
-      android:textSize="14sp"
+      android:src="@drawable/ic_audio_lang_24px"
       app:layout_constraintBottom_toBottomOf="parent"
       app:layout_constraintEnd_toEndOf="parent"
       app:layout_constraintTop_toTopOf="parent" />

diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt
@@ -151,7 +151,6 @@ class AudioFragmentTest {
     )
   }
 
-  // TODO(#1845): As updating tvAudioLanguage to image, we need to remove this test
   @Test
   fun testAudioFragment_withDefaultProfile_showsAudioLanguageAsEnglish() {
     addMediaInfo()
@@ -160,11 +159,12 @@ class AudioFragmentTest {
         internalProfileId
       )
     ).use {
-      onView(withId(R.id.tvAudioLanguage)).check(matches(withText("EN")))
+      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("en")))
     }
   }
 
-  // TODO(#1845): As updating tvAudioLanguage to image, we need to remove this test
+  // TODO(#2417): Re-enable once this test passes on robolectric
+  @RunOn(TestPlatform.ESPRESSO)
   @Test
   fun testAudioFragment_withHindiAudioLanguageProfile_showsHindiAudioLanguage() {
     addMediaInfo()
@@ -181,7 +181,7 @@ class AudioFragmentTest {
       it.onActivity {
         profileTestHelper.waitForOperationToComplete(data)
       }
-      onView(withId(R.id.tvAudioLanguage)).check(matches(withText("EN")))
+      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("hi")))
     }
   }
 
@@ -194,9 +194,9 @@ class AudioFragmentTest {
       )
     ).use {
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(isDisplayed()))
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_play_description))))
     }
   }
@@ -213,10 +213,10 @@ class AudioFragmentTest {
     ).use {
       testCoroutineDispatchers.runCurrent()
 
-      onView(withId(R.id.ivPlayPauseAudio)).perform(click())
+      onView(withId(R.id.play_pause_audio_icon)).perform(click())
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_pause_description))))
     }
   }
@@ -231,10 +231,10 @@ class AudioFragmentTest {
     ).use {
       testCoroutineDispatchers.runCurrent()
 
-      onView(withId(R.id.sbAudioProgress)).perform(setProgress(100))
+      onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100))
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_play_description))))
     }
   }
@@ -251,12 +251,12 @@ class AudioFragmentTest {
     ).use {
       testCoroutineDispatchers.runCurrent()
 
-      onView(withId(R.id.ivPlayPauseAudio)).perform(click())
+      onView(withId(R.id.play_pause_audio_icon)).perform(click())
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.sbAudioProgress)).perform(setProgress(100))
+      onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100))
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_pause_description))))
     }
   }
@@ -270,15 +270,14 @@ class AudioFragmentTest {
       )
     ).use {
       invokePreparedListener(shadowMediaPlayer)
-      onView(withId(R.id.ivPlayPauseAudio)).perform(click())
-      onView(withId(R.id.sbAudioProgress)).perform(setProgress(100))
+      onView(withId(R.id.play_pause_audio_icon)).perform(click())
+      onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100))
       onView(isRoot()).perform(orientationLandscape())
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_pause_description))))
     }
   }
 
-  // TODO(#1845): As updating tvAudioLanguage to image, we need to update this test
   @Test
   fun testAudioFragment_invokePrepared_changeDifferentLanguage_checkResetSeekBarAndPaused() {
     addMediaInfo()
@@ -289,12 +288,12 @@ class AudioFragmentTest {
     ).use {
       testCoroutineDispatchers.runCurrent()
 
-      onView(withId(R.id.ivPlayPauseAudio)).perform(click())
+      onView(withId(R.id.play_pause_audio_icon)).perform(click())
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.sbAudioProgress)).perform(setProgress(100))
+      onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100))
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.tvAudioLanguage)).perform(click())
+      onView(withId(R.id.audio_language_icon)).perform(click())
 
       val locale = Locale("es")
 
@@ -305,9 +304,9 @@ class AudioFragmentTest {
       onView(withText("OK")).inRoot(isDialog()).perform(click())
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_play_description))))
-      onView(withId(R.id.sbAudioProgress)).check(matches(withSeekBarPosition(0)))
+      onView(withId(R.id.audio_progress_seek_bar)).check(matches(withSeekBarPosition(0)))
     }
   }
 

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
@@ -399,7 +399,7 @@ class ExplorationActivityTest {
           withEffectiveVisibility(Visibility.VISIBLE)
         )
       ).inRoot(isDialog()).perform(click())
-      onView(withId(R.id.ivPlayPauseAudio)).check(matches(not(isDisplayed())))
+      onView(withId(R.id.play_pause_audio_icon)).check(matches(not(isDisplayed())))
     }
     explorationDataController.stopPlayingExploration()
   }
@@ -438,7 +438,7 @@ class ExplorationActivityTest {
 
       onView(
         allOf(
-          withId(R.id.ivPlayPauseAudio),
+          withId(R.id.play_pause_audio_icon),
           withEffectiveVisibility(Visibility.VISIBLE)
         )
       )
@@ -476,7 +476,7 @@ class ExplorationActivityTest {
       onView(withId(R.id.action_audio_player)).perform(click())
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio)).check(matches(not(isDisplayed())))
+      onView(withId(R.id.play_pause_audio_icon)).check(matches(not(isDisplayed())))
       onView(withText(context.getString(R.string.cellular_data_alert_dialog_title)))
         .check(doesNotExist())
     }
@@ -514,7 +514,7 @@ class ExplorationActivityTest {
       onView(withId(R.id.action_audio_player)).perform(click())
 
       testCoroutineDispatchers.runCurrent()
-      onView(withId(R.id.ivPlayPauseAudio)).check(matches(isDisplayed()))
+      onView(withId(R.id.play_pause_audio_icon)).check(matches(isDisplayed()))
       onView(withText(context.getString(R.string.cellular_data_alert_dialog_title)))
         .check(doesNotExist())
     }
@@ -540,13 +540,13 @@ class ExplorationActivityTest {
       onView(withId(R.id.action_audio_player)).perform(click())
       onView(
         allOf(
-          withId(R.id.ivPlayPauseAudio),
+          withId(R.id.play_pause_audio_icon),
           withEffectiveVisibility(Visibility.VISIBLE)
         )
       )
-      onView(allOf(withText("EN"), withEffectiveVisibility(Visibility.VISIBLE)))
+      onView(allOf(withId(R.id.audio_language_icon), withEffectiveVisibility(Visibility.VISIBLE)))
       waitForTheView(withDrawable(R.drawable.ic_pause_circle_filled_white_24dp))
-      onView(withId(R.id.ivPlayPauseAudio)).check(
+      onView(withId(R.id.play_pause_audio_icon)).check(
         matches(
           withDrawable(
             R.drawable.ic_pause_circle_filled_white_24dp
@@ -581,7 +581,7 @@ class ExplorationActivityTest {
       onView(withId(R.id.action_audio_player)).perform(click())
       onView(
         allOf(
-          withText("EN"),
+          withId(R.id.audio_language_icon),
           withEffectiveVisibility(Visibility.VISIBLE)
         )
       ).perform(click())
@@ -599,7 +599,7 @@ class ExplorationActivityTest {
         )
       )
       onView(withId(R.id.continue_button)).perform(click())
-      onView(withText("HI-EN")).check(matches(isDisplayed()))
+      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("hi-en")))
     }
     explorationDataController.stopPlayingExploration()
   }
@@ -633,7 +633,7 @@ class ExplorationActivityTest {
       onView(withId(R.id.submit_answer_button)).perform(click())
       Thread.sleep(1000)
 
-      onView(withId(R.id.ivPlayPauseAudio))
+      onView(withId(R.id.play_pause_audio_icon))
         .check(matches(withContentDescription(context.getString(R.string.audio_pause_description))))
     }
     explorationDataController.stopPlayingExploration()