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 : Align policy text and symbols to the left, partial fix for list items #5573

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f8143d2
Align policy text and symbols to the left, partial fix for list items
TanishMoral11 Nov 7, 2024
fdb0929
Fix text alignment and bullet point formatting in PoliciesFragment
TanishMoral11 Nov 12, 2024
b5148d4
Fix: Remove unnecessary formatting changes and maintain consistent st…
TanishMoral11 Nov 14, 2024
220580f
Added Newline At EOF
TanishMoral11 Nov 15, 2024
64fca2b
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Nov 22, 2024
8c521f3
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Dec 1, 2024
eebbadf
Fix left-align policy text
TanishMoral11 Dec 9, 2024
492284e
B
TanishMoral11 Dec 9, 2024
f54c59c
Small Change
TanishMoral11 Dec 10, 2024
28c8ea5
Add Old Comments For Future Reference
TanishMoral11 Dec 10, 2024
957b27a
Remove Ununsed Variable
TanishMoral11 Dec 16, 2024
061e0a7
Remove Ununsed Variable
TanishMoral11 Dec 16, 2024
a6624fa
Resolved All Issues
TanishMoral11 Dec 16, 2024
73e09dc
Resolved All Issues
TanishMoral11 Dec 16, 2024
642f106
Improving Formatting
TanishMoral11 Dec 16, 2024
c14a0a5
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Dec 22, 2024
5e05035
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Dec 24, 2024
eace6d8
Removed Unused Variable
TanishMoral11 Dec 25, 2024
2fada65
Merge branch 'left-align-policy-text-fix' of https://github.com/Tanis…
TanishMoral11 Dec 25, 2024
7fb000d
Merge branch 'develop' into left-align-policy-text-fix
adhiamboperes Jan 10, 2025
44bd395
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Jan 16, 2025
351c9de
Testing Purpose
TanishMoral11 Jan 16, 2025
c0b2abb
Merge branch 'left-align-policy-text-fix' of https://github.com/Tanis…
TanishMoral11 Jan 16, 2025
5b86f58
Resolved Issues
TanishMoral11 Jan 16, 2025
a669e05
Merge branch 'left-align-policy-text-fix' of https://github.com/Tanis…
TanishMoral11 Jan 16, 2025
2962e5a
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Jan 21, 2025
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
@@ -1,5 +1,7 @@
package org.oppia.android.app.policies

import android.text.SpannableString
import android.text.Spanned
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
Expand All @@ -11,6 +13,7 @@ import org.oppia.android.app.model.PolicyPage
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.databinding.PoliciesFragmentBinding
import org.oppia.android.util.parser.html.HtmlParser
import org.oppia.android.util.parser.html.LeftAlignedSymbolsSpan
import org.oppia.android.util.parser.html.PolicyType
import javax.inject.Inject

Expand Down Expand Up @@ -53,16 +56,39 @@ class PoliciesFragmentPresenter @Inject constructor(
policyWebLink = resourceHandler.getStringInLocale(R.string.terms_of_service_web_link)
}

