Page MenuHomePhabricator

[native] Navigate to a screen to create a new role
ClosedPublic

Authored by rohan on Jun 29 2023, 9:50 AM.
Tags
None
Referenced Files
F3367441: D8379.id.diff
Mon, Nov 25, 2:51 PM
Unknown Object (File)
Sat, Nov 23, 7:13 AM
Unknown Object (File)
Sat, Nov 23, 7:11 AM
Unknown Object (File)
Sat, Nov 23, 7:11 AM
Unknown Object (File)
Sun, Nov 10, 10:27 AM
Unknown Object (File)
Thu, Nov 7, 9:55 PM
Unknown Object (File)
Thu, Nov 7, 9:55 PM
Unknown Object (File)
Thu, Nov 7, 9:55 PM

Details

Summary

Following the community roles screen, clicking the button should navigate to a new screen that will allow admins to create new roles with a set of permissions. The actual screen will be filled out in a following diff, this just handles the navigation to what will be the create roles screen.

Depends on D8359

ENG-4170

Test Plan

Confirmed that the navigation works and the animation from the previous screen to the new screen is good

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/roles/create-roles-screen.react.js
20–22 ↗(On Diff #28274)

This could probably be return null, but I don't think it's really important since it won't be landed as is and will be implemented up the stack

ashoat added inline comments.
native/roles/roles-navigator.react.js
43 ↗(On Diff #28274)

As it stands today, @ted does not have a process down for making sure the copy he produces is okay to handoff to developers. Instead, every dev on the team has to add me as a reviewer to any diffs that have copy. It's a bad process and @ted is working on improving it, but this is what we have now

Specifically here, I think buttons and titles in the app should appear as "Create role". @rohan, can you confirm that's how we do it in the app today, and if so updates buttons / titles to match?

Make sure you adjust the button too, not just the title that I'm highlighting here. Ideally you adjust the button in the diff that introduces it, but if that's already landed you can do it in this diff too

native/roles/roles-navigator.react.js
43 ↗(On Diff #28274)

I think there's some inconsistency across the app:

  • Invite links:

Simulator Screen Shot - iPhone 14 Pro - 2023-07-03 at 11.03.46.png (2×1 px, 155 KB)

  • Community creation:

Simulator Screen Shot - iPhone 14 Pro - 2023-07-03 at 11.03.54.png (2×1 px, 147 KB)

Though I know you requested changes to change copy like 'Invite Links' and 'Create Role' to 'Invite links' and 'Create role', so I can match that convention and implement your suggestion

headerTitle: 'Create Role' --> headerTitle: 'Create role'

Introduce action: create_role | edit_role since I plan to re-use the screen for editing roles later.

Thanks Rohan for pointing out those inconsistencies! Would you mind putting up a separate diff to fix up the "Create Community" button too (so it's "Create community")? Figure it should only take a sec

Thanks Rohan for pointing out those inconsistencies! Would you mind putting up a separate diff to fix up the "Create Community" button too (so it's "Create community")? Figure it should only take a sec

https://phab.comm.dev/D8427

This revision is now accepted and ready to land.Jul 5 2023, 4:18 PM