-
Notifications
You must be signed in to change notification settings - Fork 526
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 #511 : Past expanded answers do not collapse in the question player #1456
Fix #511 : Past expanded answers do not collapse in the question player #1456
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.
Thanks @anandwana001! I have one overall question that I left a comment on.
app/src/main/java/org/oppia/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt
Outdated
Show resolved
Hide resolved
… question-player-previous-collapse
…e' into question-player-previous-collapse
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.
This is awesome, nice find @anandwana001! Since this is really subtle, can we add a GitHub action to test that new code doesn't use certain prohibited patterns like this import?
Yes, I can file issue regarding the check you mention with few others custom check we are thinking to good to have. |
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.
@BenHenning Are we good to merge this PR now? |
… question-player-previous-collapse
Thanks for filing the issue. I think adding a test should still be a priority since it should be possible. |
… question-player-previous-collapse
… question-player-previous-collapse
@anandwana001 Un-assigning myself until this comment related discussion gets and update #1456 (comment) |
… question-player-previous-collapse
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!
Explanation
Fixes #511
Added
QuestionPlayerActivityLocalTest
. Added test cases for the previous header expand and collapse logic both inStateFragment
andQuestionPlayer
.Test passing with these test cases
Test fails without this fix
Till here all test are passing in
StateFragmentLocalTest
Checklist