Page MenuHomePhabricator
Differential D8161

[native] Prevent the only admin role from being changed
ClosedPublic
Actions

Authored by rohan on Jun 9 2023, 11:45 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 3, 1:54 AM
Unknown Object (File)
Wed, Feb 26, 12:31 PM
Unknown Object (File)
Wed, Feb 26, 12:30 PM
Unknown Object (File)
Wed, Feb 26, 12:30 PM
Unknown Object (File)
Wed, Feb 26, 12:26 AM
Unknown Object (File)
Mon, Feb 24, 3:07 PM
Unknown Object (File)
Sun, Feb 23, 3:41 PM
Unknown Object (File)
Sun, Feb 23, 3:41 PM
View All Files
Subscribers
ashoat
• ted
tomek

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
rohan updated this revision to Diff 27642.Jun 12 2023, 5:33 AM

Rebase

Harbormaster failed remote builds in B20177: Diff 27642!Jun 12 2023, 5:35 AM
atul added a comment.Jun 12 2023, 9:33 AM

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

rohan updated this revision to Diff 27669.Jun 12 2023, 2:03 PM

Rebase

Harbormaster completed remote builds in B20199: Diff 27669.Jun 12 2023, 2:22 PM
rohan planned changes to this revision.Jun 13 2023, 9:07 AM

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

rohan updated this revision to Diff 27708.EditedJun 13 2023, 6:32 PM

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?

