Page MenuHomePhabricator

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

Authored by atul on Wed, May 24, 12:56 PM.
Referenced Files
F570378: D7968.diff
Sat, Jun 3, 2:09 AM
F557760: 6318ff.png
Wed, May 24, 5:01 PM
Unknown Object (File)
Wed, May 24, 2:51 PM



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

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

atul published this revision for review.Wed, May 24, 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.Wed, May 24, 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

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.Wed, May 24, 4:17 PM
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

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


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.Thu, May 25, 8:58 AM
195 ↗(On Diff #27015)


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