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

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
+93 −48
Open

Fix #5039 : Align policy text and symbols to the left, partial fix for list items #5573

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 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
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
2 changes: 1 addition & 1 deletion app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import org.oppia.android.app.model.PoliciesFragmentArguments
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.locale.LeftAlignedSymbolsSpan
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
1 change: 1 addition & 0 deletions utility/src/main/java/org/oppia/android/util/parser/html/BUILD.bazel
View file
Open in desktop
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",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@TanishMoral11, to answer your question regarding adding this class to the build file:

  1. Sometimes a kt lib can have multiple source files like you have added here, and for tag_handlers, but it depends on usecase. E.g., if you want to use LeftAlignedSymbolsSpan in a different part of the codepase, you will need to add a dependency on the lib, list_item_leading_margin_span, which will provide both ListItemLeadingMarginSpan and LeftAlignedSymbolsSpan. On the other hand, each class can have it's own lib if they are only used in a standalone way, like most of the other utility classes in this package and their corresponding libs in this BUILD file.

],
visibility = [
"//app:__subpackages__",
Expand Down
8 changes: 5 additions & 3 deletions ...oid/util/locale/LeftAlignedSymbolsSpan.kt → ...til/parser/html/LeftAlignedSymbolsSpan.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package org.oppia.android.util.locale
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 alignment for symbols as well
// Custom span to force LTR alignment for all text including symbols
/**
* 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,
Expand Down
16 changes: 4 additions & 12 deletions utility/src/main/java/org/oppia/android/util/parser/html/ListItemLeadingMarginSpan.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ 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.R.dimen.spacing_before_bullet
import org.oppia.android.util.locale.OppiaLocale

// TODO(#562): Add screenshot tests to check whether the drawing logic works correctly on all devices.
Expand Down Expand Up @@ -40,7 +40,7 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
private val bulletRadius = resources.getDimensionPixelSize(R.dimen.bullet_radius)

private val bulletDiameter by lazy { bulletRadius * 2 }
private val baseMargin = (16f * context.resources.displayMetrics.density).toInt()
private val baseMargin = context.resources.getDimensionPixelSize((spacing_before_bullet))

private val isRtl by lazy {
displayLocale.getLayoutDirection() == ViewCompat.LAYOUT_DIRECTION_RTL
Expand Down Expand Up @@ -119,7 +119,8 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
private val displayLocale: OppiaLocale.DisplayLocale
) : ListItemLeadingMarginSpan() {
private val resources = context.resources
private val baseMargin = (16f * context.resources.displayMetrics.density).toInt()
private val baseMargin =
context.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 =
Expand All @@ -143,15 +144,6 @@ sealed class ListItemLeadingMarginSpan : LeadingMarginSpan {
val isFirstCharacter = startCharOfSpan == start

if (isFirstCharacter) {
// Force left alignment
paint.textAlign = Paint.Align.LEFT

val textWidth = Rect().also {
paint.getTextBounds(
numberedItemPrefix, /* start= */ 0, /* end= */ numberedItemPrefix.length, it
)
}.width()

// Positioning calculation
val prefixStartX = x.toFloat() + baseMargin

Expand Down
Loading