Add a new Coral Color Picker component. #179

indra2gurjar · 2021-07-03T23:54:50Z

Description

We are adding a new ColorPicker component which can support multiple formats like - HSL, HSV, RGB, HEX, HEX3, HEX4, HEX8, Name etc.
The component is similar to ColarArea component in Spectrum - https://spectrum.adobe.com/page/color-area/

For color format conversions library '@ctrl/tinycolor' is used.
At some places we are converting hsv/hsl color string since tinycolor doesn't preserve hue at S/L boundaries.
Color conversion in different formats loose details so it's done minimal and at many places color strings are generated for hsl/hsl format.

Related Issue

https://jira.corp.adobe.com/browse/CUI-7445
https://xd.adobe.com/view/4565a1db-3df7-4483-8246-3a0e4882c842-1036/screen/e4cae491-ff98-49ce-9161-264e6abc03ef
https://stock.adobe.com/

Motivation and Context

Existing ColorInput component does support only few formats like - RGBA and color swatches only.
This new Color Picker provides user a way to select colors in the natural way where user can select a fully saturated hue(color) from ColorSlider, then user can change saturation and luminosity.
Color input-output supports most of the formats.
Alpha channel selector isn't added currently and can be added later. This isn't required for CUI-7445.
User can still change alpha manually.

How Has This Been Tested?

Only manually.
Component can be seen in action at - http://indra-imac.corp.adobe.com:9001/examples/
We are planning to add unit test in the PR.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

indra2gurjar · 2021-07-15T06:45:11Z

@Pareesh @icaraps A gentle reminder! Please help in reviewing.

Pareesh · 2021-07-20T05:05:45Z

coral-component-colorpicker/src/scripts/ColorArea.js

+  _stepY() {
+    return 0.01;
+  }
+


since all these are const values. you can add a getter like

get _minX() { return 0; }

and then you can use it as property rather than function

The major difference I see b/w property and _minX function is:
Property can be changed by the consumer of the class while _minX function always returns the const value.
We want to keep min, max, step values constant.

I don't know any benefit of accessing a property over a function call. Accessing a property is same as calling getter function.

This corresponds more to a property rather than function. Generally the the property starting with underscore is private and should not be changed, in case consumer is changing its consumer faults.

you can still avoid the change by adding an empty setter

To avoid getter/setter functions , we can simply define these constants in constructor.
e.g.
constructor(){
this._minX= 0;
this._maxX = 100;
}

Properties are meant to be set/get by consumers to change the component's behaviour.

coral-component-colorpicker/src/scripts/ColorArea.js

Pareesh · 2021-07-22T07:06:38Z

coral-component-colorpicker/src/scripts/ColorArea.js

+     }
+     if(this.y !== y) {
+       this.y = y;
+       this.trigger('change');


We will be calling a change event twice in case both value change. If possible can we trigger only one change event, when both or one of the values get changed

Pareesh · 2021-07-22T10:38:18Z

coral-component-colorpicker/src/scripts/ColorUtil.js

+   The color string
+   */
+  extractHsv(colorString) {
+    const exp = /^hsva?\((\d{1,3}),\s*(\d{1,3}%),\s*(\d{1,3}%)/;


Please add a single line comment explaining the regex, would be useful for future. Please do it for all regex as well.
Thank you

Pareesh

I didn't tried this visually apart from that LGTM.

indra2gurjar · 2021-07-23T03:05:53Z

@Pareesh You can try this here - http://indra-imac.corp.adobe.com:9001/examples/

indra2gurjar · 2021-08-11T06:28:46Z

@majornista Could you please review this for accessibility?
This can be tried at - http://indra-imac.corp.adobe.com:9001/examples/

1. ColorArea is currently just for Saturation and Brightness/Value. With luminosity, 50% is fully saturated, 100% is white and 0% is black, so its important to be clear about which values the slider controls. 2. We should use `aria-valuetext` to report the value of each slider. 3. The ColorPicker overlay should have aria-live="off", otherwise the value change to the text input announces assertively instead of the value of the slider that changed. 4. I added a degree symbol to the aria-valuetext of the Hue slider.

majornista

I opened a PR with some suggested changes for accessibility: indra2gurjar#2

fix(adobe#179): Suggested changes to ColorArea to improve accessibility

majornista · 2021-08-16T14:05:18Z

FWIW, I have a PR open for React-Spectrum that implements a ColorArea with support for HSL and RGB in addition to HSV: adobe/react-spectrum#1732. See: https://reactspectrum.blob.core.windows.net/reactspectrum/d02b605cb71505dbf074eb9f3b54c9174538bc8c/storybook/index.html?path=/story/colorarea--default-hsb-xchannel-saturation-ychannel-brightness for an example.

coral-component-colorpicker/src/scripts/ColorPicker.js

add translation provided by translation team.

@trivial

* Add a new Coral Color Picker component. * dependency at tiny * Fix axe * lint * add package-lock * update examples * @trivial * Fixed * Add formats support. * Tests ColorArea * fix(#179): Suggested changes to ColorArea to improve accessibility 1. ColorArea is currently just for Saturation and Brightness/Value. With luminosity, 50% is fully saturated, 100% is white and 0% is black, so its important to be clear about which values the slider controls. 2. We should use `aria-valuetext` to report the value of each slider. 3. The ColorPicker overlay should have aria-live="off", otherwise the value change to the text input announces assertively instead of the value of the slider that changed. 4. I added a degree symbol to the aria-valuetext of the Hue slider. * Tests ColorSliderHue * Test ColorProperties. * test * Test ColorPicker * coverage * large style * support large * Add 1 second debounce for change event. * cleanup * add translation provided by translation team. Co-authored-by: igurjar <igurjar@adobe.com> Co-authored-by: Michael Jordan <mijordan@adobe.com> Co-authored-by: Mohit Arora <mohiaror@adobe.com>

# [4.12.0](v4.11.1...v4.12.0) (2021-09-16) ### Features * **ColorPicker:** Add a new Coral Color Picker component. ([#179](#179)) ([eb1b4d6](eb1b4d6))

github-actions · 2021-09-16T16:28:40Z

🎉 This issue has been resolved in version 4.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

indra2gurjar requested review from icaraps and Pareesh July 4, 2021 00:55

indra2gurjar force-pushed the color-area branch from 3d0ec95 to c8b1d7e Compare July 4, 2021 01:12

Pareesh reviewed Jul 20, 2021

View reviewed changes

coral-component-colorpicker/src/scripts/ColorArea.js Show resolved Hide resolved

Pareesh requested review from majornista and ShreeSub July 20, 2021 05:14

Pareesh reviewed Jul 22, 2021

View reviewed changes

Pareesh approved these changes Jul 22, 2021

View reviewed changes

igurjar added 9 commits August 4, 2021 14:15

Add a new Coral Color Picker component.

ddb487b

dependency at tiny

bb8a38d

Fix axe

f8fec30

lint

efdbbe8

add package-lock

7dc2104

update examples

0f1d541

@trivial

0aa8b1d

Fixed

01023f4

Add formats support.

b55788a

indra2gurjar force-pushed the color-area branch from c9c4ddf to b55788a Compare August 4, 2021 08:45

Tests ColorArea

03e9f43

indra2gurjar force-pushed the color-area branch from 192a7f9 to 03e9f43 Compare August 11, 2021 17:11

majornista mentioned this pull request Aug 12, 2021

fix(#179): Suggested changes to ColorArea to improve accessibility indra2gurjar/coral-spectrum#2

Merged

6 tasks

majornista requested changes Aug 12, 2021

View reviewed changes

indra2gurjar and others added 2 commits August 13, 2021 10:24

Merge pull request #2 from majornista/color-area

d2c4c00

fix(adobe#179): Suggested changes to ColorArea to improve accessibility

Tests ColorSliderHue

c99853d

Test ColorProperties.

f6fd288

majornista approved these changes Aug 16, 2021

View reviewed changes

igurjar added 7 commits August 17, 2021 12:51

test

9779433

Test ColorPicker

b558633

coverage

df34d1f

large style

982cb69

support large

65dd3a0

Add 1 second debounce for change event.

3aaf299

cleanup

e9be80c

mohiaror reviewed Sep 12, 2021

View reviewed changes

coral-component-colorpicker/src/scripts/ColorPicker.js Show resolved Hide resolved

mohiaror and others added 2 commits September 16, 2021 03:27

add translation provided by translation team.

f322ecb

Merge pull request #3 from mohiaror/color-area

e91a5e5

add translation provided by translation team.

majornista merged commit 7b271c3 into adobe:master Sep 16, 2021

majornista mentioned this pull request Sep 16, 2021

feat(ColorPicker): Add a new Coral Color Picker component. (#179) #186

Merged

6 tasks

github-actions bot pushed a commit that referenced this pull request Sep 16, 2021

chore(release): 4.12.0 [skip ci]

78e6554

# [4.12.0](v4.11.1...v4.12.0) (2021-09-16) ### Features * **ColorPicker:** Add a new Coral Color Picker component. ([#179](#179)) ([eb1b4d6](eb1b4d6))

github-actions bot added the released label Sep 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new Coral Color Picker component. #179

Add a new Coral Color Picker component. #179

indra2gurjar commented Jul 3, 2021 •

edited

indra2gurjar commented Jul 15, 2021

Pareesh Jul 20, 2021

indra2gurjar Jul 21, 2021

Pareesh Jul 22, 2021 •

edited

indra2gurjar Jul 23, 2021

Pareesh Jul 22, 2021 •

edited

Pareesh Jul 22, 2021 •

edited

Pareesh left a comment

indra2gurjar commented Jul 23, 2021

indra2gurjar commented Aug 11, 2021

majornista left a comment

majornista commented Aug 16, 2021

github-actions bot commented Sep 16, 2021

Add a new Coral Color Picker component. #179

Add a new Coral Color Picker component. #179

Conversation

indra2gurjar commented Jul 3, 2021 • edited

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

indra2gurjar commented Jul 15, 2021

Pareesh Jul 20, 2021

Choose a reason for hiding this comment

indra2gurjar Jul 21, 2021

Choose a reason for hiding this comment

Pareesh Jul 22, 2021 • edited

Choose a reason for hiding this comment

indra2gurjar Jul 23, 2021

Choose a reason for hiding this comment

Pareesh Jul 22, 2021 • edited

Choose a reason for hiding this comment

Pareesh Jul 22, 2021 • edited

Choose a reason for hiding this comment

Pareesh left a comment

Choose a reason for hiding this comment

indra2gurjar commented Jul 23, 2021

indra2gurjar commented Aug 11, 2021

majornista left a comment

Choose a reason for hiding this comment

majornista commented Aug 16, 2021

github-actions bot commented Sep 16, 2021

indra2gurjar commented Jul 3, 2021 •

edited

Pareesh Jul 22, 2021 •

edited

Pareesh Jul 22, 2021 •

edited

Pareesh Jul 22, 2021 •

edited