-
Notifications
You must be signed in to change notification settings - Fork 527
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
Fixes part of #40: Added bottom padding to Question Player #3250
Conversation
@BenHenning @anandwana001 PTAL at the description and can you suggest something on how to fix these failing tests? |
I will take a look at this by tomorrow. |
@rt4914 per our discussion earlier in the week, is further input needed from me for this 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.
Hmm. I'm not keen on changing padding for the purpose of testing since that has real side effects for users. Do you have a before & after to demonstrate the effects?
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 @rt4914. LGTM.
Explanation
Fixes part of #40: Added bottom padding to Question Player
Before this PR more than 2 test cases were failing on espresso in
QuestionPlayerActivityTest
that were fixed by adding the bottom-padding which was also a requirement as per mocks.But still 2 test cases are failing because of nested recyclerview for
SelectionInputInteraction
.In both these tests we want to scroll to
index=3
item of theselection_interaction_recycler_view
but the scrolling is disabled because it's an item ofquestion_recycler_view
and in current code we cannot scroll to this specific position.Output on Pixel 3 (5.56")
Output on Pixel 3 XL (6.3")
To fix this I have filed an issue #3370
Checklist