-
Couldn't load subscription status.
- Fork 1.1k
[Explore Vis] update new color scheme for heatmap #10755
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
base: main
Are you sure you want to change the base?
[Explore Vis] update new color scheme for heatmap #10755
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10755 +/- ##
==========================================
+ Coverage 60.54% 60.55% +0.01%
==========================================
Files 4511 4511
Lines 121359 121385 +26
Branches 20266 20269 +3
==========================================
+ Hits 73473 73502 +29
+ Misses 42738 42735 -3
Partials 5148 5148
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const newSchemeColor = { | ||
| purples: '#A669E2', | ||
| oranges: '#FF4B14', | ||
| yellows: '#ff9900', | ||
| blues: darkMode ? '#006CE0' : '#003B8F', | ||
| greens: darkMode ? '#008559' : '#005237', | ||
| reds: '#EB003B', | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not hardcode color code here, it should be different for different OUI themes.
| return colors; | ||
| } | ||
|
|
||
| export const getScale = (colorSchema: ColorSchemas) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to rename this function to be more specific, like getColorRange?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, thanks for the suggestion
Signed-off-by: Qxisylolo <[email protected]>
Signed-off-by: Qxisylolo <[email protected]>
Signed-off-by: Qxisylolo <[email protected]>
Signed-off-by: Qxisylolo <[email protected]>
Signed-off-by: Qxisylolo <[email protected]>
Signed-off-by: Qxisylolo <[email protected]>
4a47a2f to
8400725
Compare
Signed-off-by: Qxisylolo <[email protected]>
| euiPaletteColorBlindColors[3], | ||
| euiPaletteColorBlindColors[9], | ||
| euiPaletteColorBlindColors[5], | ||
| euiPaletteColorBlindColors[1], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this duplicate color intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in the new theme categories, we’ve defined two types of blue: purple blue and regular blue. But euiPaletteColorBlindColors only includes one blue, I duplicated it to cover both cases.
Description
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration