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] Table UI: consistent "See all" buttons #117029

Closed
3 tasks done
banderror opened this issue Nov 1, 2021 · 11 comments · Fixed by #118940
Closed
3 tasks done

[Security Solution][Detections] Table UI: consistent "See all" buttons #117029

banderror opened this issue Nov 1, 2021 · 11 comments · Fixed by #118940
Assignees
Labels
enhancement New value added to drive a business result 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

Comments

@banderror
Copy link
Contributor

banderror commented Nov 1, 2021

Related to: #103782, #115948 (review)

Summary

We have "See all" buttons in two of our tables, which show the whole content of a cell in a popover. The purpose is the same, but there's a difference in how they look like in these tables:

Rule Management table:

Exceptions table:

Let's make them look consistently in both tables, and implement this using a common component.

Design

Figma file link

Acceptance Criteria

  • The "see all" buttons and popover look consistently in both the Rule Management and Exceptions tables.
  • It is implemented via a common component. This component is generic enough to contain custom content inside: tags, links, etc. There's a dedicated ticket for this technical debt: [Security Solution] Create a common component for table overview popovers #103782 - we will close it together with this ticket.
  • Existing custom components for "See all" functionality are replaced with the common one and removed from the repo.
@banderror banderror added enhancement New value added to drive a business result technical debt Improvement of the software architecture and operational architecture needs design Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team labels Nov 1, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

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

@vitaliidm
Copy link
Contributor

Scope of work:

  1. Replace button 'See all', so it look consistent
  2. Refactor 2 components(with rules and tags) and combine them into 1
  3. Determine the best UX for a common component

As 2 first look like pretty much straightforward, there is some input needed for the third point. @yiyangliu9286, could you please let us know what do you think and what could be the best way to go.
@banderror has highlighted couple of points:

  1. For tags: 3 tags are displayed before showing 'See all'
    For rules: 2 rules are displayed before showing 'See all'
    It makes sense to pick one number to display.

  2. Rules are displayed as block, tags as inline-block. In my opinion, there should be picked one solution - inline-block. That would allow rules to fit in one row when needed, instead of each on a new row, which is probably not the best solution.

  3. Popover element has the same max-width: 600px for both tags & links, where items displayed as inline-block, which allows to them to be fit together in one row, with width up to 600px, and then be moved to the next line.
    This behaviour is the same in both components. In my opinion nothing we need to do here, maybe if required to changed popover max-width, if needed

Attaching a screen to give a sense how popover works right now
This looks like a very extreme case of extra long rule name, but it gets carried over to the next line, if exceeds width of popover container. Smaller rules get fit in the same line
Screenshot 2021-11-08 at 20 24 06

@yiyangliu9286
Copy link

yiyangliu9286 commented Nov 9, 2021

@vitaliidm Thanks for summarizing it. Here are my general thoughts:

TL;DR: for these types of UI/UX enhancements, I feel like we should try to solve these separately by considering the actual persona and workflows that our users are landing to these different tables (1. Tags for Rules management table 2. Rules assigned to for the Exceptions table) and determine what are the best practise we would like to improve for user experience.

Here are my thoughts / exploration works for how to treat the interactions for the long & multiple tags in Rules management table (attaching Figma file for full design exploration).

  • V1. Displaying 1 tag + a "+ number" with Popover for the rest of the tags.

Screen Shot 2021-11-08 at 10 20 15 PM

However, if we go with this version of design for tags, there are a few UX limitations & problems:

  • If a custom / tag running too long, number badge will still cuts off
    • Potential solution is to set a max width for badge but some long text badge will still have truncation ... view for individual tag
  • There isn't that much space left for Rules name column, which I think is probably more important to display more text that the Tags column here.
  • Another product related question I have is that: Do we really need to show what tags are connected to this rule upfront? Or could we only show the number of tags + Popover (see V2)?

V2. Tags shown as part of rules column (Same behaviour as Stack Rules) <- I am more leaning towards this version (looping in @jethr0null for product considerations for this direction going forward)

Screen Shot 2021-11-08 at 10 32 39 PM

I feel like this is what I would suggest the design direction to go and here are some benefits w. this view:

  • Provide more column width for rule’s column vs. v1 (and current in product designs)
  • Tags placement is more discoverable and readable after rule’s name
  • Adds consistency with stack rules designs

Attaching Stack Rule's design for a reference here:
Stack Rules

@yiyangliu9286
Copy link

@vitaliidm As for Exceptions table, I think what you suggested here for displaying 2 rules makes sense because I feel like our users in general don't have too much exceptions list items to display (but correct me if I am wrong). So the row height is a less concern here. As for an overflow behaviour, instead a "See all" button, I think we could probably explore more specific content for that button, e.g. if users have 3 rules assigned to an exception list, display 2 Rules name + "+1 Rule" button w/ Popover for that table cell. For design component usage, we could possibly retain the way it is right now.

@vitaliidm
Copy link
Contributor

@yiyangliu9286, thanks for sharing the observations.

So, for rules table I will wait your further input, how we can proceed there.

Regarding exceptions table - so, the idea is to replace Select all with another button? That would hint us how many rules are hidden?

For design component usage, we could possibly retain the way it is right now.

So, the idea just to change button label if my understanding is correct.
I've updated naming just for test purpose - and it looks like this:
Screenshot 2021-11-09 at 12 32 53

Please, let me know if it's something you would like to proceed - or maybe we need some UI mockup for it

@banderror
Copy link
Contributor Author

Re: rule tags in the Management table:

@yiyangliu9286 I think the idea of being consistent with the Stack Management UI is good 👍 It looks nice and it will save some space in the table 🙂

Re: exceptions:

