Fixes #2325: A11y + Highfi changes to audio player (#3139)

* Highfi changes to audio player * Nit margin fix * Accessiblity test clear + tests added * Audio toolbar icon A11y checks * Error fix * Nit fix * Fix test failure * Removed test case * Added test suggested by Ben * Removed test * Nit fixes Co-authored-by: Rajat Talesra <[email protected]>
oppia · May 7, 2021 · 87f28a1 · 87f28a1
1 parent 68e85bb
commit 87f28a1
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 36 deletions.
diff --git a/app/src/main/res/layout/audio_fragment.xml b/app/src/main/res/layout/audio_fragment.xml
@@ -21,12 +21,10 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
     android:layout_gravity="center_horizontal"
-    android:layout_marginBottom="12dp"
     android:background="@drawable/audio_background"
     android:elevation="8dp"
     android:gravity="center_vertical"
-    android:minHeight="48dp"
-    android:padding="4dp">
+    android:minHeight="48dp">
 
     <ImageView
       android:id="@+id/play_pause_audio_icon"
@@ -36,7 +34,7 @@
       android:clickable="@{viewModel.playStatusLiveData != UiAudioPlayStatus.LOADING}"
       android:contentDescription="@{viewModel.playStatusLiveData == UiAudioPlayStatus.PLAYING ? @string/audio_pause_description : @string/audio_play_description}"
       android:onClick="@{(v) -> viewModel.togglePlayPause(viewModel.playStatusLiveData)}"
-      android:padding="8dp"
+      android:padding="12dp"
       android:src="@{viewModel.playStatusLiveData == UiAudioPlayStatus.PLAYING ? @drawable/ic_pause_circle_filled_white_24dp : @drawable/ic_play_circle_filled_white_24dp}"
       app:layout_constraintBottom_toBottomOf="parent"
       app:layout_constraintStart_toStartOf="parent"
@@ -47,7 +45,8 @@
       style="@style/AudioSeekBar"
       android:layout_width="0dp"
       android:layout_height="26dp"
-      android:layout_marginStart="16dp"
+      android:layout_marginStart="4dp"
+      android:contentDescription="@string/audio_player_seekbar_content_description"
       android:max="@{viewModel.durationLiveData}"
       android:maxHeight="4dp"
       android:minWidth="180dp"
@@ -64,17 +63,12 @@
 
     <ImageView
       android:id="@+id/audio_language_icon"
-      android:layout_width="wrap_content"
-      android:layout_height="wrap_content"
-      android:contentDescription="@{viewModel.currentLanguageCode}"
+      android:layout_width="48dp"
+      android:layout_height="48dp"
+      android:contentDescription="@string/audio_language_icon_content_description"
       android:gravity="center"
-      android:minWidth="48dp"
-      android:minHeight="48dp"
       android:onClick="@{(v) -> audioFragment.languageSelectionClicked()}"
-      android:paddingStart="8dp"
-      android:paddingTop="16dp"
-      android:paddingEnd="16dp"
-      android:paddingBottom="16dp"
+      android:padding="12dp"
       android:src="@drawable/ic_audio_lang_24px"
       app:layout_constraintBottom_toBottomOf="parent"
       app:layout_constraintEnd_toEndOf="parent"

diff --git a/app/src/main/res/layout/exploration_activity.xml b/app/src/main/res/layout/exploration_activity.xml
@@ -62,8 +62,9 @@
             android:layout_width="48dp"
             android:layout_height="48dp"
             android:layout_gravity="end"
+            android:contentDescription="@{viewModel.isAudioStreamingOn ? @string/audio_player_on : @string/audio_player_off}"
             android:scaleType="center"
-            android:src="@{viewModel.isAudioStreamingOn ? @drawable/ic_audio_streaming_on_24dp :  @drawable/ic_audio_streaming_off_24dp}"
+            android:src="@{viewModel.isAudioStreamingOn ? @drawable/ic_audio_streaming_on_24dp : @drawable/ic_audio_streaming_off_24dp}"
             android:visibility="@{viewModel.showAudioButton ? View.VISIBLE : View.GONE}" />
         </LinearLayout>
       </androidx.appcompat.widget.Toolbar>

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -449,4 +449,8 @@
   <string name="stories_for_you">Stories For You</string>
   <string name="question_player_activity_title">Practice Mode</string>
   <string name="revision_card_activity_title">Skill revision page</string>
+  <string name="audio_player_seekbar_content_description">Audio progress</string>
+  <string name="audio_language_icon_content_description">Change language</string>
+  <string name="audio_player_on">Audio, ON</string>
+  <string name="audio_player_off">Audio, OFF</string>
 </resources>
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
@@ -41,7 +41,6 @@ import org.oppia.android.app.application.ApplicationInjector
 import org.oppia.android.app.application.ApplicationInjectorProvider
 import org.oppia.android.app.application.ApplicationModule
 import org.oppia.android.app.application.ApplicationStartupListenerModule
-import org.oppia.android.app.model.AudioLanguage
 import org.oppia.android.app.model.ProfileId
 import org.oppia.android.app.player.state.hintsandsolution.HintsAndSolutionConfigModule
 import org.oppia.android.app.shim.ViewBindingShimModule
@@ -79,7 +78,6 @@ import org.oppia.android.testing.threading.TestDispatcherModule
 import org.oppia.android.testing.time.FakeOppiaClockModule
 import org.oppia.android.util.accessibility.AccessibilityTestModule
 import org.oppia.android.util.caching.testing.CachingTestModule
-import org.oppia.android.util.data.DataProviders.Companion.toLiveData
 import org.oppia.android.util.gcsresource.GcsResourceModule
 import org.oppia.android.util.logging.LoggerModule
 import org.oppia.android.util.logging.firebase.FirebaseLogUploaderModule
@@ -154,36 +152,42 @@ class AudioFragmentTest {
   }
 
   @Test
-  fun testAudioFragment_withDefaultProfile_showsAudioLanguageAsEnglish() {
+  fun testAudioFragment_seekbar_hasContentDescription() {
     addMediaInfo()
     launch<AudioFragmentTestActivity>(
       createAudioFragmentTestIntent(
         internalProfileId
       )
     ).use {
-      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("en")))
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.audio_progress_seek_bar))
+        .check(
+          matches(
+            withContentDescription(
+              context.getString(R.string.audio_player_seekbar_content_description)
+            )
+          )
+        )
     }
   }
 
-  // TODO(#2417): Re-enable once this test passes on robolectric
-  @RunOn(TestPlatform.ESPRESSO)
   @Test
-  fun testAudioFragment_withHindiAudioLanguageProfile_showsHindiAudioLanguage() {
+  fun testAudioFragment_languageIcon_hasContentDescription() {
     addMediaInfo()
-    profileTestHelper.addOnlyAdminProfile()
-    val data = profileManagementController.updateAudioLanguage(
-      profileId,
-      AudioLanguage.HINDI_AUDIO_LANGUAGE
-    ).toLiveData()
     launch<AudioFragmentTestActivity>(
       createAudioFragmentTestIntent(
         internalProfileId
       )
     ).use {
-      it.onActivity {
-        profileTestHelper.waitForOperationToComplete(data)
-      }
-      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("hi")))
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.audio_language_icon))
+        .check(
+          matches(
+            withContentDescription(
+              context.getString(R.string.audio_language_icon_content_description)
+            )
+          )
+        )
     }
   }
 

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
@@ -26,6 +26,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.isChecked
 import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
 import androidx.test.espresso.matcher.ViewMatchers.isRoot
 import androidx.test.espresso.matcher.ViewMatchers.withContentDescription
@@ -209,6 +210,69 @@ class ExplorationActivityTest {
     explorationDataController.stopPlayingExploration()
   }
 
+  @Test
+  fun testExploration_toolbarAudioIcon_defaultContentDescription_isCorrect() {
+    setUpAudioForFractionLesson()
+    launch<ExplorationActivity>(
+      createExplorationActivityIntent(
+        internalProfileId,
+        FRACTIONS_TOPIC_ID,
+        FRACTIONS_STORY_ID_0,
+        FRACTIONS_EXPLORATION_ID_0
+      )
+    ).use {
+      explorationDataController.startPlayingExploration(FRACTIONS_EXPLORATION_ID_0)
+      networkConnectionUtil.setCurrentConnectionStatus(NetworkConnectionUtil.ConnectionStatus.LOCAL)
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.action_audio_player))
+        .check(matches(withContentDescription(context.getString(R.string.audio_player_off))))
+    }
+    explorationDataController.stopPlayingExploration()
+  }
+
+  @Test
+  fun testExploration_clickAudioIcon_contentDescription_changesCorrectly() {
+    setUpAudioForFractionLesson()
+    launch<ExplorationActivity>(
+      createExplorationActivityIntent(
+        internalProfileId,
+        FRACTIONS_TOPIC_ID,
+        FRACTIONS_STORY_ID_0,
+        FRACTIONS_EXPLORATION_ID_0
+      )
+    ).use {
+      explorationDataController.startPlayingExploration(FRACTIONS_EXPLORATION_ID_0)
+      networkConnectionUtil.setCurrentConnectionStatus(NetworkConnectionUtil.ConnectionStatus.LOCAL)
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.action_audio_player)).perform(click())
+      onView(withId(R.id.action_audio_player))
+        .check(matches(withContentDescription(context.getString(R.string.audio_player_on))))
+    }
+    explorationDataController.stopPlayingExploration()
+  }
+
+  @Test
+  fun testExploration_clickAudioIconTwice_contentDescription_changesToDefault() {
+    setUpAudioForFractionLesson()
+    launch<ExplorationActivity>(
+      createExplorationActivityIntent(
+        internalProfileId,
+        FRACTIONS_TOPIC_ID,
+        FRACTIONS_STORY_ID_0,
+        FRACTIONS_EXPLORATION_ID_0
+      )
+    ).use {
+      explorationDataController.startPlayingExploration(FRACTIONS_EXPLORATION_ID_0)
+      networkConnectionUtil.setCurrentConnectionStatus(NetworkConnectionUtil.ConnectionStatus.LOCAL)
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.action_audio_player)).perform(click())
+      onView(withId(R.id.action_audio_player)).perform(click())
+      onView(withId(R.id.action_audio_player))
+        .check(matches(withContentDescription(context.getString(R.string.audio_player_off))))
+    }
+    explorationDataController.stopPlayingExploration()
+  }
+
   @Test
   fun testExploration_overflowMenu_isDisplayedSuccessfully() {
     launch<ExplorationActivity>(
@@ -222,8 +286,7 @@ class ExplorationActivityTest {
       explorationDataController.startPlayingExploration(TEST_EXPLORATION_ID_2)
       openActionBarOverflowOrOptionsMenu(context)
       onView(withText(context.getString(R.string.menu_options))).check(matches(isDisplayed()))
-      onView(withText(context.getString(R.string.menu_help)))
-        .check(matches(isDisplayed()))
+      onView(withText(context.getString(R.string.menu_help))).check(matches(isDisplayed()))
     }
     explorationDataController.stopPlayingExploration()
   }
@@ -562,7 +625,7 @@ class ExplorationActivityTest {
   // TODO (#1855): Resolve ktlint max line in app module test
   @Test
   fun testAudioWithWifi_openFractionsExploration_changeLanguage_clickNext_checkLanguageIsHinglish() { // ktlint-disable max-line-length
-    setupAudioForFraction()
+    setUpAudioForFractionLesson()
     launch<ExplorationActivity>(
       createExplorationActivityIntent(
         internalProfileId,
@@ -601,7 +664,15 @@ class ExplorationActivityTest {
         )
       )
       onView(withId(R.id.continue_button)).perform(click())
-      onView(withId(R.id.audio_language_icon)).check(matches(withContentDescription("hi-en")))
+      onView(
+        allOf(
+          withId(R.id.audio_language_icon),
+          withEffectiveVisibility(Visibility.VISIBLE)
+        )
+      ).perform(click())
+      onView(withText("Hinglish"))
+        .inRoot(isDialog())
+        .check(matches(isChecked()))
     }
     explorationDataController.stopPlayingExploration()
   }
@@ -751,7 +822,7 @@ class ExplorationActivityTest {
     }
   }
 
-  private fun setupAudioForFraction() {
+  private fun setUpAudioForFractionLesson() {
     // Only initialize the Robolectric shadows when running on Robolectric (and use reflection since
     // Espresso can't load Robolectric into its classpath).
     if (isOnRobolectric()) {