Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: Preset gradient functionality button block #17169

Open
wants to merge 4 commits into
base: update/extact-circular-option-picker-internal-component-containing-color-palette-design
from

Conversation

@jorgefilipecosta
Copy link
Member

commented Aug 23, 2019

Description

Implements: #16482
Related: #16662

This PR adds a set of Gradient related functionality and components and uses them to implement gradient functionality in the button block.
For now, we still don't have a custom gradient functionality we just have gradient presets.

Themes will be able to configure gradient presets. But we need a default set of gradients when themes did not pass their own set. I think this set is not very creative and we should look into having an awesome set :)
cc: @mapk, @kjellr

How has this been tested?

I added the button block. I used the gradient panel and I verified the preset gradients work as expected.

Screenshots

image

@jorgefilipecosta jorgefilipecosta force-pushed the update/extact-circular-option-picker-internal-component-containing-color-palette-design branch from efcc261 to 71e4eb4 Aug 23, 2019

@senadir

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2019

🎉🎉🎉🎉
Will test today & report back, thank you so much for working on this!

@senadir

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

as per today's editor chat, there has been a discussion of whether we should use classes for gradients or directly inline them.

in my point of view, I think classes are the best, most compatible solution in case of updating & or changing the visual identity

@kjellr

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Regarding the default set of gradients: I think the main use case for this will actually be outside of the button block: for black(or white)-to-transparent gradients above images in the Cover block. If we plan on adding this functionality to that block, we should include those in this default set too.

For color ones, I think we should just choose a few subtle ones that are based on the default set of colors we use. So here's an example set:

  • Pale Pink to Vivid Red
  • Pale Cyan Blue to Vivid Cyan Blue
  • Light Green Cyan to Vivid Green Cyan
  • Black to White
  • Black to Transparent
  • White to Transparent

... each with a left-to-right and top-to-bottom variant. That would result in 12 swatches, which is coincidentally the same number that we include by default for colors today.

@kjellr

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Also yes, I think these should follow the standard set by colors: so these should be defined by classnames.

If we allow for users to create their own custom gradients later down the road, those would be inlined, just like the custom colors are today.

@mapk

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

I think this set is not very creative and we should look into having an awesome set :)

In addition to some of Kjell's example above, I'd like to see a cool gradient and warm gradient if it's not too much. I can see these gradients being used as backgrounds to the Group block and Cover block.

