[EuiProgress] Add support for more colors by andreadelrio · Pull Request #4130 · elastic/eui

andreadelrio · 2020-10-13T01:53:07Z

Summary

Added more colors to EuiProgress (tint1 to tint9), warning and success.
Updated the docs example to show available colors.

Note: Colors success and secondary use the same color, I've left secondary as an option thinking of consumers that might already be using secondary in their current implementations.

Fixes #4117 #3618

Checklist

Check against all themes for compatibility in both light and dark modes
Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
- [ ] Props have proper autodocs
Added documentation
~~- [ ] Checked Code Sandbox works for the any docs examples~~
Added or updated jest tests
~~- [ ] Checked for breaking changes and labeled appropriately~~
~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
A changelog entry exists and is marked appropriately

kibanamachine · 2020-10-13T02:19:51Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

cchaos

This is a nice expansion of the color prop to allow consumers to easily color with our visualization palette. 😍

The one thing that's not quite addressed in this PR that #3618 is asking for, is to be able to completely customize the color by passing a valid hex or rgb or other valid color type. That's where this can get complicated.

I wonder if it can be done similarly to EuiIcon, where a custom color gets applied in the inline style tag as the text color like style={{ color: customColor }} and then in the CSS we use currentColor to color the fill.

The only thing is that we couldn't then apply the makeHighContrastColor() mixin for the text color, but we could either caveat the example description with this or we do have a way to increase the contrast of a color in JS for just the text portion.

src-docs/src/views/progress/progress_size_color.js

src/components/progress/progress.tsx

src/components/progress/_variables.scss

andreadelrio · 2020-10-14T17:26:48Z

This is a nice expansion of the color prop to allow consumers to easily color with our visualization palette. 😍

The one thing that's not quite addressed in this PR that #3618 is asking for, is to be able to completely customize the color by passing a valid hex or rgb or other valid color type. That's where this can get complicated.

I wonder if it can be done similarly to EuiIcon, where a custom color gets applied in the inline style tag as the text color like style={{ color: customColor }} and then in the CSS we use currentColor to color the fill.

The only thing is that we couldn't then apply the makeHighContrastColor() mixin for the text color, but we could either caveat the example description with this or we do have a way to increase the contrast of a color in JS for just the text portion.

@cchaos I didn't know about currentColor! that's cool. I've implemented most of your suggestions. The one bit missing is applying makeHighContrastColor() to valueText when they pass a custom color. You mention there's a way to do that in JS? can you point me to an example?

kibanamachine · 2020-10-14T17:47:11Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

cchaos · 2020-10-14T18:31:21Z

You mention there's a way to do that in JS? can you point me to an example?

I looked through some of our components where I thought we were using it, but we don't seem to have this exact implementation of changing the actual color. So I think we could just put a callout in the docs saying that we don't do any adjustment for contrast when applying the custom color to the text.

kibanamachine · 2020-10-14T21:51:12Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

cchaos

Updates are great. Here's a code review.

Looks like we're missing tests for the color prop. You'll also want to be sure to add one specifically for the custom color as well.

CHANGELOG.md

src-docs/src/views/progress/progress_colors.js

src/components/progress/_variables.scss

src/components/progress/_progress.scss

src/components/progress/progress.tsx

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2020-10-15T20:39:11Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

… into colorful-progress

kibanamachine · 2020-10-15T23:36:58Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

kibanamachine · 2020-10-15T23:44:00Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

andreadelrio · 2020-10-16T03:02:15Z

@cchaos thanks for the review. I addressed the feedback and added tests (we were also missing one for size).

cchaos

Nice! Looks great! I tested in Chrome, FF, and Safari and they all render the proper color. 👍

@chandlerprall Want to take a quick peak at the JS side?

src-docs/src/views/progress/progress_example.js

cchaos · 2020-10-16T14:19:01Z

src/components/progress/progress.test.tsx

  });
+
+  describe('color', () => {
+    [...COLORS, '#885522'].forEach((color) => {


Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2020-10-16T17:25:19Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

cchaos · 2020-10-19T18:03:16Z

I think CI is failing on Prettier of the progress_example.js file. Might want to do a quick re-save of that file to get it formatted.

… into colorful-progress

kibanamachine · 2020-10-19T19:00:15Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

chandlerprall

Changes LGTM!

kibanamachine · 2020-10-26T17:22:22Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4130/

andreadelrio added 4 commits October 12, 2020 17:51

add more colors to EuiProgress

392cc84

Merge remote-tracking branch 'upstream/master' into colorful-progress

29359f1

cleanup

a4dd431

cl

ac76f67

andreadelrio requested a review from cchaos October 13, 2020 01:58

cchaos reviewed Oct 13, 2020

View reviewed changes

src-docs/src/views/progress/progress_size_color.js Outdated Show resolved Hide resolved

src/components/progress/progress.tsx Outdated Show resolved Hide resolved

src/components/progress/_variables.scss Show resolved Hide resolved

andreadelrio added 2 commits October 14, 2020 10:00

allow custom colors

9c0f068

valueText support for custom colors

9400abb

add callout

2dfa8ac

andreadelrio requested a review from cchaos October 14, 2020 21:38

update prop description and renamed tint to vis

9d99749

cchaos suggested changes Oct 15, 2020

View reviewed changes

Update CHANGELOG.md

664f927

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

andreadelrio added 4 commits October 15, 2020 16:08

pr feedback

25dffee

using keysOf

e671dbb

Merge branch 'colorful-progress' of https://github.com/andreadelrio/eui…

ff0436e

… into colorful-progress

cleanup

42b0b9d

andreadelrio requested a review from cchaos October 16, 2020 03:02

cchaos approved these changes Oct 16, 2020

View reviewed changes

cchaos requested a review from chandlerprall October 16, 2020 14:56

Update src-docs/src/views/progress/progress_example.js

02186fe

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

andreadelrio added 4 commits October 19, 2020 11:19

Merge remote-tracking branch 'upstream/master' into colorful-progress

ded2ad6

update cl

0865e16

Merge branch 'colorful-progress' of https://github.com/andreadelrio/eui…

3a54b15

… into colorful-progress

example

1e4de17

chandlerprall approved these changes Oct 26, 2020

View reviewed changes

andreadelrio added 2 commits October 26, 2020 09:53

Merge remote-tracking branch 'upstream/master' into colorful-progress

625c7ec

update cl

cf10add

andreadelrio merged commit 7857f26 into elastic:master Oct 26, 2020

andreadelrio mentioned this pull request Oct 26, 2020

[EuiProgress] Allow for customization of colors #3618

Closed

thompsongl mentioned this pull request Oct 27, 2020

Upgrade EUI to v30.1.1 elastic/kibana#81499

Merged

Conversation

andreadelrio commented Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Oct 13, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andreadelrio commented Oct 14, 2020

Uh oh!

kibanamachine commented Oct 14, 2020

Uh oh!

cchaos commented Oct 14, 2020

Uh oh!

kibanamachine commented Oct 14, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Oct 15, 2020

Uh oh!

kibanamachine commented Oct 15, 2020

Uh oh!

kibanamachine commented Oct 15, 2020

Uh oh!

andreadelrio commented Oct 16, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cchaos Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Oct 16, 2020

Uh oh!

cchaos commented Oct 19, 2020

Uh oh!

kibanamachine commented Oct 19, 2020

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Oct 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andreadelrio commented Oct 13, 2020 •

edited

Loading