-
Notifications
You must be signed in to change notification settings - Fork 58
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
BottomSheet with scroll view inside - ideal version #1884
Comments
Let's define what we mean by "ideal version". As far as I understand the only observable change of this issue will be:
But I am curious about navigation capabilities as well. Is it possible to present the BottomSheet by using system's navigation capabilities? So we can push/pop screens inside the BottomSheet with using all system provided transitions/animations. It can be applied to Color settings as well, currently it is using a kind of imitation navigation. But I have a worry about brining the navigation capabilities together with custom height. Our BottomSheet is currently using custom height, if we present the BottomSheet like any other modal in iOS it'll be displayed with a standard height. Similar to what we see here: Classic editor, link settings: Normally on iOS, you need to write custom code to be able to present a modal with custom height, and I doubt that ability is given us via react-native navigation. This needs some experimenting.
My other question would be,
I want to hear from @iamthomasbishop as well. Is there a list of capabilities we wish BottomSheet had? |
@pinarol Yea, I agree custom heights is the critical piece. We need to have a “short” sheet height so that it obstructs visibility of the canvas as little as possible. Obviously this isn’t possible 100% of the time, but showing a half-screen or shorter sheet guarantees much more visibility I think one of the teams working on WPiOS and WPAndroid are working on fully native bottom sheets, complete with custom heights. It’d be worth touching base to see what we can utilize, if anything. // cc @pinarol @maxme @yaelirub @leandroalonso If we can’t utilize anything there and have to make our current implementation work (or come up with a new RN-based solution), then something like what @lukewalczak has built for color settings would be more than acceptable. So while I’d certainly love to have fully-native page transitions, nesting, and the low maintenance cost, our current approach is a close second. |
@yaelirub @leandroalonso are you going to ship that as independent libraries? Let's imagine we have a RN wrapper around this new native bottom sheets (using the same API on Android and iOS). Would it be easy to port our current usage of bottom sheets (in existing blocks) using this RN wrapper? |
@iamthomasbishop the current bottom sheet is missing the hide/increase gesture with scroll view. It wasn't a feature we needed for the Prepublishing Nudges project thus we didn't implement. I know that @bjtitus had plans to work on that but I'm not sure of the timeline. Right now the Bottom Sheet won't solve the initial problem stated in this issue I'm afraid. @maxme it is currently in the WordPressUI library for iOS. But I don't see any problem in extracting it to its own pod/repo if it's necessary for Gutenberg. @jd-alexander has more information about the state of Bottom Sheet on the Android side. |
It wouldn't be easy without solving navigation, the new Bottom Sheet won't support navigating between different react-native views. Plus, even if we ignore navigation, implementing a wrapper for the new native bottom sheet is quite some work as well. |
We had a hard time implementing the working BottomSheet with a scroll view inside and swipe to hide gestures. Basically there was an issue with gesture handler inside the
Modal
. We managed to make it work for iOS but not for Android. There is PR with the first version - WordPress/gutenberg#18574 - please check comments, there are a lot of details about why we need that.We need that PR -software-mansion/react-native-gesture-handler#937 to be released to implement the ideal version for iOS and Android.
The text was updated successfully, but these errors were encountered: