[Security Solution] Changes rules table tag display #77102

dplumlee · 2020-09-09T18:53:22Z

Summary

Changes rules table tag display to show 3 initial tags and then display the full list in a scrollable popover

Screenshots

Limits width of tag popover (has title hover)

Displays a popover button if the tag amount is over 3

Popover limits tag length and is scrollable

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
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-09-09T18:59:37Z

Pinging @elastic/siem (Team:SIEM)

yctercero · 2020-09-10T20:15:42Z

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx

+                <EuiBadge
+                  color="hollow"
+                  key={`${tag}-${i}`}
+                  data-test-subj={`rules-table-column-tags-${i}`}


Could we add a quick test in x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.test.tsx that tests that all tags are rendered/visible?

yctercero

LGTM! Just one small comment on adding a quick unit test if you can.

spong · 2020-09-16T21:50:12Z

@elasticmachine merge upstream

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx

spong · 2020-09-18T00:25:30Z

For this I think we'll still want some sort of overflow option as things can get quite hairy with vertical space when there's quite a few tags (which sounds like the protections team will be adding plenty of in the coming release 😅 ):

There's two bounds to cover here:

When a tag is reeeeeeally long
When there are a lot of tags

And looks like there's five touch points for these bounds:

Rule Creation / Edit Rules
Rule Details
Rules Table
Rules Table Tag Selection
Detection Alerts Table

Going over these:

Rules Creation looks good on both fronts! 👍 Long tags are truncated per line, and a fair number of tags display without too much vertical space (can always set a max height and enable scroll if this becomes an issue)

Rule Details fairs pretty well too, but we're working with quite a bit more space here. Doesn't look like reeeeeeally long tags truncate, but this should be a corner case.

Rules Table, as above, is probably where this will be a pain point for our users. Reeeeeeally long tags truncate per line and have hover title hover text 👍, so that's all good, but many tags will keep expanding the row height. For this, I think the easiest and nicest solution for our users would be to cap the row height, and then expose the overflow tags in a scrollable popover -- could just reuse the tag selection component w/ selection disabled.
Rules Table Tag Selection looks good, but gets a bit unwieldy when dealing with reeeeeeally long tags. As above, this is a corner case, but it'd be an easy fix to set a max-width on the popover and ensure there's either horizontal scroll or at least title hover text.

Detection Alerts Table looks to be having the same issue as Rule Name ([SIEM] Signal's rule name is not showing properly on signals table #65764) and just shows the empty value --. On expansion of the alert, the tags display fairly well, one per line, with no truncation, but as mentioned above this should be a corner case.

All that said, no need to fix all these in this much-appreciated enhancement! 😅 I do think we should address item 3. though as with the influx of tags from protections this release the row-heights are going to be all over the place if we don't set a max, so this would probably be the most impactful to users.

dplumlee · 2020-10-01T21:42:35Z

@spong @yctercero I refactored a lot of this including the overall design to address most of the issues garrett brought up so thought i'd re-request reviews

...plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx

spong

LGTM! 👍 Thanks for cleaning up the UX so things stay manageable on the All Rules table @dplumlee! 🙂

kibanamachine · 2020-10-02T22:18:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1cbc331

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`securitySolution`	1978	1979	+1

async chunks size

id	before	after	diff
`securitySolution`	10.3MB	10.3MB	+5.3KB

History

💚 Build #78993 succeeded 8f139cf
💔 Build #78963 failed bb163a9
💔 Build #75000 failed 50f3060
💚 Build #73801 succeeded 40f31ac
💛 Build #73505 was flaky cac806b

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

* master: (128 commits) add core-js production dependency (elastic#79395) Add support for sharing saved objects to all spaces (elastic#76132) [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038) load js-yaml lazily (elastic#79092) skip flaky suite (elastic#77278) Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341) [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988) [Security Solutions][Detection Engine] Updates the edit rules page to:wq! only have what is selected for editing (elastic#79233) Cleanup yarn.lock from duplicates (elastic#66617) [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052) [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291) [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358) [babel/register] remove from build (take 2) (elastic#79379) [Security Solution] Changes rules table tag display (elastic#77102) define integrationTestRoot in config file and use to define screensho… (elastic#79247) Revert "[babel/register] remove from build (elastic#79176)" skip flaky suite (elastic#75241) [Uptime] Synthetics UI (elastic#77960) [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812) [babel/register] remove from build (elastic#79176) ...

* master: (288 commits) add core-js production dependency (elastic#79395) Add support for sharing saved objects to all spaces (elastic#76132) [Alerting UI] Display a banner to users when some alerts have failures, added alert statuses column and filters (elastic#79038) load js-yaml lazily (elastic#79092) skip flaky suite (elastic#77278) Fix agentPolicyUpdateEventHandler() to use app context soClient for creation of actions (elastic#79341) [Security Solution] Untitled Timeline created when first action is to add note (elastic#78988) [Security Solutions][Detection Engine] Updates the edit rules page to only have what is selected for editing (elastic#79233) Cleanup yarn.lock from duplicates (elastic#66617) [kbn/optimizer] implement more efficient auto transpilation for node (elastic#79052) [Ingest Manager] Rename Fleet setup and requirement, Fleet => Central… (elastic#79291) [core/server/plugins] don't run discovery in dev server parent process (take 2) (elastic#79358) [babel/register] remove from build (take 2) (elastic#79379) [Security Solution] Changes rules table tag display (elastic#77102) define integrationTestRoot in config file and use to define screensho… (elastic#79247) Revert "[babel/register] remove from build (elastic#79176)" skip flaky suite (elastic#75241) [Uptime] Synthetics UI (elastic#77960) [Security Solution] [Detections] Only display actions options if user has "read" privileges (elastic#78812) [babel/register] remove from build (elastic#79176) ...

elasticmachine · 2021-09-23T14:25:54Z

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

dplumlee added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 9, 2020

dplumlee self-assigned this Sep 9, 2020

dplumlee added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 9, 2020

dplumlee marked this pull request as ready for review September 9, 2020 18:59

dplumlee requested review from a team as code owners September 9, 2020 18:59

dplumlee force-pushed the rules-table-tag-display branch from cac806b to 40f31ac Compare September 10, 2020 17:20

yctercero reviewed Sep 10, 2020

View reviewed changes

yctercero approved these changes Sep 10, 2020

View reviewed changes

spong reviewed Sep 17, 2020

View reviewed changes

x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx Outdated Show resolved Hide resolved

dplumlee marked this pull request as draft September 21, 2020 19:40

dplumlee added 4 commits October 1, 2020 15:34

changes tag display

e154e85

tries new approach

d5d82bd

switches to non stacked tag display

6feb5d6

adds tests and addresses comments

bb163a9

dplumlee force-pushed the rules-table-tag-display branch from 50f3060 to bb163a9 Compare October 1, 2020 21:35

dplumlee marked this pull request as ready for review October 1, 2020 21:40

dplumlee requested review from spong and yctercero October 1, 2020 21:41

dplumlee added 2 commits October 1, 2020 16:00

deletes extra file

5c9865a

adds back data-test-subj

8f139cf

spong reviewed Oct 2, 2020

View reviewed changes

...plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx Show resolved Hide resolved

spong reviewed Oct 2, 2020

View reviewed changes

...plugins/security_solution/public/detections/pages/detection_engine/rules/all/tag_display.tsx Outdated Show resolved Hide resolved

spong approved these changes Oct 2, 2020

View reviewed changes

switches to sorted tags

1cbc331

dplumlee merged commit bd80d3c into elastic:master Oct 2, 2020

dplumlee deleted the rules-table-tag-display branch October 2, 2020 22:19

dplumlee mentioned this pull request Oct 2, 2020

[7.x] [Security Solution] Changes rules table tag display (#77102) #79380

Merged

dplumlee added a commit that referenced this pull request Oct 3, 2020

[Security Solution] Changes rules table tag display (#77102) (#79380)

33d3784

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Changes rules table tag display #77102

[Security Solution] Changes rules table tag display #77102

dplumlee commented Sep 9, 2020 •

edited

Loading

elasticmachine commented Sep 9, 2020

yctercero Sep 10, 2020

yctercero left a comment

spong commented Sep 16, 2020

spong commented Sep 18, 2020 •

edited

Loading

dplumlee commented Oct 1, 2020

spong left a comment

kibanamachine commented Oct 2, 2020

elasticmachine commented Sep 23, 2021

[Security Solution] Changes rules table tag display #77102

[Security Solution] Changes rules table tag display #77102

Conversation

dplumlee commented Sep 9, 2020 • edited Loading

Summary

Screenshots

Checklist

For maintainers

elasticmachine commented Sep 9, 2020

yctercero Sep 10, 2020

Choose a reason for hiding this comment

yctercero left a comment

Choose a reason for hiding this comment

spong commented Sep 16, 2020

spong commented Sep 18, 2020 • edited Loading

dplumlee commented Oct 1, 2020

spong left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 2, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

History

elasticmachine commented Sep 23, 2021

dplumlee commented Sep 9, 2020 •

edited

Loading

spong commented Sep 18, 2020 •

edited

Loading