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

Jump to bottom

Fix #4008: [RTL] Correctly align text in TextViews #4009

Merged
merged 5 commits into from
Nov 23, 2021
Merged

Fix #4008: [RTL] Correctly align text in TextViews #4009

merged 5 commits into from
Nov 23, 2021

Conversation

veena14cs
Copy link
Contributor

@veena14cs veena14cs commented Nov 22, 2021 •
edited

Explanation

This PR fixes #4008. This PR fixes text alignment issues in the Textview for RTL devices.

Screenshot LTR and RTL

Mobile
Screenshot_1637598123....Screenshot_1637598090

Screenshot_1637598117....Screenshot_1637598096

Screenshot_1637597975....Screenshot_1637598010
Screenshot_1637597979.....Screenshot_1637598006

Screenshot_1637597966......Screenshot_1637598016
Screenshot_1637597962....Screenshot_1637598021

Tablet

Screenshot_1637604141....Screenshot_1637604115

Screenshot_1637604107....Screenshot_1637604147

Screenshot_1637605462....Screenshot_1637605498

Screenshot_1637605444....Screenshot_1637605507
Screenshot_1637606202.....Screenshot_1637605513

Screenshot_1637606190....Screenshot_1637605519

Essential Checklist

  • 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 files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR does not contain any unnecessary code changes from Android Studio (reference).
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

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)
  • Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
  • Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

@veena14cs veena14cs added this to the RTL Support milestone Nov 22, 2021
rt4914
rt4914 suggested changes Nov 22, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@veena14cs Based on whatever style you are introducing please lookout for
layout_width, layout_height and layout_gravity.

app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-port/hints_summary.xml Outdated Show resolved Hide resolved
app/src/main/res/layout-sw600dp-port/topic_lessons_story_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-port/topic_lessons_story_summary.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/solution_summary.xml Show resolved Hide resolved
app/src/main/res/layout/solution_summary.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/solution_summary.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/topic_lessons_story_summary.xml Show resolved Hide resolved
app/src/main/res/layout/topic_lessons_story_summary.xml Outdated Show resolved Hide resolved
@rt4914 rt4914 assigned veena14cs and unassigned rt4914 Nov 22, 2021
@veena14cs veena14cs assigned rt4914 and unassigned veena14cs Nov 23, 2021
rt4914
rt4914 suggested changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@veena14cs PTAL

app/src/main/res/layout-land/hints_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-land/hints_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-port/hints_summary.xml Show resolved Hide resolved
app/src/main/res/layout-sw600dp-port/topic_lessons_story_summary.xml Show resolved Hide resolved
rt4914
rt4914 approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM

@rt4914 rt4914 assigned veena14cs and unassigned rt4914 Nov 23, 2021
@oppiabot oppiabot bot added the PR: LGTM label Nov 23, 2021
@oppiabot
Copy link

oppiabot bot commented Nov 23, 2021

Hi @veena14cs, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

@veena14cs
Copy link
Contributor Author

LGTM

Thanks @rt4914.

@veena14cs veena14cs merged commit 10fa6cd into oppia:develop Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

Assignees

@veena14cs veena14cs

Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix Text alignment for RTL.
2 participants
@veena14cs @rt4914