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

[SIEM] Detection Engine Create Rule Design Review #54319

Closed
36 of 39 tasks
MichaelMarcialis opened this issue Jan 9, 2020 · 2 comments
Closed
36 of 39 tasks

[SIEM] Detection Engine Create Rule Design Review #54319

MichaelMarcialis opened this issue Jan 9, 2020 · 2 comments
Assignees

Comments

@MichaelMarcialis
Copy link
Contributor

MichaelMarcialis commented Jan 9, 2020

Design Review

Thanks so much for making the previously requested changes to the "Create new rule" page for the detection engine, @XavierM! This is really looking awesome.

With the most recent PR, I wanted to make sure I provided a thorough design review of the page. As that PR has already been merged, I'm creating a separate issue with all my notes. Please feel free to have a look and let me know if you have any questions.

Documentation

Originating issue
Originating PR

Designs

Figma Wireframes
Figma Updated Mockups

Action Items

General

  • All step headers have hover styles that underline the text and show a pointer cursor. This should not happen, as interacting with this area does nothing.
    header-hover

  • Step headers should utilize sentence casing. Please change “Define Rule” to “Define rule”, “About Rule” to “About rule” and “Schedule Rule” to “Schedule rule”. (X)

  • All form fields in each step panel should be left-aligned with the beginning of the step header text, as indicated in the wireframes and mockups. Currently, they are left-aligned with the numbered/checked circle. Can this be corrected? (P)

  • It appears as though all of the text input and textarea elements perform validation on key down or up. I think the common pattern in Kibana is to do form validation on blur. Can we change to this? Right now, it’s a bit overwhelming to be constantly told that the field is incorrect as I’m actively typing/changing it. (X)

  • It appears that all “Optional” text is using the default text color. Can we change this text to be the subdued/euiColorDarkShade color? (P)

  • The spacing in between each step panel appears to be smaller than indicated in the wireframes/mockups. Can we change to 24px space in between each step panel to mimic the design?

  • The EuiHorizontalRule under each step panel header is a different/incorrect margin size versus the one used in the step panel footer. Can the one under the header be changed to margin="m"?

Define Rule

  • When altering the “Index patterns” field, the alignment of the label changes and shifts up. Doing so misaligns the label, as it doesn’t share the same baseline as the “Reset to default index patterns” text. (P) use IconButton
    index-patterns-altered

  • The “Index patterns” field button to “Reset to default index patterns” is missing the prefixed refresh icon indicated in the mockups. Can we add that in? (P)

  • The “Index patterns” field validation text uses an incorrect plural. Suggest changing to “A minimum of one index pattern is required.” (X)

  • Traditionally, form field labels appear to turn blue when the related field is focused by the user. It appears that the "Custom query" field does not do this currently. Can it be changed to mimic other field label behavior? (X/P)

  • The “Custom query” field also has the same label/button baseline alignment issues described above. (X/P)

  • Focusing the “Custom query” field brings up the autocomplete menu. The autocomplete menu appears to be very short vertically and also increases the height of the “Define rule” step when opened. I assume this is done because there is an overflow: hidden; somewhere and you’re attempting to prevent the autocomplete menu from cutting off? If so, I think the best way to address this is see if we can avoid having the containing overflow: hidden; at all, so the autocomplete menu reacts normally (both in height and being able to overflow out of the step panel). (P)
    custom-query-autocomplete

- [ ] Clicking the “+ Add filter” button shows a momentary flash of the add filter menu appearing in the top-left corner of the viewport, before correctly placing itself by the button that triggered it. Can we do anything to prevent this flash of incorrect positioning?
custom-query-flash

- [ ] When saving a current query via the “SAVED QUERIES” menu in the “Custom query” field, the modal that appears shows up behind the “SAVED QUERIES” menu. Ideally it would appear behind the modal, as it does in the Discover app.

About Rule

  • In the “Severity” field, it appears as though the EuiHealth component within the field is off-center vertically. Can this be corrected? (P)

  • The height of the “Timeline template” field dropdown seems to be much shorter than I last saw it (now only displaying two timelines at a time). Would it be possible to increase the height of this to support showing around five timelines at least? (X)

  • I love your addition of the “Only favorites” filter button to the “Timeline template” field dropdown. Can we just alter the layout slightly and possibly add an indicator showing which timeline is favorited versus unfavorited in the list? I’ve updated my Figma design to reflect this request. (X)
    image

  • Selected check icons and subsequent text are not aligned as shown in the EuiSelectable documentation examples and the Figma design updates in the “Timeline template” field dropdown. Would it be possible to update the alignment as shown in the Figma designs? (X)

  • Can the “Add reference” button text for the “Reference URLs” field be changed to “Add reference URL”? (X)

  • Can the “False positives” field label be renamed to “False positive examples” and the subsequent “Add false positive” button text be changed to “Add false positive example”? (X)