This revision is now accepted and ready to land.Jun 14 2023, 10:17 AM
rohan added inline comments.Jun 14 2023, 10:30 AM
native/roles/change-roles-screen.react.js
282 ↗(On Diff #27708)

Oh yeah that's from debugging let me remove that

rohan updated this revision to Diff 27768.Jun 14 2023, 10:32 AM

Remove unused green color

Harbormaster completed remote builds in B20257: Diff 27768.Jun 14 2023, 10:49 AM
rohan updated this revision to Diff 27951.Jun 21 2023, 8:07 AM

Rebase

This revision was landed with ongoing or failed builds.Jun 21 2023, 8:42 AM
Closed by commit rCOMMbf2371062015: [native] Prevent the only admin role from being changed (authored by rohan). · Explain Why
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMMbf2371062015: [native] Prevent the only admin role from being changed.
Harbormaster completed remote builds in B20397: Diff 27951.Jun 21 2023, 9:41 AM

Revision Contents
Changeset List

Diff 27969

View Options

native/roles/change-roles-header-right-button.react.js

Show First 20 Lines • Show All 135 Lines • ▼ Show 20 Linesconst light = Object.freeze({
subthreadsModalClose: designSystemColors.shadesBlack60, subthreadsModalClose: designSystemColors.shadesBlack60,
subthreadsModalBackground: designSystemColors.shadesWhite80, subthreadsModalBackground: designSystemColors.shadesWhite80,
subthreadsModalSearch: '#00000008', subthreadsModalSearch: '#00000008',
messageLabel: designSystemColors.shadesBlack100, messageLabel: designSystemColors.shadesBlack100,
modalSeparator: designSystemColors.shadesWhite60, modalSeparator: designSystemColors.shadesWhite60,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesBlack100, inviteLinkLinkColor: designSystemColors.shadesBlack100,
inviteLinkButtonBackground: designSystemColors.shadesWhite60, inviteLinkButtonBackground: designSystemColors.shadesWhite60,
emojiKeyboardKnob: designSystemColors.shadesWhite90,
emojiKeyboardCategoryIcon: designSystemColors.shadesBlack70,
emojiKeyboardContainerActive: designSystemColors.shadesWhite90,
ashoatUnsubmitted
Not Done
Inline Actions

Do we really need one-offs here? Why can't we find an existing color family that works with our designs?

We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app

ashoat: Do we really need one-offs here? Why can't we find an existing color family that works with our…
ashoatUnsubmitted
Not Done
Inline Actions

Meant to tag @ted

ashoat: Meant to tag @ted
}); });
export type Colors = $Exact<typeof light>; export type Colors = $Exact<typeof light>;
const dark: Colors = Object.freeze({ const dark: Colors = Object.freeze({
blockQuoteBackground: '#A9A9A9', blockQuoteBackground: '#A9A9A9',
blockQuoteBorder: designSystemColors.shadesBlack60, blockQuoteBorder: designSystemColors.shadesBlack60,
codeBackground: designSystemColors.shadesBlack100, codeBackground: designSystemColors.shadesBlack100,
disabledButton: designSystemColors.shadesBlack80, disabledButton: designSystemColors.shadesBlack80,
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Linesconst dark: Colors = Object.freeze({
typeaheadTooltipBackground: '#1F1F1f', typeaheadTooltipBackground: '#1F1F1f',
typeaheadTooltipBorder: designSystemColors.shadesBlack80, typeaheadTooltipBorder: designSystemColors.shadesBlack80,
typeaheadTooltipText: 'white', typeaheadTooltipText: 'white',
messageLabel: designSystemColors.shadesWhite60, messageLabel: designSystemColors.shadesWhite60,
modalSeparator: designSystemColors.shadesBlack80, modalSeparator: designSystemColors.shadesBlack80,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesWhite80, inviteLinkLinkColor: designSystemColors.shadesWhite80,
inviteLinkButtonBackground: designSystemColors.shadesBlack80, inviteLinkButtonBackground: designSystemColors.shadesBlack80,
emojiKeyboardKnob: designSystemColors.shadesWhite100,
emojiKeyboardCategoryIcon: designSystemColors.shadesWhite70,
emojiKeyboardContainerActive: designSystemColors.shadesBlack60,
}); });
const colors = { light, dark }; const colors = { light, dark };
const colorsSelector: (state: AppState) => Colors = createSelector( const colorsSelector: (state: AppState) => Colors = createSelector(
(state: AppState) => state.globalThemeInfo.activeTheme, (state: AppState) => state.globalThemeInfo.activeTheme,
(theme: ?GlobalTheme) => { (theme: ?GlobalTheme) => {
const explicitTheme = theme ? theme : 'light'; const explicitTheme = theme ? theme : 'light';
return colors[explicitTheme]; return colors[explicitTheme];
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines
View Options

native/roles/change-roles-screen.react.js

Show First 20 Lines • Show All 135 Lines • ▼ Show 20 Linesconst light = Object.freeze({
subthreadsModalClose: designSystemColors.shadesBlack60, subthreadsModalClose: designSystemColors.shadesBlack60,
subthreadsModalBackground: designSystemColors.shadesWhite80, subthreadsModalBackground: designSystemColors.shadesWhite80,
subthreadsModalSearch: '#00000008', subthreadsModalSearch: '#00000008',
messageLabel: designSystemColors.shadesBlack100, messageLabel: designSystemColors.shadesBlack100,
modalSeparator: designSystemColors.shadesWhite60, modalSeparator: designSystemColors.shadesWhite60,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesBlack100, inviteLinkLinkColor: designSystemColors.shadesBlack100,
inviteLinkButtonBackground: designSystemColors.shadesWhite60, inviteLinkButtonBackground: designSystemColors.shadesWhite60,
emojiKeyboardKnob: designSystemColors.shadesWhite90,
emojiKeyboardCategoryIcon: designSystemColors.shadesBlack70,
emojiKeyboardContainerActive: designSystemColors.shadesWhite90,
ashoatUnsubmitted
Not Done
Inline Actions

Do we really need one-offs here? Why can't we find an existing color family that works with our designs?

We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app

ashoat: Do we really need one-offs here? Why can't we find an existing color family that works with our…
ashoatUnsubmitted
Not Done
Inline Actions

Meant to tag @ted

ashoat: Meant to tag @ted
}); });
export type Colors = $Exact<typeof light>; export type Colors = $Exact<typeof light>;
const dark: Colors = Object.freeze({ const dark: Colors = Object.freeze({
blockQuoteBackground: '#A9A9A9', blockQuoteBackground: '#A9A9A9',
blockQuoteBorder: designSystemColors.shadesBlack60, blockQuoteBorder: designSystemColors.shadesBlack60,
codeBackground: designSystemColors.shadesBlack100, codeBackground: designSystemColors.shadesBlack100,
disabledButton: designSystemColors.shadesBlack80, disabledButton: designSystemColors.shadesBlack80,
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Linesconst dark: Colors = Object.freeze({
typeaheadTooltipBackground: '#1F1F1f', typeaheadTooltipBackground: '#1F1F1f',
typeaheadTooltipBorder: designSystemColors.shadesBlack80, typeaheadTooltipBorder: designSystemColors.shadesBlack80,
typeaheadTooltipText: 'white', typeaheadTooltipText: 'white',
messageLabel: designSystemColors.shadesWhite60, messageLabel: designSystemColors.shadesWhite60,
modalSeparator: designSystemColors.shadesBlack80, modalSeparator: designSystemColors.shadesBlack80,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesWhite80, inviteLinkLinkColor: designSystemColors.shadesWhite80,
inviteLinkButtonBackground: designSystemColors.shadesBlack80, inviteLinkButtonBackground: designSystemColors.shadesBlack80,
emojiKeyboardKnob: designSystemColors.shadesWhite100,
emojiKeyboardCategoryIcon: designSystemColors.shadesWhite70,
emojiKeyboardContainerActive: designSystemColors.shadesBlack60,
}); });
const colors = { light, dark }; const colors = { light, dark };
const colorsSelector: (state: AppState) => Colors = createSelector( const colorsSelector: (state: AppState) => Colors = createSelector(
(state: AppState) => state.globalThemeInfo.activeTheme, (state: AppState) => state.globalThemeInfo.activeTheme,
(theme: ?GlobalTheme) => { (theme: ?GlobalTheme) => {
const explicitTheme = theme ? theme : 'light'; const explicitTheme = theme ? theme : 'light';
return colors[explicitTheme]; return colors[explicitTheme];
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines