search

expo / react-native-action-sheet

A cross-platform ActionSheet for React Native
MIT License
1.37k stars 224 forks source link

[Android] No way to close the action sheet using the device back button #187

Closed reubenab closed 2 years ago

reubenab commented 3 years ago

Hi! Would it be possible to add a flag to the action sheet props that would handle an Android native back button press? Ideally it would function the same way as the cancel button and just close the action sheet.

reubenab commented 3 years ago

Never mind, this is more lkely an issue on our end. Given our native infra, it would be cool to be able to imperatively close the action sheet - is there some way you could expose this behavior?

bradbyte commented 3 years ago

This has been requested before (https://github.com/expo/react-native-action-sheet/issues/104), we could do it for Android but I'm not aware of an API equivalent for iOS.

reubenab commented 3 years ago

Thanks for pointing to that request @bradbumbalough ! Would it be possible to expose something like this just for Android for now? We don't have a use case for iOS anyway (since there's no hardware back button)

bogdanciobanu commented 3 years ago

I've discovered something quite interesting. This behavior is actually built in through 2 mechanisms: when you press outside of the actionsheet and when pressing the hardware back button on Android. When either of these 2 actions happen the cancel button is "pressed".

But what if you don't want to add a special cancel button? One idea comes from reading the _selectCancelButton method: https://github.com/expo/react-native-action-sheet/blob/5549e36a92d8afcc0ce2bf6a601e83209b5ce080/src/ActionSheet/index.tsx#L208-L222 If you pass something that's not a number for this prop you'll get the desired behavior. However, it's not ideal because the typescript type for the props require cancelButtonIndex to be a number.

The second option is to pass a special value as cancelButtonIndex, -1 for example. However, in this case you need to handle the special case in the method called when a button is pressed.

One simple fix would be that in _selectCancelButton to have the default behavior of closing the actionSheet when cancelButtonIndex is undefined. What do you think, @bradbumbalough ?

bradbyte commented 2 years ago

Ok, I spent some time looking into this and comparing with iOS. This would introduce a UX difference from iOS. iOS requires a cancelIndex in order for background taps to work. Hardware back button for Android works fine, but you need to include a cancel button. For now I'm keeping this in sync with iOS.