Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size #5290

Vishwajith-Shettigar · 2023-12-25T06:02:49Z

Explanation

Support resizable reading text in resume lesson, question player and revision card.

Before	After
before.webm	device-2024-01-02-150914.webm
tablet_before.webm	tablet_after.webm

Question Player

questionPLayer.webm

Essential Checklist

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: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

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)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Vishwajith-Shettigar · 2023-12-25T14:52:54Z

@adhiamboperes, PTAL.

adhiamboperes

Thanks @Vishwajith-Shettigar!!

Please update the PR description and add before/after screenshots for mobile and tablets so that I can verify that the solution works.

oppiabot · 2024-01-02T07:37:24Z

Unassigning @adhiamboperes since the review is done.

oppiabot · 2024-01-02T07:37:27Z

Hi @Vishwajith-Shettigar, it looks like some changes were requested on this pull request by @adhiamboperes. PTAL. Thanks!

oppiabot · 2024-01-02T10:01:53Z

Unassigning @Vishwajith-Shettigar since a re-review was requested. @Vishwajith-Shettigar, please make sure you have addressed all review comments. Thanks!

Vishwajith-Shettigar · 2024-01-02T10:18:32Z

@adhiamboperes, PTAL.

adhiamboperes

Thanks @Vishwajith-Shettigar! This is an intyeresting solution.

I have some thoughts:

When thinking about possible User journey, it seems less likely that a user may want to change the text size on the resume lesson page, and more that they would expect it to show the same as the text size they selected the last time they were in a lesson.
As such, we would want to not add the option to change the text sie in the toolbar, but to retrieve the saved exploration reading text size, and apply it to this activity as well.

Please let me know if you have questions on how to go about this.

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivity.kt

adhiamboperes

Thanks @Vishwajith-Shettigar!

I have taken another full pass at this and left some comments. Most of them revolve around removing changes that do not contribute to the feature.

adhiamboperes · 2024-04-09T13:36:48Z

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivity.kt

@@ -110,7 +110,6 @@ class ResumeLessonActivity :
  }

  override fun onBackPressed() {
-    super.onBackPressed()


I believe this is needed for this lifecycle method

adhiamboperes · 2024-04-17T15:14:26Z

app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt

  }

  /** Action for onOptionsItemSelected. */
  fun handleOnOptionsItemSelected(itemId: Int): Boolean {
+    setReadingTextSizeMedium()


I think this should remain the same as the current selected font size, for consistency purposes.

We are only scaling content of the app, i don't think scaling option or help activity is necessary.
Would like to hear your opinion.

I don't really understand your comment in response to mine.

If we remove setReadingTextSizeMedium() from line no 135, option activity and help activity font size will also scale.
Below i have attached screen recording, PTAL.

OptionActivity.webm

app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivity.kt

app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardActivityPresenter.kt

app/src/main/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentPresenter.kt

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivity.kt

adhiamboperes · 2024-04-17T21:26:52Z

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivity.kt

@@ -87,6 +90,7 @@ class ResumeLessonActivity :
    parentScreen: ExplorationActivityParams.ParentScreen,
    isCheckpointingEnabled: Boolean
  ) {
+    resumeLessonActivityPresenter.setReadingTextSizeNormal()


I believe there is no need to reset the size here.

If we don't set text size medium it will affect the exploration tool bar title text size.

Resume.webm

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivityPresenter.kt

adhiamboperes · 2024-04-30T13:27:56Z

app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt

+      onView(withId(R.id.revision_card_explanation_text)).check(
+        matches(
+          withFontSize(
+            round(
+              context.resources.getDimension(
+                R.dimen.revision_card_fragment_font_scale_content_extra_large_text_size
+              )
+            )
+          )
+        )
+      )
+    }
+  }


In our discussion we had agreed to hardcode the values, e.g below.

Suggested change

onView(withId(R.id.revision_card_explanation_text)).check(

matches(

withFontSize(

round(

context.resources.getDimension(

R.dimen.revision_card_fragment_font_scale_content_extra_large_text_size

)

)

)

)

)

}

}

onView(withId(R.id.revision_card_explanation_text)).check(

matches(

withFontSize(

62.0F

)

)

)

I came up with 62F by logging the calculated value in the FontSizeMatcher.kt util class and checking logcat:

