Page MenuHomePhabricator
Differential D8523

[native] Display an action sheet with the 'edit role' option
ClosedPublic
Actions

Authored by rohan on Jul 17 2023, 10:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 28, 6:55 AM
Unknown Object (File)
Sat, Nov 23, 2:31 PM
Unknown Object (File)
Sat, Nov 23, 2:31 PM
Unknown Object (File)
Sat, Nov 23, 2:31 PM
Unknown Object (File)
Sat, Nov 23, 12:50 PM
Unknown Object (File)
Sat, Nov 9, 2:45 PM
Unknown Object (File)
Sat, Nov 9, 2:34 PM
Unknown Object (File)
Sat, Nov 9, 10:48 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
ginsu
Commits
rCOMM019b1d22ea64: [native] Display an action sheet with the 'edit role' option
Summary

The first option we display to the user is an "Edit role" item, where clicking it will take them to the role name / permissions screen. This diff just handles the action sheet, but the next one will
handle responding to pressing the 'Edit role' option. Down the line, I will also be adding the 'Delete' option to the action sheet.

ENG-4368.

Depends on D8522

Test Plan

Verified that on iOS and Android, the action sheet looks as expected.

Screenshot 2023-07-17 at 1.41.51 PM.png (1×1 px, 635 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan created this revision.Jul 17 2023, 10:44 AM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 17 2023, 10:44 AM
rohan edited the test plan for this revision. (Show Details)Jul 17 2023, 10:45 AM
rohan updated this revision to Diff 28762.Jul 17 2023, 10:55 AM

Clean up options/icons logic

rohan updated this revision to Diff 28764.Jul 17 2023, 10:58 AM

Revert changes

Harbormaster completed remote builds in B20978: Diff 28761.Jul 17 2023, 11:02 AM
rohan added a child revision: D8524: [native] Navigate to the role creation screen with pre-populated fields for editing roles.Jul 17 2023, 11:11 AM
Harbormaster completed remote builds in B20979: Diff 28762.Jul 17 2023, 11:12 AM
Harbormaster completed remote builds in B20981: Diff 28764.Jul 17 2023, 11:19 AM
rohan requested review of this revision.Jul 17 2023, 11:19 AM
ginsu accepted this revision.Jul 20 2023, 3:24 PM

accepting so I don't block this diff and also my concerns are outside the scope of this diff; however, it is becoming very obvious that we need our own shared action sheet component that abstracts a lot of the similar patterns surrounding the actions sheet (only showing cancel for iOS in options, the cancelButtonIndex, the userInterfaceStyle, etc). This task should help with addressing this concern too since we should be able to deprecate most of this logic in favor for our own custom bottomsheet.

This revision is now accepted and ready to land.Jul 20 2023, 3:24 PM
rohan updated this revision to Diff 28938.Jul 21 2023, 7:55 AM

Rebase

rohan added a comment.Jul 21 2023, 7:59 AM
In D8523#252704, @ginsu wrote:

accepting so I don't block this diff and also my concerns are outside the scope of this diff; however, it is becoming very obvious that we need our own shared action sheet component that abstracts a lot of the similar patterns surrounding the actions sheet (only showing cancel for iOS in options, the cancelButtonIndex, the userInterfaceStyle, etc). This task should help with addressing this concern too since we should be able to deprecate most of this logic in favor for our own custom bottomsheet.

Yeah I completely agree with this, I think converting to a custom bottomsheet (or at the very least in the short term having a shared action sheet component if the bottomsheet won't be prioritized) is going to help keep things consistent, and also make the conversion from actionsheet --> bottomsheet a lot easier

Harbormaster completed remote builds in B21116: Diff 28938.Jul 21 2023, 8:17 AM
rohan updated this revision to Diff 29329.Jul 31 2023, 12:03 PM

Rebase

This revision was landed with ongoing or failed builds.Jul 31 2023, 6:15 PM
Closed by commit rCOMM019b1d22ea64: [native] Display an action sheet with the 'edit role' option (authored by rohan). · Explain Why
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM019b1d22ea64: [native] Display an action sheet with the 'edit role' option.

Revision Contents
Changeset List

PathSize
native/
roles/
67 lines

Diff 28762

View Options

native/roles/role-panel-entry.react.js

Loading...