Skip to content

Commit

Permalink
Fix #3843 Split Concatenated TextView (#5151)
Browse files Browse the repository at this point in the history
## Explanation
Fix #3843 Instead of using 1 textview to display a concatenated string,
now have 1 textview for story progress, 1 for bar separator, and 1 for
topic progress.

## 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

|LTR|RTL|
|---|---|

|![Screenshot_20230913_170309](https://github.com/oppia/oppia-android/assets/74568012/5d10ee41-41cb-46f1-b37d-b368e247c108)|![Screenshot_20230913_170148](https://github.com/oppia/oppia-android/assets/74568012/d5ee32ce-1899-4e5c-825c-3070f5e132ad)|

|![Screenshot_20230913_165312](https://github.com/oppia/oppia-android/assets/74568012/4435074e-41b3-4b0b-9dd1-7cfc1b3bfd72)|![Screenshot_20230913_170218](https://github.com/oppia/oppia-android/assets/74568012/476060ea-15b8-4636-afa0-89fdb130d05b)|




## Tests
![Screenshot 2023-09-15
101749](https://github.com/oppia/oppia-android/assets/74568012/2ee79906-22e8-42a0-b3b6-b71676b50f92)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
XichengSpencer and adhiamboperes authored Sep 22, 2023
1 parent c28b8ee commit cfce93c
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,40 @@ class NavigationDrawerHeaderViewModel @Inject constructor(
val profile = ObservableField(Profile.getDefaultInstance())
private var ongoingTopicCount = DEFAULT_ONGOING_TOPIC_COUNT
private var completedStoryCount = DEFAULT_COMPLETED_STORY_COUNT
val profileProgressText: ObservableField<String> = ObservableField(computeProfileProgressText())
val profileTopicProgressText: ObservableField<String> =
ObservableField(computeProfileTopicProgressText())
val profileStoryProgressText: ObservableField<String> =
ObservableField(computeProfileStoryProgressText())

fun onHeaderClicked() {
routeToProfileProgressListener.routeToProfileProgress(profile.get()!!.id.internalId)
}

fun setOngoingTopicProgress(ongoingTopicCount: Int) {
this.ongoingTopicCount = ongoingTopicCount
profileProgressText.set(computeProfileProgressText())
profileTopicProgressText.set(computeProfileTopicProgressText())
}

fun setCompletedStoryProgress(completedStoryCount: Int) {
this.completedStoryCount = completedStoryCount
profileProgressText.set(computeProfileProgressText())
profileStoryProgressText.set(computeProfileStoryProgressText())
}

private fun computeProfileProgressText(): String {
// TODO(#3843): Either combine these strings into one or use separate views to display them.
val completedStoryCountText =
resourceHandler.getQuantityStringInLocaleWithWrapping(
R.plurals.completed_story_count, completedStoryCount, completedStoryCount.toString()
)
val ongoingTopicCountText =
resourceHandler.getQuantityStringInLocaleWithWrapping(
R.plurals.ongoing_topic_count, ongoingTopicCount, ongoingTopicCount.toString()
)
val barSeparator = resourceHandler.getStringInLocale(R.string.bar_separator)
return "$completedStoryCountText$barSeparator$ongoingTopicCountText"
private fun computeProfileStoryProgressText(): String {
return resourceHandler.getQuantityStringInLocaleWithWrapping(
R.plurals.completed_story_count,
completedStoryCount,
completedStoryCount.toString()
)
}

fun getBarSeparator() = resourceHandler.getStringInLocale(R.string.bar_separator)

private fun computeProfileTopicProgressText(): String {
return resourceHandler.getQuantityStringInLocaleWithWrapping(
R.plurals.ongoing_topic_count,
ongoingTopicCount,
ongoingTopicCount.toString()
)
}
}
51 changes: 41 additions & 10 deletions app/src/main/res/layout/nav_header_navigation_drawer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
<com.google.android.material.imageview.ShapeableImageView
android:layout_width="64dp"
android:layout_height="64dp"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:shapeAppearanceOverlay="@style/ShapeAppearanceOverlay.RoundedShape"
profile:src="@{viewModel.profile.avatar}" />

<TextView
Expand All @@ -44,16 +44,47 @@
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp" />

<TextView
android:id="@+id/profile_progress_text_view"
style="@style/TextViewStart"
<LinearLayout
android:id="@+id/progress_linear_layout"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.profileProgressText}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp" />
android:orientation="horizontal">

<TextView
android:id="@+id/profile_story_progress_text_view"
style="@style/TextViewStart"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.profileStoryProgressText}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp" />

<TextView
android:id="@+id/profile_topic_separator"
style="@style/TextViewStart"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.getBarSeparator()}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp" />

<TextView
android:id="@+id/profile_topic_progress_text_view"
style="@style/TextViewStart"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="8dp"
android:fontFamily="sans-serif"
android:text="@{viewModel.profileTopicProgressText}"
android:textColor="@color/component_color_shared_secondary_4_text_color"
android:textSize="14sp" />
</LinearLayout>
</LinearLayout>
</layout>
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class NavigationDrawerActivityProdTest {
}

@Test
fun testNavDrawer_openNavDrawer_oneTopicInProgress_profileProgressIsDisplayedCorrectly() {
fun testNavDrawer_openNavDrawer_oneTopicInProgress_profileStoryProgressIsDisplayedCorrectly() {
storyProfileTestHelper.markCompletedRatiosStory1Exp0(
ProfileId.newBuilder().setInternalId(
internalProfileId
Expand All @@ -270,10 +270,32 @@ class NavigationDrawerActivityProdTest {
it.openNavigationDrawer()
onView(
allOf(
withId(R.id.profile_progress_text_view),
isDescendantOfA(withId(R.id.header_linear_layout))
withId(R.id.profile_story_progress_text_view),
isDescendantOfA(withId(R.id.progress_linear_layout))
)
).check(matches(withText("1 Story Completed")))
}
}

@Test
fun testNavDrawer_openNavDrawer_oneTopicInProgress_profileTopicProgressIsDisplayedCorrectly() {
storyProfileTestHelper.markCompletedRatiosStory1Exp0(
ProfileId.newBuilder().setInternalId(
internalProfileId
).build(),
timestampOlderThanOneWeek = false
)
launch<NavigationDrawerTestActivity>(
createNavigationDrawerActivityIntent(internalProfileId)
).use {
testCoroutineDispatchers.runCurrent()
it.openNavigationDrawer()
onView(
allOf(
withId(R.id.profile_topic_progress_text_view),
isDescendantOfA(withId(R.id.progress_linear_layout))
)
).check(matches(withText("1 Story Completed | 1 Topic in Progress")))
).check(matches(withText("1 Topic in Progress")))
}
}

Expand Down

0 comments on commit cfce93c

Please sign in to comment.