Page MenuHomePhabricator

[native] Introduce `ColorSelector` component
ClosedPublic

Authored by abosh on Apr 20 2022, 5:31 PM.
Tags
None
Referenced Files
F2899950: D3798.id11836.diff
Sat, Oct 5, 8:39 AM
Unknown Object (File)
Mon, Sep 23, 12:56 PM
Unknown Object (File)
Mon, Sep 23, 12:56 PM
Unknown Object (File)
Wed, Sep 11, 7:40 AM
Unknown Object (File)
Wed, Sep 11, 7:40 AM
Unknown Object (File)
Wed, Sep 11, 7:40 AM
Unknown Object (File)
Wed, Sep 11, 7:40 AM
Unknown Object (File)
Wed, Sep 11, 7:39 AM

Details

Summary

Part of the first step in redesigning the ColorPicker component on native to match the ColorSelector component on web introduced by @atul. Depends on D3790

ColorSelector is a new component that will work with ColorSelectorButton to replace ColorPicker on native. This diff handles color layout, and adding onPress handling and active state functionality will be applied in later diffs.

See the full plan for the ColorPicker redesign in this Linear issue.

Test Plan

Tested on iOS 15.4 and works as expected:

Screen Shot 2022-04-20 at 10.27.59 AM.png (1×1 px, 593 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

abosh added 1 blocking reviewer(s): atul.
abosh edited the test plan for this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 20 2022, 5:36 PM
Harbormaster failed remote builds in B8374: Diff 11686!

Updated memoization to memoize array passed into style prop for the View in ColorSelectorButton.

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 20 2022, 7:29 PM
Harbormaster failed remote builds in B8375: Diff 11687!
atul requested changes to this revision.Apr 24 2022, 2:20 PM

Looks like there's some sort of git issue here. Feel free to re-request review once the issues have been resolved!

native/components/color-selector-button.react.js
11–17 ↗(On Diff #11687)

Looks like you have some sort of git issue here, you'll need to rebase or update this diff or something

This revision now requires changes to proceed.Apr 24 2022, 2:20 PM
atul accepted this revision.EditedApr 25 2022, 7:04 AM

Looks good visually to me.

Something I noticed (outside the scope of this diff, but thanks to the provided screenshot) is that there's not great contrast between the modal and the view "behind it."

It would probably be a good idea to add some sort of outline or drop shadow to the Modal component to make that distinction clear.

It seems like it'd be another good starter task for you (@yayabosh). I'll create a Linear task and link it here.

edit: https://linear.app/comm/issue/ENG-1040/[native]-increase-contrast-between-modal-and-background-view

native/components/color-selector.react.js
31 ↗(On Diff #11836)

Could maybe rename this colorButtons or something?

This revision is now accepted and ready to land.Apr 25 2022, 7:04 AM
This revision was automatically updated to reflect the committed changes.
abosh marked an inline comment as done.