Skip to content

Commit

Permalink
Fixes #4737: No meaningful label of the revision screen navigation ca…
Browse files Browse the repository at this point in the history
…rds during talkback. (#5113)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fixes #4737 

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing


[talkback.webm](https://github.com/oppia/oppia-android/assets/92685493/c897e62f-3c8f-467f-88a8-e792f2974998)

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
  • Loading branch information
Akshatkamboj14 and adhiamboperes authored Aug 2, 2023
1 parent a888729 commit 37265c5
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ VIEW_MODELS_WITH_RESOURCE_IMPORTS = [
"src/main/java/org/oppia/android/app/topic/lessons/ChapterSummaryViewModel.kt",
"src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt",
"src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerViewModel.kt",
"src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardViewModel.kt",
"src/main/java/org/oppia/android/app/utility/RatioExtensions.kt",
"src/main/java/org/oppia/android/app/walkthrough/topiclist/topiclistviewmodel/WalkthroughTopicSummaryViewModel.kt",
]
Expand Down Expand Up @@ -365,7 +366,6 @@ VIEW_MODELS = [
"src/main/java/org/oppia/android/app/topic/practice/TopicPracticeViewModel.kt",
"src/main/java/org/oppia/android/app/topic/revision/revisionitemviewmodel/TopicRevisionItemViewModel.kt",
"src/main/java/org/oppia/android/app/topic/revision/TopicRevisionViewModel.kt",
"src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardViewModel.kt",
"src/main/java/org/oppia/android/app/walkthrough/end/WalkthroughFinalViewModel.kt",
"src/main/java/org/oppia/android/app/walkthrough/topiclist/topiclistviewmodel/WalkthroughTopicHeaderViewModel.kt",
"src/main/java/org/oppia/android/app/walkthrough/topiclist/WalkthroughTopicItemViewModel.kt",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package org.oppia.android.app.topic.revisioncard
import androidx.appcompat.app.AppCompatActivity
import androidx.lifecycle.LiveData
import androidx.lifecycle.Transformations
import org.oppia.android.R
import org.oppia.android.app.model.EphemeralRevisionCard
import org.oppia.android.app.model.EphemeralSubtopic
import org.oppia.android.app.model.EphemeralTopic
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.topic.RouteToRevisionCardListener
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ObservableViewModel
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.topic.TopicController
Expand Down Expand Up @@ -36,6 +38,7 @@ class RevisionCardViewModel private constructor(
val topicId: String,
val subtopicId: Int,
val profileId: ProfileId,
private val appLanguageResourceHandler: AppLanguageResourceHandler,
val subtopicListSize: Int
) : ObservableViewModel() {

Expand Down Expand Up @@ -100,6 +103,21 @@ class RevisionCardViewModel private constructor(
} ?: ""
}

/** Returns the content description of the subtopic. */
fun computeContentDescriptionText(subtopicLiveData: LiveData<EphemeralSubtopic>): String {
return when (subtopicLiveData) {
previousSubtopicLiveData -> appLanguageResourceHandler.getStringInLocaleWithWrapping(
R.string.previous_subtopic_talkback_text,
computeTitleText(previousSubtopicLiveData.value)
)
nextSubtopicLiveData -> appLanguageResourceHandler.getStringInLocaleWithWrapping(
R.string.next_subtopic_talkback_text,
computeTitleText(nextSubtopicLiveData.value)
)
else -> ""
}
}

private fun processPreviousSubtopicData(
topicLiveData: AsyncResult<EphemeralTopic>
): EphemeralSubtopic {
Expand Down Expand Up @@ -157,6 +175,7 @@ class RevisionCardViewModel private constructor(
private val topicController: TopicController,
private val oppiaLogger: OppiaLogger,
@TopicHtmlParserEntityType private val entityType: String,
private val appLanguageResourceHandler: AppLanguageResourceHandler,
private val translationController: TranslationController
) {
/** Returns a new [RevisionCardViewModel]. */
Expand All @@ -175,6 +194,7 @@ class RevisionCardViewModel private constructor(
topicId,
subtopicId,
profileId,
appLanguageResourceHandler,
subtopicListSize
)
}
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout-sw600dp/revision_card_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
android:paddingEnd="@dimen/topic_revision_summary_view_subtopic_title_text_view_padding_end"
android:paddingBottom="16dp"
android:text="@{viewModel.computeTitleText(viewModel.previousSubtopicLiveData)}"
android:contentDescription="@{viewModel.computeContentDescriptionText(viewModel.previousSubtopicLiveData)}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp"
android:textStyle="bold"
Expand Down Expand Up @@ -166,6 +167,7 @@
android:paddingEnd="@dimen/topic_revision_summary_view_subtopic_title_text_view_padding_end"
android:paddingBottom="16dp"
android:text="@{viewModel.computeTitleText(viewModel.nextSubtopicLiveData)}"
android:contentDescription="@{viewModel.computeContentDescriptionText(viewModel.nextSubtopicLiveData)}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp"
android:textStyle="bold"
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout/revision_card_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
android:paddingEnd="@dimen/topic_revision_summary_view_subtopic_title_text_view_padding_end"
android:paddingBottom="16dp"
android:text="@{viewModel.computeTitleText(viewModel.previousSubtopicLiveData)}"
android:contentDescription="@{viewModel.computeContentDescriptionText(viewModel.previousSubtopicLiveData)}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp"
android:textStyle="bold"
Expand Down Expand Up @@ -164,6 +165,7 @@
android:paddingEnd="@dimen/topic_revision_summary_view_subtopic_title_text_view_padding_end"
android:paddingBottom="16dp"
android:text="@{viewModel.computeTitleText(viewModel.nextSubtopicLiveData)}"
android:contentDescription="@{viewModel.computeContentDescriptionText(viewModel.nextSubtopicLiveData)}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp"
android:textStyle="bold"
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -623,4 +623,6 @@
<string name="nps_passive_feedback_question">Thanks for responding! How can we provide a better experience?</string>
<string name="nps_detractor_feedback_question">Help us improve your experience! Please share the primary reason for your score:</string>
<string name="nps_score_question">On a scale from 0–10, how likely are you to recommend %s to a friend or colleague?</string>
<string name="previous_subtopic_talkback_text">The previous subtopic is %s</string>
<string name="next_subtopic_talkback_text">The next subtopic is %s</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.test.espresso.intent.Intents.intended
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
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.isRoot
import androidx.test.espresso.matcher.ViewMatchers.withId
Expand Down Expand Up @@ -183,6 +184,50 @@ class RevisionCardFragmentTest {
Intents.release()
}

@Test
fun testRevisionCard_previousSubtopicTitle_whatIsAFraction_hasCorrectContentDescription() {
launch<RevisionCardActivity>(
createRevisionCardActivityIntent(
context,
profileId.internalId,
FRACTIONS_TOPIC_ID,
subtopicId = 2,
FRACTIONS_SUBTOPIC_LIST_SIZE
)
).use {
testCoroutineDispatchers.runCurrent()
onView(withId(R.id.prev_subtopic_title)).check(
matches(
ViewMatchers.withContentDescription(
"The previous subtopic is What is a Fraction?"
)
)
)
}
}

@Test
fun testRevisionCard_nextSubtopicTitle_mixedNumbers_hasCorrectContentDescription() {
launch<RevisionCardActivity>(
createRevisionCardActivityIntent(
context,
profileId.internalId,
FRACTIONS_TOPIC_ID,
subtopicId = 2,
FRACTIONS_SUBTOPIC_LIST_SIZE
)
).use {
testCoroutineDispatchers.runCurrent()
onView(withId(R.id.next_subtopic_title)).check(
matches(
ViewMatchers.withContentDescription(
"The next subtopic is Mixed Numbers"
)
)
)
}
}

@Test
fun testRevisionCardTest_initialise_openBottomSheet_showsBottomSheet() {
launch<ExplorationActivity>(
Expand Down

0 comments on commit 37265c5

Please sign in to comment.