class FontSizeMatcher(private val expectedSize: Float) : TypeSafeMatcher<View?>(View::class.java) { override fun matchesSafely(target: View?): Boolean { if (target !is TextView) { return false } println("Textsize ${target.textSize}") return target.textSize.equals(expectedSize) }

You'll be able to find the appropriate values for other scale sizes too. Hope this helps.

adhiamboperes

I resolved some comments after verifying, and left some that I believe are still valid. PTAL.

oppiabot · 2024-05-07T14:11:30Z

Hi @Vishwajith-Shettigar, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot · 2024-05-21T07:05:16Z

Hi @Vishwajith-Shettigar, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

Vishwajith-Shettigar added 9 commits December 25, 2023 11:16

fixed reading text size for lesson activity

d4c9168

fixed reading text size for lesson activity

e46ca6b

fixed reading text size for lesson activity

380562b

fixed reading text size for lesson activity

8386f62

fixed reading text size for lesson activity

a158e5f

fixed reading text size for revision

1074505

fixed reading text size for revision

3c83b32

set reading text size medium for revision option items

e5ba6c3

lint issue fix

e393543

Vishwajith-Shettigar marked this pull request as ready for review December 25, 2023 14:52

Vishwajith-Shettigar requested review from a team as code owners December 25, 2023 14:52

Vishwajith-Shettigar requested a review from adhiamboperes December 25, 2023 14:52

Vishwajith-Shettigar assigned adhiamboperes Dec 25, 2023

Vishwajith-Shettigar added 2 commits December 27, 2023 09:20

lint issue fix

7e634ad

fix lint issue

35a9b51

Vishwajith-Shettigar requested a review from a team as a code owner December 28, 2023 03:45

adhiamboperes requested changes Jan 2, 2024

View reviewed changes

oppiabot bot unassigned adhiamboperes Jan 2, 2024

oppiabot bot assigned Vishwajith-Shettigar and adhiamboperes and unassigned Vishwajith-Shettigar Jan 2, 2024

Vishwajith-Shettigar requested a review from adhiamboperes January 2, 2024 10:19

adhiamboperes requested changes Jan 3, 2024

View reviewed changes

app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonActivity.kt Outdated Show resolved Hide resolved

adhiamboperes assigned Vishwajith-Shettigar Jan 3, 2024

adhiamboperes requested changes Apr 17, 2024

View reviewed changes

adhiamboperes assigned Vishwajith-Shettigar and unassigned adhiamboperes Apr 17, 2024

Vishwajith-Shettigar mentioned this pull request Apr 26, 2024

Story Text Size in other learner screens. #1491

Open

5 tasks

Vishwajith-Shettigar added 2 commits April 27, 2024 15:55

support text scaling question player

4306527

clean up

fef59c7

Vishwajith-Shettigar marked this pull request as draft April 27, 2024 10:31

Vishwajith-Shettigar changed the title ~~Fix #4362: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size~~ Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size Apr 27, 2024

adhiamboperes reviewed Apr 30, 2024

View reviewed changes

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 7, 2024

Vishwajith-Shettigar removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 8, 2024

Vishwajith-Shettigar and others added 14 commits May 9, 2024 16:04

Merge branch 'develop' into readingsize

0de4260

test fix

4e473c6

test fix

c723f4c

tests fix

58a1055

tests fix

9088d30

tests added for question player

b68b6fc

tests fix

8d5c612

tests fixed

cc7bf87

test changes

45b38ef

klint

45df691

test changes

6b8a9a8

fix

e197107

marks test as flaky

27f077f

mark test as flaky

ce57386

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 21, 2024

adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label May 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size #5290

Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size #5290

Vishwajith-Shettigar commented Dec 25, 2023 •

edited

Vishwajith-Shettigar commented Dec 25, 2023

adhiamboperes left a comment

oppiabot bot commented Jan 2, 2024

oppiabot bot commented Jan 2, 2024

oppiabot bot commented Jan 2, 2024

Vishwajith-Shettigar commented Jan 2, 2024

adhiamboperes left a comment

adhiamboperes left a comment

adhiamboperes Apr 9, 2024

adhiamboperes Apr 17, 2024

Vishwajith-Shettigar Apr 23, 2024

adhiamboperes Apr 30, 2024

Vishwajith-Shettigar May 10, 2024

adhiamboperes Apr 17, 2024

Vishwajith-Shettigar May 10, 2024

adhiamboperes Apr 30, 2024

Vishwajith-Shettigar May 10, 2024

adhiamboperes left a comment

oppiabot bot commented May 7, 2024

oppiabot bot commented May 21, 2024

Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size #5290

Are you sure you want to change the base?

Fix #4362, #1491: When reading text size is extra large, resume lesson page and revision tabs content is seen in normal size #5290

Conversation

Vishwajith-Shettigar commented Dec 25, 2023 • edited

Explanation

Essential Checklist

For UI-specific PRs only

Vishwajith-Shettigar commented Dec 25, 2023

adhiamboperes left a comment

Choose a reason for hiding this comment

oppiabot bot commented Jan 2, 2024

oppiabot bot commented Jan 2, 2024

oppiabot bot commented Jan 2, 2024

Vishwajith-Shettigar commented Jan 2, 2024

adhiamboperes left a comment

Choose a reason for hiding this comment

adhiamboperes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adhiamboperes left a comment

Choose a reason for hiding this comment

oppiabot bot commented May 7, 2024

oppiabot bot commented May 21, 2024

Vishwajith-Shettigar commented Dec 25, 2023 •

edited