[native] Refactor out EnumSettingsOption into its own component
ClosedPublic
Actions

Authored by rohan on Jun 22 2023, 11:34 AM.

Details

Reviewers

atul

Commits

rCOMMa0fe061a40fe: [native] Refactor out EnumSettingsOption into its own component

Summary

A EnumSettingsOption counterpart is used on native during community creation, however it's built into the community configuration screen directly since we don't use it anywhere else on native. I'm going to need something similar for selecting role permissions, so I just made a pretty clean refactor into its own component. I recall there was some talk about needing to confirm the actual designs for checked / unchecked, but for now I've just copied it straight over without any design changes.

The reason icon is an optional prop is because at the moment, the designs for role permissions don't require an icon so it made more sense to just make it optional entirely.

Test Plan

Ran through the community creation flow and logged into another user to confirm that the 'announcement root' setting actually applied (video below). Also I'll show what it looks like if an icon is not provided.

Simulator Screen Shot - iPhone 14 Pro - 2023-06-22 at 14.36.17.png (2×1 px, 145 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Jun 22 2023, 11:34 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 22 2023, 11:34 AM

rohan edited the summary of this revision. (Show Details)Jun 22 2023, 11:35 AM

rohan edited the test plan for this revision. (Show Details)

rohan attached a referenced file: F602403: Screen Recording 2023-06-22 at 2.25.07 PM.mov. (Show Details)

Reduce marginLeft

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

rohan attached a referenced file: F602439: Simulator Screen Shot - iPhone 14 Pro - 2023-06-22 at 14.36.17.png. (Show Details)

rohan added inline comments.Jun 22 2023, 11:39 AM

native/community-creation/community-configuration.react.js
144 ↗	(On Diff #28013)	This is longer than 80 characters since it's a one-line description

Preemptively requesting review for CI failures

Harbormaster failed remote builds in B20435: Diff 28012!Jun 22 2023, 11:56 AM

Harbormaster failed remote builds in B20436: Diff 28013!

Looks good, thanks for taking care of this. Let's re-run CI before landing.

native/community-creation/community-configuration.react.js
144 ↗	(On Diff #28013)	We can still split the string so we don't exceed 80 character width. I think that'd be the preferred style for the codebase.
native/components/enum-settings-option.react.js
22 ↗	(On Diff #28013)	We can destructure `props` once so we don't need to write `props.icon`, `props.enumValue`, etc
33–36 ↗	(On Diff #28013)	Personally prefer const checkBoxFill = props.enumValue ? ( <View style={styles.enumCheckBoxFill} /> ) : null; but totally arbitrary, whatever you prefer
37–47 ↗	(On Diff #28013)	Could do something like const infoContainerStyle = React.useMemo( () => props.icon ? styles.enumInfoContainer : { ...styles.enumInfoContainer, marginLeft: 10 }, [props.icon, styles.enumInfoContainer], ); but whatever you prefer