-
Notifications
You must be signed in to change notification settings - Fork 510
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 #3583: Merge topic_practice_footer_view into single layout file. #3644
Fix #3583: Merge topic_practice_footer_view into single layout file. #3644
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.
@rishidyno There are 4 files which needs to be merged into 1.
Also as mentioned in issue please add before/after screenshots of this UI in app in mobile portrait/landscape and tablet portrait/landscape mode.
@rt4914 I have added Screenshots also I wanted to mention that I tried merging it into one XML file but I ran into some issues. I added these two in
Also i added these two in
But when I tried to run the app in that activity it crashed in tablet portrait/landscape so I kept the tablet portrait and landscape file. I could not get a complete understanding of why they crashed. But I think it might be due to other layout elements surrounding them. I could only delete the landscape file. Please guide me on what to do now. |
@rishidyno After you have done the changes for by removing the tablet xml file also, rebuild the entire project, uninstall the previous app from tablet device and then install the app. It should work properly after that. |
Thanks a lot @rt4914 I followed what you said and I was able to merge it into a single layout file. |
…' into merge-topic_practice_footer_view
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.
Left two comments, PTAL @rishidyno.
Also, note that if you want any reviewers to add their review, you should assign them the PR. Reviewers only add their reviews to the PRs assigned to them.
Ptal @rt4914
Got it 🙂 |
Unassigning @rishidyno since a re-review was requested. @rishidyno, please make sure you have addressed all review comments. Thanks! |
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 @rishidyno
Thanks @prayutsu |
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.
@rishidyno Code is good. Just nit changes suggested.
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.
Explanation
Fix #3583
Checklist