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.

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 #5039: Terms of Service and Privacy Policy content should still be left-aligned, even if the app is in an RTL language #5296

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,27 @@ class PoliciesFragmentPresenter @Inject constructor(
policyWebLink = resourceHandler.getStringInLocale(R.string.terms_of_service_web_link)
}

binding.policyDescriptionTextView.textAlignment = View.TEXT_ALIGNMENT_TEXT_START
binding.policyDescriptionTextView.text = htmlParserFactory.create(
policyOppiaTagActionListener = this,
displayLocale = resourceHandler.getDisplayLocale()
displayLocale = resourceHandler.getDisplayLocale(),
supportLtr = true
).parseOppiaHtml(
policyDescription,
binding.policyDescriptionTextView,
supportsLinks = true,
supportsConceptCards = false
)

binding.policyWebLinkTextView.textAlignment = View.TEXT_ALIGNMENT_TEXT_START
binding.policyWebLinkTextView.text = htmlParserFactory.create(
gcsResourceName = "",
entityType = "",
entityId = "",
imageCenterAlign = false,
customOppiaTagActionListener = null,
resourceHandler.getDisplayLocale()
resourceHandler.getDisplayLocale(),
supportLtr = true
).parseOppiaHtml(
policyWebLink,
binding.policyWebLinkTextView,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class HtmlParser private constructor(
private val cacheLatexRendering: Boolean,
customOppiaTagActionListener: CustomOppiaTagActionListener?,
policyOppiaTagActionListener: PolicyOppiaTagActionListener?,
displayLocale: OppiaLocale.DisplayLocale
displayLocale: OppiaLocale.DisplayLocale,
private val supportLtr: Boolean = false
) {
private val conceptCardTagHandler by lazy {
ConceptCardTagHandler(
Expand All @@ -55,11 +56,11 @@ class HtmlParser private constructor(
consoleLogger
)
}
private val bulletTagHandler by lazy { LiTagHandler(context, displayLocale) }
private val bulletTagHandler by lazy { LiTagHandler(context, displayLocale, supportLtr) }
private val imageTagHandler by lazy { ImageTagHandler(consoleLogger) }

private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
(displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL) && !supportLtr
}

/**
Expand Down Expand Up @@ -223,7 +224,8 @@ class HtmlParser private constructor(
entityId: String,
imageCenterAlign: Boolean,
customOppiaTagActionListener: CustomOppiaTagActionListener? = null,
displayLocale: OppiaLocale.DisplayLocale
displayLocale: OppiaLocale.DisplayLocale,
supportLtr: Boolean = false
): HtmlParser {
return HtmlParser(
context,
Expand All @@ -236,7 +238,8 @@ class HtmlParser private constructor(
cacheLatexRendering = enableCacheLatexRendering.value,
customOppiaTagActionListener,
null,
displayLocale
displayLocale,
supportLtr = supportLtr
)
}

Expand All @@ -248,7 +251,9 @@ class HtmlParser private constructor(
*/
fun create(
policyOppiaTagActionListener: PolicyOppiaTagActionListener? = null,
displayLocale: OppiaLocale.DisplayLocale
displayLocale: OppiaLocale.DisplayLocale,
supportLtr: Boolean = false

): HtmlParser {
return HtmlParser(
context = context,
Expand All @@ -261,7 +266,8 @@ class HtmlParser private constructor(
cacheLatexRendering = false,
customOppiaTagActionListener = null,
policyOppiaTagActionListener = policyOppiaTagActionListener,
displayLocale = displayLocale
displayLocale = displayLocale,
supportLtr = supportLtr
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const val CUSTOM_LIST_OL_TAG = "oppia-ol"
*/
class LiTagHandler(
private val context: Context,
private val displayLocale: OppiaLocale.DisplayLocale
private val displayLocale: OppiaLocale.DisplayLocale,
private val supportLtr: Boolean = false
) : CustomHtmlContentHandler.CustomTagHandler {
private val pendingLists = Stack<ListTag<*, *>>()
private val latestPendingList: ListTag<*, *>?
Expand Down Expand Up @@ -52,7 +53,12 @@ class LiTagHandler(
// Actually place the spans only if the root tree has been finished (as the entirety of the
// tree is needed for analysis).
val closingList = pendingLists.pop().also { it.recordList() }
if (pendingLists.isEmpty()) closingList.finishListTree(output, context, displayLocale)
if (pendingLists.isEmpty()) closingList.finishListTree(
output,
context,
displayLocale,
supportLtr
)
}
CUSTOM_LIST_LI_TAG -> latestPendingList?.closeItem(output)
}
Expand Down Expand Up @@ -122,8 +128,13 @@ class LiTagHandler(
* Recursively replaces all marks for this root list (and all its children) with renderable
* spans in the provided [text].
*/
fun finishListTree(text: Editable, context: Context, displayLocale: OppiaLocale.DisplayLocale) =
finishListRecursively(parentSpan = null, text, context, displayLocale)
fun finishListTree(
text: Editable,
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
supportLtr: Boolean
) =
finishListRecursively(parentSpan = null, text, context, displayLocale, supportLtr)

/**
* Returns a new mark of type [M] for this tag.
Expand All @@ -136,22 +147,23 @@ class LiTagHandler(
parentSpan: ListItemLeadingMarginSpan?,
text: Editable,
context: Context,
displayLocale: OppiaLocale.DisplayLocale
displayLocale: OppiaLocale.DisplayLocale,
supportLtr: Boolean = false
) {
val childrenToProcess = childrenLists.toMutableMap()
markRangesToReplace.forEach { (startMark, endMark) ->
val styledSpan = startMark.toSpan(
parentSpan, context, displayLocale, peerItemCount = markRangesToReplace.size
parentSpan, context, displayLocale, peerItemCount = markRangesToReplace.size, supportLtr
)
text.replaceMarksWithSpan(startMark, endMark, styledSpan)
childrenToProcess.remove(startMark)?.finishListRecursively(
parentSpan = styledSpan, text, context, displayLocale
parentSpan = styledSpan, text, context, displayLocale, supportLtr
)
}

// Process the remaining children that are not lists themselves.
childrenToProcess.values.forEach {
it.finishListRecursively(parentSpan = null, text, context, displayLocale)
it.finishListRecursively(parentSpan = null, text, context, displayLocale, supportLtr)
}
}

Expand Down Expand Up @@ -188,7 +200,8 @@ class LiTagHandler(
parentSpan: ListItemLeadingMarginSpan?,
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
peerItemCount: Int
peerItemCount: Int,
supportLtr: Boolean
): S

/** Marks the opening tag location of a list item inside an <ul> element. */
Expand All @@ -202,8 +215,15 @@ class LiTagHandler(
parentSpan: ListItemLeadingMarginSpan?,
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
peerItemCount: Int
) = ListItemLeadingMarginSpan.UlSpan(parentSpan, context, indentationLevel, displayLocale)
peerItemCount: Int,
supportLtr: Boolean
) = ListItemLeadingMarginSpan.UlSpan(
parentSpan,
context,
indentationLevel,
displayLocale,
supportLtr
)
}

/** Marks the opening tag location of a list item inside an <ol> element. */
Expand All @@ -215,14 +235,16 @@ class LiTagHandler(
parentSpan: ListItemLeadingMarginSpan?,
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
peerItemCount: Int
peerItemCount: Int,
supportLtr: Boolean
): ListItemLeadingMarginSpan.OlSpan {
return ListItemLeadingMarginSpan.OlSpan(
parentSpan,
context,
numberedItemPrefix = "${displayLocale.toHumanReadableString(number)}.",
longestNumberedItemPrefix = "${displayLocale.toHumanReadableString(peerItemCount)}.",
displayLocale
displayLocale,
supportLtr
)
}
}
Expand All @@ -236,7 +258,8 @@ class LiTagHandler(
parentSpan: ListItemLeadingMarginSpan?,
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
peerItemCount: Int
peerItemCount: Int,
supportLtr: Boolean
) = error("Ending marks cannot be converted to spans.")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
context: Context,
private val indentationLevel: Int,
private val displayLocale: OppiaLocale.DisplayLocale,
private val supportLtr: Boolean = false
) : ListItemLeadingMarginSpan() {
private val resources = context.resources
private val bulletRadius = resources.getDimensionPixelSize(R.dimen.bullet_radius)
Expand All @@ -44,7 +45,7 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {

private val bulletDiameter by lazy { bulletRadius * 2 }
private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
(displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL) && !supportLtr
}
private val clipBounds by lazy { Rect() }

Expand Down Expand Up @@ -120,7 +121,8 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
context: Context,
private val numberedItemPrefix: String,
private val longestNumberedItemPrefix: String,
private val displayLocale: OppiaLocale.DisplayLocale
private val displayLocale: OppiaLocale.DisplayLocale,
private val supportLtr: Boolean = false
) : ListItemLeadingMarginSpan() {
private val resources = context.resources
private val spacingBeforeText = resources.getDimensionPixelSize(R.dimen.spacing_before_text)
Expand All @@ -132,7 +134,7 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
2 * longestNumberedItemPrefix.length + spacingBeforeText

private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
(displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL) && !supportLtr
}

override fun drawLeadingMargin(
Expand Down
Loading