Details

Reviewers

atul
ginsu

Commits

rCOMMbf2371062015: [native] Prevent the only admin role from being changed

Summary

Just like on web, we want to make sure the last remaining admin in a community cannot remove themselves from the role until someone else is assigned the Admins role

Depends on D8160

Test Plan

Please see the video below that demonstrates this

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Jun 9 2023, 11:45 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 9 2023, 11:45 AM

rohan edited the test plan for this revision. (Show Details)Jun 9 2023, 11:47 AM

rohan attached a referenced file: F580304: Simulator Screen Recording - iPhone 14 Pro - 2023-06-09 at 14.45.44.mp4. (Show Details)

Harbormaster completed remote builds in B20140: Diff 27590.Jun 9 2023, 12:02 PM

rohan requested review of this revision.Jun 9 2023, 12:02 PM

Rebase

Harbormaster failed remote builds in B20177: Diff 27642!Jun 12 2023, 5:35 AM

(Shellcheck CI job should pass after pulling in latest changes.)

Rebase

Harbormaster completed remote builds in B20199: Diff 27669.Jun 12 2023, 2:22 PM

Planning changes to hold off on review until feedback on D8141 is addressed

Refactor the way we indicate a role change cannot be possible to be similar to web (block the user from getting into an invalid state all together)

rohan attached a referenced file: F586189: Simulator Screen Recording - iPhone 14 Pro - 2023-06-13 at 21.29.51.mp4. (Show Details)Jun 13 2023, 6:37 PM

Harbormaster completed remote builds in B20230: Diff 27708.Jun 13 2023, 6:50 PM

atul accepted this revision.Jun 14 2023, 10:17 AM

atul added a subscriber: • ted.

atul added inline comments.

native/roles/change-roles-screen.react.js
166–173 ↗	(On Diff #27708)	This seems like it would be a great candidate of something that could be included in design system... maybe `NoticeBox`? With `InfoBox`, `WarningBox`, ErrorBox` variants that have different icons/colors? Nothing actionable with this diff, just a thought CC @ashoat, @ted
282 ↗	(On Diff #27708)	What's this for? Didn't catch it in the screen recording, maybe a layout debugging artifact that got left in? If it is something we're using, can we use a color from the design system instead?