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

Add a filter to hide silenced alerts #319

Merged
merged 1 commit into from Apr 28, 2016

Conversation

mpchadwick
Copy link
Contributor

An implementation for the feature request in #313.

Just uses CSS display: none to hide the silenced alerts. Worth noting that the alert group node will not be removed if all alerts are silenced (the UI feels like an accordion in this case).

Before Filtering

image

After Filtering

image

@matthiasr
Copy link

matthiasr commented Apr 22, 2016 via email

@fabxc
Copy link
Contributor

fabxc commented Apr 22, 2016

Nice, thanks! 👍

</fieldset>
</form>

<div id="alert-groups">
<div id="alert-groups" ng-class="{'hide-silenced': hideSilenced}">
Copy link
Member

Choose a reason for hiding this comment

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

Can this just be ng-hide="hideSilenced"? Then you don't need any CSS... same for the other one in alert.html.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@juliusv Hmm...I think that would hide the entire alert-groups node. In order to avoid the DOM / CSS approach we'd probably need to do something like this on the alert-item (untested)...

ng-hide="alert.silenced && hideSilenced"

hideSilenced is currently a property on AlertsCtrls $scope. So we'd need to make it available in AlertCtrl either via $rootScope or a service. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, the nested nature of this wasn't apparent to me. All makes sense then, I guess keep it like it is.

@mpchadwick
Copy link
Contributor Author

@matthiasr I initially thought about floating and using that real estate, but then when I added it initially at the bottom I thought it looked fine. If we start having tons of filters we would probably want the two columns (and maybe even show more / show less). I thought it might look weird / lonely to have a second column with a lone checkbox. I can add a screenshot later though.

@mpchadwick
Copy link
Contributor Author

@matthiasr updated to use 2 columns for the filters. I tested it out and do think it looks better after all

image

@fabxc anything holding back the merge here?

@matthiasr
Copy link

matthiasr commented Apr 28, 2016 via email

@fabxc
Copy link
Contributor

fabxc commented Apr 28, 2016

Thanks!

@fabxc fabxc merged commit 6493dd5 into prometheus:master Apr 28, 2016
@fabxc fabxc mentioned this pull request Apr 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants