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.

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

[Security Solution][Detections] Improves UX for Rules and Exceptions tables #118940

Merged

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Nov 17, 2021

Issues:

Summary

  • Rules & Rules Monitoring tables now truncate text in columns and display tooltips
  • Exceptions table now has tooltips and proper truncation for columns
  • Tags column completely redesigned for Rules table: now it doesn't display tags but only button(labeled by number of tags) with popover. See Figma
  • Popover component redesigned. Created a common component PopoverItems instead of 2 similar DisplayTags & ExceptionOverflow. For reference [Security Solution] Create a common component for table overview popovers #103782
  • Exceptions table reconfigured to use Actions column properly. I.e. using property actions in column config, rather having action as column itself Eui table actions guide
  • Capitalized Rule status name in Rules tables

Rules

Before

Screenshot 2021-11-19 at 10 18 54

After

Screenshot 2021-11-18 at 19 23 04

Rules Monitoring

Before

Screenshot 2021-11-19 at 10 19 11

After

Screenshot 2021-11-18 at 19 23 38

Exceptions

Before

Screenshot 2021-11-19 at 10 18 24

After

Screenshot 2021-11-18 at 19 21 56

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@vitaliidm
Copy link
Contributor Author

@elasticmachine merge upstream

@vitaliidm vitaliidm added enhancement New value added to drive a business result Feature:Rule Exceptions Security Solution Detection Rule Exceptions area Feature:Rule Management Security Solution Detection Rule Management area Feature:Rule Monitoring Security Solution Detection Rule Monitoring area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team v8.1.0 labels Nov 18, 2021
@vitaliidm vitaliidm self-assigned this Nov 18, 2021
@vitaliidm vitaliidm changed the title [Security Solution][Detections] Enhances Rules are UI tables [Draft] [Security Solution][Detections] Improves UX for Rules and Exceptions tables Nov 19, 2021
@vitaliidm
Copy link
Contributor Author

@elasticmachine merge upstream

@spong
Copy link
Member

spong commented Nov 22, 2021

These are great improvements @vitaliidm -- thank you! In testing I opened this issue for setting max-length on our inputs and providing better truncation/overflow within the Rule Creation/Edit/Details pages.

While these test strings are definitely not in good-faith of actual user values 😅 , I do wonder if a scrollable popover (vs current tooltip) would be a better general purpose solution for these types of situations. While less of a concern for Rule Name (as shown below), this would probably be beneficial for Descriptions, Investigation Guides, and other free text fields, etc. Could always just do it as a fallback to the tooltip if field is > certain length.

No changes requested here, just commenting 🙂 cc @yiyangliu9286

@@ -363,6 +381,7 @@ export const getMonitoringColumns = (
</EuiText>
),
width: '14%',
truncateText: true,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like there's some overlap with the tooltip hover icon:

Looks to be just the EuiTooltip ones -- the Last Gap custom one seems to be 👍 though:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, fixed that


const PopoverWrapper = styled(EuiBadgeGroup)`
max-height: 200px;
max-width: 600px;
Copy link
Member

@spong spong Nov 22, 2021

Choose a reason for hiding this comment

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

All hail max-width (and height)! 😎 Keeps this popup in check for ridiculously large (and large number of) tags 👍

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, tested locally and performed cursory code review -- LGTM! 👍 🚀 Thanks for all the UI/UX enhancement and general code cleanup + tests here @vitaliidm! 🙂

I did leave a couple nits (overlap w/ column tooltips, and using , instead of CSS if possible), but they're not not blockers. Also as mentioned in a separate comment, created this issue for addressing some max-length/overflow issues we've got floating around that I found in testing.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2754 2755 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 4.5MB 4.5MB +69.0B

History

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

cc @vitaliidm

Copy link
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@vitaliidm vitaliidm merged commit 7927bd4 into elastic:main Nov 24, 2021
@vitaliidm vitaliidm added the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 24, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 24, 2021
…tables (elastic#118940)

[Security Solution][Detections] Improves UX for Rules and Exceptions tables
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 24, 2021
…tables (#118940) (#119585)

[Security Solution][Detections] Improves UX for Rules and Exceptions tables

Co-authored-by: Vitalii Dmyterko <[email protected]>
@vitaliidm vitaliidm deleted the security-solution/ui-tables-enhancements branch November 24, 2021 12:28
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
…tables (#118940)

[Security Solution][Detections] Improves UX for Rules and Exceptions tables
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…tables (elastic#118940)

[Security Solution][Detections] Improves UX for Rules and Exceptions tables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed enhancement New value added to drive a business result Feature:Rule Exceptions Security Solution Detection Rule Exceptions area Feature:Rule Management Security Solution Detection Rule Management area Feature:Rule Monitoring Security Solution Detection Rule Monitoring area release_note:enhancement Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture v8.0.0 v8.1.0
Projects
None yet
6 participants