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] fix issue related to local storage on expandable flyout non-alert document #180391

Merged

Conversation

PhilippeOberti
Copy link
Contributor

Summary

This PR fixes a small issue introduced in this previous PR, where if the user clicks on the Overview tab, that selection is saved in local storage. When the user opens a non-alert document that does not have the Overview tab (see details in this PR), the view does not default back to the Table tab.

Previous (broken) behavior:

Screen.Recording.2024-04-09.at.10.21.40.AM.mov

Fixed behavior:

Screen.Recording.2024-04-09.at.10.17.32.AM.mov

Checklist

@PhilippeOberti PhilippeOberti requested a review from a team as a code owner April 9, 2024 15:26
@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team labels Apr 9, 2024
@elasticmachine
Copy link
Contributor

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

@@ -58,7 +58,17 @@ export const RightPanel: FC<Partial<RightPanelProps>> = memo(({ path }) => {
const defaultTab = tabsDisplayed[0].id;
const tabSavedInlocalStorage = storage.get(FLYOUT_STORAGE_KEYS.RIGHT_PANEL_SELECTED_TABS);

if (!path) return tabSavedInlocalStorage || defaultTab;
if (!path) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm considering wrapping this whole useMemo (and the previous one calculating the tabDisplayed) into a hook to make this unit testable.
What do you think @christineweng

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@christineweng I implemented the hook and its unit test in this last commit

@PhilippeOberti PhilippeOberti force-pushed the fix-local-storage-default-path branch from 92edb72 to e9d7f1b Compare April 10, 2024 18:24
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5302 5303 +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 17.0MB 17.0MB +796.0B

History

  • 💔 Build #202196 failed 92edb7294b54985168a50df71e492c79c34d3711

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

Copy link
Contributor

@christineweng christineweng left a comment

Choose a reason for hiding this comment

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

Desk tested and LGTM! thanks for the fix and unit tests!

@PhilippeOberti PhilippeOberti merged commit 8bbd815 into elastic:main Apr 10, 2024
34 checks passed
@PhilippeOberti PhilippeOberti deleted the fix-local-storage-default-path branch April 10, 2024 21:48
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Apr 10, 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.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants