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.

Sign up for GitHub

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

Jump to bottom

[Security Solution][Notes] - update notes management page columns #194860

Merged
merged 1 commit into from
Oct 9, 2024
Merged

[Security Solution][Notes] - update notes management page columns #194860

merged 1 commit into from
Oct 9, 2024

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Oct 3, 2024 •
edited by kibanamachine
Loading

Summary

This PR makes a small change to the Notes management page. It focuses on the following items:

  • moving the Actions column to the left of the table to follow the latest mocks
  • adding a new open flyout icon to the actions column, to allow the users to open the flyout for the document the note may have been associated with. If the note has a value in its eventId property, we display the icon, otherwise we don't.
  • replacing the Timeline columns that contained the Open timeline links with another new open timeline icon in the actions column, to let the user open the saved timeline associated with the note. If the note has a value in its timelineId propterty, we display the icon, otherwise we don't.
  • make some slight tweaks to the delete icon to better follow the mocks linked above. Also we're now using the confirmation modal when deleting notes in the flyout Notes tab as well as in the Timeline Notes tab
  • use an avatar instead of the name of the user who created the note
  • re-organize the order of the columns to be like the mocks

Note: one difference with the mocks is we do not render the 3-dot menu, because at this time we do not have the Export note functionality, so having a dropdown menu with a single entry does not make sense

View event and open timeline actions

Screen.Recording.2024-10-03.at.4.28.56.PM.mov

Delete notes in notes management, flyout and timeline

Screen.Recording.2024-10-03.at.4.32.17.PM.mov

Checklist

#189879

@PhilippeOberti PhilippeOberti added backport release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Oct 3, 2024
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner October 3, 2024 16:10
@elasticmachine
Copy link
Contributor

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

@PhilippeOberti PhilippeOberti force-pushed the note-management-actions branch 2 times, most recently from 7158ae9 to 41ab859 Compare October 3, 2024 18:34
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner October 3, 2024 18:34
@PhilippeOberti PhilippeOberti marked this pull request as draft October 3, 2024 19:11
@PhilippeOberti PhilippeOberti force-pushed the note-management-actions branch from 41ab859 to 15c50f2 Compare October 3, 2024 21:38
@PhilippeOberti PhilippeOberti changed the title [Security Solution][Notes] - update notes management page actions [Security Solution][Notes] - update notes management page columns Oct 3, 2024
@PhilippeOberti PhilippeOberti removed the request for review from a team October 3, 2024 21:44
@PhilippeOberti PhilippeOberti marked this pull request as ready for review October 3, 2024 21:44
@PhilippeOberti PhilippeOberti force-pushed the note-management-actions branch 2 times, most recently from b9cbe8c to c5853ff Compare October 3, 2024 22:26
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5920 5918 -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 20.5MB 20.5MB -7.3KB

History

  • 💔 Build #239281 failed b9cbe8cd4dd694f1f696f6ee767f65d1983dc167
  • 💔 Build #239275 failed 15c50f2385826b521faa4e85596ff470b5617cb9
  • 💔 Build #239210 failed 7158ae999f80c37377515d510e6b089f18f60161
  • 💔 Build #239204 failed 8bcc07b2c7dc060e7ea11e758c0596a367ef3126

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

@PhilippeOberti PhilippeOberti force-pushed the note-management-actions branch from c5853ff to 309020a Compare October 8, 2024 20:14
christineweng
christineweng approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@christineweng christineweng left a comment •
edited by PhilippeOberti
Loading

Choose a reason for hiding this comment

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

Desk tested and LGTM!

Do you think it would be better to open the notes tab when user clicks expand?
I made a related comment in the other PR, if users can assess the notes easily, then the note content popover wouldn't need to show all the content? just a thought!

@PhilippeOberti PhilippeOberti force-pushed the note-management-actions branch from 309020a to 8a21394 Compare October 9, 2024 19:29
@PhilippeOberti
Copy link
Contributor Author

Desk tested and LGTM!

Do you think it would be better to open the notes tab when user clicks expand? I made a related comment in the other PR, if users can assess the notes easily, then the note content popover wouldn't need to show all the content? just a thought!

We could open the notes tab, but we do not have the ability yet to scroll to the correct note, so if a user had many notes on that document, we would see a bunch of notes in the Notes tab...
Another solution would be to have a flyout just for that note, but I'm going to stick with the current behavior for now as this is what UIUX recommended.

I will improve the UI of the popover though per your comment on the other PR. Paul and I were discussing it this morning. I have an idea on how to make it look better!

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #8 / Configure updates field options correctly when not required

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5930 5928 -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 20.6MB 20.6MB -7.3KB

History

  • 💔 Build #240442 failed 309020abaf31d1ffe3032f9aa8b88494491bd960

@PhilippeOberti PhilippeOberti merged commit 3fa70e1 into elastic:main Oct 9, 2024
41 checks passed
@PhilippeOberti PhilippeOberti deleted the note-management-actions branch October 9, 2024 21:41
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11263516126

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 9, 2024
@PhilippeOberti
[Security Solution][Notes] - update notes management page columns (el…
38f8453 
…astic#194860)

(cherry picked from commit 3fa70e1)
@kibanamachine kibanamachine mentioned this pull request Oct 9, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 9, 2024
@kibanamachine @PhilippeOberti
[8.x] [Security Solution][Notes] - update notes management page colum…
4944b91 
…ns (#194860) (#195683)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Notes] - update notes management page columns
(#194860)](#194860)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-09T21:41:02Z","message":"[Security
Solution][Notes] - update notes management page columns
(#194860)","sha":"3fa70e122c6a3c77edea3f2c47980403c1835256","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","v9.0.0","Team:Threat
Hunting:Investigations","v8.16.0"],"title":"[Security Solution][Notes] -
update notes management page
columns","number":194860,"url":"https://github.com/elastic/kibana/pull/194860","mergeCommit":{"message":"[Security
Solution][Notes] - update notes management page columns
(#194860)","sha":"3fa70e122c6a3c77edea3f2c47980403c1835256"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194860","number":194860,"mergeCommit":{"message":"[Security
Solution][Notes] - update notes management page columns
(#194860)","sha":"3fa70e122c6a3c77edea3f2c47980403c1835256"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <[email protected]>
@PhilippeOberti PhilippeOberti mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christineweng christineweng christineweng approved these changes

Assignees
No one assigned
Labels
backport release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@PhilippeOberti @elasticmachine @kibana-ci @kibanamachine @christineweng