Page MenuHomePhabricator

[native] fix BottomSheetBackdrop animation
ClosedPublic

Authored by ginsu on Oct 5 2023, 11:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 9:01 AM
Unknown Object (File)
Fri, Nov 22, 8:52 AM
Unknown Object (File)
Fri, Nov 22, 8:45 AM
Unknown Object (File)
Fri, Nov 22, 3:22 AM
Unknown Object (File)
Wed, Nov 20, 5:41 PM
Unknown Object (File)
Wed, Nov 20, 5:13 AM
Unknown Object (File)
Sun, Nov 10, 12:45 AM
Unknown Object (File)
Fri, Nov 8, 1:13 PM
Subscribers

Details

Summary

With the switch from BottomSheetModal to basic BottomSheet, there was a slight regression with the animation of the backdrop overlay. This diff addresses this regression by using the overlay animation from react-navigation, and replacing the contents of our Backdrop component with a TouchableWithoutFeedback that will close the bottom sheet on press (which will then trigger the navigating back callback)

This is step 2 in the list below
Outlined below are the steps I will take in this stack (each point here will be it's own diff):

  1. Replace out of box BottomSheetModal with basic BottomSheet (Unfortunately, BottomSheetModal does not play nice with nested navigator)
  2. Polish up the BottomSheetBackdrop animation
  3. Introduce User Profile Bottom Sheet Navigators (this is so we can navigate to User Profile Avatar Modal from the User Profile Bottom Sheet)
  4. Introduce the User Profile Avatar Modal component and all the necessary things (like route names) that we will need to navigate to this new screen
  5. Factor out the User Avatar component in UserProfile into it's own separate component to keep things better organized

Depends on D9373

Test Plan

Please see the demo videos below

Before:

After:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/bottom-sheet/bottom-sheet-backdrop.react.js
19–21 ↗(On Diff #31689)

In D9260, I brought concerns up about a bug where if I quickly pressed the bottom sheet backdrop again before the bottom sheet was fully open the onChange callback would not get called even if the bottom sheet was in a closed state. To address this, I set this conditional statement to make sure that we can only close the bottom sheet when it is fully open so onChange will always get called

update

native/bottom-sheet/bottom-sheet-backdrop.react.js
17–20 ↗(On Diff #31690)

In D9260, I brought concerns up about a bug where if I quickly pressed the bottom sheet backdrop again before the bottom sheet was fully open the onChange callback would not get called even if the bottom sheet was in a closed state. To address this, I set this conditional statement to make sure that we can only close the bottom sheet when it is fully open so onChange will always get called

ginsu requested review of this revision.Oct 5 2023, 12:10 PM
This revision is now accepted and ready to land.Oct 5 2023, 4:19 PM

rebase one more time before landing (had a phab error when attempted to recently land)

This revision was landed with ongoing or failed builds.Oct 5 2023, 6:38 PM
This revision was automatically updated to reflect the committed changes.