Skip to content

Commit

Permalink
Fix #4845: Dark mode support to Equations Text (#4866)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation

Fixes #4845 : Dark mode support to Equations Text

<!--
- 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.
  -->

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

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


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

---------

Co-authored-by: Ben Henning <ben@oppia.org>
  • Loading branch information
MohitGupta121 and BenHenning authored Mar 21, 2023
1 parent cbda9d1 commit 0b26c24
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 7 deletions.
2 changes: 1 addition & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ git_repository(
# min target SDK version to be compatible with Oppia.
git_repository(
name = "kotlitex",
commit = "6b7db8ff9e0f4a70bdaa25f482143e038fd0c301",
commit = "43139c140833c7120f351d63d74b42c253d2b213",
remote = "https://github.com/oppia/kotlitex",
shallow_since = "1647554845 -0700",
)
Expand Down
1 change: 1 addition & 0 deletions utility/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ android_library(
custom_package = "org.oppia.android.util",
manifest = "src/main/AndroidManifest.xml",
resource_files = glob(["src/main/res/**/*.xml"]),
visibility = ["//visibility:public"],
)

# Visibility for migrated utility tests.
Expand Down
2 changes: 1 addition & 1 deletion utility/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ dependencies {
'androidx.lifecycle:lifecycle-livedata-ktx:2.2.0-alpha03',
'androidx.work:work-runtime-ktx:2.4.0',
'com.github.oppia:androidsvg:4bc1d26412f0fb9fd4ef263fa93f6a64f4d4dbcf',
'com.github.oppia:kotlitex:6b7db8ff9e0f4a70bdaa25f482143e038fd0c301',
'com.github.oppia:kotlitex:43139c140833c7120f351d63d74b42c253d2b213',
'com.github.bumptech.glide:glide:4.11.0',
'com.google.dagger:dagger:2.24',
'com.google.firebase:firebase-analytics-ktx:17.5.0',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.oppia.android.util.parser.html

import android.app.Application
import android.content.Context
import android.text.Spannable
import android.text.SpannableStringBuilder
Expand Down Expand Up @@ -134,7 +135,8 @@ class HtmlParser private constructor(
consoleLogger,
context.assets,
htmlContentTextView.lineHeight.toFloat(),
cacheLatexRendering
cacheLatexRendering,
context as? Application ?: context.applicationContext as Application
)
if (supportsConceptCards) {
handlersMap[CUSTOM_CONCEPT_CARD_TAG] = conceptCardTagHandler
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package org.oppia.android.util.parser.html

import android.app.Application
import android.content.res.AssetManager
import android.text.Editable
import android.text.Spannable
import android.text.style.ImageSpan
import androidx.core.content.res.ResourcesCompat
import io.github.karino2.kotlitex.view.MathExpressionSpan
import org.json.JSONObject
import org.oppia.android.util.R
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.parser.html.CustomHtmlContentHandler.ImageRetriever.Type.BLOCK_IMAGE
import org.oppia.android.util.parser.html.CustomHtmlContentHandler.ImageRetriever.Type.INLINE_TEXT_IMAGE
Expand All @@ -24,7 +27,8 @@ class MathTagHandler(
private val consoleLogger: ConsoleLogger,
private val assetManager: AssetManager,
private val lineHeight: Float,
private val cacheLatexRendering: Boolean
private val cacheLatexRendering: Boolean,
private val application: Application
) : CustomHtmlContentHandler.CustomTagHandler {
override fun handleTag(
attributes: Attributes,
Expand Down Expand Up @@ -65,7 +69,15 @@ class MathTagHandler(
)
} else {
MathExpressionSpan(
content.rawLatex, lineHeight, assetManager, isMathMode = !useInlineRendering
content.rawLatex,
lineHeight,
assetManager,
isMathMode = !useInlineRendering,
ResourcesCompat.getColor(
application.resources,
R.color.component_color_shared_equation_color,
/* theme = */ null
)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ kt_android_library(
":math_latex_model",
"//third_party:com_github_bumptech_glide_glide",
"//third_party:io_github_karino2_kotlitex",
"//utility:resources",
"//utility/src/main/java/org/oppia/android/util/logging:console_logger_injector_provider",
"//utility/src/main/java/org/oppia/android/util/threading:dispatcher_injector_provider",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import android.text.Spannable
import android.text.SpannableStringBuilder
import android.text.StaticLayout
import android.text.TextPaint
import androidx.core.content.res.ResourcesCompat
import com.bumptech.glide.Priority
import com.bumptech.glide.load.DataSource
import com.bumptech.glide.load.Options
Expand All @@ -26,6 +27,7 @@ import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.oppia.android.util.R
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.ConsoleLoggerInjectorProvider
import org.oppia.android.util.threading.DispatcherInjectorProvider
Expand Down Expand Up @@ -109,7 +111,16 @@ class MathBitmapModelLoader private constructor(
// creation can still happen in parallel, and those are the more expensive steps.
val span = withContext(CoroutineScope(blockingDispatcher).coroutineContext) {
MathExpressionSpan(
model.rawLatex, model.lineHeight, application.assets, !model.useInlineRendering
model.rawLatex,
model.lineHeight,
application.assets,
!model.useInlineRendering,
// TODO(#1523): Test color parameter in MathBitmapModelLoader
ResourcesCompat.getColor(
application.resources,
R.color.component_color_shared_equation_color,
/* theme = */null
)
).also { it.ensureDrawable() }
}
val renderableText = SpannableStringBuilder("\uFFFC").apply {
Expand All @@ -121,6 +132,7 @@ class MathBitmapModelLoader private constructor(
// since the width isn't necessarily known ahead of time).
// Any TextPaint can be used since the span will use its own.
val textPaint = TextPaint()

@Suppress("DEPRECATION") // This call is necessary for the supported min API version.
val staticTextLayout =
StaticLayout(
Expand Down
5 changes: 5 additions & 0 deletions utility/src/main/res/values-night/color_palette.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- TODO(#59): Consolidate color resource files once Bazel migration is done -->
<resources>
<color name="color_palette_shared_equation_color">@color/color_def_white</color>
</resources>
6 changes: 6 additions & 0 deletions utility/src/main/res/values/color_defs.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- TODO(#59): Consolidate color resource files once Bazel migration is done -->
<resources>
<color name="color_def_black">#000000</color>
<color name="color_def_white">#FFFFFF</color>
</resources>
5 changes: 5 additions & 0 deletions utility/src/main/res/values/color_palette.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- TODO(#59): Consolidate color resource files once Bazel migration is done -->
<resources>
<color name="color_palette_shared_equation_color">@color/color_def_black</color>
</resources>
5 changes: 5 additions & 0 deletions utility/src/main/res/values/component_colors.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- TODO(#59): Consolidate color resource files once Bazel migration is done -->
<resources>
<color name="component_color_shared_equation_color">@color/color_palette_shared_equation_color</color>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.oppia.android.util.parser.html

import android.app.Application
import android.content.Context
import android.graphics.Color
import android.text.Html
import android.text.Spannable
import android.text.style.ImageSpan
Expand Down Expand Up @@ -34,6 +35,7 @@ import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.LoggerModule
import org.oppia.android.util.parser.html.CustomHtmlContentHandler.CustomTagHandler
import org.oppia.android.util.parser.html.CustomHtmlContentHandler.ImageRetriever
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import javax.inject.Inject
import javax.inject.Singleton
Expand Down Expand Up @@ -246,6 +248,33 @@ class MathTagHandlerTest {
verifyNoMoreInteractions(mockImageRetriever) // No cached image loading.
}

@Test
fun testParseHtmlDayColor_withMathMarkup_cachingOff_getEquationDayColor() {
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithUncachedMathSupport
)

val equationColor = parsedHtml.getSpansFromWholeString(MathExpressionSpan::class)
assertThat(equationColor[0].equationColor).isEqualTo(Color.BLACK)
}

@Config(qualifiers = "night")
@Test
fun testParseHtmlNightColor_withMathMarkup_cachingOff_getEquationNightColor() {
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithUncachedMathSupport
)

val equationColor = parsedHtml.getSpansFromWholeString(MathExpressionSpan::class)
assertThat(equationColor[0].equationColor).isEqualTo(Color.WHITE)
}

@Test
fun testParseHtml_noTagHandler_withMathMarkup_doesNotIncludeImageSpan() {
val parsedHtml =
Expand Down Expand Up @@ -306,7 +335,13 @@ class MathTagHandlerTest {

private fun createMathTagHandler(cacheLatexRendering: Boolean): MathTagHandler {
// Pick an arbitrary line height since rendering doesn't actually happen in tests.
return MathTagHandler(consoleLogger, context.assets, lineHeight = 10.0f, cacheLatexRendering)
return MathTagHandler(
consoleLogger,
context.assets,
lineHeight = 10.0f,
cacheLatexRendering,
application = context.applicationContext as Application
)
}

private fun <T : Any> Spannable.getSpansFromWholeString(spanClass: KClass<T>): Array<T> =
Expand Down

0 comments on commit 0b26c24

Please sign in to comment.