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 34 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
34 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
54c9517
Merge remote-tracking branch 'upstream/develop' into left-align-polic…
TanishMoral11 Jan 27, 2025
a5a6a9d
Resolved Merge Conflict
TanishMoral11 Jan 27, 2025
d3734a6
Reformating
TanishMoral11 Jan 27, 2025
4a1296a
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Jan 28, 2025
5bd46c0
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Jan 31, 2025
3075506
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Feb 5, 2025
37efff5
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Feb 7, 2025
058259d
Merge branch 'develop' into left-align-policy-text-fix
TanishMoral11 Feb 18, 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
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 All @@ -86,4 +90,4 @@ class PoliciesFragmentPresenter @Inject constructor(
(activity as RouteToPoliciesListener).onRouteToPolicies(PolicyPage.TERMS_OF_SERVICE)
}
}
}
}
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 All @@ -78,6 +79,7 @@ class HtmlParser private constructor(
supportsLinks: Boolean = false,
supportsConceptCards: Boolean = false
): Spannable {

var htmlContent = rawString

// Canvas does not support RTL, it always starts from left to right in RTL due to which compound drawables are
Expand Down Expand Up @@ -123,17 +125,11 @@ class HtmlParser private constructor(
}

val imageGetter = urlImageParserFactory?.create(
htmlContentTextView,
gcsResourceName,
entityType,
entityId,
imageCenterAlign
htmlContentTextView, gcsResourceName, entityType, entityId, imageCenterAlign
)

val htmlSpannable = CustomHtmlContentHandler.fromHtml(
htmlContent,
imageGetter,
computeCustomTagHandlers(supportsConceptCards, htmlContentTextView)
htmlContent, imageGetter, computeCustomTagHandlers(supportsConceptCards, htmlContentTextView)
)

val urlPattern = Patterns.WEB_URL
Expand Down Expand Up @@ -228,43 +224,22 @@ class HtmlParser private constructor(
entityId: String,
imageCenterAlign: Boolean,
customOppiaTagActionListener: CustomOppiaTagActionListener? = null,
displayLocale: OppiaLocale.DisplayLocale
displayLocale: OppiaLocale.DisplayLocale,
supportLtr: Boolean = false
): HtmlParser {
return HtmlParser(
context = context,
urlImageParserFactory = urlImageParserFactory,
gcsResourceName = gcsResourceName,
entityType = entityType,
entityId = entityId,
imageCenterAlign = imageCenterAlign,
consoleLogger = consoleLogger,
cacheLatexRendering = enableCacheLatexRendering.value,
customOppiaTagActionListener = customOppiaTagActionListener,
policyOppiaTagActionListener = null,
displayLocale = displayLocale
)
}

/**
* Returns a new [HtmlParser] with the empty entity type and ID for loading images,
* doesn't require GCS properties and imageCenterAlign set to false
* optionally specified [CustomOppiaTagActionListener] for handling custom Oppia tag events.
*/
fun create(
displayLocale: OppiaLocale.DisplayLocale
): HtmlParser {
return HtmlParser(
context = context,
urlImageParserFactory = urlImageParserFactory,
gcsResourceName = "",
entityType = "",
entityId = "",
imageCenterAlign = false,
consoleLogger = consoleLogger,
context,
urlImageParserFactory,
gcsResourceName,
entityType,
entityId,
imageCenterAlign,
consoleLogger,
cacheLatexRendering = enableCacheLatexRendering.value,
customOppiaTagActionListener = null,
policyOppiaTagActionListener = null,
displayLocale = displayLocale
customOppiaTagActionListener,
null,
displayLocale,
supportLtr = supportLtr
)
}

Expand All @@ -276,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 @@ -289,8 +266,9 @@ class HtmlParser private constructor(
cacheLatexRendering = false,
customOppiaTagActionListener = null,
policyOppiaTagActionListener = policyOppiaTagActionListener,
displayLocale = displayLocale
displayLocale = displayLocale,
supportLtr = supportLtr
)
}
}
}
}
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 Expand Up @@ -291,4 +314,4 @@ class LiTagHandler(
private fun <T : Mark<*>> Spannable.addMark(mark: T) =
setSpan(mark, length, length, Spanned.SPAN_MARK_MARK)
}
}
}
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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable appears unused.

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
Copy link
Collaborator

Choose a reason for hiding this comment

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

This and displayLocale above are nolonger used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done .

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 Expand Up @@ -179,4 +181,4 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {

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