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][Timeline] extract and cleanup timeline flyout component #173880

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Dec 21, 2023

## Notes

This is a follow up PR to this one which should be merged first. The changes can be reviewed individually though, by ignoring the first commit and focusing only on the second.

This PR is part of a long-running effort to clean up the timeline code. I originally had opened #172408 but the file count kept growing and so did the complexity for the reviewers. Of that original PR I will create probably 5-6 smaller ones.

Summary

This very PR focuses on extracting the TimelineFlyout component:

  • extract the component out of the flyout folder as timeline is no longer a flyout, but a modal
  • rename the folder and file to wrapper and TimelineWrapper respectively to better represent what the component actually does
  • add documentation and cleanup unit tests

The PR also make the following small modifications

  • rename SecuritySolutionBottomBar to Timeline has the component actually contains the bottom and the timeline modal
  • merge 2 selectors files into 1

Absolutely no visual or behavior changes introduced here!

Checklist

The following PR will focus on the FlyoutBottomBar component.

@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team labels Dec 21, 2023
@PhilippeOberti PhilippeOberti requested review from a team as code owners December 21, 2023 20:56
@PhilippeOberti PhilippeOberti force-pushed the timeline-cleanup-part-2 branch from 844a591 to 2f255f2 Compare December 21, 2023 22:03
@PhilippeOberti PhilippeOberti force-pushed the timeline-cleanup-part-2 branch from 2f255f2 to 2e72277 Compare December 26, 2023 22:37
@PhilippeOberti
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -90,7 +90,7 @@ export const SecuritySolutionTemplateWrapper: React.FC<Omit<KibanaPageTemplatePr
{isTimelineBottomBarVisible && (
<KibanaPageTemplate.BottomBar data-test-subj="timeline-bottom-bar-container">
<EuiThemeProvider colorMode={globalColorMode}>
<SecuritySolutionBottomBar />
<Timeline />
Copy link
Contributor

Choose a reason for hiding this comment

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

Such a simple change, yet so beautiful ❤️ 😄

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes :) Looks great!

@michaelolo24
Copy link
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4869 4868 -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 11.4MB 11.4MB -1.1KB
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 470 469 -1

Total ESLint disabled count

id before after diff
securitySolution 542 541 -1

History

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

@PhilippeOberti PhilippeOberti merged commit 26472b6 into elastic:main Jan 9, 2024
37 checks passed
@PhilippeOberti PhilippeOberti deleted the timeline-cleanup-part-2 branch January 9, 2024 15:41
@kibanamachine kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 9, 2024
nreese pushed a commit to nreese/kibana that referenced this pull request Jan 10, 2024
delanni pushed a commit to delanni/kibana that referenced this pull request Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants