[CTI] updates Alert Summary UI #107081

ecezalp · 2021-07-28T19:59:59Z

Summary

updates indentation of field values on the Summary tab
places ActionCell component at the end of CTI values

indicator match rule with multiple imphash hits

custom query rule with small browser window

small browser window (hidden icons)

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

ecezalp · 2021-07-28T20:03:31Z

@elasticmachine merge upstream

x-pack/plugins/security_solution/public/common/components/event_details/event_details.tsx

rylnd · 2021-07-28T21:12:42Z

...security_solution/public/common/components/event_details/cti_details/threat_summary_view.tsx

@@ -81,11 +86,20 @@ const EnrichmentDescription: React.FC<ThreatSummaryItem['description']> = ({
          </RightMargin>
        </>
      )}
+      <ActionCell
+        data={(data ?? { field: fieldName }) as EventFieldsData}


Can we simply build an appropriate EventFieldsData from the existing parameters and what we know about these fields? I suspect that we don't need to pass data through all these layers like this.

I am seeking @angorayc's guidance on how to approach this piece as I haven't been able to find an implementation that utilizes EventFieldsData type (with no casting)

It might be simplest to "flatten" ActionCell's props to just what it needs; it looks like only a few properties from EventFieldsData are used in there (field, type, value, isObjectArray, and format), and some of those may be optional .

...urity_solution/public/common/components/event_details/table/use_action_cell_data_provider.ts

...security_solution/public/common/components/event_details/cti_details/threat_details_view.tsx

monina-n · 2021-07-29T16:54:34Z

@ecezalp some design feedback for the actions in the summary tab. Please let us know what you think!

In order to to avoid the awkward wrapping of the action cell when a field values wraps to two or more lines, design is proposing this new behavior:

Action cell will always appear to the very right of the value
We'll have a pre-defined break point on the right where lines will wrap so there will always be space on the right for the action cell's full width

This way, the action cell is clearly associated with the hovered over value and has a consistnet behavior on how it appears.

hover.gif.mov

cc: @yiyangliu9286

elasticmachine · 2021-07-29T19:41:11Z

Pinging @elastic/security-solution (Team: SecuritySolution)

ecezalp · 2021-07-29T19:42:28Z

@monina-n @yiyangliu9286 please find the new screenshots where the icons are always in a single line

ecezalp · 2021-08-03T15:55:12Z

@elasticmachine merge upstream

rylnd

Approving! Layout looks good, behavior seems good. A few points to address before merging:

There's some dead code; just a few lines that could be cleaned up
I had nits/questions about some hardcoded widths/heights
Having to thread through a parallel, redundant set of data for these action fields isn't great
- Not a requirement here, but a better approach might be to pass e.g. a buildActions function prop to ThreatSummaryView or some other kind of control inversion

One other UX concern: the interaction on hover isn't nice: you have to hover the blank area to the right of a field's value to see the actions available; hovering anywhere else has no effect. This is bad enough that I would consider it a bug.

rylnd · 2021-08-03T17:20:43Z

...security_solution/public/common/components/event_details/cti_details/threat_summary_view.tsx

@@ -6,15 +6,19 @@
 */

 import styled from 'styled-components';
-import React from 'react';
+import { get } from 'lodash/fp';


Was there a reason for preferring the FP version of this function?

no reason - updated to lodash

...security_solution/public/common/components/event_details/cti_details/threat_summary_view.tsx

rylnd · 2021-08-03T17:35:32Z

...security_solution/public/common/components/event_details/cti_details/threat_summary_view.tsx

@@ -50,22 +56,23 @@ const EnrichmentTitle: React.FC<ThreatSummaryItem['title']> = ({ title, type })
 const EnrichmentDescription: React.FC<ThreatSummaryItem['description']> = ({
  timelineId,
  eventId,
-  fieldName,


We're still generating this value in buildThreatSummaryItems, but it's now unused.

thanks for the catch! removed fieldName from ThreatSummaryItem type

rylnd · 2021-08-03T17:46:20Z

x-pack/plugins/security_solution/public/common/components/event_details/helpers.tsx

@@ -213,7 +219,7 @@ export const getSummaryColumns = (
      field: 'title',
      truncateText: false,
      render: getTitle,
-      width: '33%',
+      width: '220px',


Is there significance to this value? Was this specified by design, or is this intended to be more dynamic than fixed?

there is no particular significance, it is to ensure that the indentation is fixed. Previously this value was 160px for both AlertSummary and ThreatSummary items. Recently it was changed to 33% for AlertSummary only. To ensure the consistency between AlertSummary and ThreatSummary values, a change needed to be made. Viewing the designs, I thought that a fixed 220px was good to go. We can get confirmation from @yiyangliu9286 for this one.

For the design changes in 7.15, we made the "value" field indent 8px from each section header since we have added the section header in 7.15, so which setting that allows the "value" field to be indented at the right position will work here.

rylnd · 2021-08-03T17:46:47Z

x-pack/plugins/security_solution/public/common/components/hover_actions/index.tsx

@@ -40,6 +40,7 @@ export const AdditionalContent = styled.div`
 AdditionalContent.displayName = 'AdditionalContent';

 const StyledHoverActionsContainer = styled.div<{ $showTopN: boolean; $showOwnFocus: boolean }>`
+  min-width: 138px;


Ditto on questioning the magic number here

This was to ensure that the Icon Row never wraps as per Monina's request here

rylnd · 2021-08-03T17:57:49Z

x-pack/plugins/security_solution/public/common/components/event_details/helpers.tsx

@@ -196,9 +198,13 @@ export const onEventDetailsTabKeyPressed = ({
  }
 };

+const StyledH5 = styled.h5`
+  line-height: 1.7rem;


Is the intention here to give more spacing between table rows? If so, should this be applied to StyledEuiInMemoryTable so that all rows are at least a min height?

yeah so this is a good question - previously the table in Alert Summary tab and the table in Threat Intel tab had the same line height, and at a certain point they went out of sync. I looked through the changes and I was unable to identify how this change took place (didn't see any CSS associated with the change, and the tables are the same, with the same compressed prop that makes the rows closer to each other) - it could be that the ActionCell component is enlarging the row height in the Alert Summary tab now that I think about it. I added this explicit line height to ensure that all rows have the same line height.

ecezalp · 2021-08-03T19:06:41Z

There's some dead code; just a few lines that could be cleaned up

removed, thanks for noticing!

I had nits/questions about some hardcoded widths/heights

looped in Yiyang for the column one to get the desired indentation percentage / value, answered the other two questions with no code changes, please let me know if further changes would be desired

Having to thread through a parallel, redundant set of data for these action fields isn't great

Not a requirement here, but a better approach might be to pass e.g. a buildActions function prop to ThreatSummaryView or some other kind of control inversion

leaving this for now, will come back at the end of sprint if there is any spare time remaining

One other UX concern: the interaction on hover isn't nice: you have to hover the blank area to the right of a field's value to see the actions available; hovering anywhere else has no effect. This is bad enough that I would consider it a bug.

@monina-n @yiyangliu9286 can provide more insight on this one, I am happy to implement any other changes as desired

kibanamachine · 2021-08-03T21:47:52Z

💚 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`	6.4MB	6.4MB	+1.7KB

History

💚 Build #142702 succeeded 885df9d
💚 Build #141821 succeeded 3da36e9
💔 Build #141803 failed deb5694
💔 Build #141778 failed 1675427
💔 Build #141761 failed b8d8573

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

cc @ecezalp

kibanamachine · 2021-08-03T21:50:27Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

Co-authored-by: Ece Özalp <[email protected]>

monina-n · 2021-08-09T17:05:46Z

hey @ecezalp! sorry for the late response. in terms of a response to:

One other UX concern: the interaction on hover isn't nice: you have to hover the blank area to the right of a field's value to see the actions available; hovering anywhere else has no effect. This is bad enough that I would consider it a bug.

I couldn't tell from the screenshots, but the user should be able to hover over the field itself to reveal the hover action menu, not just over the space where the hover action menu is itself. I think if it appears when a user hovers on both areas (above the value and above where the action menu appears), that should work

ecezalp · 2021-08-10T20:03:12Z

hey @ecezalp! sorry for the late response. in terms of a response to:

One other UX concern: the interaction on hover isn't nice: you have to hover the blank area to the right of a field's value to see the actions available; hovering anywhere else has no effect. This is bad enough that I would consider it a bug.

I couldn't tell from the screenshots, but the user should be able to hover over the field itself to reveal the hover action menu, not just over the space where the hover action menu is itself. I think if it appears when a user hovers on both areas (above the value and above where the action menu appears), that should work

according to our slack conversation, @angorayc will be taking care of this feature as a part of her UX changes. Thanks for bringing it up!

angorayc · 2021-08-11T13:15:14Z

wip #108192

updates Alert Summary UI

25c2809

ecezalp requested a review from a team as a code owner July 28, 2021 19:59

ecezalp marked this pull request as draft July 28, 2021 20:00

ecezalp self-assigned this Jul 28, 2021

ecezalp added auto-backport Deprecated - use backport:version if exact versions are needed Team: CTI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.15.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Jul 28, 2021

Merge branch 'master' into 1474-2

386f212

rylnd reviewed Jul 28, 2021

View reviewed changes

x-pack/plugins/security_solution/public/common/components/event_details/event_details.tsx Show resolved Hide resolved

rylnd reviewed Jul 28, 2021

View reviewed changes

...urity_solution/public/common/components/event_details/table/use_action_cell_data_provider.ts Outdated Show resolved Hide resolved

rylnd reviewed Jul 28, 2021

View reviewed changes

...security_solution/public/common/components/event_details/cti_details/threat_details_view.tsx Show resolved Hide resolved

ecezalp requested a review from angorayc July 29, 2021 12:18

ecezalp added 2 commits July 29, 2021 09:57

incorporates Angela's changes

c71a35b

adds timeline support

1126fee

ecezalp added 3 commits July 29, 2021 15:17

fixes test

b8d8573

Merge branch 'master' into 1474-2

d1fe3a9

remove unused prop

1675427

ecezalp marked this pull request as ready for review July 29, 2021 19:41

fix type

43ef43d

ecezalp changed the title ~~updates Alert Summary UI~~ [CTI] updates Alert Summary UI Jul 29, 2021

kibanamachine added 2 commits July 29, 2021 16:56

Merge branch 'master' into 1474-2

deb5694

Merge branch 'master' into 1474-2

3da36e9

ecezalp requested a review from rylnd August 3, 2021 13:24

Merge branch 'master' into 1474-2

885df9d

rylnd approved these changes Aug 3, 2021

View reviewed changes

ecezalp added 2 commits August 3, 2021 15:07

addresses comments

9976670

bring back lodash/fp to fix typeerror

aea2218

ecezalp enabled auto-merge (squash) August 3, 2021 19:34

ecezalp merged commit c6a7062 into elastic:master Aug 3, 2021

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 3, 2021

[CTI] updates Alert Summary UI (elastic#107081)

b3774e8

kibanamachine mentioned this pull request Aug 3, 2021

[7.x] [CTI] updates Alert Summary UI (#107081) #107605

Merged

kibanamachine added a commit that referenced this pull request Aug 4, 2021

[CTI] updates Alert Summary UI (#107081) (#107605)

9234cba

Co-authored-by: Ece Özalp <[email protected]>

streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021

[CTI] updates Alert Summary UI (elastic#107081)

b80b6af

angorayc mentioned this pull request Aug 11, 2021

[Security Solution] flyout UI adjustment #108192

Merged

3 tasks

elasticmachine unassigned ecezalp Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CTI] updates Alert Summary UI #107081

[CTI] updates Alert Summary UI #107081

ecezalp commented Jul 28, 2021 •

edited

Loading

ecezalp commented Jul 28, 2021

rylnd Jul 28, 2021

ecezalp Jul 28, 2021

rylnd Jul 28, 2021 •

edited

Loading

monina-n commented Jul 29, 2021

elasticmachine commented Jul 29, 2021

ecezalp commented Jul 29, 2021

ecezalp commented Aug 3, 2021

rylnd left a comment

rylnd Aug 3, 2021

ecezalp Aug 3, 2021 •

edited

Loading

rylnd Aug 3, 2021

ecezalp Aug 3, 2021

rylnd Aug 3, 2021

ecezalp Aug 3, 2021

yiyangliu9286 Aug 3, 2021

rylnd Aug 3, 2021

ecezalp Aug 3, 2021

rylnd Aug 3, 2021

ecezalp Aug 3, 2021 •

edited

Loading

ecezalp commented Aug 3, 2021 •

edited

Loading

kibanamachine commented Aug 3, 2021

kibanamachine commented Aug 3, 2021

monina-n commented Aug 9, 2021

ecezalp commented Aug 10, 2021

angorayc commented Aug 11, 2021

[CTI] updates Alert Summary UI #107081

[CTI] updates Alert Summary UI #107081

Conversation

ecezalp commented Jul 28, 2021 • edited Loading

Summary

Checklist

ecezalp commented Jul 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rylnd Jul 28, 2021 • edited Loading

Choose a reason for hiding this comment

monina-n commented Jul 29, 2021

elasticmachine commented Jul 29, 2021

ecezalp commented Jul 29, 2021

ecezalp commented Aug 3, 2021

rylnd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecezalp Aug 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecezalp Aug 3, 2021 • edited Loading

Choose a reason for hiding this comment

ecezalp commented Aug 3, 2021 • edited Loading

kibanamachine commented Aug 3, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Aug 3, 2021

💚 Backport successful

monina-n commented Aug 9, 2021

ecezalp commented Aug 10, 2021

angorayc commented Aug 11, 2021

ecezalp commented Jul 28, 2021 •

edited

Loading

rylnd Jul 28, 2021 •

edited

Loading

ecezalp Aug 3, 2021 •

edited

Loading

ecezalp Aug 3, 2021 •

edited

Loading

ecezalp commented Aug 3, 2021 •

edited

Loading