Added a BottomSheetFragment to bottomsheet-commons that works like a DialogFragment. #79
Conversation
| public static int getContainerId(Fragment fragment) { | ||
| return fragment.mContainerId; | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure the best way to handle this. DialogFragment makes use of this field that's not part of the public api to determine if it's showing in a dialog or normally. Other possible solutions are to use reflection or find some other way to obtain the same information without that field.
|
Sorry about the delayed response, looking forward to reviewing this! Going to try to tonight. |
|
I like this a lot. The code itself looks sound, but I'd like to propose investigating an alternative approach if you haven't already. Requiring the user to extend our own BottomSheetFragment isn't ideal, as many developers prefer to use their own base implementations for fragments. Sure, sheets are intended to be simple, but it would be understandable for them to still prefer their own base implementations anyway. In light of this, could we take a similar approach to This would be two parts:
I've written up an implementation of this based on your PR here: https://github.com/Flipboard/bottomsheet/tree/z/bottomsheetfragmentdelegate Let me know what you think! You too @markrietveld |
| } | ||
|
|
||
| @Override | ||
| public void onSheetStateChanged(BottomSheetLayout.State state) { |
There was a problem hiding this comment.
On second thought, I think it would be better not to set this. Could be confusing if the user set a listener in their activity and finds out that it's never called! They can still do the same thing in the fragment with getBottomSheetLayout().setOnSheetStateChangeListener()
|
Providing a delegate seems completely reasonable. I didn't do it originally since I was staying close to DialogFragment, but we can be better than them. |
|
Cool, pending @markrietveld's thoughts then |
|
Merged with the delgate approach. Thanks! |
After trying a few things I believe this is the best solution to support fragments in a bottom sheet. Fragments have a more complex lifecycle so it's far simpler to handle everything in the fragment itself instead of trying to deal with it in a view or other helper component. This implementation actually follows very closely to the support lib's DialogFragment.