Listen for back button press #14
Conversation
| if (event.isTracking() && !event.isCanceled()) { | ||
| if (isSheetShowing()) { | ||
| if (getState() == BottomSheetLayout.State.EXPANDED) { | ||
| peekSheet(); |
There was a problem hiding this comment.
I do want to bring this up again. In my case I'm using the bottom sheet for sharing. If you press back you want to completely cancel the share (what the default share sheet on lollipop does). If this is hard-coded to peek instead of dismiss, it will be vary hard to change.
There was a problem hiding this comment.
I agree that there should be more flexibility here. I didn't want to make any assumptions, but only supply a solution to your issue posted. I will leave it to Flipboard to decide the best way to integrate.
There was a problem hiding this comment.
Fair, just wanted to make sure this is documented as an issue on the pull request.
There was a problem hiding this comment.
For the purpose of discussion, what do you believe the best solution would be? A boolean field that toggles full dismiss vs peek dismiss? Can you identify other types of dismiss that could be desired? I struggled to find good naming, which was the main deterrent. setAlwaysPeekDismiss(boolean), setAlwaysFullDismiss(boolean), setPeekOnDismiss(boolean)? Those aren't great. Also, what do you believe should be the default?
There was a problem hiding this comment.
I feel that fully dismissing should be the default, I'm not sure I can think of any use cases where you'd want that not to happen, as you start with it off screen. I guess it would make sense to go back to peeking if it was somehow locked to peek on the screen, but that isn't a feature this lib implements. I'd go with passing the existing state enum instead of a boolean setBackDismissMode(State.PEEKED) or setBackDismissMode(State.HIDDEN).
There was a problem hiding this comment.
I can see many use cases for wanting peek on dismiss. E.g. Tapping an item in peek mode expands to full.
Avoid enums in Android, but perhaps using IntDef makes the most sense.
There was a problem hiding this comment.
Agree about avoiding enums, but the State enum is already there.
Conflicts: bottomsheet-sample/src/main/java/com/flipboard/bottomsheet/sample/MainActivity.java
Instead of assuming that a back press should change the state from EXPANDED to PEEKED, this can now be controlled through BottomSheetLayout.setDismissMode(State). The default is HIDDEN, which completely dismisses the Bottom Sheet when back is pressed in EXPANDED state. Updates MenuActivity sample to demonstrate BottomSheetLayout.setDismissMode(State).
|
Sorry for the late response. There's been some good discussion, and we're going to mull over it tonight and tomorrow. In the meantime, @iPaulPro would you mind rebasing on the current master? We just merged a fairly significant change to the sample structure. Also please fill out the Individual Contributor License Agreement if you haven't already. |
|
@hzsweers No worries. I already merged the latest master, so you should see the big green button. :-) I also submitted the CLA yesterday morning (before the PR). |
| } | ||
| return true; | ||
| } else if (event.getAction() == KeyEvent.ACTION_UP) { | ||
| KeyEvent.DispatcherState dispatcherState = getKeyDispatcherState(); |
There was a problem hiding this comment.
in the above block the variable is just referenced as state. Could the names be consistent? Either name is fine for me.
|
Just a couple small comments otherwise it looks good. Sorry for the extended delay. |
Listen for back button press
|
Merged and updated the API after |
|
Sorry about that! Been super busy. Thanks :-) |
|
No worries, thanks for the PR! This is released now in v1.3 |
Removes the need to listen for
onBackPressedin theActivity. Sample updated to reflect this. Fixes #10.