-
Notifications
You must be signed in to change notification settings - Fork 499
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 #3474: Merge faq single activity into single layout file. #3526
Fix #3474: Merge faq single activity into single layout file. #3526
Conversation
@rt4914 I have added the required screenshots and all tests have passed. Please, review my PR |
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.
Generally LGTM, just have one comment.
@rt4914 @anandwana001 Please review my PR. |
@rishidyno PR mostly looks good to me, can you please add the screenshots in a table so that it is easy to compare? |
Assign me once all test case gets pass. |
Ptal @anandwana001 Assigning this to you. |
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 some more comments, PTAL @rishidyno
Now we are having 2 versions of faq_single_activity we don't need to mention fillViewport attribute so i have deleted this file
@rishidyno Please reply to the open comments so that reviewer can know what exactly you did for that particular comment. |
@rt4914 All addressed changes are done. I have retained all the attributes needed and deleted unnecessary ones. |
Sir! sorry for the inconvenience. I will take care of this in the future. |
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.
Ptal @rt4914 All tests have passed and I have updated the project. |
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. |
Thanks @rt4914 |
Explanation
Fix #3474
Checklist