search

wordpress-mobile / gutenberg-mobile

Mobile version of Gutenberg - native iOS and Android
GNU General Public License v2.0
256 stars 57 forks source link

BottomSheet with scroll view inside - ideal version #1884

Closed dratwas closed 3 years ago

dratwas commented 4 years ago

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 - https://github.com/WordPress/gutenberg/pull/18574 - please check comments, there are a lot of details about why we need that.

We need that PR -https://github.com/software-mansion/react-native-gesture-handler/pull/937 to be released to implement the ideal version for iOS and Android.

pinarol commented 4 years ago

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: (As you see, even if the content is mostly empty the modal is displayed with the standard modal height.)

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?

iamthomasbishop commented 4 years ago

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:

@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.

maxme commented 4 years ago

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.

@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?

leandroalonso commented 4 years ago

@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.

pinarol commented 4 years ago

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?

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.