-
Notifications
You must be signed in to change notification settings - Fork 517
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 #3451: [RTL] High-fi Add support for RTL in binding adapters #3452
[RunAllTests] Fix #3451: [RTL] High-fi Add support for RTL in binding adapters #3452
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veena14cs PTAL, thanks.
app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/databinding/StateAssemblerMarginBindingAdapters.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/databinding/StateAssemblerPaddingBindingAdapters.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java
Outdated
Show resolved
Hide resolved
...haredTest/java/org/oppia/android/app/databinding/StateAssemblerPaddingBindingAdaptersTest.kt
Outdated
Show resolved
Hide resolved
…eAssemblerPaddingBindingAdaptersTest.kt Co-authored-by: Ben Henning <[email protected]>
@BenHenning In that case do I need to go ahead with the changes or revert it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@veena14cs I think you might not have fully followed the suggestions in my earlier comment. PTAL at the follow-ups.
I have corrected. PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @veena14cs. This LGTM!
Explanation
This PR fixes #3451. This PR fixes the custom attributes used in data binding adapters to set margin or padding.
This fix was mainly for
content
andfeedback
items of Exploration player.This PR also fixes #3205.
Screenshot LTR and RTL
Mobile:
......
......
Tablet:
....
.....
Expresso Tests
Screenshots of tests that fails as expected when RTL code is temporarily removed.
Checklist