-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution][threat hunting explore] EUI refresh: Remove custom color hex #204631
Conversation
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.
The change in getSeverityColor
logic will change the color palette in Borealis quite drastically (medium severity will become pink for example). I don't think that's how we should handle the migration. We should rely on the new severity palette elastic/eui#8247 and decide what to do about the missing green color in this palette. Either continue using euiColorVis0
or accept our low severity alerts/vulnerabilities to be in blue, as "info" severity signals
export const SEVERITY_COLOR = { | ||
critical: '#E7664C', | ||
high: '#DA8B45', | ||
medium: '#D6BF57', | ||
low: '#54B399', | ||
} as const; | ||
|
||
export const getSeverityColor = (euiTheme: EuiThemeComputed) => | ||
({ | ||
critical: euiTheme.colors.vis.euiColorVis9, |
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.
The Borealis data viz color palette changed, see https://docs.google.com/document/d/1IAKbasq1nDfqd2IU3KdP8cwD3uCCAwkIekKRq7zgyWg/edit?tab=t.0#heading=h.lgyarxm75dlu , so that vis9 is not red but yellow and vis5 is not yellow but pink. These colors don't work for severity
There is Severity Palette introduced by the EUI team #203387 , but it is missing green color, which is currently how our low severity alerts/vulnerabilities are being show. There is a discussion with Security solution on how to go about it, see https://elastic.slack.com/archives/C060SDYTZ96/p1734636026436679
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.
Thanks @maxcold , I could find that we are adding more euiColorSeverity
color for different severity levels. This PR might have to wait until those colors becomes available. Have we got a clear idea of which euiColorSeverity
token should be used for critical, high, medium and low?
I thought this (Security/Vulnerability) was the mapping, but they were not in the list of euiColorSeverity
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.
@angorayc the team is moving from the variety of colors illustrated by your screenshot to a unified palette available here elastic/eui#8247 , but I haven't seen exact colors defined for Security severity levels from this palette. If I understand correctly, it's what @codearos is doing currently with the design team
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.
Given that we are still waiting for the severity colors, I have dropped the changes for severity color in this PR. Will do that in a separate PR when the decision is made.
export const SEVERITY_COLOR = { | ||
critical: '#E7664C', | ||
high: '#DA8B45', | ||
medium: '#D6BF57', | ||
low: '#54B399', | ||
} as const; | ||
|
||
export const getSeverityColor = (euiTheme: EuiThemeComputed) => |
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 see that @CAWilson94 is doing a similar work here, so I wonder if there would be a value in extracting this logic into a more generic function/hook that could be moved into our common folder.
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.
@PhilippeOberti @CAWilson94
It seems that the new severity colors are still pending for decision, do you think it's worthwhile if I revert the severity color changes in our PRs, and open another pr especially for severity color changes?
So that we can also move the getSeverityColor hook into a common color and use it from there? and it won't block changes that doesn't involved the severity colors?
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.
Yup I like the idea!
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 have dropped the changes for severity colors in this PR. Let's do that in a separate PR when the decision is made.
@@ -192,8 +191,8 @@ export const BarChartComponent: React.FC<BarChartComponentProps> = ({ | |||
const legendItems: LegendItem[] = useMemo( | |||
() => | |||
barChart != null && stackByField != null | |||
? barChart.map((d, i) => ({ | |||
color: d.color ?? (i < defaultLegendColors.length ? defaultLegendColors[i] : undefined), |
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.
defaultLegendColors
is removed as we don't want hard coded colors:
x-pack/solutions/security/plugins/security_solution/public/common/components/matrix_histogram/utils.ts
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.
packages/kbn-babel-preset/styled_components_files.js
LGTM
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.
Nice work! Thanks for all the collab on severity colours also 🚀
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.
LGTM! Thanks for this changes Angela 😎 💘
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
|
LGTM, thank you Angela! |
…m color hex (elastic#204631) ## Summary elastic#202498 elastic#202503 1. This PR does **Not** include the severity color change. It will be implemented in a follow up PR once color tokens have been decided. 2. This PR updates the type `GetLensAttributes` to accept `euiTheme`, so all the functions in this type are updated accordingly. https://github.com/elastic/kibana/pull/204631/files#diff-abe20658865cad59eadcff945552b40832d96da0264ed89ddd5ab25ded1420a3R30 ---- ## To test: Please verify if visualizations are displayed properly. ### Running Kibana with the Borealis theme In order to run Kibana with `Borealis`, you'll need to do the following: 1. Set the following in kibana.dev.yml: `uiSettings.experimental.themeSwitcherEnabled: true` 4. Run Kibana with the following environment variable set: ```KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start``` 5. This will expose a toggle under Stack Management > Advanced Settings > Theme version, which you can use to toggle between Amsterdam and Borealis. ![Image](https://github.com/user-attachments/assets/78d64946-43fc-4400-bbb1-229d900b7f05) ---- ### Explore <img width="2557" alt="host_after" src="https://github.com/user-attachments/assets/f69b6e2a-58f6-4ed4-9f38-dcdbcf9919ed" /> <img width="1281" alt="users_after" src="https://github.com/user-attachments/assets/0eec8e9c-e678-4a66-83ee-4b8d11380b8e" /> <img width="2552" alt="network_dns" src="https://github.com/user-attachments/assets/19f06d2a-6e51-419b-9f89-233bfa5727ba" /> <img width="2557" alt="network_after" src="https://github.com/user-attachments/assets/3b90c5e2-23a1-4f15-a2d0-f9290a39af30" /> ---- ### Dashboards <img width="2557" alt="dashboard_overview" src="https://github.com/user-attachments/assets/c1873359-fee9-42c6-addd-fe2bc1c98aee" /> <img width="2558" alt="dashboard_detection_response_2" src="https://github.com/user-attachments/assets/f6853710-0938-402b-b326-fa00d586b5d6" /> <img width="2559" alt="dashboard_detection_response_1" src="https://github.com/user-attachments/assets/4eb75526-9a57-46e9-b090-b53353956ea1" /> ---- ### Alerts <img width="2555" alt="alerts_chart_collapsed" src="https://github.com/user-attachments/assets/6ecf5dd5-a785-4701-900b-0454f024b36d" /> <img width="2554" alt="summary" src="https://github.com/user-attachments/assets/1731a6ea-ef2b-4d7d-bf21-4041e59f0ad4" /> <img width="2559" alt="trend" src="https://github.com/user-attachments/assets/b9a741d1-a359-4273-9555-850cdcbc8932" /> <img width="2557" alt="counts" src="https://github.com/user-attachments/assets/a6193ccc-86b8-4974-ad9f-9417e200e859" /> <img width="1281" alt="treemap" src="https://github.com/user-attachments/assets/7b6e163a-a660-4bb1-a6de-88e21934b98a" /> ---- ### Rules preview <img width="2556" alt="Screenshot 2024-12-18 at 13 45 33" src="https://github.com/user-attachments/assets/47099c18-86ee-455a-a5af-ebd6a29904a5" /> ---- --------- Co-authored-by: kibanamachine <[email protected]>
## Summary Previous changes applied same tokens for Borealis and Amsterdam: #204631 (comment) PR above causes color changes to the current theme, after discussing with UX, we decide to maintain different color tokens until Borealis is launched. This PR should revert the color changed on Amsterdam by the previous PR and only apply the new color for Borealis. | Current and Amsterdam | Borealis | |-------------------------|----------| |Source: Hard coded: `#d36186`|Source: `euiColorVis4` - `#EE72A6` | |Dest: Hard coded: `#9170b8` |Dest: `euiColorVis2` - `#61A2FF`| ### Host IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2560" alt="host_IPs" src="https://github.com/user-attachments/assets/c0f9f317-fb02-4c96-8422-c1d2484f4636" />|<img width="2560" alt="host_bor_light" src="https://github.com/user-attachments/assets/451d6604-1d7c-4a2e-82c3-74b2499852d2" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/ac45a3ac-ecaf-46b6-91d9-68704d8639ee" />|<img width="2553" alt="host_bor_dark" src="https://github.com/user-attachments/assets/e34e56fd-8202-4a3c-80c1-996718320fd8" />| ### Network IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2557" alt="network_IPs" src="https://github.com/user-attachments/assets/0e0b33d9-55b7-41a5-8910-11b80e539398" />|<img width="2559" alt="network_bor_light" src="https://github.com/user-attachments/assets/f616b3ab-5032-4e69-b67e-cde39b88ea5f" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/9613a49f-f0c6-4b63-aa56-c960fac175fc" />|<img width="2560" alt="network_bor_dark" src="https://github.com/user-attachments/assets/911f0509-43ea-428a-94d6-9ce01f5425ac" />| ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…m color hex (elastic#204631) ## Summary elastic#202498 elastic#202503 1. This PR does **Not** include the severity color change. It will be implemented in a follow up PR once color tokens have been decided. 2. This PR updates the type `GetLensAttributes` to accept `euiTheme`, so all the functions in this type are updated accordingly. https://github.com/elastic/kibana/pull/204631/files#diff-abe20658865cad59eadcff945552b40832d96da0264ed89ddd5ab25ded1420a3R30 ---- ## To test: Please verify if visualizations are displayed properly. ### Running Kibana with the Borealis theme In order to run Kibana with `Borealis`, you'll need to do the following: 1. Set the following in kibana.dev.yml: `uiSettings.experimental.themeSwitcherEnabled: true` 4. Run Kibana with the following environment variable set: ```KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start``` 5. This will expose a toggle under Stack Management > Advanced Settings > Theme version, which you can use to toggle between Amsterdam and Borealis. ![Image](https://github.com/user-attachments/assets/78d64946-43fc-4400-bbb1-229d900b7f05) ---- ### Explore <img width="2557" alt="host_after" src="https://github.com/user-attachments/assets/f69b6e2a-58f6-4ed4-9f38-dcdbcf9919ed" /> <img width="1281" alt="users_after" src="https://github.com/user-attachments/assets/0eec8e9c-e678-4a66-83ee-4b8d11380b8e" /> <img width="2552" alt="network_dns" src="https://github.com/user-attachments/assets/19f06d2a-6e51-419b-9f89-233bfa5727ba" /> <img width="2557" alt="network_after" src="https://github.com/user-attachments/assets/3b90c5e2-23a1-4f15-a2d0-f9290a39af30" /> ---- ### Dashboards <img width="2557" alt="dashboard_overview" src="https://github.com/user-attachments/assets/c1873359-fee9-42c6-addd-fe2bc1c98aee" /> <img width="2558" alt="dashboard_detection_response_2" src="https://github.com/user-attachments/assets/f6853710-0938-402b-b326-fa00d586b5d6" /> <img width="2559" alt="dashboard_detection_response_1" src="https://github.com/user-attachments/assets/4eb75526-9a57-46e9-b090-b53353956ea1" /> ---- ### Alerts <img width="2555" alt="alerts_chart_collapsed" src="https://github.com/user-attachments/assets/6ecf5dd5-a785-4701-900b-0454f024b36d" /> <img width="2554" alt="summary" src="https://github.com/user-attachments/assets/1731a6ea-ef2b-4d7d-bf21-4041e59f0ad4" /> <img width="2559" alt="trend" src="https://github.com/user-attachments/assets/b9a741d1-a359-4273-9555-850cdcbc8932" /> <img width="2557" alt="counts" src="https://github.com/user-attachments/assets/a6193ccc-86b8-4974-ad9f-9417e200e859" /> <img width="1281" alt="treemap" src="https://github.com/user-attachments/assets/7b6e163a-a660-4bb1-a6de-88e21934b98a" /> ---- ### Rules preview <img width="2556" alt="Screenshot 2024-12-18 at 13 45 33" src="https://github.com/user-attachments/assets/47099c18-86ee-455a-a5af-ebd6a29904a5" /> ---- --------- Co-authored-by: kibanamachine <[email protected]>
…06254) ## Summary Previous changes applied same tokens for Borealis and Amsterdam: elastic#204631 (comment) PR above causes color changes to the current theme, after discussing with UX, we decide to maintain different color tokens until Borealis is launched. This PR should revert the color changed on Amsterdam by the previous PR and only apply the new color for Borealis. | Current and Amsterdam | Borealis | |-------------------------|----------| |Source: Hard coded: `#d36186`|Source: `euiColorVis4` - `#EE72A6` | |Dest: Hard coded: `#9170b8` |Dest: `euiColorVis2` - `#61A2FF`| ### Host IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2560" alt="host_IPs" src="https://github.com/user-attachments/assets/c0f9f317-fb02-4c96-8422-c1d2484f4636" />|<img width="2560" alt="host_bor_light" src="https://github.com/user-attachments/assets/451d6604-1d7c-4a2e-82c3-74b2499852d2" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/ac45a3ac-ecaf-46b6-91d9-68704d8639ee" />|<img width="2553" alt="host_bor_dark" src="https://github.com/user-attachments/assets/e34e56fd-8202-4a3c-80c1-996718320fd8" />| ### Network IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2557" alt="network_IPs" src="https://github.com/user-attachments/assets/0e0b33d9-55b7-41a5-8910-11b80e539398" />|<img width="2559" alt="network_bor_light" src="https://github.com/user-attachments/assets/f616b3ab-5032-4e69-b67e-cde39b88ea5f" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/9613a49f-f0c6-4b63-aa56-c960fac175fc" />|<img width="2560" alt="network_bor_dark" src="https://github.com/user-attachments/assets/911f0509-43ea-428a-94d6-9ce01f5425ac" />| ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
…m color hex (elastic#204631) ## Summary elastic#202498 elastic#202503 1. This PR does **Not** include the severity color change. It will be implemented in a follow up PR once color tokens have been decided. 2. This PR updates the type `GetLensAttributes` to accept `euiTheme`, so all the functions in this type are updated accordingly. https://github.com/elastic/kibana/pull/204631/files#diff-abe20658865cad59eadcff945552b40832d96da0264ed89ddd5ab25ded1420a3R30 ---- ## To test: Please verify if visualizations are displayed properly. ### Running Kibana with the Borealis theme In order to run Kibana with `Borealis`, you'll need to do the following: 1. Set the following in kibana.dev.yml: `uiSettings.experimental.themeSwitcherEnabled: true` 4. Run Kibana with the following environment variable set: ```KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start``` 5. This will expose a toggle under Stack Management > Advanced Settings > Theme version, which you can use to toggle between Amsterdam and Borealis. ![Image](https://github.com/user-attachments/assets/78d64946-43fc-4400-bbb1-229d900b7f05) ---- ### Explore <img width="2557" alt="host_after" src="https://github.com/user-attachments/assets/f69b6e2a-58f6-4ed4-9f38-dcdbcf9919ed" /> <img width="1281" alt="users_after" src="https://github.com/user-attachments/assets/0eec8e9c-e678-4a66-83ee-4b8d11380b8e" /> <img width="2552" alt="network_dns" src="https://github.com/user-attachments/assets/19f06d2a-6e51-419b-9f89-233bfa5727ba" /> <img width="2557" alt="network_after" src="https://github.com/user-attachments/assets/3b90c5e2-23a1-4f15-a2d0-f9290a39af30" /> ---- ### Dashboards <img width="2557" alt="dashboard_overview" src="https://github.com/user-attachments/assets/c1873359-fee9-42c6-addd-fe2bc1c98aee" /> <img width="2558" alt="dashboard_detection_response_2" src="https://github.com/user-attachments/assets/f6853710-0938-402b-b326-fa00d586b5d6" /> <img width="2559" alt="dashboard_detection_response_1" src="https://github.com/user-attachments/assets/4eb75526-9a57-46e9-b090-b53353956ea1" /> ---- ### Alerts <img width="2555" alt="alerts_chart_collapsed" src="https://github.com/user-attachments/assets/6ecf5dd5-a785-4701-900b-0454f024b36d" /> <img width="2554" alt="summary" src="https://github.com/user-attachments/assets/1731a6ea-ef2b-4d7d-bf21-4041e59f0ad4" /> <img width="2559" alt="trend" src="https://github.com/user-attachments/assets/b9a741d1-a359-4273-9555-850cdcbc8932" /> <img width="2557" alt="counts" src="https://github.com/user-attachments/assets/a6193ccc-86b8-4974-ad9f-9417e200e859" /> <img width="1281" alt="treemap" src="https://github.com/user-attachments/assets/7b6e163a-a660-4bb1-a6de-88e21934b98a" /> ---- ### Rules preview <img width="2556" alt="Screenshot 2024-12-18 at 13 45 33" src="https://github.com/user-attachments/assets/47099c18-86ee-455a-a5af-ebd6a29904a5" /> ---- --------- Co-authored-by: kibanamachine <[email protected]>
…06254) ## Summary Previous changes applied same tokens for Borealis and Amsterdam: elastic#204631 (comment) PR above causes color changes to the current theme, after discussing with UX, we decide to maintain different color tokens until Borealis is launched. This PR should revert the color changed on Amsterdam by the previous PR and only apply the new color for Borealis. | Current and Amsterdam | Borealis | |-------------------------|----------| |Source: Hard coded: `#d36186`|Source: `euiColorVis4` - `#EE72A6` | |Dest: Hard coded: `#9170b8` |Dest: `euiColorVis2` - `#61A2FF`| ### Host IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2560" alt="host_IPs" src="https://github.com/user-attachments/assets/c0f9f317-fb02-4c96-8422-c1d2484f4636" />|<img width="2560" alt="host_bor_light" src="https://github.com/user-attachments/assets/451d6604-1d7c-4a2e-82c3-74b2499852d2" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/ac45a3ac-ecaf-46b6-91d9-68704d8639ee" />|<img width="2553" alt="host_bor_dark" src="https://github.com/user-attachments/assets/e34e56fd-8202-4a3c-80c1-996718320fd8" />| ### Network IPs: | Current and Amsterdam | Borealis | |-------------------------|----------| |<img width="2557" alt="network_IPs" src="https://github.com/user-attachments/assets/0e0b33d9-55b7-41a5-8910-11b80e539398" />|<img width="2559" alt="network_bor_light" src="https://github.com/user-attachments/assets/f616b3ab-5032-4e69-b67e-cde39b88ea5f" />| |<img width="2558" alt="host_IPs_dark" src="https://github.com/user-attachments/assets/9613a49f-f0c6-4b63-aa56-c960fac175fc" />|<img width="2560" alt="network_bor_dark" src="https://github.com/user-attachments/assets/911f0509-43ea-428a-94d6-9ce01f5425ac" />| ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
Summary
#202498
#202503
GetLensAttributes
to accepteuiTheme
, so all the functions in this type are updated accordingly.https://github.com/elastic/kibana/pull/204631/files#diff-abe20658865cad59eadcff945552b40832d96da0264ed89ddd5ab25ded1420a3R30
To test:
Please verify if visualizations are displayed properly.
Running Kibana with the Borealis theme
In order to run Kibana with
Borealis
, you'll need to do the following:Set the following in kibana.dev.yml:
uiSettings.experimental.themeSwitcherEnabled: true
Run Kibana with the following environment variable set:
KBN_OPTIMIZER_THEMES="borealislight,borealisdark,v8light,v8dark" yarn start
This will expose a toggle under Stack Management > Advanced Settings > Theme version, which you can use to toggle between Amsterdam and Borealis.
Explore
Dashboards
Alerts
Rules preview