[Security Solution][Resolver] Word-break long titles in related event… #75926

bkimmel · 2020-08-25T20:33:35Z

… description lists

^ and yes, that's ironic.

In order to make meta info titles in the related detail panel more readable and to ensure that they don't overflow onto the data entry next to them, we're changing the style direction to overflow-wrap: word-break on the title container. We're also salting the title text with U+200B (Zero-Width spaces) around punctuation/word boundaries so that when the titles wrap, they wrap around periods, commas, etc. to improve the readability. We're also adding a test to the selector that does that work to confirm that the breaking characters are being set correctly. On the front-end, for User Agents that support it, we're replacing the U+200B spaces with <wbr /> tags because the tags don't get picked up on copy/paste. In the case that the isn't supported, the U+200B are kept as a fallback with the same line-breaking behavior.

Summary

Long property titles in the Related Event Panel description lists can overflow onto the data entry like so:

This PR word-breaks the title at the boundary of its element so that it wraps and extends the box it's in instead of overflowing the data entry as pictured above.

Screenshot

using the GeneratedText component to prevent the descriptive name from triggering a horizontal scroll:

… description lists

bkimmel · 2020-08-25T20:41:29Z

@elasticmachine merge upstream

elasticmachine · 2020-08-25T20:42:05Z

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

elasticmachine · 2020-08-25T20:42:05Z

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

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

oatkiller

Thoughts on using one of these instead? https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap

…kimmel/kibana into resolver/fix-dl-title-overflow

bkimmel · 2020-08-25T22:23:22Z

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

@@ -23,6 +23,19 @@ import * as selectors from '../../store/selectors';
 import { useResolverDispatch } from '../use_resolver_dispatch';
 import { PanelContentError } from './panel_content_error';

+/**


❔ It's really only for display purposes, but should we move it to a selector?

bkimmel · 2020-08-26T00:43:26Z

Thoughts on using one of these instead? https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap

I got that here now: f9c5278#diff-6b7ec89ef2cc83904892488878bf7257R24

bkimmel · 2020-08-26T00:44:54Z

x-pack/plugins/security_solution/public/resolver/store/data/selectors.ts

+  // Seed boundaries beetween word and non-word characters with Zero-width spaces to allow for text to be wrapped when it overflow its container.
+
+  return potentiallyLongString.replace(/(\w\W)|(\W\w)/g, (boundary: string) => {
+    return `${boundary[0]}\u200b${boundary[1]}`;


❔ @oatkiller I ended up pulling this into the selector here. Is this OK?

bkimmel · 2020-08-26T01:26:11Z

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

+    formattedDate,
+  ] = useSelector(selectors.relatedEventDisplayInfoByEntityAndSelfId)(
+    processEntityId,
+    relatedEventId
  );



ℹ️ pulled a bunch of this junk into a selector

I see the rationale of having view specific logic like this tied with the component rather than the data. Even though the it doesn't actually change any of the raw data in state, I think of selectors as specifically handling some kind of expensive/calculative logic rather than anything display related. Maybe a bit off here, but I consider this more fitting in the assets.tsx file. I don't think that name is particularly accurate anymore, but the concept of:
here are functions/components that, given some data, apply visual changes
is more representative. The name should be modified/changed when styles are cleaned up, but I think that may be a better compromise that will still allow this to be testable?

Thinking out loud a bit, I think something a la a style-utils or something folder with separate file for visual components (cube assets etc...), text modifications, color modifications, and whatever else might make sense or not...probably something I missed here 😂 . Anyways, not really for this PR, but just thinking how we can begin to improve how we handle styles/visual changes

I think most of this stuff here does things that Robert has referred to in the past as "business logic" like counting things, filtering things out, selecting which category to display, etc. (and prefers that they be done in selectors) but I completely agree about thinking a little more deeply about how we organize visual components. I ended up bringing the "break-hinting" stuff back up to the front, b/c @kqualters-elastic made a good point that putting it in the selector just creates another place it can break (and Robert already had that GeneratedText thing done) but that might be a good first entry in that style-utils (until it finds a home in EUI or something later).

bkimmel · 2020-08-26T01:26:47Z

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

 import * as event from '../../../../common/endpoint/models/event';
 import { ResolverEvent } from '../../../../common/endpoint/types';
 import * as selectors from '../../store/selectors';
 import { useResolverDispatch } from '../use_resolver_dispatch';
 import { PanelContentError } from './panel_content_error';

-/**


ℹ️ moved to selector

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

michaelolo24 · 2020-08-26T20:50:34Z

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx

@@ -90,6 +56,49 @@ const TitleHr = memo(() => {
 });
 TitleHr.displayName = 'TitleHR';

+const GeneratedText = React.memo(function ({ children }) {


Referencing @kqualters-elastic comment from slack, I would cc a member of the EUI team to see about moving this logic there and getting that process started

Yeah, I think that's a good idea for the long term, too. Seems like it would be useful in a lot of places. @chandlerprall has always been helpful, maybe he could give us an idea.

for any context since I still have a terrible habit of saying too much there and not enough here: long term imo it might make sense to have this logic live in https://github.com/elastic/eui/tree/master/src/components/text as text_wrap.tsx and text_wrap.test.tsx, then used in the base text component conditionally based on the presence of a prop. Add a few snapshot tests that verify the presence of 's where you expect them, add as many test cases as needed

Feel free to open an issue in EUI for discussion! It's possible we have this solved via a css utility class.

x-pack/plugins/security_solution/public/resolver/store/data/selectors.ts

bkimmel · 2020-08-26T22:40:07Z

@monina-n approved earlier via slack conversation

kibanamachine · 2020-08-27T00:44:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9bc1de8

Build metrics

async chunks size

id	value	diff	baseline
securitySolution	9.5MB	+10.9KB	9.5MB

page load bundle size

id	value	diff	baseline
securitySolution	807.8KB	+1.0B	807.8KB

History

💔 Build #70715 failed bccb7ab
💔 Build #70712 failed e7c1134
💚 Build #70626 succeeded 0be1d08
💚 Build #70594 succeeded 9efb55a
💚 Build #70587 succeeded b64164f

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

#75926) (#76092) * [Security Solution][Resolver] Word-break long titles in related event description lists * word-break long titles at non-word boundaries Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

[Security Solution][Resolver] Word-break long titles in related event…

b1e2bac

… description lists

bkimmel marked this pull request as ready for review August 25, 2020 20:35

bkimmel requested review from a team as code owners August 25, 2020 20:35

Merge branch 'master' into resolver/fix-dl-title-overflow

dd77617

bkimmel added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Resolver Security Solution Resolver feature Team:Endpoint Data Visibility Team managing the endpoint resolver labels Aug 25, 2020

bkimmel commented Aug 25, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx Show resolved Hide resolved

oatkiller reviewed Aug 25, 2020

View reviewed changes

Brent Kimmel added 2 commits August 25, 2020 17:41

word-break long titles at non-word boundaries

889c2f2

Merge branch 'resolver/fix-dl-title-overflow' of https://github.com/b…

69d3bf8

…kimmel/kibana into resolver/fix-dl-title-overflow

bkimmel commented Aug 25, 2020

View reviewed changes

Brent Kimmel added 3 commits August 25, 2020 18:25

grammar

2ba7272

Move stuff to selectors

61a84a7

remove unused helpers

5c9b508

bkimmel commented Aug 26, 2020

View reviewed changes

add test for new selector

a96fb7b

bkimmel commented Aug 26, 2020

View reviewed changes

Brent Kimmel added 4 commits August 25, 2020 21:46

change to CSS standard

f9c5278

Replace ZWSP with <wbr/> when UA allows

0c80365

move display prep to function

ff229ec

fix comment

8389676

kqualters-elastic reviewed Aug 26, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/view/panels/related_event_detail.tsx Outdated Show resolved Hide resolved

Brent Kimmel added 5 commits August 26, 2020 10:34

remove wbr check

4c48fc5

move breakers back to view

2a10e03

using GeneratedText

b64164f

using GeneratedText for description too

9efb55a

run descriptive name -> GeneratedText too

0be1d08

michaelolo24 reviewed Aug 26, 2020

View reviewed changes

michaelolo24 approved these changes Aug 26, 2020

View reviewed changes

kqualters-elastic reviewed Aug 26, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/store/data/selectors.ts Outdated Show resolved Hide resolved

K Qualters Review: move types

e7c1134

kqualters-elastic approved these changes Aug 26, 2020

View reviewed changes

CI Typecheck

bccb7ab

CI Lint

9bc1de8

bkimmel merged commit 4294232 into elastic:master Aug 27, 2020

bkimmel deleted the resolver/fix-dl-title-overflow branch August 27, 2020 01:06

bkimmel mentioned this pull request Aug 27, 2020

[7.x] [Security Solution][Resolver] Word-break long titles in related event… (#75926) #76092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Resolver] Word-break long titles in related event… #75926

[Security Solution][Resolver] Word-break long titles in related event… #75926

bkimmel commented Aug 25, 2020 •

edited

Loading

bkimmel commented Aug 25, 2020

elasticmachine commented Aug 25, 2020

elasticmachine commented Aug 25, 2020

oatkiller left a comment

bkimmel Aug 25, 2020

bkimmel commented Aug 26, 2020 •

edited

Loading

bkimmel Aug 26, 2020

bkimmel Aug 26, 2020

michaelolo24 Aug 26, 2020 •

edited

Loading

michaelolo24 Aug 26, 2020

bkimmel Aug 26, 2020

bkimmel Aug 26, 2020

michaelolo24 Aug 26, 2020

bkimmel Aug 26, 2020

kqualters-elastic Aug 26, 2020

chandlerprall Aug 27, 2020

bkimmel commented Aug 26, 2020

kibanamachine commented Aug 27, 2020

[Security Solution][Resolver] Word-break long titles in related event… #75926

[Security Solution][Resolver] Word-break long titles in related event… #75926

Conversation

bkimmel commented Aug 25, 2020 • edited Loading

Summary

Screenshot

bkimmel commented Aug 25, 2020

elasticmachine commented Aug 25, 2020

elasticmachine commented Aug 25, 2020

oatkiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkimmel commented Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 Aug 26, 2020 • 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

bkimmel commented Aug 26, 2020

kibanamachine commented Aug 27, 2020

💚 Build Succeeded

Build metrics

async chunks size

page load bundle size

History

bkimmel commented Aug 25, 2020 •

edited

Loading

bkimmel commented Aug 26, 2020 •

edited

Loading

michaelolo24 Aug 26, 2020 •

edited

Loading