Page MenuHomePhabricator

[native] introduce barebones bottom sheet component
ClosedPublic

Authored by ginsu on Aug 25 2023, 11:15 AM.
Tags
None
Referenced Files
F3369720: D8955.id30330.diff
Mon, Nov 25, 11:23 PM
Unknown Object (File)
Fri, Nov 15, 11:08 PM
Unknown Object (File)
Fri, Nov 15, 3:57 AM
Unknown Object (File)
Mon, Nov 11, 1:08 PM
Unknown Object (File)
Sat, Nov 9, 8:29 AM
Unknown Object (File)
Oct 26 2024, 1:28 AM
Unknown Object (File)
Oct 20 2024, 3:41 PM
Unknown Object (File)
Oct 20 2024, 3:41 PM

Details

Summary

This components introduces the barebones bottom sheet component. This component is meant to be a generic component that is feature agnostic and can be reused for any future bottom sheet we might want to create. I am very much working off the idea that this is far from the finished bottom sheet component, and some stuff here might not look super polished and a fair amount of this will change. I thought for review this would be easier to follow the progression of this component as I add the bits and pieces to it rather than just getting the final thing.

This diff also introduces a new folder called bottom-sheets. I thought this would be appropriate to add given that we likely will be using bottom sheets a lot in our app soon (profiles, message toolitps, any other place we use actionsheet) and it would make sense to have a folder to house all of this.

Here is design for reference:

Screenshot 2023-08-25 at 2.19.48 PM.png (1×734 px, 134 KB)

Depends on D8935

Test Plan

Please see the demo video to see how the bottom sheet looks/behaves so far

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/bottom-sheets/bottom-sheet.react.js
17 ↗(On Diff #30330)

This is hardcoded based on what I saw in the bottom sheet docs, this will change in a subsequent diff

44 ↗(On Diff #30330)

We need to forward the ref here since in order to call the present function for the bottom sheet we need to pass in the ref into the component.

General Usage:
https://gorhom.github.io/react-native-bottom-sheet/modal/usage

Methods:
https://gorhom.github.io/react-native-bottom-sheet/modal/methods

Hooks:
https://gorhom.github.io/react-native-bottom-sheet/modal/hooks

Looks good to me. Don't have any experience forwarding refs so adding @tomek as blocking to take another look.

This revision is now accepted and ready to land.Aug 27 2023, 10:46 AM
This revision now requires review to proceed.Aug 27 2023, 10:56 AM
tomek added inline comments.
native/bottom-sheets/bottom-sheet.react.js
12 ↗(On Diff #30330)

Is there a way to type ref? Maybe something like React.Ref<typeof BottomSheetModal>?

33 ↗(On Diff #30330)

Quite surprising to see a foreground color used as a background

This revision is now accepted and ready to land.Aug 28 2023, 1:44 AM
ginsu added inline comments.
native/bottom-sheets/bottom-sheet.react.js
33 ↗(On Diff #30330)

cc @ted

address comments / rebase before landing

native/bottom-sheets/bottom-sheet.react.js
33 ↗(On Diff #30330)

Spoke w/ @ted IRL about this. Agree that it is strange to use "foreground" as a background color; however, we tried modalBackground and the color is too dark. @ted and I are working on cleaning up the colors this month and we will try and address this

https://linear.app/comm/issue/ENG-4485/clean-up-the-rest-of-the-native-colors

This revision was landed with ongoing or failed builds.Sep 6 2023, 11:07 AM
This revision was automatically updated to reflect the committed changes.