Fix #4845: Dark mode support to Equations Text #4866

MohitGupta121 · 2023-02-04T10:24:50Z

Explanation

Fix #4845 : Dark mode support to Equations Text

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

Equation Text Screenshots

Equation Text Screen Recording

Equation.Dark.Mode.mp4

Equation Color Test

Day Mode Test

Night Mode Test

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)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

MohitGupta121

@rt4914 PTAL on the darkmode of equation-text if there is any changes.

@BenHenning I updated the Colors as we discuss in last meeting. Only now I left with kotlitex PR.

MohitGupta121 · 2023-02-04T17:55:36Z

The workflow Tests are failing as the library is not updated as per my changes but I raised PR for that changes directly into kotlitex library.
So please ignore the test failing for now.

MohitGupta121 · 2023-02-06T15:45:52Z

@BenHenning I make PR for Kotlitex Library Please first review and merge that to avoid build and workflow tests failures oppia/kotlitex#1

BenHenning

Thanks @MohitGupta121. oppia/kotlitex#1 is now merged, so this PR can be updated to point to the new latest commit to the fork.

Beyond that, is there also a way to add tests to verify that the correct color is being passed to the span for day/night modes? I think we can set this in Robolectric tests via config.

utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt

utility/src/main/java/org/oppia/android/util/parser/math/MathBitmapModelLoader.kt

utility/src/main/java/org/oppia/android/util/parser/html/HtmlParser.kt

MohitGupta121 · 2023-02-07T04:33:49Z

Beyond that, is there also a way to add tests to verify that the correct color is being passed to the span for day/night modes? I think we can set this in Robolectric tests via config.

About the tests we need to done that in this PR itself, I think tests will blocking this issue to clear. Just a suggestion.
What you think?

BenHenning · 2023-02-07T04:38:41Z

Beyond that, is there also a way to add tests to verify that the correct color is being passed to the span for day/night modes? I think we can set this in Robolectric tests via config.

About the tests we need to done that in this PR itself, I think tests will blocking this issue to clear. Just a suggestion. What you think?

Sorry I'm not sure that I'm following--what exactly is blocking adding tests?

MohitGupta121 · 2023-02-07T04:42:47Z

Sorry I'm not sure that I'm following--what exactly is blocking adding tests?

Okay sure, I'll add tests for that also. Only blocking is that I have not have clear idea about that tests but I try in this so that it helps me in future issues also.

MohitGupta121 · 2023-02-07T08:15:51Z

@BenHenning I not understand why this https://github.com/oppia/oppia-android/actions/runs/4110540436/jobs/7093464931#step:13:1542 tests is failing as in my local R is getting from utility and it's working fine.

BenHenning

Thanks @MohitGupta121! Took another pass--PTAL.

utility/src/main/java/org/oppia/android/util/parser/math/BUILD.bazel

utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt

utility/src/main/java/org/oppia/android/util/parser/math/MathBitmapModelLoader.kt

utility/src/test/java/org/oppia/android/util/parser/html/MathTagHandlerTest.kt

utility/src/main/java/org/oppia/android/util/parser/math/MathBitmapModelLoader.kt

rt4914

For codeowner files.

… darkmode-equations

MohitGupta121

@BenHenning PTAL, I make all changes as per review.

BenHenning

Thanks @MohitGupta121. This LGTM!

BenHenning · 2023-03-20T23:57:50Z

Updating from develop & enabling auto-merge.

BenHenning · 2023-03-20T23:59:12Z

Also @MohitGupta121 please note the edits I made to your PR title & description for the correct verb tenses to use when referring to the issues being fixed. Part of this is to make sure GitHub closes the issues being fixed correctly, and the other is for consistency with other PRs (which is ultimately based on a Git best practice when referring to issues being fixed).

oppiabot · 2023-03-21T00:00:42Z

Unassigning @BenHenning since they have already approved the PR.

oppiabot · 2023-03-21T00:00:47Z

Hi @MohitGupta121, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

MohitGupta121 · 2023-03-21T03:28:38Z

@BenHenning Yes Thanks, noted the changes that you made. Yes the issue is also closed now.

## Explanation Fixes oppia#4845 : Dark mode support to Equations Text  ## Essential Checklist  - [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 ### Equation Text Screenshots <img src="https://user-images.githubusercontent.com/76530270/216761443-21a973e8-20f9-4fbb-a19d-595530dfebb9.jpg" height="400" style="max-width: 100%"> <img src="https://user-images.githubusercontent.com/76530270/216761463-b4a49932-7b9c-4b2a-b3e6-96a5a693f058.jpg" height="400" style="max-width: 100%"> ### Equation Text Screen Recording https://user-images.githubusercontent.com/76530270/216761782-027744b7-a0df-41ca-8547-2e7f52eaea0c.mp4 ## Equation Color Test #### Day Mode Test ![image](https://user-images.githubusercontent.com/76530270/222653431-05f7e726-c8f0-4677-a48e-8ca37c5ea8e4.png) #### Night Mode Test ![image](https://user-images.githubusercontent.com/76530270/222653513-458e25ce-9508-480e-bdfc-a4ede5e8a0ed.png)  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 --------- Co-authored-by: Ben Henning <ben@oppia.org>

MohitGupta121 added 4 commits February 2, 2023 11:00

Fixed Equation dark mode color added colors in utility res.

bf9ad69

Merge remote-tracking branch 'upstream/develop' into darkmode-equations

51af3aa

Workflow fixed

