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

Improve message display #71

Conversation

Marc-Assmann
Copy link
Contributor

Currently the metric values are only displayed in case "Show Always" is checked.
This causes all messages to be displayed even when there is no warning.

This pull request replaces the checkbox with a drop down for a finer grained configuration on when to display a metric value next along with the alias:

  • Never - The value will never be displayed, only the alias in warning or critical state
  • When Critical - The alias + the value will be displayed in critical state only
  • When Warning - The alias + the value will be displayed in warning or critical state
  • Always - The alias + the value will always be displayed, regardless critical and warning state

In order to keep the current behaviour, the default setting is "Never".
When upgrading from a previous version, "Show Always" is converted to "Always".

On the UI, "Display Type" is renamed to "Display Position" to make that setting better understandable in its context.

@tomer-amir-vonage
Copy link
Contributor

Hi Marc,

thanks for contributing!
sounds like a good addition to this plugin. We have a bit of a backlog in pull requests, so I'll review it once we're done with the previous PRs, but for now, there is a small issue that I'm not sure I like,

When you call the default behavior 'Never' it sounds like the measurement will never be displayed.
It's good that you added a tooltip that explains the behavior, but could you try to find a better name?

Thanks!

@Marc-Assmann Marc-Assmann force-pushed the improve_message_display branch from fe0fff6 to 89ea945 Compare September 21, 2017 11:17
@YehonatanVonage YehonatanVonage changed the base branch from master to feature/messageDisplay December 4, 2017 13:12
@YehonatanVonage YehonatanVonage merged commit 6729146 into Vonage:feature/messageDisplay Dec 4, 2017
@YehonatanVonage
Copy link
Contributor

Hi @Marc-Assmann,
Thanks for the contribution. I merged your code to a side branch, so I can add some more fixes and features relevant to your commit.

Your changes will be available in the next version.

@Marc-Assmann
Copy link
Contributor Author

Hi @YehonatanVonage,
Thank's a lot for picking this up!
In the meanwhile I have tried to rebase the pull request but didn't know on how to improve the wording as requested by TomerAmirV above.
But I see you now solved it by adding finer grained control on the display of the alias as well which looks very good!
Best regards,
Marc

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.

3 participants