Color Picker - Gradient Editing

[jauyong]

As a user I want to be able to set color gradients

This ticket is a followup to #106

This ticket has a followup in #403 which deals with visually displaying and manipulating the gradient shape and color stops on the selected element/page background.

This leaves this ticket dealing with only the color picker and gradient manipulation inside the color picker.

Feature brief

For this we need a way to store a pattern in data, where the Pattern can be either opaque or some gradient.

After a lot of discussion with @swissspidy and @dvoytenko, we've decided to use a data format throughout the application, but to make sure it's as compact as possible for the common case of just an opaque color. This data format will then be assigned to anything that can have a user-configurable color (whether or not it can be a gradient or not) and passed to the color picker.

Helper functions will let components get the background CSS value from a Pattern object.

Format

Percentage: float(0..1)
Dimension: int(0..255)
Color: object(
	r: Dimension,
	g: Dimension,
	b: Dimension,
	a: Percentage(default: 1),
)
Stop: object(
	color: Color,
	stop: Percentage,
)
PatternType: string('solid','linear','radial','conic')
Pattern: object(
	type: PatternType(default: 'solid'),
        // the following attribute is only used for type=opaque
	color: Color,
        // the following attribute is only used for type=linear,radial,conic
	stops: array(Stop),
        // the following attribute is only used for type=linear,conic
	rotation: Percentage(default: 0),
        // the following attribute is only used for type=radial,conic
	center: array(Percentage)[2](default: [.5,.5]),
        // the following attribute is only used for type=radial
	size: array(Percentage)[2](default: [.5,.5]),
)

Examples:

// transparent
e.background = null;
e.background = {
  type: 'transparent',
};
// converted to a CSS property:
//   background-color: transparent;

// pure blue - note that the type is excluded as 'solid' is the default
e.background = {
  color: {r:0,g:0,b:255},
};
// converted to a CSS property:
//   background-color: #00f;

// simple linear gradient from red to blue
e.background = {
  type:'linear',
  stops: [
    {color:{r:255,g:0,b:0}, stop: 0},
    {color:{r:0,g:0,b:255}, stop: 1}
  ],
};
// converted to a CSS property:
//   background-image: linear-gradient(#f00, #00f);

// conic gradient with 3 stops starting at .2 rotation (72 degrees)
e.background = {
  type:'conic',
  stops: [
    {color:{r:255,g:0,b:0},stop:.1},
    {color:{r:0,g:255,b:0},stop:.3},
    {color:{r:0,g:0,b:255}, stop: 1}
  ],
  rotation: .2,
};
// converted to a CSS property:
//   background-image: conic-gradient(from .2turn, #f00 .1turn, #0f0 .3turn, #00f);

// elongated radial gradient in the top left corner with 4 stops starting at transparent
e.background = {
  type:'radial',
  stops: [
    {color:{r:255,g:0,b:0,a:0},stop:.1},
    {color:{r:255,g:0,b:0},stop:.3},
    {color:{r:0,g:255,b:0},stop:.3},
    {color:{r:0,g:0,b:255}, stop: 1},
  ],
  size:[.5,.2],
  center:[0,0],
};
// converted to a CSS property:
//   background-image: radial-gradient(ellipse 50% 20% at 0% 0%, rgba(255,0,0,0) 10%, #f00 30%, #0f0 30%, #00f 100%);

Todo

Before considering adding gradient support to the color picker:

• Create format in JSDoc (including gradients)
• Create helper functions converting to CSS (including gradients)
• Modify existing color picker to use this input and output format
• Configure color picker to allow transparent and/or gradient where applicable
• Create migration for all existing colors to this format
• Replace any color selector with color picker in design panels
• Use helper functions in display and output components

Then implementing gradients is "simply" adding the desired functionality to the color picker and the rest of the functionality should follow:

• Create internal reducer
• Unit test reducer
• Add color type picker (solid/linear/radial/conic)
• Add new gradient stop component
• Add stop reverse button
• Add gradient 90° rotation button

Acceptance Criteria

1. The user is able to set linear color gradients
2. The user is able to set radial color gradients
3. The user is able to set color stops
4. The user is able to reverse color stops
5. The user is able to rotate the gradient direction 90 degrees clockwise

Design**

[barklund]

@jauyong @kmyram, I've upgraded this to an L based on the above brief - it included a bit more complexity than expected due to the required format definition and data migration.

[dvoytenko]

