Page MenuHomePhabricator

[native] Introduce `CommunityCreationNavigator`
ClosedPublic

Authored by atul on May 23 2023, 1:19 PM.
Tags
None
Referenced Files
F2087185: D7950.id26987.diff
Sun, Jun 23, 2:24 PM
Unknown Object (File)
Thu, Jun 20, 5:35 PM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Unknown Object (File)
Tue, Jun 18, 8:23 AM
Subscribers

Details

Summary

Heavily inspired by RegistrationNavigator, this diff introduces CommunityCreationNavigator. This navigator will "handle" the screens of the community creation flow. Specifically the community configuration screen and the optional "Add members" screen.

WARNING: Please let me know if this is the wrong approach. My understanding is that because we have > 1 screen in sequence as part of our flow we should create a new navigation stack. HOWEVER, I have a very very limited understanding of navigation on native, so please correct me if that assumption is incorrect and I should approach this differently.
Test Plan
  1. Set isCommunityCreationButtonEnabled to true.
  2. Open community drawer and press "Create community."
  3. Observe that CommunityConfiguration is displayed (by looking at screen "title").

Simulator Screenshot - Fresh iPhone 14 Pro - 2023-05-23 at 16.26.55.png (2×1 px, 86 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.May 23 2023, 1:40 PM
ashoat requested changes to this revision.May 23 2023, 2:16 PM

High-level approach is great, but some comments below

native/community-creation/community-creation-navigator.react.js
31 ↗(On Diff #26914)

I'm not sure since I don't have the link to the Figma designs handy, but if the title is different for different screens, this might be something you want to specify to CommunityConfiguration directly:

<CommunityCreation.Screen
  name={CommunityConfigurationRouteName}
  component={CommunityConfiguration}
  options={{ headerTitle: 'Create a community' }}
/>

(Memoized of course)

32 ↗(On Diff #26914)

I use white directly in the registration flow, but that flow is built for dark mode only. I think you'll need to call useColors in CommunityCreationNavigator, and construct screenOptions with the appropriate color there

native/navigation/root-navigator.react.js
207 ↗(On Diff #26914)

Why include disableGesturesScreenOptions here? Feels like it would be better with gestures enabled

This revision now requires changes to proceed.May 23 2023, 2:16 PM
atul marked 3 inline comments as done.May 24 2023, 7:18 AM
atul added inline comments.
native/community-creation/community-creation-navigator.react.js
31 ↗(On Diff #26914)

Thanks for catching that, made the change

32 ↗(On Diff #26914)

Updated to use colors.panelForegroundLabel

native/navigation/root-navigator.react.js
207 ↗(On Diff #26914)

No good reason, removed that

ashoat added inline comments.
native/community-creation/community-creation-navigator.react.js
50–52 ↗(On Diff #26985)

Doesn't really matter, and partly my fault for suggesting memoization... but this could be defined outside the component

This revision is now accepted and ready to land.May 24 2023, 7:21 AM
atul marked 3 inline comments as done.May 24 2023, 7:23 AM
atul added inline comments.
native/community-creation/community-creation-navigator.react.js
50–52 ↗(On Diff #26985)

True, will make that change

pull communityConfigurationOptions out of component