58f42f0

Lint import error fixed

b188c40

MohitGupta121 requested review from BenHenning and rt4914 as code owners February 4, 2023 10:24

MohitGupta121 assigned rt4914 and BenHenning Feb 4, 2023

MohitGupta121 commented Feb 4, 2023

View reviewed changes

MohitGupta121 mentioned this pull request Feb 4, 2023

Add dark mode support to Equations Text #4845

Closed

BenHenning requested changes Feb 7, 2023

View reviewed changes

BenHenning assigned MohitGupta121 and unassigned BenHenning Feb 7, 2023

MohitGupta121 added 2 commits February 7, 2023 09:19

Merge remote-tracking branch 'upstream/develop' into darkmode-equations

3b78da0

Point Kotlitex to latest commit, improve readability of parameters.

88928cf

MohitGupta121 added 9 commits February 8, 2023 15:31

Refac: Link Bazel Migration Issue with this PR.

0ff1366

Fixed Unresolved resources R. in Bazel

7b868e0

Lint Check Fixed under math bazel

55f3bfc

Minor Lint check fixed

49cced4

bazel resources visibility fixed

561fc78

bazel lint fixed

e79ebea

bazel visiblity try

7e761c0

bazel visiblity fixed

6fb63cc

bazel visiblity fixed

82b7392

Merge branch 'develop' into darkmode-equations

8aa9357

BenHenning reviewed Mar 11, 2023

View reviewed changes

BenHenning assigned MohitGupta121 and unassigned BenHenning Mar 11, 2023

MohitGupta121 and others added 2 commits March 11, 2023 10:43

Merge branch 'develop' into darkmode-equations

78d7b56

Fix some review changes.

92c8692

rt4914 approved these changes Mar 11, 2023

View reviewed changes

rt4914 removed their assignment Mar 11, 2023

MohitGupta121 and others added 3 commits March 14, 2023 22:44

Merge branch 'develop' into darkmode-equations

3b2cf1f

Merge branch 'develop' of https://github.com/oppia/oppia-android into…

cc263b5

… darkmode-equations

Fixed Ben's Review changes

dc0dfd8

MohitGupta121 removed their assignment Mar 15, 2023

MohitGupta121 requested a review from BenHenning March 15, 2023 08:10

MohitGupta121 assigned BenHenning Mar 15, 2023

MohitGupta121 commented Mar 15, 2023

View reviewed changes

BenHenning approved these changes Mar 20, 2023

View reviewed changes

Merge branch 'develop' into darkmode-equations

dffff4f

BenHenning changed the title ~~Fixed #4845 : Dark mode support to Equations Text~~ Fix #4845: Dark mode support to Equations Text Mar 20, 2023

BenHenning enabled auto-merge (squash) March 20, 2023 23:59

oppiabot bot unassigned BenHenning Mar 21, 2023

oppiabot bot added the PR: LGTM label Mar 21, 2023

oppiabot bot assigned MohitGupta121 Mar 21, 2023

BenHenning merged commit 0b26c24 into oppia:develop Mar 21, 2023

MohitGupta121 deleted the darkmode-equations branch March 21, 2023 03:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4845: Dark mode support to Equations Text #4866

Fix #4845: Dark mode support to Equations Text #4866

MohitGupta121 commented Feb 4, 2023 •

edited

Loading

MohitGupta121 left a comment

MohitGupta121 commented Feb 4, 2023 •

edited

Loading

MohitGupta121 commented Feb 6, 2023 •

edited

Loading

BenHenning left a comment

MohitGupta121 commented Feb 7, 2023

BenHenning commented Feb 7, 2023

MohitGupta121 commented Feb 7, 2023

MohitGupta121 commented Feb 7, 2023

BenHenning left a comment

rt4914 left a comment

MohitGupta121 left a comment

BenHenning left a comment

BenHenning commented Mar 20, 2023 •

edited

Loading

BenHenning commented Mar 20, 2023

oppiabot bot commented Mar 21, 2023

oppiabot bot commented Mar 21, 2023

MohitGupta121 commented Mar 21, 2023

Fix #4845: Dark mode support to Equations Text #4866

Fix #4845: Dark mode support to Equations Text #4866

Conversation

MohitGupta121 commented Feb 4, 2023 • edited Loading

Explanation

Essential Checklist

For UI-specific PRs only

Equation Text Screenshots

Equation Text Screen Recording

Equation Color Test

Day Mode Test

Night Mode Test

MohitGupta121 left a comment

Choose a reason for hiding this comment

MohitGupta121 commented Feb 4, 2023 • edited Loading

MohitGupta121 commented Feb 6, 2023 • edited Loading

BenHenning left a comment

Choose a reason for hiding this comment

MohitGupta121 commented Feb 7, 2023

BenHenning commented Feb 7, 2023

MohitGupta121 commented Feb 7, 2023

MohitGupta121 commented Feb 7, 2023

BenHenning left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

MohitGupta121 left a comment

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

BenHenning commented Mar 20, 2023 • edited Loading

BenHenning commented Mar 20, 2023

oppiabot bot commented Mar 21, 2023

oppiabot bot commented Mar 21, 2023

MohitGupta121 commented Mar 21, 2023

MohitGupta121 commented Feb 4, 2023 •

edited

Loading

MohitGupta121 commented Feb 4, 2023 •

edited

Loading

MohitGupta121 commented Feb 6, 2023 •

edited

Loading

BenHenning commented Mar 20, 2023 •

edited

Loading