Skip to content

Commit

Permalink
Fix #5032 Images in Arabic (RTL) lessons are right-aligned rather tha…
Browse files Browse the repository at this point in the history
…n center-aligned. (#5212)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes #5032, To make the image center-aligned, we are adding styling to
the HTML content image and removing the left margin by setting
drawableLeft bound to 0 in RTL.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [ ] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only

### Before
LTR Potrait


![beforeLTRpotrait](https://github.com/oppia/oppia-android/assets/76042077/298fc2c0-7512-40dc-8d56-444fe55f4998)

LTR Landscape


![LTRlandscape](https://github.com/oppia/oppia-android/assets/76042077/94e16ccb-052c-4723-b26e-00fa0591270d)

RTL Potrait


![beforeRTLpotrait](https://github.com/oppia/oppia-android/assets/76042077/59efc1c5-c695-442a-9549-8aa353d61aed)

RTL Landscape 


![RTLlandscape](https://github.com/oppia/oppia-android/assets/76042077/dae3c7b8-ce4e-4968-8dea-c4bc23eaf75f)



### After

RTL Potrait



![Potrait_after](https://github.com/oppia/oppia-android/assets/76042077/d5115f63-f71c-44fa-bb60-639e8f8abcef)

RTL Landscape


![landscapeafter](https://github.com/oppia/oppia-android/assets/76042077/9a37d6d1-dc80-489e-827c-8b208fa33d67)

LTR Potrait


![LTRpotraitafter](https://github.com/oppia/oppia-android/assets/76042077/4d51cf7c-68eb-4924-aaaa-64cb44391081)

LTR Landscape


![LTRLandscape](https://github.com/oppia/oppia-android/assets/76042077/802dcd37-5adb-4f5d-8a30-cfa110d76885)


<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
  • Loading branch information
Vishwajith-Shettigar authored Oct 31, 2023
1 parent 6ac4f9c commit 7272dc7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,26 @@ 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
// not center aligned. To avoid this situation check if RTL is enabled and set the textDirection.
if (isRtl) {
htmlContentTextView.textDirection = View.TEXT_DIRECTION_RTL

val regex = Regex("""<oppia-noninteractive-image [^>]*>.*?</oppia-noninteractive-image>""")
val modifiedHtmlContent = rawString.replace(regex) {
val oppiaImageTag = it.value
"""<div style="text-align: center;">$oppiaImageTag</div>"""
}
htmlContent = modifiedHtmlContent
} else {
htmlContentTextView.textDirection = View.TEXT_DIRECTION_LTR
}

htmlContentTextView.invalidate()

var htmlContent = rawString
if ("\n\t" in htmlContent) {
htmlContent = htmlContent.replace("\n\t", "")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import android.view.View
import android.view.ViewGroup
import android.view.ViewTreeObserver
import android.widget.TextView
import androidx.core.view.ViewCompat
import com.bumptech.glide.request.target.CustomTarget
import com.bumptech.glide.request.transition.Transition
import org.oppia.android.util.R
Expand Down Expand Up @@ -234,6 +235,11 @@ class UrlImageParser private constructor(
private val autoResizeImage: Boolean
) : AutoAdjustingImageTarget<T, D>(targetConfiguration) {

private fun isRTLMode(): Boolean {
return ViewCompat.getLayoutDirection(htmlContentTextView) == ViewCompat
.LAYOUT_DIRECTION_RTL
}

override fun computeBounds(
context: Context,
drawable: D,
Expand Down Expand Up @@ -304,11 +310,13 @@ class UrlImageParser private constructor(
drawableWidth *= multipleFactor
}
}
val drawableLeft = if (imageCenterAlign) {

val drawableLeft = if (imageCenterAlign && !isRTLMode()) {
calculateInitialMargin(maxAvailableWidth, drawableWidth)
} else {
0f
}

val drawableTop = 0f
val drawableRight = drawableLeft + drawableWidth
val drawableBottom = drawableTop + drawableHeight
Expand Down

0 comments on commit 7272dc7

Please sign in to comment.