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][Resolver] Analyzed event styling #77115

Merged
merged 5 commits into from
Sep 14, 2020
Merged

[Security Solution][Resolver] Analyzed event styling #77115

merged 5 commits into from
Sep 14, 2020

Conversation

michaelolo24
Copy link
Contributor

Summary

1. Adds styling for the node that was selected from the timeline for Analysis.

analyzed_node

2. Selection outline for nodes are now all the same (i.e. running and terminated nodes have same darker blue outline)

Screen Shot 2020-09-09 at 4 42 21 PM

Checklist

Delete any items that are not applicable to this PR.

@michaelolo24 michaelolo24 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver Feature:Resolver Security Solution Resolver feature v7.10.0 labels Sep 9, 2020
@michaelolo24 michaelolo24 requested review from a team as code owners September 9, 2020 20:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Resolver)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

michaelolo24
michaelolo24 commented Sep 9, 2020
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/assets.tsx
@@ -475,7 +475,7 @@ export const useResolverTheme = (): {
),
isLabelFilled: false,
labelButtonFill: 'primary',
strokeColor: `${theme.euiColorPrimary}33`, // 33 = 20% opacity
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maintaining the same outline color since the more translucent outline now signifies the analyzed event

michaelolo24
michaelolo24 commented Sep 9, 2020
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx
@@ -351,6 +358,20 @@ const UnstyledProcessEventDot = React.memo(
height={markerSize * 1.5}
className="backing"
/>
{isOrigin && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a second backing layer for the origin rather than modifying the existing one to maintain the outline effect on top of the more translucent line. If there is a better way to do this lmk.

bkimmel
bkimmel reviewed Sep 10, 2020
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx
{isOrigin && (
<use
xlinkHref={`#${SymbolIds.processCubeActiveBacking}`}
fill="transparent" // Transparent so we don't double up on the default hover
Copy link
Contributor

Choose a reason for hiding this comment

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

ℹ️ Does this make it through the use shadow boundary? This is something I have to look up again and again every time it comes up: https://tympanus.net/codrops/2015/07/16/styling-svg-use-content-css/

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, so I guess the fill does get through the boundary using the presentation attribute on the outer use.

bkimmel
bkimmel approved these changes Sep 10, 2020
View reviewed changes
Copy link
Contributor

@bkimmel bkimmel left a comment

Choose a reason for hiding this comment

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

Nice!

@michaelolo24
Copy link
Contributor Author

@elasticmachine merge upstream

jonathan-buttner
jonathan-buttner approved these changes Sep 10, 2020
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx Outdated
@@ -99,7 +100,9 @@ const UnstyledProcessEventDot = React.memo(
*/
timeAtRender: number;
}) => {
const resolverComponentInstanceID = useSelector(selectors.resolverComponentInstanceID);
const resolverComponentInstanceID = useSelector((state: ResolverState) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own edification, what's the difference with using the arrow function vs just passing in selectors.resolverComponentInstanceID? Does it not pass the state by default?

Copy link
Contributor Author

@michaelolo24 michaelolo24 Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, it doesn't change anything actually. I just thought some of these weren't updated based on some other useSelector changes I remembered seeing, but those changes were primarily due to how useSelector handles thunks, but these aren't thunks so it was an unnecessary change. I reverted it 😄

oatkiller
oatkiller reviewed Sep 10, 2020
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx Outdated
@@ -384,6 +401,15 @@ const UnstyledProcessEventDot = React.memo(
color={colorMap.descriptionText}
isDisplaying={isShowingDescriptionText}
>
{isOrigin && (
<>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe put the 'analyzed event' text, the dot, and the descriptionText into a single formatted message. This will allow translators to, for example, swap out the dot with something locale specific, or reverse the order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, makes sense. I've updated it

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
securitySolution 10.0MB +1.4KB 10.0MB

History

  • 💔 Build #73825 failed fb249df6d12dccd11a4dbc9d1f8b7afab9ce5f12
  • 💔 Build #73787 failed 12fd862351acdc71cf992205aa4832d48b960b94
  • 💚 Build #73721 succeeded 2c2273c4686ecdf7ae736e89482a5c1ae587be93
  • 💚 Build #73534 succeeded b808ad0e4bb89d93b6faa33456aad07316d96574

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

@michaelolo24 michaelolo24 merged commit 5c457d1 into elastic:master Sep 14, 2020
@michaelolo24 michaelolo24 deleted the analyzed-event-styling branch September 14, 2020 16:56
@michaelolo24 michaelolo24 mentioned this pull request Sep 14, 2020
Merged
michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Sep 14, 2020
@michaelolo24
[Security Solution][Resolver] Analyzed event styling (elastic#77115)
77b9a54
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
@gmmorris
Merge branch 'master' into actions/test-connector
91ea9c5 
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
@gmmorris
Merge branch 'master' into alerts/improve-find-pref
b4729a4 
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
michaelolo24 added a commit that referenced this pull request Sep 14, 2020
@michaelolo24
[7.x] [Security Solution][Resolver] Analyzed event styling (#77115) (#…
9310afc 
…77375)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkimmel bkimmel bkimmel approved these changes

@oatkiller oatkiller oatkiller left review comments

@monina-n monina-n monina-n approved these changes

@jonathan-buttner jonathan-buttner jonathan-buttner approved these changes

Assignees
No one assigned
Labels
Feature:Resolver Security Solution Resolver feature release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@michaelolo24 @elasticmachine @kibanamachine @oatkiller @bkimmel @jonathan-buttner @monina-n