-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Add guided tour to document details flyout #180318
Conversation
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations) |
Pinging @elastic/fleet (Team:Fleet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fleet change 👍
71e4f5c
to
8a5106c
Compare
Here's a behavior that I think we could handle a bit better maybe: Situation: user opens the flyout for the first time from a link shared with her/him. That link has the flyout open on the table tab with the left section already expanded. Current behavior: the first step of the tour (show overview tab) is displayed but when the user clicks next nothing happens Expected behavior: the tour should apply the correct actions to navigate through the flyout, in this case select the overview tab to continue (similar to how we open the left section to continue the tour) Video to show the issue: Screen.Recording.2024-04-09.at.2.49.29.PM.movWhat do you think? Could we support this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments, overall the functionality seems to be working fine with a couple of weird edge cases!
...gins/security_solution/public/flyout/document_details/shared/components/tour_step_config.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/flyout/document_details/left/tabs/test_ids.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/flyout/document_details/left/tabs/test_ids.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/flyout/document_details/right/test_ids.ts
Outdated
Show resolved
Hide resolved
...gins/security_solution/public/flyout/document_details/shared/components/tour_step_config.tsx
Outdated
Show resolved
Hide resolved
...k/plugins/security_solution/public/flyout/document_details/shared/components/flyout_tour.tsx
Outdated
Show resolved
Hide resolved
...k/plugins/security_solution/public/flyout/document_details/shared/components/flyout_tour.tsx
Show resolved
Hide resolved
...k/plugins/security_solution/public/flyout/document_details/shared/components/flyout_tour.tsx
Outdated
Show resolved
Hide resolved
Another weird case that I believe you mentioned during our standup is when opening a flyout from the alerts table, then investigating in timeline and from there opening another flyout, we now have 2 tours rendered. Screen.Recording.2024-04-09.at.4.18.55.PM.mov |
@PhilippeOberti I can see 2 tours when both flyouts are open, but their steps are the same.. As discussed, to prevent all kinds of edge cases and reduce code complexity, I have limited the tour to be shown on alerts only (alert table and cases). This also prevents the dual tour situation. |
The code looks much easier to follow now, thanks for simplifying the logic! There is only one scenario that is still a bit weird and we might be able to fix it up:
The tour gets stuck when trying to show the expand details button. Closing and reopening the flyout unblocks it. Screen.Recording.2024-04-11.at.5.25.00.PM.movThis scenario could happen if a user opens the flyout for the first time using a link provided by someone. Maybe we consider this as too much of an edge case and we decide to ignore the issue? |
This could also happen when we point to expand details, and user click it first. I think we should address it.
My first take on it was to add the left section tours in the right. there is duplication, but what it does is, if the flyout is already expanded, the right section actually can find the next anchor (since it is already present on the page), and the tour can go on. I don't know if this is an anti-pattern, let me know what you think! Another option, depending on how you decide to do the local storage, is we can have an observable for monitor the local storage change and trigger a render. We can always force it to close left panel and reopen, then we have a flash, if that's more obvious and annoying? Screen.Recording.2024-04-12.at.12.48.36.PM.mov |
0fd0f86
to
2349bb1
Compare
At that stage, what's the point of having 2 tours then? If we have both right and left steps in the right tour, do we still need the left tour?
Monitoring local storage wouldn't be enough I think. We don't need to have any entry in the local storage as the left section of the flyout can also be opened when a url is shared.
I think avoiding a close/reopen with a flash would be ideal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for making all the changes. If you don't mind adding the comment before merging that'd be great!
x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.tsx
Show resolved
Hide resolved
e571221
to
7768142
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…iew tab (#181202) ## Summary This PR fixes a bug introduced by #180318. If user has not completed the flyout tour and click exit. Upon refresh, the tabs in alerts flyout always reset to overview. This is because the tour step is stuck in step 1 and it will keep calling `goToOverview` tab to start the tour. This PR adds additional guard to check the tour is active before navigating.
…iew tab (elastic#181202) ## Summary This PR fixes a bug introduced by elastic#180318. If user has not completed the flyout tour and click exit. Upon refresh, the tabs in alerts flyout always reset to overview. This is because the tour step is stuck in step 1 and it will keep calling `goToOverview` tab to start the tour. This PR adds additional guard to check the tour is active before navigating. (cherry picked from commit 9ea94d1)
…o overview tab (#181202) (#181352) # Backport This will backport the following commits from `main` to `8.14`: - [[Security Solution][Alert Details] Fix alert flyout reseting to overview tab (#181202)](#181202) <!--- 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-04-22T18:29:03Z","message":"[Security Solution][Alert Details] Fix alert flyout reseting to overview tab (#181202)\n\n## Summary\r\n\r\nThis PR fixes a bug introduced by\r\nhttps://github.com//pull/180318.\r\n\r\nIf user has not completed the flyout tour and click exit. Upon refresh,\r\nthe tabs in alerts flyout always reset to overview. This is because the\r\ntour step is stuck in step 1 and it will keep calling `goToOverview` tab\r\nto start the tour. This PR adds additional guard to check the tour is\r\nactive before navigating.","sha":"9ea94d10b6588c93fea9cd08bfbc73f017b0c76c","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat Hunting","Team:Threat Hunting:Investigations","v8.14.0","v8.15.0"],"title":"[Security Solution][Alert Details] Fix alert flyout reseting to overview tab","number":181202,"url":"https://github.com/elastic/kibana/pull/181202","mergeCommit":{"message":"[Security Solution][Alert Details] Fix alert flyout reseting to overview tab (#181202)\n\n## Summary\r\n\r\nThis PR fixes a bug introduced by\r\nhttps://github.com//pull/180318.\r\n\r\nIf user has not completed the flyout tour and click exit. Upon refresh,\r\nthe tabs in alerts flyout always reset to overview. This is because the\r\ntour step is stuck in step 1 and it will keep calling `goToOverview` tab\r\nto start the tour. This PR adds additional guard to check the tour is\r\nactive before navigating.","sha":"9ea94d10b6588c93fea9cd08bfbc73f017b0c76c"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181202","number":181202,"mergeCommit":{"message":"[Security Solution][Alert Details] Fix alert flyout reseting to overview tab (#181202)\n\n## Summary\r\n\r\nThis PR fixes a bug introduced by\r\nhttps://github.com//pull/180318.\r\n\r\nIf user has not completed the flyout tour and click exit. Upon refresh,\r\nthe tabs in alerts flyout always reset to overview. This is because the\r\ntour step is stuck in step 1 and it will keep calling `goToOverview` tab\r\nto start the tour. This PR adds additional guard to check the tour is\r\nactive before navigating.","sha":"9ea94d10b6588c93fea9cd08bfbc73f017b0c76c"}}]}] BACKPORT--> Co-authored-by: christineweng <[email protected]>
… 8.14 (#198708) ## Summary We [added](#180318) a guided tour to the expandable flyout back in `8.14`. It is time to remove it as enough users have seen it. No UI changes other than the tour not showing up. ### How to test - clear local storage (more specifically remove the `securitySolution.documentDetails.newFeaturesTour.v8.14` key - open a alert or event details flyout and verify that the tour does not show up ### 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 Should help solve #197492
… 8.14 (elastic#198708) ## Summary We [added](elastic#180318) a guided tour to the expandable flyout back in `8.14`. It is time to remove it as enough users have seen it. No UI changes other than the tour not showing up. ### How to test - clear local storage (more specifically remove the `securitySolution.documentDetails.newFeaturesTour.v8.14` key - open a alert or event details flyout and verify that the tour does not show up ### 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 Should help solve elastic#197492 (cherry picked from commit 0aec5a8) # Conflicts: # x-pack/packages/security-solution/common/src/flyout/common/components/flyout_navigation.tsx # x-pack/plugins/security_solution/public/flyout/document_details/shared/utils/tour_step_config.tsx # x-pack/plugins/translations/translations/fr-FR.json # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
… 8.14 (elastic#198708) ## Summary We [added](elastic#180318) a guided tour to the expandable flyout back in `8.14`. It is time to remove it as enough users have seen it. No UI changes other than the tour not showing up. ### How to test - clear local storage (more specifically remove the `securitySolution.documentDetails.newFeaturesTour.v8.14` key - open a alert or event details flyout and verify that the tour does not show up ### 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 Should help solve elastic#197492 (cherry picked from commit 0aec5a8) # Conflicts: # x-pack/packages/security-solution/common/src/flyout/common/components/flyout_navigation.tsx # x-pack/plugins/security_solution/public/flyout/document_details/shared/utils/tour_step_config.tsx # x-pack/plugins/translations/translations/fr-FR.json # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
… 8.14 (elastic#198708) ## Summary We [added](elastic#180318) a guided tour to the expandable flyout back in `8.14`. It is time to remove it as enough users have seen it. No UI changes other than the tour not showing up. ### How to test - clear local storage (more specifically remove the `securitySolution.documentDetails.newFeaturesTour.v8.14` key - open a alert or event details flyout and verify that the tour does not show up ### 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 Should help solve elastic#197492 (cherry picked from commit 0aec5a8) # Conflicts: # x-pack/packages/security-solution/common/src/flyout/common/components/flyout_navigation.tsx # x-pack/plugins/security_solution/public/flyout/document_details/shared/utils/tour_step_config.tsx # x-pack/plugins/translations/translations/fr-FR.json # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
… 8.14 (elastic#198708) ## Summary We [added](elastic#180318) a guided tour to the expandable flyout back in `8.14`. It is time to remove it as enough users have seen it. No UI changes other than the tour not showing up. ### How to test - clear local storage (more specifically remove the `securitySolution.documentDetails.newFeaturesTour.v8.14` key - open a alert or event details flyout and verify that the tour does not show up ### 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 Should help solve elastic#197492 (cherry picked from commit 0aec5a8) # Conflicts: # x-pack/packages/security-solution/common/src/flyout/common/components/flyout_navigation.tsx # x-pack/plugins/security_solution/public/flyout/document_details/shared/utils/tour_step_config.tsx # x-pack/plugins/translations/translations/fr-FR.json # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
…ced in 8.14 (#198708) (#198871) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Alert Details] - remove flyout tour introduced in 8.14 (#198708)](#198708) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Philippe Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-04T21:02:07Z","message":"[Security Solution][Alert Details] - remove flyout tour introduced in 8.14 (#198708)\n\n## Summary\r\n\r\nWe [added](#180318) a guided tour\r\nto the expandable flyout back in `8.14`. It is time to remove it as\r\nenough users have seen it.\r\n\r\nNo UI changes other than the tour not showing up.\r\n\r\n### How to test\r\n\r\n- clear local storage (more specifically remove the\r\n`securitySolution.documentDetails.newFeaturesTour.v8.14` key\r\n- open a alert or event details flyout and verify that the tour does not\r\nshow up\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\r\n\r\nShould help solve https://github.com/elastic/kibana/issues/197492","sha":"0aec5a82dbf0e27d1ca7d416e54e028e629f3a1c","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","Team:Threat Hunting:Investigations","backport:version","v8.17.0"],"number":198708,"url":"https://github.com/elastic/kibana/pull/198708","mergeCommit":{"message":"[Security Solution][Alert Details] - remove flyout tour introduced in 8.14 (#198708)\n\n## Summary\r\n\r\nWe [added](#180318) a guided tour\r\nto the expandable flyout back in `8.14`. It is time to remove it as\r\nenough users have seen it.\r\n\r\nNo UI changes other than the tour not showing up.\r\n\r\n### How to test\r\n\r\n- clear local storage (more specifically remove the\r\n`securitySolution.documentDetails.newFeaturesTour.v8.14` key\r\n- open a alert or event details flyout and verify that the tour does not\r\nshow up\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\r\n\r\nShould help solve https://github.com/elastic/kibana/issues/197492","sha":"0aec5a82dbf0e27d1ca7d416e54e028e629f3a1c"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198708","number":198708,"mergeCommit":{"message":"[Security Solution][Alert Details] - remove flyout tour introduced in 8.14 (#198708)\n\n## Summary\r\n\r\nWe [added](#180318) a guided tour\r\nto the expandable flyout back in `8.14`. It is time to remove it as\r\nenough users have seen it.\r\n\r\nNo UI changes other than the tour not showing up.\r\n\r\n### How to test\r\n\r\n- clear local storage (more specifically remove the\r\n`securitySolution.documentDetails.newFeaturesTour.v8.14` key\r\n- open a alert or event details flyout and verify that the tour does not\r\nshow up\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\r\n\r\nShould help solve https://github.com/elastic/kibana/issues/197492","sha":"0aec5a82dbf0e27d1ca7d416e54e028e629f3a1c"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Summary
This PR adds a guided tour for the new expandable flyout in alerts table.
Where it will show up:
✅ Alerts page
✅ Alerts tab in Cases
Tour will not show up in:
❌ rule creation
❌ flyout in timeline
❌ event table.
Screen.Recording.2024-04-04.at.4.36.17.PM.mov
How to test
To test guided tour in event and timeline, enable
expandableEventFlyoutEnabled
,expandableTimelineFlyoutEnabled
respectively.https://github.com/elastic/security-team/issues/8134
Checklist