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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Prebuilt rules diffing style adjustments #173187

Merged
merged 5 commits into from
Dec 14, 2023
Merged

[Security Solution] Prebuilt rules diffing style adjustments #173187

merged 5 commits into from
Dec 14, 2023

Conversation

nikitaindik
Copy link
Contributor

@nikitaindik nikitaindik commented Dec 12, 2023 •
edited by kibanamachine
Loading

Resolves: #173264
Addresses: #169160

Summary

Tweaks diff styling so that it's more readable in both light and dark modes.

Light mode

Scherm­afbeelding 2023-12-13 om 17 37 07

Dark mode

Scherm­afbeelding 2023-12-13 om 17 41 03

@nikitaindik nikitaindik self-assigned this Dec 12, 2023
@joepeeples joepeeples mentioned this pull request Dec 12, 2023
Closed
@nikitaindik nikitaindik mentioned this pull request Dec 13, 2023
Closed
@nikitaindik nikitaindik added enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Rule Details Security Solution Detection Rule Details page v8.12.0 labels Dec 13, 2023
@nikitaindik nikitaindik mentioned this pull request Dec 13, 2023
Closed
15 tasks
@nikitaindik nikitaindik marked this pull request as ready for review December 13, 2023 15:13
@nikitaindik nikitaindik requested a review from a team as a code owner December 13, 2023 15:13
@nikitaindik nikitaindik requested a review from dplumlee December 13, 2023 15:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@nikitaindik nikitaindik force-pushed the json-diff-highlighting branch from 54e2bdc to c99d64a Compare December 13, 2023 16:28
@banderror banderror added Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area and removed Feature:Rule Details Security Solution Detection Rule Details page labels Dec 13, 2023
dplumlee
dplumlee approved these changes Dec 13, 2023
View reviewed changes
Copy link
Contributor

@dplumlee dplumlee left a comment

Choose a reason for hiding this comment

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

Just left one nit, I think this looks good for the most part. There are a lot of "magic numbers" but I'm not sure we can avoid that when modifying third party components to fit our need. Designs look good in use too, tested with every field type.

...tion/public/detection_engine/rule_management/components/rule_details/json_diff/diff_view.tsx Outdated Show resolved Hide resolved
@nikitaindik nikitaindik force-pushed the json-diff-highlighting branch from c99d64a to 9b18fd2 Compare December 14, 2023 11:05
@nikitaindik nikitaindik enabled auto-merge (squash) December 14, 2023 11:07
@nikitaindik nikitaindik added the ci:cloud-deploy Create or update a Cloud deployment label Dec 14, 2023
@nikitaindik nikitaindik merged commit 37af741 into elastic:main Dec 14, 2023
39 checks passed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4832 4833 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 13.1MB 13.1MB +1.3KB

History

  • 💛 Build #183468 was flaky c99d64a38718a09e31315af5c35ec6577e179af2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @nikitaindik

@kibanamachine kibanamachine mentioned this pull request Dec 14, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 14, 2023
@nikitaindik
[Security Solution] Prebuilt rules diffing style adjustments (elastic…
704f6ca 
…#173187)

**Resolves: elastic#173264
**Addresses: elastic#169160

## Summary
Tweaks diff styling so that it's more readable in both light and dark
modes.

---

#### Light mode
<img width="1178" alt="Scherm­afbeelding 2023-12-13 om 17 37 07"
src="https://github.com/elastic/kibana/assets/15949146/fe8620c7-407a-4355-8863-4a5a8e1425ea">

#### Dark mode
<img width="1174" alt="Scherm­afbeelding 2023-12-13 om 17 41 03"
src="https://github.com/elastic/kibana/assets/15949146/a8df3b88-a482-455f-91d3-5e08f1381b8c">

(cherry picked from commit 37af741)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 14, 2023
@kibanamachine @nikitaindik
[8.12] [Security Solution] Prebuilt rules diffing style adjustments (#…
df14319 
…173187) (#173373)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] Prebuilt rules diffing style adjustments
(#173187)](#173187)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nikita
Indik","email":"nikita.indik@elastic.co"},"sourceCommit":{"committedDate":"2023-12-14T12:09:16Z","message":"[Security
Solution] Prebuilt rules diffing style adjustments
(#173187)\n\n**Resolves:
https://github.com/elastic/kibana/issues/173264**\r\n**Addresses:
https://github.com/elastic/kibana/issues/169160**\r\n\r\n##
Summary\r\nTweaks diff styling so that it's more readable in both light
and dark\r\nmodes.\r\n\r\n---\r\n\r\n#### Light mode\r\n<img
width=\"1178\" alt=\"Scherm­afbeelding 2023-12-13 om 17 37
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/fe8620c7-407a-4355-8863-4a5a8e1425ea\">\r\n\r\n\r\n####
Dark mode\r\n<img width=\"1174\" alt=\"Scherm­afbeelding 2023-12-13 om
17 41
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/a8df3b88-a482-455f-91d3-5e08f1381b8c\">","sha":"37af741c6e03afe9dc877600fe47960b5bac8f50","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["enhancement","release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","ci:cloud-deploy","v8.12.0","v8.13.0"],"number":173187,"url":"https://github.com/elastic/kibana/pull/173187","mergeCommit":{"message":"[Security
Solution] Prebuilt rules diffing style adjustments
(#173187)\n\n**Resolves:
https://github.com/elastic/kibana/issues/173264**\r\n**Addresses:
https://github.com/elastic/kibana/issues/169160**\r\n\r\n##
Summary\r\nTweaks diff styling so that it's more readable in both light
and dark\r\nmodes.\r\n\r\n---\r\n\r\n#### Light mode\r\n<img
width=\"1178\" alt=\"Scherm­afbeelding 2023-12-13 om 17 37
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/fe8620c7-407a-4355-8863-4a5a8e1425ea\">\r\n\r\n\r\n####
Dark mode\r\n<img width=\"1174\" alt=\"Scherm­afbeelding 2023-12-13 om
17 41
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/a8df3b88-a482-455f-91d3-5e08f1381b8c\">","sha":"37af741c6e03afe9dc877600fe47960b5bac8f50"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173187","number":173187,"mergeCommit":{"message":"[Security
Solution] Prebuilt rules diffing style adjustments
(#173187)\n\n**Resolves:
https://github.com/elastic/kibana/issues/173264**\r\n**Addresses:
https://github.com/elastic/kibana/issues/169160**\r\n\r\n##
Summary\r\nTweaks diff styling so that it's more readable in both light
and dark\r\nmodes.\r\n\r\n---\r\n\r\n#### Light mode\r\n<img
width=\"1178\" alt=\"Scherm­afbeelding 2023-12-13 om 17 37
07\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/fe8620c7-407a-4355-8863-4a5a8e1425ea\">\r\n\r\n\r\n####
Dark mode\r\n<img width=\"1174\" alt=\"Scherm­afbeelding 2023-12-13 om
17 41
03\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/a8df3b88-a482-455f-91d3-5e08f1381b8c\">","sha":"37af741c6e03afe9dc877600fe47960b5bac8f50"}}]}]
BACKPORT-->

Co-authored-by: Nikita Indik <nikita.indik@elastic.co>
@banderror banderror added release_note:feature Makes this part of the condensed release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Jan 4, 2024
@banderror banderror mentioned this pull request Jan 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dplumlee dplumlee dplumlee approved these changes

Assignees

@nikitaindik nikitaindik

Labels
ci:cloud-deploy Create or update a Cloud deployment enhancement New value added to drive a business result Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area release_note:feature Makes this part of the condensed release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.12.0 v8.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Security Solution] Tweak JSON diffs highlighting styles to improve readability
6 participants
@nikitaindik @elasticmachine @kibana-ci @kibanamachine @dplumlee @banderror