- [ ] Though my original wireframes included the trash icon button to delete every row for “Reference URLs”, “False positive examples”, and “MITRE ATTACK threats”, in retrospect, it feels odd to allow users to delete the first row of inputs (especially if there is only one available). I had altered this in my updated mockups but neglected to list it for you in my previous list of changes. Would it be possible to only include the trash icon button for only the second and greater rows? That way the first row will always be visible and not look like a bug (as it can no longer be removed). (X)

  • All subordinate actions (“Add reference URL”, “Add false positive example”, “Add MITRE ATT&CK threat”) appear to be larger than designed in both icon and font size. Can we change this to use a 12x12 icon and 12px font size? As EUI doesn’t seem to natively support these smaller sized icon buttons, I previously created a linkIcon component that should be able to be used here as well. (X)

  • For both the “Reference URLs” and “False positive examples” fields, if the user begins to type a string in the input and then goes back and deletes it, the entire input element is suddenly removed. This could be very confusing for a user who is just potentially editing their text. Can we leave the input element present, even if the user deletes text that they had in it? The field should only be removed when clicking the trash button icon. (X)
    reference-url-delete

  • The subordinate actions for “Add reference URL” and “Add false positive example” do not appear to work unless the preceding field is filled. This confused me at first, as I thought it was broken when I attempted to add multiple at once. Can we allow users to add reference URLs and false positive examples without requiring they have the fields filled first? (X)

  • If a user has not yet selected a “MITRE ATT&CK tactic” for a given row, the related “MITRE ATT&CK technique” should be disabled without a placeholder, as indicated in the updated mockups. Only once a tactic has been selected, the technique field should be enabled with accompanying placeholder. (X)

Schedule Rule

  • Both time unit select elements for “Rule run interval & look-back` and “Additional look-back” appear as if nested inside and overflowing out the bottom of the preceding time integer input. Can we clean this up to make them appear adjacent rather than nested?

  • [ ] The “Optional” text for the “Additional look-back” field is not right aligned properly with the right edge of the form field below. Can this be addressed?

  • The helper text below the “Additional look-back” field appears to be breaking a line at an arbitrary point. Can this just be allowed to stretch the width of the given space? Or if not, perhaps confined to the width of the form fields? (X maybe)

Completed Step Summaries

  • After completing a step, the completed step summaries don't appear to be formatted as indicated in the wireframes and mockups. For example:

    • Alignment should again be left-aligned with step header text, not with the checked circle. (X)

    - [ ] The field titles font size appears larger than the designs.

    • The “Query” field in the “Define rule” step should be renamed “Custom query” (to match the text to the field it derives from). (P)

~~ - [ ] The query filters in the “Define rule” step are listed apart from the “Custom query” field. The designs show them being combined. ~~

~~ - [ ] The “About rule” step summary’s items appear to be organized by appearance vertically. The wireframes indicated the items should be laid out horizontally, with some exceptions in the layout (such as the “Description” field taking up two columns of width).~~

  • The “Description” field appears in a disabled textarea, which is not semantically correct and doesn't match the designs. (X/P)

- [ ] The “Description” field does not extend across two columns, as indicated in the wireframes.

  • The “Severity” field’s value appears to be lower case, when it should be sentence case (i.e. “low” vs “Low”). (P)

  • The “Risk score” field is missing from the completed step summary. (X)

  • The “Reference URLs” field appears to be using an EuiListGroup component. I’d prefer to stick with the simple bulleted list of links as indicated in the wireframes. (P)

  • The “Reference URLs” field is the wrong font size. (P)

  • The “Reference URLs” field will show an empty space for a second URL, if the user has added a second reference URL row, but neglected to type anything in. (P)

  • Can we change the “False positives” field label to be “False positive examples”? (P)

  • The “False positive examples” field is using an EuiBadge component for each value, which feels odd because the values are expected to be like sentences. Can we change this to a simple bulleted list, as indicated in the wireframes? (P)

  • Can we change the “MITRE ATT&CK” field label to be “MITRE ATTACK tm”? (X)

  • The “MITRE ATT&CK” field’s tactic value is the wrong font size and weight. (P)

  • The “MITRE ATT&CK” field’s technique values are missing the prefixed icon. (P)

@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@MichaelMarcialis
Copy link
Contributor Author

MichaelMarcialis commented Jan 23, 2020

Closing this issue, as my full design review (#55753) for detections supersedes this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants