-
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] Prepare Rule Management owned pages for Borealis #206122
Conversation
0d230db
to
7e4c02a
Compare
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
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 @nikitaindik, checked all pages owned by us in:
- Amsterdam light
- Amsterdam dark
- Borealis light
- Borealis dark
Aside from the severity and warning badges you mention in the description, I think there's some slightly iffy contrast things on the MITRE coverage overview page in dark mode but those are existing issues with our current theme as well, and there have been discussions related to that elsewhere.
If we really wanted to address it though, I think outlining the techniques in dark mode with a light border would help differentiate the empty techniques from the 1-3 techniques coloring, but I'll leave that up to you - we could probably do a more exhaustive update of the colors on that page at some point.
7e4c02a
to
fb0b784
Compare
@dplumlee I agree. I think this needs a more comprehensive approach. Having five shades of the same color might be difficult to distinguish, especially for users with limited vision. These cards could be a good candidate for the severity palette currently being developed by EUI. For now, I’ve applied light text to the darkest cards to improve contrast. Here’s how it looks in both Amsterdam and Borealis. |
💔 Build Failed
Failed CI StepsHistory
cc @nikitaindik |
…lastic#206122) **Resolves: elastic#204737** **Related PR (fixes severity indicator colors): elastic#206276** ## Summary This PR introduces some of the necessary changes to support the Borealis UI theme on Rule Management team-owned pages. You can find a list of what needs to be done in this [issue](elastic#199715) description under "Requested Code Changes". ## Changes in this PR - Replaced deprecated `euiThemeVars` with `useEuiTheme`. - Fixed incorrect "disabled" button color for the rule delete bulk action. ## Changes that are needed, but are outside of this PR's scope - Fix incorrect severity colors in Borealis. Will be resolved by this Threat Hunting [PR](elastic#206276). <img width="93" alt="Schermafbeelding 2025-01-13 om 13 46 26" src="https://github.com/user-attachments/assets/d35407d7-77f9-4d7d-a6b8-3dfe4f8a0e9e" /> - Fix rule execution status indicator appearing too dark in the Borealis theme. This issue will be addressed in EUI, but there is no draft PR yet, as discussions are still ongoing between the design team and EUI folks to determine the best approach. <img width="326" alt="borealis" src="https://github.com/user-attachments/assets/3324448c-be13-4074-bfdc-c6837fb2bc6c" /> ## Testing You can find the theme switcher in "Stack Management" -> "Advanced Settings". Then you can test if everything looks correct in: - Amsterdam theme, light mode - Amsterdam theme, dark mode - Borealis theme, light mode - Borealis theme, dark mode Work started on: 08-01-2025
Resolves: #204737
Related PR (fixes severity indicator colors): #206276
Summary
This PR introduces some of the necessary changes to support the Borealis UI theme on Rule Management team-owned pages. You can find a list of what needs to be done in this issue description under "Requested Code Changes".
Changes in this PR
euiThemeVars
withuseEuiTheme
.Changes that are needed, but are outside of this PR's scope
Testing
You can find the theme switcher in "Stack Management" -> "Advanced Settings". Then you can test if everything looks correct in:
Work started on: 08-01-2025