Cool

  • Vivid Green Cyan (# 00d084) to Vivid Cyan Blue (# 0693e3)

Warm

  • Luminous Vivid Amber (# fcb900) to Luminous Vivid Orange (# ff6900)

Screen Shot 2019-08-28 at 4 45 16 PM

as per today's editor chat, there has been a discussion of whether we should use classes for gradients or directly inline them.
in my point of view, I think classes are the best, most compatible solution in case of updating & or changing the visual identity

I'd also agree that classnames would be wonderful here. 👍

@jorgefilipecosta jorgefilipecosta force-pushed the update/extact-circular-option-picker-internal-component-containing-color-palette-design branch from 71e4eb4 to 914631f Aug 29, 2019

@jorgefilipecosta jorgefilipecosta force-pushed the add/preset-gradient-functionality-button-block branch from ff41c4b to 0b4f447 Aug 29, 2019

@jorgefilipecosta jorgefilipecosta force-pushed the update/extact-circular-option-picker-internal-component-containing-color-palette-design branch 3 times, most recently from ccb0f00 to 89d00c5 Aug 29, 2019

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

commented Aug 30, 2019

Hi @mapk, @kjellr, I updated the PR to use the suggested backgrounds.
We have now many predefined gradients. Should we also include some radial gradients (in that case probably we need to remove some of the current gradients)?
We have a problem right now we can not visually differentiate "Black to White" from
"Black to Transparent" as the background of the gradient picker is also white. Maybe we can try to add small png bellow as background with gray and white squares to indicate transparency?
Regarding the class usage, I update the attribute to be "customGradient" so right now we will always set custom values. This change allows us to quickly right after adding the classes mechanism. I'm not adding it right now because this PR is already huge and if I increase its size, even more, it becomes impossible to review.
This PR depends on #17154 which is pure code refactor I would like a review in that PR first so we can advance with this one :)

@kjellr

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

Hi @mapk, @kjellr, I updated the PR to use the suggested backgrounds.

I think I'm actually still seeing the old colors here?

Screen Shot 2019-09-04 at 1 42 14 PM

We have a problem right now we can not visually differentiate "Black to White" from
"Black to Transparent" as the background of the gradient picker is also white. Maybe we can try to add small png bellow as background with gray and white squares to indicate transparency?

Yeah, that makes sense. Something like this:

Frame 3

We could draw that grid as an SVG behind the gradient. Here's a sample SVG to try.

Trying this out, I'm wondering if it actually makes sense to have these exist in their own panel in the first place? Since these gradients are colors, I'd expect to find them appended to the rest of the color swatches:

Frame 4

@jorgefilipecosta jorgefilipecosta force-pushed the add/preset-gradient-functionality-button-block branch from 0b4f447 to 3256474 Sep 6, 2019

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2019

Hi @kjellr,

I think I'm still seeing the old colors here?

Sorry, I missed to push my update it should be fixed now.

We have a problem right now we can not visually differentiate "Black to White" from
"Black to Transparent" as the background of the gradient picker is also white. Maybe we can try to add small png bellow as background with gray and white squares to indicate transparency?

Yeah, that makes sense. Something like this:
Frame 3

I will try to add this transparency mark.

Trying this out, I'm wondering if it actually makes sense to have these exist in their own panel in the first place? Since these gradients are colors, I'd expect to find them appended to the rest of the color swatches:

Soon we will have a custom gradients functionality I opted for a separate panel to not make a single panel very huge.
We could open the custom gradients panel in a Popover (although none of the designs proposed suggests that), but if we open in a popover we will need a Custom gradient button and custom color button and I'm not sure if having the two buttons is intuitive.
Having the colors and gradients together means 24 color options by default, I guess it may be too many options for a single panel.

@mapk

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

I updated the PR to use the suggested backgrounds.

Looking through, I only see a few. Are we missing the gradients I had suggested earlier?

Here's what I'm seeing right now. I notice the color white by itself is included there. And two of the Black-to-white gradients are duplicated.

btn-grads

@jorgefilipecosta jorgefilipecosta force-pushed the add/preset-gradient-functionality-button-block branch from 3256474 to 6127f7e Sep 7, 2019

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2019

Looking through, I only see a few. Are we missing the gradients I had suggested earlier?

Hi @mapk the gradients you suggested Vivid Green Cyan (# 00d084) to Vivid Cyan Blue (# 0693e3) and Luminous Vivid Amber (# fcb900) to Luminous Vivid Orange (# ff6900) was there, but I wrongly set one of the transparency values the problem was fixed.

Screenshot 2019-09-07 at 18 40 02

Screenshot 2019-09-07 at 18 40 09

Here's what I'm seeing right now. I notice the color white by itself is included there. And two of the Black-to-white gradients are duplicated.

Hi @mapk the color white is White to Transparent, and one of the Black-to-white gradients is, in fact, a black to transparent. Both white and transparent were suggested as noted before we will need a way to indicate transparency (the typical white and dark squares). I hope to add this functionality very soon.
For now, to test if it gradients work well, we can nest the button inside a group and change the background color of the group.

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

commented Sep 7, 2019

The transparency indication was added now we can differentiate between white and transparent gradients:
Screenshot 2019-09-07 at 19 43 19

Props to @kjellr for providing the transparency SVG!

@kjellr

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2019

Thank you, @jorgefilipecosta! This is looking great. A few things to note:

The transparency SVG is looking great. I'm seeing it applied to selected colors, even when they aren't transparent though:

transparent


Seeing these in action, I think we can trim out the Light Green Cyan to Vivid Green Cyan color swatches. Those are really subtle, and it's hard to tell they're gradients at a glance.


The White to Transparent options need a little adjusting so that their base color is white the whole time. Something like linear-gradient(90deg, rgba(255, 255, 255, 1) 0%, rgba(255, 255, 255, 0) 100%), with a transparent background color.

Current:
Screen Shot 2019-09-09 at 1 47 27 PM

Should be:
Screen Shot 2019-09-09 at 1 48 26 PM


Also, I wonder if we can reorder some of the items, so that similar gradients are right next to each other?

Current:

Screen Shot 2019-09-09 at 1 49 35 PM

Suggestion:

Screen Shot 2019-09-09 at 1 51 21 PM

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

Hi @kjellr, I applied all the suggested improvements for the default gradients. Regarding the transparency problem on the active option, I spent some time trying to find a solution that did not evolve a significant refactor of the styles in this component and unfortunately, I did not found a "decent" solution. I will try to look into this problem again tomorrow with fresh eyes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.