-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 for issue #1436 list icon allows user only to show the list, but not to make it disappear #1443
Conversation
…the current state of the list. It is false by default when you open the nearby activity.
Codecov Report
@@ Coverage Diff @@
## master #1443 +/- ##
=========================================
- Coverage 3.3% 3.3% -0.01%
=========================================
Files 128 128
Lines 6839 6842 +3
Branches 669 671 +2
=========================================
Hits 226 226
- Misses 6598 6601 +3
Partials 15 15
Continue to review full report at Codecov.
|
Thanks for this PR @brok85 , just after we decide this button is a good idea, I will test and merge this. |
// Handle item selection | ||
switch (item.getItemId()) { | ||
case R.id.action_display_list: | ||
bottomSheetBehaviorForDetails.setState(BottomSheetBehavior.STATE_HIDDEN); | ||
bottomSheetBehavior.setState(BottomSheetBehavior.STATE_EXPANDED); | ||
if(!opened){ |
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.
I would remove "opened" variable at all, since someone else may forgot to set it, and everything can be a mess. Instead you can use bottomSheetBehavior.getState(), to check if it is opened or not.
Thanks for your first time contribution @brok85 , I will review this again just after you fixed reviewed point. |
…pand or collapse the bottom list.
@neslihanturan thanks for the suggestion. I pushed an updated version using getState. the bottomSheetBahavior state technically can be different from hidden, collapsed, or expanded. It can also be "dragging" or "settling" but I don't thing those two states are ever assigned in this case. That is why they are not included in the new if statement. Let me know if the new version is ok or if I should make other changes. |
Thanks @brok85 ! |
…the current state of the list. It is false by default when you open the nearby activity.
Description
Fixes #1436
{Describe the changes made and why they were made.}
Created a boolean variable that defaults to false (because the list is collapsed by default) when the nearby activity is opened. Then, it changes value according to the current state of the list.
Tests performed
Tested on LG V10, API 24, master 2.7.x
Tested on {API level & name of device/emulator}, with {build variant, e.g. ProdDebug}.
{Please test your PR at least once before submitting.}
tested several times and seems to work properly.
Screenshots showing what changed