binding.policyDescriptionTextView.text = htmlParserFactory.create(
val parsedHtmlDescription = htmlParserFactory.create(
policyOppiaTagActionListener = this,
displayLocale = resourceHandler.getDisplayLocale()
).parseOppiaHtml(
policyDescription,
binding.policyDescriptionTextView,
rawString = policyDescription,
htmlContentTextView = binding.policyDescriptionTextView,
supportsLinks = true,
supportsConceptCards = false
)

binding.policyDescriptionTextView.apply {
layoutDirection = View.LAYOUT_DIRECTION_LTR
textAlignment = View.TEXT_ALIGNMENT_TEXT_START
textDirection = View.TEXT_DIRECTION_LTR
setSingleLine(false)
setMaxLines(Int.MAX_VALUE)
}

val spannableString = SpannableString(parsedHtmlDescription)
parsedHtmlDescription.split("\n").forEachIndexed { _, line ->
val lineStart = parsedHtmlDescription.indexOf(line)
if (line.trimStart().startsWith("•")) {
val bulletIndex = lineStart + line.indexOf("•")
spannableString.setSpan(
LeftAlignedSymbolsSpan(),
bulletIndex,
bulletIndex + 1,
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE
)
}
}
binding.policyDescriptionTextView.text = spannableString

binding.policyWebLinkTextView.text = htmlParserFactory.create(
gcsResourceName = "",
entityType = "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ kt_android_library(
name = "list_item_leading_margin_span",
srcs = [
"ListItemLeadingMarginSpan.kt",
"LeftAlignedSymbolsSpan.kt",
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
],
visibility = [
"//app:__subpackages__",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.oppia.android.util.parser.html

import android.graphics.Canvas
import android.graphics.Paint
import android.text.style.ReplacementSpan

/**
* Custom span to force LTR (left-to-right) alignment for all text,
* including symbols, regardless of the system's text direction.
*/
class LeftAlignedSymbolsSpan : ReplacementSpan() {
override fun getSize(
paint: Paint,
text: CharSequence?,
start: Int,
end: Int,
fm: Paint.FontMetricsInt?
): Int {
return paint.measureText(text, start, end).toInt()
}

override fun draw(
canvas: Canvas,
text: CharSequence,
start: Int,
end: Int,
x: Float,
top: Int,
y: Int,
bottom: Int,
paint: Paint
) {
val originalAlignment = paint.textAlign
paint.textAlign = Paint.Align.LEFT

// Draw the bullet point at the exact x position
canvas.drawText(text, start, end, x, y.toFloat(), paint)

// Restore original alignment
paint.textAlign = originalAlignment
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class LiTagHandler(
context: Context,
displayLocale: OppiaLocale.DisplayLocale,
peerItemCount: Int
) = ListItemLeadingMarginSpan.UlSpan(parentSpan, context, indentationLevel, displayLocale)
) = ListItemLeadingMarginSpan.UlSpan(parentSpan, context, indentationLevel)
}

/** Marks the opening tag location of a list item inside an <ol> element. */
Expand All @@ -221,9 +221,7 @@ class LiTagHandler(
return ListItemLeadingMarginSpan.OlSpan(
parentSpan,
context,
numberedItemPrefix = "${displayLocale.toHumanReadableString(number)}.",
longestNumberedItemPrefix = "${displayLocale.toHumanReadableString(peerItemCount)}.",
displayLocale
numberedItemPrefix = "${displayLocale.toHumanReadableString(number)}."
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@ package org.oppia.android.util.parser.html
import android.content.Context
import android.graphics.Canvas
import android.graphics.Paint
import android.graphics.Rect
import android.graphics.RectF
import android.text.Layout
import android.text.Spanned
import android.text.style.LeadingMarginSpan
import androidx.core.view.ViewCompat
import org.oppia.android.util.R
import org.oppia.android.util.locale.OppiaLocale
import kotlin.math.max
import org.oppia.android.util.R.dimen.spacing_before_bullet

// TODO(#562): Add screenshot tests to check whether the drawing logic works correctly on all devices.

Expand All @@ -35,18 +32,12 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
override val parent: ListItemLeadingMarginSpan?,
context: Context,
private val indentationLevel: Int,
private val displayLocale: OppiaLocale.DisplayLocale,
) : ListItemLeadingMarginSpan() {
private val resources = context.resources
private val bulletRadius = resources.getDimensionPixelSize(R.dimen.bullet_radius)
private val spacingBeforeText = resources.getDimensionPixelSize(R.dimen.spacing_before_text)
private val spacingBeforeBullet = resources.getDimensionPixelSize(R.dimen.spacing_before_bullet)

private val bulletDiameter by lazy { bulletRadius * 2 }
private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
}
private val clipBounds by lazy { Rect() }
private val baseMargin = context.resources.getDimensionPixelSize((spacing_before_bullet))

override fun drawLeadingMargin(
canvas: Canvas,
Expand All @@ -69,16 +60,15 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
val previousStyle = paint.style
val bulletDrawRadius = bulletRadius.toFloat()

val indentedX = parentAbsoluteLeadingMargin + spacingBeforeBullet
val bulletCenterLtrX = indentedX + bulletDrawRadius
val bulletCenterX = if (isRtl) {
// See https://stackoverflow.com/a/21845993/3689782 for 'right' property exclusivity.
val maxDrawX = if (canvas.getClipBounds(clipBounds)) {
clipBounds.right - 1
} else canvas.width - 1
maxDrawX - bulletCenterLtrX
} else bulletCenterLtrX
// Force left alignment
paint.textAlign = Paint.Align.LEFT

// Positioning calculation

val bulletCenterLtrX = x.toFloat() + baseMargin * (indentationLevel + 1)
val bulletCenterX = bulletCenterLtrX
val bulletCenterY = (top + bottom) / 2f

when (indentationLevel) {
0 -> {
// A solid circle is used for the outermost bullet.
Expand Down Expand Up @@ -111,29 +101,17 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
}

override fun getLeadingMargin(first: Boolean) =
bulletDiameter + spacingBeforeBullet + spacingBeforeText
baseMargin * (indentationLevel + 2)
}

/** A subclass of [LeadingMarginSpan] that shows nested list span for <ol> tags. */
class OlSpan(
override val parent: ListItemLeadingMarginSpan?,
context: Context,
private val numberedItemPrefix: String,
private val longestNumberedItemPrefix: String,
private val displayLocale: OppiaLocale.DisplayLocale
private val numberedItemPrefix: String
) : ListItemLeadingMarginSpan() {
private val resources = context.resources
private val spacingBeforeText = resources.getDimensionPixelSize(R.dimen.spacing_before_text)
private val spacingBeforeNumberPrefix =
resources.getDimensionPixelSize(R.dimen.spacing_before_number_prefix)

// Try to use a computed margin, but otherwise guess if there's no guaranteed spacing.
private var computedLeadingMargin =
2 * longestNumberedItemPrefix.length + spacingBeforeText

private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
}
private val baseMargin =
context.resources.getDimensionPixelSize((R.dimen.spacing_before_number_prefix))

override fun drawLeadingMargin(
canvas: Canvas,
Expand All @@ -153,30 +131,15 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
val isFirstCharacter = startCharOfSpan == start

if (isFirstCharacter) {
val textWidth = Rect().also {
paint.getTextBounds(
numberedItemPrefix, /* start= */ 0, /* end= */ numberedItemPrefix.length, it
)
}.width()
val longestTextWidth = Rect().also {
paint.getTextBounds(
longestNumberedItemPrefix,
/* start= */ 0,
/* end= */ longestNumberedItemPrefix.length,
it
)
}.width()
computedLeadingMargin = longestTextWidth + spacingBeforeNumberPrefix + spacingBeforeText

// Compute the prefix's start x value such that it is right-aligned with other numbers in
// the list.
val indentedX = parentAbsoluteLeadingMargin + spacingBeforeNumberPrefix
val endAlignedX = (max(textWidth, longestTextWidth) - textWidth) + indentedX
val prefixStartX = if (isRtl) canvas.width - endAlignedX - 1 else endAlignedX
canvas.drawText(numberedItemPrefix, prefixStartX.toFloat(), baseline.toFloat(), paint)
// Positioning calculation
val prefixStartX = x.toFloat() + baseMargin

// Draw the numbered prefix
canvas.drawText(numberedItemPrefix, prefixStartX, baseline.toFloat(), paint)
}
}

override fun getLeadingMargin(first: Boolean) = computedLeadingMargin
override fun getLeadingMargin(first: Boolean) =
baseMargin * 2
}
}
Loading