Page MenuHomePhabricator
Differential D9274

[native] make bottomsheet snap point dynamic
ClosedPublic
Actions

Authored by ginsu on Sep 24 2023, 11:44 PM.
Tags
None
Referenced Files
F3341262: D9274.diff
Thu, Nov 21, 10:47 PM
Unknown Object (File)
Mon, Nov 11, 6:19 PM
Unknown Object (File)
Mon, Oct 28, 9:40 PM
Unknown Object (File)
Sat, Oct 26, 1:12 AM
Unknown Object (File)
Oct 20 2024, 3:41 PM
Unknown Object (File)
Oct 20 2024, 3:41 PM
Unknown Object (File)
Oct 20 2024, 3:41 PM
Unknown Object (File)
Oct 20 2024, 3:41 PM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
inka
Commits
rCOMM050b1af776c0: [native] make bottomsheet snap point dynamic
Summary

This diff replaces the hardcoded snap point I set earlier with a snap point that is based off the height of the bottomsheet container. With this new dynamic snap point, the bottom sheet will always update and change it's snap point whenever the height of the container changes (for example when we add or remove a button on the user profile component)

Depends on D9273

Test Plan

Please see the demo videos below

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Sep 24 2023, 11:44 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 24 2023, 11:44 PM
ginsu edited the summary of this revision. (Show Details)Sep 24 2023, 11:52 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
ginsu added a parent revision: D9273: [native] factor out UserProfileRelationshipButton condtionals.
ginsu attached a referenced file: F772686: Screen Recording 2023-09-25 at 2.50.56 AM.mov. (Show Details)
ginsu added inline comments.Sep 24 2023, 11:56 PM
native/user-profile/user-profile-relationship-button.react.js
55 ↗(On Diff #31383)

Since we want to measure the height of the user profile bottom sheet before the elements/components get painted on the screen, we should use a useLayoutEffect here

https://chat.openai.com/c/81792e1b-b517-47e3-9b0c-1eda21d8a8f9

https://react.dev/reference/react/useLayoutEffect#measuring-layout-before-the-browser-repaints-the-screen

Harbormaster completed remote builds in B22813: Diff 31383.Sep 25 2023, 12:00 AM
ginsu requested review of this revision.Sep 25 2023, 12:00 AM
atul accepted this revision.Sep 26 2023, 2:06 PM
atul added inline comments.
native/user-profile/user-profile.react.js
52 ↗(On Diff #31383)

Eh we can just use insets.bottom directly within the useLayoutEffect right?

This revision is now accepted and ready to land.Sep 26 2023, 2:06 PM
ginsu updated this revision to Diff 31477.Sep 27 2023, 10:58 PM

address @atul's feedback

Harbormaster completed remote builds in B22886: Diff 31477.Sep 27 2023, 11:15 PM
ginsu updated this revision to Diff 31636.Oct 4 2023, 10:06 AM

rebase before landing

ginsu mentioned this in D9358: [native] introduce user profile constants.Oct 4 2023, 10:10 AM
Harbormaster completed remote builds in B22972: Diff 31636.Oct 4 2023, 10:24 AM
Closed by commit rCOMM050b1af776c0: [native] make bottomsheet snap point dynamic (authored by ginsu). · Explain WhyOct 4 2023, 10:29 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM050b1af776c0: [native] make bottomsheet snap point dynamic.
ginsu mentioned this in rCOMM6a421e536baa: [native] introduce user profile constants.Oct 4 2023, 8:50 PM

Revision Contents
Changeset List

PathSize
native/
bottom-sheet/
9 lines
user-profile/
30 lines
46 lines

Diff 31636

View Options

native/bottom-sheet/bottom-sheet.react.js

Loading...
View Options

native/user-profile/user-profile-relationship-button.react.js

Loading...
View Options

native/user-profile/user-profile.react.js

Loading...