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

[RunAllTests] Fix #3633: [RTL] High fi Onboarding images supports RTL #3635

Merged
merged 35 commits into from
Aug 7, 2021

Conversation

veena14cs
Copy link
Contributor

@veena14cs veena14cs commented Aug 2, 2021

Explanation

This PR fixes #3633. The PR focuses on fixing RTL for Onboading images

Screenshot LTR and RTL

Mobile Portrait:

Screenshot_1627853193..... Screenshot_1627853346
Screenshot_1627853198.....Screenshot_1627853351

Screenshot_1627853201.....Screenshot_1627853355

Screenshot_1627853205.......Screenshot_1627853359

Mobile Landscape:

Screenshot_1627853246......Screenshot_1627853385

Screenshot_1627853243........Screenshot_1627853381
Screenshot_1627853239.......Screenshot_1627853376

Screenshot_1627853228....Screenshot_1627853368

Tablet Portrait:
Screenshot_1627853506......Screenshot_1627853600
Screenshot_1627853510....Screenshot_1627853598
Screenshot_1627853513....Screenshot_1627853595
Screenshot_1627853519.....Screenshot_1627853590

Tablet Landscape:
Screenshot_1627853539....Screenshot_1627853572
Screenshot_1627853535....Screenshot_1627853575
Screenshot_1627853531.....Screenshot_1627853579
Screenshot_1627853526.....Screenshot_1627853582

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@veena14cs veena14cs requested a review from rt4914 as a code owner August 2, 2021 09:59
@veena14cs veena14cs added this to the RTL Support milestone Aug 2, 2021
@rt4914
Copy link
Contributor

rt4914 commented Aug 3, 2021

@veena14cs I think this is incorrect. In RTL landscape we should only reverse the background and not the foreground.
You can email Chantel/Sean (keeping everyone in cc) for the images where the background is flipped RTL wise and foreground is still the same.

@rt4914 rt4914 removed their assignment Aug 3, 2021
@anandwana001 anandwana001 changed the title Fix #3633: [RTL] High fi Onboarding images supports RTL [RunAllTests] Fix #3633: [RTL] High fi Onboarding images supports RTL Aug 3, 2021
@veena14cs veena14cs assigned rt4914 and unassigned veena14cs Aug 6, 2021
@veena14cs
Copy link
Contributor Author

@veena14cs I think this is incorrect. In RTL landscape we should only reverse the background and not the foreground.
You can email Chantel/Sean (keeping everyone in cc) for the images where the background is flipped RTL wise and foreground is still the same.

As discussed in the email thread, we are keeping RTL for the entire image as shown in the screenshots in the PR description. In that case @rt4914 please review this PR.

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.

Approving and also filed an issue for the same. #3655

@rt4914 rt4914 merged commit be4fcbc into oppia:develop Aug 7, 2021
@veena14cs veena14cs deleted the fix-onboarding-images branch August 9, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RTL] High-fi Add support for RTL on Onboarding Images.
2 participants