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

[Alerting UI]Changed rules table to support visual indication for disabled and muted alerts #104190

Merged

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Jul 1, 2021

Resolves #80310 #80313
Current PR covers the next list of changes:

  • Added a switch column to the front of each row to quickly allow users to see and update whether the rule is Enabled or not.
  • Truncated tags column with hover tooltip which shows the rest of hidden values.
  • Extended column for 'Actions' count with 'Muted' badge indicator.
  • Show 2 row actions on hover (Edit & Delete). Overflow menu remains regardless of hover (Follows EUI table example)
  • Overflow menu options are: Mute/Unmute, Enable/Disable, Edit rule, Delete rule (should have confirmation modal to delete)
  • Updated the coloring for the statuses 'Pending' and Active as 'Healthy' color to stay consistent with RAC changes.
  • Renamed Disable to Enable on the Rule Details page.

Here is an updated mockup with the follow changes:
Screen Shot 2021-07-08 at 8 43 03 PM
Screen Shot 2021-07-07 at 8 32 44 PM

Screen Shot 2021-07-08 at 10 44 51 PM

@YulNaumenko YulNaumenko self-assigned this Jul 1, 2021
@YulNaumenko YulNaumenko added v7.15.0 v8.0.0 Feature:Alerting/RulesManagement Issues related to the Rules Management UX Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) release_note:skip Skip the PR/issue when compiling release notes labels Jul 7, 2021
@YulNaumenko YulNaumenko marked this pull request as ready for review July 7, 2021 19:25
@YulNaumenko YulNaumenko requested review from a team as code owners July 7, 2021 19:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

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

cc @YulNaumenko

@mikecote mikecote self-requested a review July 8, 2021 15:10
@YulNaumenko YulNaumenko requested a review from mdefazio July 8, 2021 18:09
@YulNaumenko
Copy link
Contributor Author

@elasticmachine merge upstream

@spalger
Copy link
Contributor

spalger commented Jul 8, 2021

jenkins, test this

(had to abort for Jenkins upgrade)

Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

I left a couple of questions, but LGTM overall 👍 looking forward to these enhancements!

@mdefazio
Copy link
Contributor

mdefazio commented Jul 9, 2021

We should probably change the rule detail view for a disabled rule to use the empty panel page layout instead of a callout. I will mock this up.

At the very least, the copy in the callout will need to be updated. We still refer to the 'Disabled' switch. And I would prefer to avoid the icon usage of the arrow to direct them. (Which is why the empty page layout will be better here).

Is there a scenario where the user will reach this screen without the privileges to enable the rule?

@mikecote
Copy link
Contributor

mikecote commented Jul 9, 2021

@mdefazio

Is there a scenario where the user will reach this screen without the privileges to enable the rule?

Yes, it would be when the user has readonly access to rules (done via feature privileges).

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

These updates look fantastic! The only thing I noticed was that when you click the trash can icon on the hover over, it doesn't trigger the delete action, it triggers the mute all action.

@YulNaumenko
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

The UI updates look good. There is still ongoing discussions/decisions around this table through the RAC project, so I will open another issue when we have more definitive direction and how to align these tables. But I think these updates achieve the goal of this PR.

One issue i'm noticing that I'm not sure is related to this PR or not is the delay in enabling/disabling a rule. (See gif). I'm wondering if we need to disable the toggle when loading the new state so the user doesn't go click happy like I'm doing here. Subsequently, kibana crashed after about 10-15x of flipping this on and off.

Screen Recording 2021-07-19 at 11 24 59 AM

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

Awesome! Such a great set of enhancements. LGTM

I noticed one thing when playing with it. The new "inline edit" ends up leaving the hover for the "Edit" label on the UI, after the save, when the "Updated" notification is displayed.

image

The edit hover seems very "sticky" in general. Even if I immediately "cancel" out of the edit, and make sure the mouse will not end up over the edit label, it still appears. Actually, it looks like the edit and delete buttons are still available as well, after the cancel. So maybe it's the edit/delete appear-on-hover buttons that are the problem. Fixable in a follow-on issue, if too big for this one.

@YulNaumenko
Copy link
Contributor Author

Awesome! Such a great set of enhancements. LGTM

I noticed one thing when playing with it. The new "inline edit" ends up leaving the hover for the "Edit" label on the UI, after the save, when the "Updated" notification is displayed.

image

The edit hover seems very "sticky" in general. Even if I immediately "cancel" out of the edit, and make sure the mouse will not end up over the edit label, it still appears. Actually, it looks like the edit and delete buttons are still available as well, after the cancel. So maybe it's the edit/delete appear-on-hover buttons that are the problem. Fixable in a follow-on issue, if too big for this one.

Thank you for the catch @pmuellr. It's easy to fix 👍

@YulNaumenko YulNaumenko requested a review from ymao1 July 20, 2021 13:24
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM! I am also seeing the issue that @mdefazio pointed out where flipping between enable/disable quickly will crash Kibana. I'm fine with making a followup issue to fix that separate though.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 359 360 +1

Async chunks

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

id before after diff
triggersActionsUi 1.6MB 1.6MB +3.2KB

History

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

cc @YulNaumenko

@YulNaumenko YulNaumenko merged commit d91c6d0 into elastic:master Jul 20, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Jul 20, 2021
…abled and muted alerts (elastic#104190)

* [Alerting UI]Changed rules table to support visual indication for disabled or muted alerts

* changed columns styles due to the mockup

* added tests

* fixed quotas

* fixed popover

* fixed due to the lates UI updates

* fixed errors

* moved enabled to a separate component

* fixed tests

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.scss

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* removed test code

* fixed tests

* fixed due to comments

* fixed due to comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
YulNaumenko added a commit that referenced this pull request Jul 20, 2021
…abled and muted alerts (#104190) (#106282)

* [Alerting UI]Changed rules table to support visual indication for disabled or muted alerts

* changed columns styles due to the mockup

* added tests

* fixed quotas

* fixed popover

* fixed due to the lates UI updates

* fixed errors

* moved enabled to a separate component

* fixed tests

* fixed due to comments

* Update x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.scss

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* removed test code

* fixed tests

* fixed due to comments

* fixed due to comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@mikecote mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting/RulesManagement Issues related to the Rules Management UX release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual indication for disabled or muted alerts
8 participants