As for Exceptions table, I think what you suggested here for displaying 2 rules makes sense because I feel like our users in general don't have too much exceptions list items to display (but correct me if I am wrong). So the row height is a less concern here. As for an overflow behaviour, instead a "See all" button, I think we could probably explore more specific content for that button, e.g. if users have 3 rules assigned to an exception list, display 2 Rules name + "+1 Rule" button w/ Popover for that table cell. For design component usage, we could possibly retain the way it is right now.

Maybe something like that:

Rules assigned to
[5 rules] My rule name, My other rule nam...

or

Rules assigned to
My rule name, My other rule nam... [5 rules]
  • will always consume no more than 1 line of text
  • a separate "Number of rules" column would not be needed
  • consistency with tags like in Stack Management (a label that shows the full number of items)

@yiyangliu9286
Copy link

yiyangliu9286 commented Nov 11, 2021

Re: rule tags in the Management table:

@yiyangliu9286 I think the idea of being consistent with the Stack Management UI is good 👍 It looks nice and it will save some space in the table 🙂

Re: exceptions:

As for Exceptions table, I think what you suggested here for displaying 2 rules makes sense because I feel like our users in general don't have too much exceptions list items to display (but correct me if I am wrong). So the row height is a less concern here. As for an overflow behaviour, instead a "See all" button, I think we could probably explore more specific content for that button, e.g. if users have 3 rules assigned to an exception list, display 2 Rules name + "+1 Rule" button w/ Popover for that table cell. For design component usage, we could possibly retain the way it is right now.

Maybe something like that:

Rules assigned to
[5 rules] My rule name, My other rule nam...
or

Rules assigned to
My rule name, My other rule nam... [5 rules]

  • will always consume no more than 1 line of text
  • a separate "Number of rules" column would not be needed
  • consistency with tags like in Stack Management (a label that shows the full number of items)

Thanks for confirming with the rule tags in the Management table. @vitaliidm Michelle has also agreed with this proposal so I think we are good and aligned with the proposed design for Tags and we can move forward to implement this change. :)

Regarding to @banderror's proposal for exceptions, I totally agree with the proposal and think is a good direction to go for, @vitaliidm here is the mock for the Exceptions table (Figma file is here):
Exceptions

@banderror banderror changed the title [Security Solution][Detections] Table UI: consistent "See all" buttons (Draft) [Security Solution][Detections] Table UI: consistent "See all" buttons Nov 11, 2021
@vitaliidm vitaliidm self-assigned this Nov 11, 2021
@yiyangliu9286
Copy link

yiyangliu9286 commented Nov 11, 2021

Just a note @vitaliidm, after chatted with @jethr0null, we have agreed that for Tags treatment in Rule management table we will go with tags badge design same with Stack management, just showing the number of tags in the table + Popover to show an overall view of the full tags for that rule. See attached designs:
Screen Shot 2021-11-09 at 1 33 46 PM

@vitaliidm
Copy link
Contributor

@yiyangliu9286

I have a couple questions regarding proposed UX for exceptions table:

  1. Actions icons in Figma design are colourful. But according to https://elastic.github.io/eui/#/tabular-content/tables#adding-actions-to-table, they should greyed out, and become only colourful when user hover over them. I've also seen same behaviour in other tables. Is it ok with you for me align icons behaviour according EUI docs?

This how it would look like with proposed change
Screenshot 2021-11-15 at 20 43 36

  1. It looks like, it's impossible to achieve functionality as in mocks by CSS only. It would require to use Javascript to compute dynamically how to truncate the rule name. I.e. to compute element width, if it overflows its parent and is hidden. Then set computed width by JS as inline style.
    [Short name][The first very long ...] [+1 rule]
    Another case to cover would be to determine whether there is a space for a second rule name at all, or if first element is too long and takes all available space) Example below:
    [Short name][The first very long ...] [+1 rule]
    [The first very long long long rul...] [+2 rules]

JS use is required, because to use text truncation, parent's(to text string) element must have fixed width. In our case we have 2 elements(rule names), which do not have fixed width. So it has to be computed, to be able to achieve functionality as in mocks.

After discussion with @elastic/security-detections-response-rules team, we agreed that it doesn't make sense to use JS for styling if we can avoid it. That would help not to overcomplicate codebase and keep it maintainable.

So I propose to slightly amend the way we truncate it, but still to keep it in one line. But without the need for JS use instead.

1. Display always only one rule name

Screenshot 2021-11-15 at 18 22 38

So, instead of displaying 2 rules and render the rest in popover - we always display only one.
Rule names are usually fairly long, so second rule name would be definitely truncated. In some cases there even will be not a place to fit a second rule name due to a long first one.

2. Put rule names into equals grid/flex cells

Screenshot 2021-11-15 at 18 55 00

We always display 2 rule names, but they both get truncated.
Pros: equal space to see both rules
Cons: if one rule is not getting truncated, we could see some empty spaces between rule names.

3. Add fading to truncated line

Screenshot 2021-11-15 at 18 30 59

We still can display 2 rule names, but we also apply fading out, to the right, where word is truncated.
The con of this approach is - if the first rule name is too long, second one can be completely hidden.

I personally think, the first option could be the best approach to implement. @yiyangliu9286 , can you please let know what do you think, and which solution is preferable? Thank you

@yiyangliu9286
Copy link

@vitaliidm Thanks for reviewing the design and posted those questions.

  1. Agreed that for actions icons in the Exceptions table, we should aligned with EUI guidelines to use greyed out icons. https://elastic.github.io/eui/#/tabular-content/tables#adding-actions-to-table.
    See updated design:

Exceptions

  1. Agreed that we should go with option 1 Display always only one rule name (and use the +n badge + Popover to display the rest of the rules if there are more than 1 rule in the Rules assigned to column) to avoid using JS for styling if we can. From a visual perspective I think this would give users more UX friendly and consistent experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants