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] fix wrong add to favorite within timeline guided tour #175993

Merged

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Jan 31, 2024

Summary

This PR fixes the timeline guided tour. The bug must have been introduced in this refactor PR, but looking at the prior code, I'm not sure how things were correctly working before. We already had the favorite button shared and both had the same id...

The code is now working correctly

Screen.Recording.2024-01-31.at.9.00.11.AM.mov

Notes: I decided to use a isPartOfGuidedTour: boolean prop but originally had a different idea: using something like guidedTourId: string (guidedTourId would be the actual tour id). I didn't like the fact that the parent component had to know about that information though, so decided against it. If you think this would be better I'll make the change!

#175952

TODO:

  • add unit test to verify for guided tour

Checklist

@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.13.0 labels Jan 31, 2024
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner January 31, 2024 15:05
@elasticmachine
Copy link
Contributor

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

@PhilippeOberti PhilippeOberti force-pushed the fix-timeline-tour-favorite branch from b8afd14 to b241037 Compare January 31, 2024 21:36
@kibana-ci
Copy link
Collaborator

💚 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 11.4MB 11.4MB +220.0B

History

  • 💚 Build #190590 succeeded b8afd1464bb43eb76f860abb5f53f7e4512d24b0
  • 💔 Build #190523 failed 5b80a87d4514d313710ec795a7767fd5da6f0ddc

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

@PhilippeOberti PhilippeOberti merged commit 80962bb into elastic:main Jan 31, 2024
41 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 31, 2024
@PhilippeOberti PhilippeOberti deleted the fix-timeline-tour-favorite branch January 31, 2024 23:21
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 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.

5 participants