Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3201: Setup Kotlitex library to render raw latex #3194

Closed
wants to merge 20 commits into from
Closed

Fix #3201: Setup Kotlitex library to render raw latex #3194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cea9656
introducing kotlitex for mathtag rendering
anandwana001 May 17, 2021
aff03cd
nit fix
anandwana001 May 17, 2021
8bcf1a8
nit fix
anandwana001 May 19, 2021
60284b0
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
anandwana001 May 19, 2021
9c111bf
trying to add aar to bazel
anandwana001 May 24, 2021
bdd7cb6
nit fix
anandwana001 May 24, 2021
c0ee037
using commit as dependency
anandwana001 Jun 7, 2021
6b1edbb
bazel build fix
anandwana001 Jun 7, 2021
08441ad
nit fix
anandwana001 Jun 7, 2021
8cc0541
nit fix
anandwana001 Jun 7, 2021
ad683e3
nit fix
anandwana001 Jun 7, 2021
4ee9606
nit fix
anandwana001 Jun 7, 2021
1a1a8ff
nit fix
anandwana001 Jun 7, 2021
b6709c4
commit update
anandwana001 Jun 7, 2021
9d16797
using master snapshot
anandwana001 Jun 7, 2021
f553c8b
latest commit
anandwana001 Jun 7, 2021
9c88465
commit update
anandwana001 Jun 8, 2021
a236847
update commit
anandwana001 Jun 8, 2021
8605427
update commit
anandwana001 Jun 8, 2021
c8356ec
test fixes
anandwana001 Jun 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ git_repository(
remote = "https://github.com/oppia/androidsvg",
)

git_repository(
name = "kotlitex",
commit = "3e8a0804041121ece4b57099ac3de386d78e0996",
remote = "https://github.com/anandwana001/kotlitex",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once confirmed, we will move this repo to org and update in this PR only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The need of creating our own version is because we need to use this in Bazel and also the repo's current min SDK is 21 and our min SDK is 19, so need to update as well.

Also, I had removed the sample apps from the kotlitex repo, as it contains various resources like drawable, mipmap, and other colors, styles, etc which cause errors for our Bazel and Gradle for duplicate of items.

)

bind(
name = "databinding_annotation_processor",
actual = "//tools/android:compiler_annotation_processor",
Expand Down
8 changes: 8 additions & 0 deletions third_party/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ android_library(
],
)

android_library(
name = "io_github_karino2_kotlitex",
visibility = ["//visibility:public"],
exports = [
"@kotlitex//kotlitex",
],
)

