Page MenuHomePhabricator

[native] Make `enumCheckBox` functional in `CommunityConfiguration`
ClosedPublic

Authored by atul on May 24 2023, 12:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 12, 12:08 AM
Unknown Object (File)
Fri, Mar 8, 7:43 PM
Unknown Object (File)
Fri, Mar 8, 7:43 PM
Unknown Object (File)
Tue, Mar 5, 11:47 PM
Unknown Object (File)
Tue, Mar 5, 11:47 PM
Unknown Object (File)
Tue, Mar 5, 11:47 PM
Unknown Object (File)
Tue, Mar 5, 9:20 PM
Unknown Object (File)
Tue, Mar 5, 9:20 PM
Subscribers

Details

Summary

Have the enumCheckBox in CommunityConfiguration toggle announcementSetting when tapped.


Depends on D7967

Test Plan

Looks as expected:

Simulator Screenshot - Fresh iPhone 14 Pro - 2023-05-24 at 15.52.22.png (2×1 px, 138 KB)

Simulator Screenshot - Fresh iPhone 14 Pro - 2023-05-24 at 15.52.35.png (2×1 px, 138 KB)

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D7968 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.May 24 2023, 1:00 PM
atul edited the test plan for this revision. (Show Details)
atul edited the summary of this revision. (Show Details)
ashoat requested changes to this revision.May 24 2023, 4:17 PM
ashoat added a subscriber: ted.

Requesting changes for design iteration. Really wish this was captured in the designs we got from @ted before the hand-off

native/community-creation/community-configuration.react.js
191–196 ↗(On Diff #27015)

I can't find designs on Figma for what the box is supposed to look like checked. @atul, do you have any? If not, we should really talk to Ted about this... the placeholder text color was also missing, and all-in-all this amounts to designs that were not sufficiently fleshed out before being handed off to eng.

Given he's out right now, we'll have to make a call on our own. My view is that the margin should be increased (so decrease height / width), and the borderRadius should be adjusted to match the ratio of enumCheckBoxFill to enumCheckBox size.

Can you show me what height: 20, width: 20, borderRadius: 2.1875 looks like?

195 ↗(On Diff #27015)

This color looks too dark in light mode in my opinion... it's kind of distracting. It seems like we have two light mode issues now: panelSecondaryForegroundBorder and panelForegroundSecondaryLabel, right? We should probably create a separate task under DES-71 for @ted to concretely investigate those colors and see can simply be updated to more appropriate values without degrading the experience wherever else they're being used today.

This revision now requires changes to proceed.May 24 2023, 4:17 PM
native/community-creation/community-configuration.react.js
191–196 ↗(On Diff #27015)

I based it off of what we have on web:

6318ff.png (346×732 px, 35 KB)

Realizing that in my implementation on native I'm "filling up" more of the checkbox than we are on native. I'll adjust that.

Can you show me what height: 20, width: 20, borderRadius: 2.1875 looks like?

Will do

native/community-creation/community-configuration.react.js
191–196 ↗(On Diff #27015)

Here's how it looks:

Simulator Screenshot - Fresh iPhone 14 Pro - 2023-05-25 at 08.36.01.png (2×1 px, 139 KB)

height: 20, width: 20, borderRadius: 2.1875

Thanks!!

native/community-creation/community-configuration.react.js
195 ↗(On Diff #27015)

Can you create a separate task under DES-71 as described above before landing? That supertask is tracking all mobile app colors.js investigations that @ted needs to do

This revision is now accepted and ready to land.May 25 2023, 8:58 AM
native/community-creation/community-configuration.react.js
195 ↗(On Diff #27015)

Done

This revision was landed with ongoing or failed builds.May 26 2023, 8:10 AM
This revision was automatically updated to reflect the committed changes.