Thanks for the IB @barklund. Trying to catch up on this and it feels like a sizable departure from the current state. Basically, we no longer only care about backgroundColor. Instead, this is background property. Or the type is Pattern as you are suggesting. Your proposal is thus makes sense. I don't quite understand though why we still have color+opacity in Figma though:

@samitron7 can you confirm that the opacity would still be there, even though it doesn't apply in many cases. Or are we planning to hide opacity for non-opaque patterns?

Re: Pattern type. I think the proposal is solid. Couple of notes:

1. type: 'transparent' could be presented as background == null or even color: rgba(0, 0, 0, 0).
2. I don't know if we're winning anything by encoding a color as {r:0,g:0,b:255,a:1}. I think a string is just as good here.
3. I did a quick search and saw a couple of CSS gradient value parsers, so in theory we could pack it in a string as well. But that might be diminishing returns.
4. Nit: maybe "solid" would be better than "opaque" since we use opaque word a lot in different contexts.

[barklund]

1. type: 'transparent' could be presented as background == null or even color: rgba(0, 0, 0, 0).

Yeah, that's probably the best choice. I was a bit worried about some fields have null mean transparent and others having null mean "default color" (e.g. page background defaulting to white), but we can just properly default all variables and only allow null for values, where transparent is an option (e.g. text field background).

2. I don't know if we're winning anything by encoding a color as {r:0,g:0,b:255,a:1}. I think a string is just as good here.

It's just because the color picker internally use this split, that it made sense to also store it this way. But sure, a string is more compact, however we'd need to unpack it every time the color picker is opened.

I'm good with either, it's a cheap calculation either way.

3. I did a quick search and saw a couple of CSS gradient value parsers, so in theory we could pack it in a string as well. But that might be diminishing returns.

That's definitely also an option, but then this intermediate format would still be used - just only inside the color picker. It would make migration easier and would make repeated renders a lot faster. Again, I'm good with either.

4. Nit: maybe "solid" would be better than "opaque" since we use opaque word a lot in different contexts.

We do? I have no strong feeling here.

[barklund]

Hm, thinking it over, a string based strategy is probably much simpler and safer to do. The only trick is knowing how to apply the color to CSS in a given case when you just have a the string, but a small helper could do the trick then - applying the color string to either background-color, background-image, color or even stroke or fill as necessary. And null always means transparent.

Then the whole complex object and state, etc, is only necessary inside the color picker component and will be calculated from the input string and updated internally. Then it makes sense to do it as a reducer, having actions such as updateFromString, setSolid/setRadial/setLinear/setConic, addStop, setCurrentStop, setCurrentColor, etc. And the reducer then always resolves to a set of variables mimicking the above previous brief (type, color, stops, angle, center, size), a single internal state (currentStop) plus the generated color string as well (to be sent to onChange callback).

I'll rewrite the above brief to reflect this idea in stead.

[dvoytenko]

Regardless whether you find it easier or not to use a single string for the whole type, I'd still say there would be benefit from having a string color. I feel that sooner or later we might get into hsla() and such.

[samitron7]

Hi everyone involved in working on colors. Just note the following:

1. We need a global slider to allow the user to change the opacity of an element
2. But in certain situations we also additionally need to add opacity that is attached to the color swatches in the design panel (ie possibly text and textbox)

I'm working on this now and will have it by monday

[dvoytenko]

Do we really need an additional color opacity number outsider of the color picker?

…

On Fri, Feb 28, 2020, 2:54 PM samitron7 ***@***.***> wrote: Hi everyone involved in working on colors. Just note the following: 1. We need a global slider to allow the user to change the opacity of an element 2. But in certain situations we also additionally need to add opacity that is attached to the color swatches in the design panel (ie possibly text and textbox) I'm working on this now and will have it by monday — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#263>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFRIIO5JULMWOZ4E3E7AHLRFGIZ5ANCNFSM4KRU5EJQ> .

[dvoytenko]

@barklund One additional confirmation. It looks like we also want a separate opacity component for gradient as well. We can model it whichever way it's convenient for us. It works as a opacity multiplier for each color-step.

In other words:

• type=solid: rgba
• type=gradient: gradientDefinition + colorOpacityMultipler

When outputting, for now we could, I think, use opacity: ${colorOpacityMultiplier}. However, if eventually we get new features that involve DOM element nesting, such use of opacity will be incorrect and we'd have to multiply colors in the gradient steps. So, perhaps, it'd be best for us to do the color opacity multiplication right away.

Does it make our string-only idea harder?