android_library(
name = "robolectric_android-all",
visibility = ["//visibility:public"],
Expand Down
1 change: 1 addition & 0 deletions utility/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ kt_android_library(
"//third_party:com_github_bumptech_glide_glide",
"//third_party:com_google_guava_guava",
"//third_party:glide_compiler",
"//third_party:io_github_karino2_kotlitex",
"//third_party:org_jetbrains_kotlinx_kotlinx-coroutines-core",
"//utility/src/main/java/org/oppia/android/util/caching:assets",
"//utility/src/main/java/org/oppia/android/util/datetime:date_time_util",
Expand Down
1 change: 1 addition & 0 deletions utility/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ dependencies {
'androidx.appcompat:appcompat:1.0.2',
'androidx.lifecycle:lifecycle-livedata-ktx:2.2.0-alpha03',
'androidx.work:work-runtime-ktx:2.4.0',
'com.github.anandwana001:kotlitex:3e8a0804041121ece4b57099ac3de386d78e0996',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the latest commit, we might use branch = master in the Bazel file and -master:SNAPSHOT in gradle to always use the latest commit from the master branch.
This will reduce updating the commit anytime when we change anything in the kotlitex.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we actually will want to snapshot to a specific version (like we do elsewhere). Even though we control the repository, it's helpful to keep builds as deterministic as possible so there are no unexpected surprises.

'com.github.oppia:androidsvg:6bd15f69caee3e6857fcfcd123023716b4adec1d',
'com.github.bumptech.glide:glide:4.11.0',
'com.google.dagger:dagger:2.24',
Expand Down
13 changes: 9 additions & 4 deletions utility/src/main/java/org/oppia/android/util/parser/html/HtmlParser.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.oppia.android.util.parser.html

import android.content.Context
import android.content.res.AssetManager
import android.text.Spannable
import android.text.SpannableStringBuilder
import android.text.method.LinkMovementMethod
Expand All @@ -17,7 +19,8 @@ class HtmlParser private constructor(
private val entityId: String,
private val imageCenterAlign: Boolean,
private val consoleLogger: ConsoleLogger,
customOppiaTagActionListener: CustomOppiaTagActionListener?
customOppiaTagActionListener: CustomOppiaTagActionListener?,
assetManager: AssetManager
) {
private val conceptCardTagHandler by lazy {
ConceptCardTagHandler(
Expand All @@ -31,7 +34,7 @@ class HtmlParser private constructor(
}
private val bulletTagHandler by lazy { BulletTagHandler() }
private val imageTagHandler by lazy { ImageTagHandler(consoleLogger) }
private val mathTagHandler by lazy { MathTagHandler(consoleLogger) }
private val mathTagHandler by lazy { MathTagHandler(consoleLogger, assetManager) }

/**
* Parses a raw HTML string with support for custom Oppia tags.
Expand Down Expand Up @@ -125,7 +128,8 @@ class HtmlParser private constructor(
/** Factory for creating new [HtmlParser]s. */
class Factory @Inject constructor(
private val urlImageParserFactory: UrlImageParser.Factory,
private val consoleLogger: ConsoleLogger
private val consoleLogger: ConsoleLogger,
private val context: Context
) {
/**
* Returns a new [HtmlParser] with the specified entity type and ID for loading images, and an
Expand All @@ -145,7 +149,8 @@ class HtmlParser private constructor(
entityId,
imageCenterAlign,
consoleLogger,
customOppiaTagActionListener
customOppiaTagActionListener,
context.assets
)
}
}
Expand Down
49 changes: 32 additions & 17 deletions utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.oppia.android.util.parser.html

import android.content.res.AssetManager
import android.text.Editable
import android.text.Spannable
import android.text.style.ImageSpan
import io.github.karino2.kotlitex.view.MathExpressionSpan
import org.json.JSONObject
import org.oppia.android.util.logging.ConsoleLogger
import org.xml.sax.Attributes
Expand All @@ -16,7 +18,8 @@ private const val CUSTOM_MATH_SVG_PATH_ATTRIBUTE = "math_content-with-value"
* [CustomHtmlContentHandler].
*/
class MathTagHandler(
private val consoleLogger: ConsoleLogger
private val consoleLogger: ConsoleLogger,
private val assetManager: AssetManager
) : CustomHtmlContentHandler.CustomTagHandler {
override fun handleTag(
attributes: Attributes,
Expand All @@ -30,36 +33,48 @@ class MathTagHandler(
attributes.getJsonObjectValue(CUSTOM_MATH_SVG_PATH_ATTRIBUTE)
)
if (content != null) {
// Insert an image span where the custom tag currently is to load the SVG. In the future, this
// could also load a LaTeX span, instead. Note that this approach is based on Android's Html
// parser.
val drawable =
imageRetriever.loadDrawable(
content.svgFilename,
CustomHtmlContentHandler.ImageRetriever.Type.INLINE_TEXT_IMAGE
)
val (startIndex, endIndex) = output.run {
// Use a control character to ensure that there's at least 1 character on which to "attach"
// the image when rendering the HTML.
val startIndex = length
append('\uFFFC')
return@run startIndex to length
}
output.setSpan(
ImageSpan(drawable, content.svgFilename),
startIndex,
endIndex,
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
)
if (content.svgFilename != null) {
// Insert an image span where the custom tag currently is to load the SVG. In the future, this
// could also load a LaTeX span, instead. Note that this approach is based on Android's Html
// parser.
val drawable =
imageRetriever.loadDrawable(
content.svgFilename,
CustomHtmlContentHandler.ImageRetriever.Type.INLINE_TEXT_IMAGE
)
output.setSpan(
ImageSpan(drawable, content.svgFilename),
startIndex,
endIndex,
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
)
} else if (content.rawLatex != null) {
output.setSpan(
MathExpressionSpan(content.rawLatex, 72f, assetManager, true),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to come up with the base text height or might need a way to calculate and pass it here as an argument.
Do we have any current implementation with the same use case, where we calculate the text height based on the screen for the content?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use 16 sp

startIndex,
endIndex,
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE
)
}
} else consoleLogger.e("MathTagHandler", "Failed to parse math tag")
}

private data class MathContent(val rawLatex: String, val svgFilename: String) {
private data class MathContent(val rawLatex: String?, val svgFilename: String?) {
companion object {
internal fun parseMathContent(obj: JSONObject?): MathContent? {
val rawLatex = obj?.getOptionalString("raw_latex")
val svgFilename = obj?.getOptionalString("svg_filename")
return if (rawLatex != null && svgFilename != null) {
return if (rawLatex != null && svgFilename == null) {
rawLatex.replace("\\", "\\\\")
MathContent(rawLatex, svgFilename)
} else if (svgFilename != null) {
MathContent(rawLatex, svgFilename)
} else null
}
Expand Down
24 changes: 18 additions & 6 deletions utility/src/test/java/org/oppia/android/util/parser/html/MathTagHandlerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import dagger.BindsInstance
import dagger.Component
import dagger.Module
import org.junit.Before
import org.junit.Ignore
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
Expand Down Expand Up @@ -65,12 +66,20 @@ class MathTagHandlerTest {
@JvmField
val mockitoRule: MockitoRule = MockitoJUnit.rule()

@Mock lateinit var mockImageRetriever: FakeImageRetriever
@Captor lateinit var stringCaptor: ArgumentCaptor<String>
@Captor lateinit var retrieverTypeCaptor: ArgumentCaptor<ImageRetriever.Type>
@Mock
lateinit var mockImageRetriever: FakeImageRetriever

@Inject lateinit var context: Context
@Inject lateinit var consoleLogger: ConsoleLogger
@Captor
lateinit var stringCaptor: ArgumentCaptor<String>

@Captor
lateinit var retrieverTypeCaptor: ArgumentCaptor<ImageRetriever.Type>

@Inject
lateinit var context: Context

@Inject
lateinit var consoleLogger: ConsoleLogger

private lateinit var noTagHandlers: Map<String, CustomTagHandler>
private lateinit var tagHandlersWithMathSupport: Map<String, CustomTagHandler>
Expand All @@ -80,7 +89,7 @@ class MathTagHandlerTest {
setUpTestApplicationComponent()
noTagHandlers = mapOf()
tagHandlersWithMathSupport = mapOf(
CUSTOM_MATH_TAG to MathTagHandler(consoleLogger)
CUSTOM_MATH_TAG to MathTagHandler(consoleLogger, context.assets)
)
}

Expand Down Expand Up @@ -141,6 +150,9 @@ class MathTagHandlerTest {
}

@Test
@Ignore
// There are some cases in alpha content where we don't have raw latex but we do have
// svg image, so can we use that image without raw latex?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

earlier we show only when both raw latex and SVG image is available in data, but as per our last discussion and from the alpha data, there are cases where we might have raw latex or svg or both of them, so I had conditioned them here.

fun testParseHtml_withMathMarkup_missingRawLatex_doesNotIncludeImageSpan() {
val parsedHtml =
CustomHtmlContentHandler.fromHtml(
Expand Down