-
Notifications
You must be signed in to change notification settings - Fork 506
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
Fix #4015 : User name TextView not properly aligned in profile_edit_fragment.xml Screen fix #4023
Conversation
@rushikeshsuryawanshi Considering you are changing the UI. Please add RTL screenshots too. https://github.com/oppia/oppia-android/wiki/RTL-Guidelines#testing-app-for-rtl-layouts |
@rushikeshsuryawanshi add a line of explaination in the PR explaining your idea and approach , also mark the checklist as [x] remove the space so that tick mark is shown.Take care to pick the issues after anyone has assigned you as there may be a possibility of someone else working on it. Fore present I have assigned this issue to you. |
@rushikeshsuryawanshi also you can take reference of other PR while writing title or opening comment, like here you need to write Fix #issue_number: and then PR title ( ref - #4004 ) |
I've made changes as per your instruction. It's my First Contribution. |
@rushikeshsuryawanshi RTL after portrait image does not look correct. The gravity/alignment of the text should be right instead of left. |
@rt4914 |
Hi @rushikeshsuryawanshi, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
@rt4914 Could you please review my PR and merge ? |
@rushikeshsuryawanshi if you want your PR to be reviewed you need to assign it back to reviewer. write PTAL @name only then he will know that your PR is to be reviewed |
PTAL @rt4914 |
@bkaur-bkj Okay thanks. This is my first PR so, new to these things. Is that correct now ? |
Hi. As of today, some main reviewers have taken time off for the next few weeks, so it may take a little while before we can look at this PR. We appreciate your patience while some of our team members recharge. We'll be fully returning on 4 January 2021. |
Hi @rushikeshsuryawanshi, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
. |
Hi @rushikeshsuryawanshi, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
. |
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.
@rushikeshsuryawanshi Apologies for the delayed review most of our teammates were on year-end break. Thanks a lot for being patient.
This reverts commit 462d8b0
PTAL @rt4914 |
Unassigning @rushikeshsuryawanshi since a re-review was requested. @rushikeshsuryawanshi, please make sure you have addressed all review comments. Thanks! |
@rushikeshsuryawanshi Check this comment : #4023 (comment) |
@rushikeshsuryawanshi make sure to assign the reviewer back after your changes are done. write PTAL @Reviewer name to assign them only then we will be notified |
and then unassign yourself too, I have done this for now |
@bkaur-bkj Thanks ! I didn't knew this. |
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, thanks.
Unassigning @rt4914 since they have already approved the PR. |
@rt4914 here one Robolectic test is failing, pls do let us know what can be done |
Looks like build is failing unexpectedly as it does not point to any specific test failure. Re-running the jobs. |
…dit_fragment.xml Screen fix (oppia#4023) * Fixed issue oppia#4015 * Text Direction Fix * Removed TextDirection * Revert "Removed TextDirection" This reverts commit 462d8b0 * Removed Text Direction. Changed style
Explanation
Fixes #4015
Text View wasn't constrained to the right and was set to wrap_content, which made text to come out of the view.
Setting width to 0dp, And constraining it's right to right of the parent view, with some padding solves the issue.
Adding Text Direction : Locale, fixes the gravity issue
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then:
Before Portrait
After Portrait
Before Landscape
After Landscape
RTL Before Portrait
RTL After Portrait
RTL Before Landscape
RTL After Landscape