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

Revamped alert browser list page changes #1038

Merged
merged 16 commits into from
Apr 16, 2024

Conversation

shavidissa
Copy link
Collaborator

Includes update to show the revamped alert browser list changes.

@shavidissa shavidissa requested a review from mmihaylovam April 15, 2024 16:57
Copy link
Collaborator

@mmihaylovam mmihaylovam left a comment

Choose a reason for hiding this comment

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

Hi Shavi,
Approving but could you please have a look at my comments?
Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the callouts shouldn't be punctuated because they are not complete sentences but object names.
Only "Click to see the menu." is a complete sentence but, for consistency, we can change it to something like "Alert menu".
Not sure that "Number of alerts that fired" is correct because it's one alert here. I think it should be something like "Number of alert firings.
Not sure what the brackets mean in the last callout.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we update the alert_firing_severe.png image, too?

For each alert, the Alerts Browser includes detailed information. For example, an alert that is firing looks like this:
-->
You can expand each alert to view detailed information. For example, when you expand an alert that is firing, the detailed information looks like this:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably, we can add a green box around Delete?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably, we can add a green box around Edit?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably, we can add a green box around Show firings?

@shavidissa
Copy link
Collaborator Author

Thank you! Great feedback. I will address these in a separate PR.

@shavidissa shavidissa merged commit af915b0 into master Apr 16, 2024
3 checks passed
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.

2 participants