kibana
kibana copied to clipboard
elastic
[Security Solution][Alert details] - save state of left panel (collapsed/expanded) to local storage - observable approach
Summary
This PR continues adding user behavior saved to local storage for the expandable flyout. After persisting which section of the overview tab is expanded/collapses and after persisting which tab of the right section is selected, this PR adds the code to persist if the left section is expanded or not.
A small difference though, is contrary to the first 2 persistence, here we only persists the fact that the left section is expanded while the flyout is open. This allows users to navigate between alerts (by clicking in the open flyout icon button in the alerts table for example, or soon by using the pagination within the flyout) and not loose the current state of the flyout. As soon as the user clicks on the close flyout button in the top-right corner, we clear the value from local storage. This means that the next time the user opens the flyout, the left section will not be expanded. This is by design.
Note that the normal flyout (meaning the flyout open from NOT from timeline) and the timeline flyout maintain their own state in local storage.
Approach
This approach implements an observable that the kbn-expandable-flyout package provides through its API. The observable is triggered every time the flyout closes. Components can listen to the observable and run the code they need. I like this approach because it keeps the code to run on flyout close in the component that care about it, and outside the kbn-expandable-flyout package. The downside is we're running the risk to have developer forgetting to unsubscribe from the observable and therefore introducing memory leaks...
~~There is a very similar PR that uses a slightly different approach. It saves the functions in memory within the package. Please take a look and let me know which approach looks the best!~~
TODO
- [x] write unit tests for the hooks
- [x] add documentation to the flyout package to explain how to use the
onCloseAPI
Notes:
- when the user closes the browser tab or navigates to a different page without closing the flyout, the state is NOT reset (meaning a expanded section will automatically open up again next time the user opens a flyout
Saving state and clearing after closing flyout
https://github.com/elastic/kibana/assets/17276605/73e46b1b-9b2a-4f0d-bdc5-dab3843e5dd3
Security Solution and Timeline flyouts saving their state separately
https://github.com/elastic/kibana/assets/17276605/73983096-14d9-4df4-880b-a136d54f11e7
The 3 options to clear local storage:
- when the user closes the left panel
- when the user closes the flyout
- when the user navigates to a different page or closes the browser
https://github.com/elastic/kibana/assets/17276605/6afd88b6-c553-49cf-a5d9-2fee50112c62
Checklist
- [x] Documentation was added for features that require explanation or tutorials
- [x] Unit or functional tests were updated or added to match the most common scenarios
https://github.com/elastic/security-team/issues/7670 Will help https://github.com/elastic/kibana/issues/179520
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)
The downside is we're running the risk to have developer forgetting to unsubscribe from the observable and therefore introducing memory leaks...
I'm curious. Why do we need an observable in this case? Wouldn't a callback (onClose) be sufficient and less complicated in terms of state management?
The downside is we're running the risk to have developer forgetting to unsubscribe from the observable and therefore introducing memory leaks...
I'm curious. Why do we need an observable in this case? Wouldn't a callback (
onClose) be sufficient and less complicated in terms of state management?
did you get the chance to look at the second approach in the other PR linked in the description above? This is the second implementation I managed to get to work.
I couldn't find any other solutions... if you have any ideas I'm all ears!! 😆
:broken_heart: Build Failed
- Buildkite Build
- Commit: e057d6e4662ed8464c103ed2526aed2ee23c3ee9
- Interpreting CI Failures
Failed CI Steps
Metrics [docs]
Module Count
Fewer modules leads to a faster build time
| id | before | after | diff |
|---|---|---|---|
securitySolution |
5482 | 5484 | +2 |
Async chunks
Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app
| id | before | after | diff |
|---|---|---|---|
securitySolution |
15.3MB | 15.3MB | +1.2KB |
Unknown metric groups
API count
| id | before | after | diff |
|---|---|---|---|
@kbn/expandable-flyout |
38 | 39 | +1 |
ESLint disabled line counts
| id | before | after | diff |
|---|---|---|---|
securitySolution |
519 | 520 | +1 |
Total ESLint disabled count
| id | before | after | diff |
|---|---|---|---|
securitySolution |
597 | 598 | +1 |
History
- :yellow_heart: Build #206470 was flaky 5b39dea4c7cef3edce2be0efba773dff1038cf82
- :broken_heart: Build #206469 failed 789720d8184f7fcbc67f42361de14b62c1ffcb4e
- :broken_heart: Build #203078 failed d6ee11b8d4d5ff36d9b8c7adc0775e6551bcee12
To update your PR or re-run it, just comment with:
@elasticmachine merge upstream
:broken_heart: Build Failed
- Buildkite Build
- Commit: 57ccc0b7a0f676e54675c4a15ded4782c216343a
- Interpreting CI Failures
Failed CI Steps
History
- :broken_heart: Build #210191 failed f22b0b8546c43fb367d6f098ab3dcc9afe46b6b4
- :broken_heart: Build #209976 failed e57736c6ecf93c9933b08689a5e31debaedaeecc
- :broken_heart: Build #209705 failed f2a048f58be5984fea3ee67434150f93309914d1
- :green_heart: Build #209324 succeeded 91b3f18c793fed0ad793cdcfd2cf1a45929accf4
- :broken_heart: Build #208511 failed e057d6e4662ed8464c103ed2526aed2ee23c3ee9
To update your PR or re-run it, just comment with:
@elasticmachine merge upstream
@christineweng and I were discussing the multiple edge cases for the left section expanded. We decided to close this PR and I opened a new one that focuses only on providing the onClose API.
That way we defer the left section localStorage work to later, when we have more data on how we want things to work, and we can focus the discussion on the implementation inside the kbn-expandable-flyout package.