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

[Security Solution] Update icons and timeline tabs when visualization in flyout is enabled #195687

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

christineweng
Copy link
Contributor

@christineweng christineweng commented Oct 9, 2024

Summary

When advanced setting securitySolution:enableVisualizationsInFlyout is enabled, clicking on analyzer or session view button should open the visualizations in flyout, the related tabs are also removed from timeline.

Screen.Recording.2024-10-09.at.5.03.56.PM.mov

Checklist

@christineweng christineweng added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development v8.16.0 labels Oct 9, 2024
@christineweng christineweng self-assigned this Oct 9, 2024
@christineweng christineweng marked this pull request as ready for review October 9, 2024 22:57
@christineweng christineweng requested review from a team as code owners October 9, 2024 22:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

code LGTM and desk tested everything seems to be working perfectly!

As you said in Slack, it would be good to have other people test this before merging, as it might impact more things than you and I thought...

I did find a weird UI issue (not introduced by you, it is on main as well) on the cases page alerts tab actions column
Screenshot 2024-10-09 at 6 34 37 PM
I will investigate that bug!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 20.6MB 20.6MB +8.0KB

cc @christineweng

@christineweng christineweng added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development labels Oct 10, 2024
Copy link
Contributor

@logeekal logeekal left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 . Thank you.

ENABLE_VISUALIZATIONS_IN_FLYOUT_SETTING
);

const { navigateToAnalyzer } = useNavigateToAnalyzer({
Copy link
Contributor

@logeekal logeekal Oct 11, 2024

Choose a reason for hiding this comment

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

I am thinking if It makes sense to delegate the responsibility of opening Analyzer either in tab or in flyout to useNavigateToAnalyzer.

With that logic becomes consolidated in one place and easy to refactor. But i am not too strongly opiniated about it. Just something to think about. I also understand if you would want to keep the current code as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@logeekal thank you for reviewing! i could see your point, the intention of the hooks was to reduce the duplication of openFlyout calls. i would think when it comes to removing the existing analyzer overlay, the code removal would be the same in either places. But I'll think more about it!

Given there is another PR depending on this one, I will merge it as is

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense. 👍

@christineweng christineweng merged commit bd22f13 into elastic:main Oct 11, 2024
55 of 56 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11294652716

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 11, 2024
… in flyout is enabled (elastic#195687)

## Summary

When advanced setting `securitySolution:enableVisualizationsInFlyout` is
enabled, clicking on analyzer or session view button should open the
visualizations in flyout, the related tabs are also removed from
timeline.

https://github.com/user-attachments/assets/2502c1f5-f71b-4027-87a3-36cc73ea9451

### Checklist

- [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

(cherry picked from commit bd22f13)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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 Oct 11, 2024
…zation in flyout is enabled (#195687) (#195945)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Update icons and timeline tabs when visualization
in flyout is enabled
(#195687)](#195687)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT
[{"author":{"name":"christineweng","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-11T15:00:42Z","message":"[Security
Solution] Update icons and timeline tabs when visualization in flyout is
enabled (#195687)\n\n## Summary\r\n\r\nWhen advanced setting
`securitySolution:enableVisualizationsInFlyout` is\r\nenabled, clicking
on analyzer or session view button should open the\r\nvisualizations in
flyout, the related tabs are also removed
from\r\ntimeline.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2502c1f5-f71b-4027-87a3-36cc73ea9451\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"bd22f1370fc55179ea6f2737176570176f700b6e","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting","Team:Threat
Hunting:Investigations","backport:prev-minor","v8.16.0"],"title":"[Security
Solution] Update icons and timeline tabs when visualization in flyout is
enabled","number":195687,"url":"https://github.com/elastic/kibana/pull/195687","mergeCommit":{"message":"[Security
Solution] Update icons and timeline tabs when visualization in flyout is
enabled (#195687)\n\n## Summary\r\n\r\nWhen advanced setting
`securitySolution:enableVisualizationsInFlyout` is\r\nenabled, clicking
on analyzer or session view button should open the\r\nvisualizations in
flyout, the related tabs are also removed
from\r\ntimeline.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2502c1f5-f71b-4027-87a3-36cc73ea9451\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"bd22f1370fc55179ea6f2737176570176f700b6e"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195687","number":195687,"mergeCommit":{"message":"[Security
Solution] Update icons and timeline tabs when visualization in flyout is
enabled (#195687)\n\n## Summary\r\n\r\nWhen advanced setting
`securitySolution:enableVisualizationsInFlyout` is\r\nenabled, clicking
on analyzer or session view button should open the\r\nvisualizations in
flyout, the related tabs are also removed
from\r\ntimeline.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2502c1f5-f71b-4027-87a3-36cc73ea9451\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"bd22f1370fc55179ea6f2737176570176f700b6e"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: christineweng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants