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 EuiColorPalettePicker #3192

Merged
merged 59 commits into from
Jun 2, 2020
Merged

Conversation

elizabetdev
Copy link
Contributor

@elizabetdev elizabetdev commented Mar 30, 2020

Summary

Closes #2795

This PR adds a Color Palette Picker component called EuiColorPalettePicker.

While working on this PR I tried to address a few suggestions:

  • The palette type gradient should be an array of objects [{stop: 100, color: '#54B399'}] rather than a CSS linear-gradient
  • The fixed palette should be a gradient rather than different flex items with a background. The reason for changing this is that the flex items create a small empty space on the right side. Also the box-shadow: inset doesn't work with the flex items.
  • Custom options should follow the pattern of the EuiSuperSelect. I tried and the code became complex and for this reason, I created a text option. I think it is a simpler solution and also we don't lose the purpose of the palette picker

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation 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
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

1 similar comment
@kibanamachine
Copy link

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

@elizabetdev elizabetdev force-pushed the color-palette-picker branch from 203a517 to c3ae30b Compare May 12, 2020 14:36
@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@elizabetdev elizabetdev requested review from cchaos and thompsongl May 28, 2020 14:02
@kibanamachine
Copy link

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

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 LGTM! We need to get this into Kibana ASAP 😆

It looks like there's a still a couple unchecked items in the PR checklist though that you might want to cover.

@kibanamachine
Copy link

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

@elizabetdev
Copy link
Contributor Author

Thanks @cchaos, I'm covering now the unchecked items from the PR checklist and I'll be waiting for another review from @thompsongl because I changed a few TS things.

@elizabetdev
Copy link
Contributor Author

jenkins test this

@kibanamachine
Copy link

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

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're close here. I just want to solidify the EuiColorPalettePickerPaletteProps interface so it's a bit more clear to consumers how to display what they want.

@thompsongl
Copy link
Contributor

PR: elizabetdev#5

@kibanamachine
Copy link

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

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. @miukimiu will have some follow-up work on the palette display bit, which may also enable more flexibility with option types (fixed palettes with ColorStop values, for instance). For now, we're protected against error states and malformed options.

@elizabetdev elizabetdev merged commit c7795f1 into elastic:master Jun 2, 2020
daveyholler pushed a commit to daveyholler/eui that referenced this pull request Jun 3, 2020
* Adding initial code

* Adding initial code

* Adding getLinearGradient method

* Improving palettes

* initial tests

* More improvements

* Deleting palettes file

* Adding display togles

* Simplifying button

* Custom option value

* Update some types

* getFixedLinearGradient

* Text as an option

* Improving props description

* Improving sass

* Adding selectionDisplay

* More examples

* Addressing pr review

* Update src-docs/src/views/color_picker/color_palette_picker.tsx

Co-authored-by: Caroline Horn <[email protected]>

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: Caroline Horn <[email protected]>

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: Caroline Horn <[email protected]>

* Extending EuiSuperSelectProp

* Simplifying structure

* Changelog

* Improving example

* Improving snippet

* Adding more tests

* Improving props description

* Improving docs text

* Improve text

* Addressing review

* Update src/components/color_picker/utils.ts

Co-authored-by: Greg Thompson <[email protected]>

* Update src/components/color_picker/utils.ts

Co-authored-by: Greg Thompson <[email protected]>

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: Greg Thompson <[email protected]>

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: Greg Thompson <[email protected]>

* Update src/components/color_picker/utils.ts

Co-authored-by: Greg Thompson <[email protected]>

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: Greg Thompson <[email protected]>

* Improving TS

* Improving palette doc

* Palette doc

* Changing name of gray palette

* Slimming the description a bit

* removing console log

* more strict types

* Update src/components/color_picker/color_palette_picker/color_palette_picker.tsx

Co-authored-by: cchaos <[email protected]>
Co-authored-by: Caroline Horn <[email protected]>
Co-authored-by: Greg Thompson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Color Palette Picker component and widget
5 participants