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

Collapsible plugin output #3870

Merged
merged 9 commits into from Jul 29, 2019

Conversation

@nilmerg
Copy link
Member

commented Jul 18, 2019

The following details now collapse:

  • Plugin output
  • Plugin perfdata
  • Custom variables
  • Notes

Plugin output and perfdata are grouped by their check_command name. For example, expanding disk's output on host localhost expands it for every other host which has this check.
All custom variables are grouped by their object's type. Once custom variables are expanded for a service every other service has its variables also expanded. The same applies for notes.

resolves #3566

@nilmerg

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2019

@dnsmichi @Thomas-Gelf Custom variables also?

@dnsmichi

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

@nilmerg would be nice, yes. Some environments have a long list of CVs (or those which a JSON encoded and rendered as large blobs).

@nilmerg nilmerg force-pushed the feature/collapsible-plugin-output-3566 branch from 864e352 to 3dc9ffe Jul 18, 2019

@nilmerg nilmerg requested a review from lippserd Jul 19, 2019

@lippserd
Copy link
Member

left a comment

Hi,

I played around with this a little bit and found the following things which we should fix/discuss:

  • Reloading the page seems to clear the storage.
  • I would not add extra toggles for dicts in the custom variables section. The whole section should be either collapsed or expanded. I don't think that we should require the user to click more than one time in order to see the custom variables. Further, it clutters the UI.
  • Using the check command for plugin output and performance data absolutely makes sense. So it does for custom variables and services. But what do we do with hosts? One has to expand the custom variables for every host separately.
  • I think it makes sense to add a preference for enabling/disabling this feature.
  • We have this greyish background when loading takes too long. This now looks odd. See screenshot.
  • The plugin output in list views should be collapsed/cut as well. Having the full output in list views and a collapsed one in the detail area does not make sense. See screenshot.
  • The list views show full five perf data pies. The detail area has the shadow over the fifth one. This should be raised to six I think. See screenshot.
  • We should have something smarter than "collapse if there are more than 5 lines". It feels wrong to just expand a single line for example. Maybe something like "collapse if there are more than 5 lines to collapse".
  • Expanded perf data flickers in chrome. I tested this with an auto-refresh interval of 1 second.

Cheers,
Eric

collapsible-containers

@nilmerg nilmerg force-pushed the feature/collapsible-plugin-output-3566 branch from 3dc9ffe to e348480 Jul 25, 2019

@nilmerg nilmerg force-pushed the feature/collapsible-plugin-output-3566 branch from e348480 to 91a8bdf Jul 26, 2019

@nilmerg

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2019

Fixed bugs and applied the changes we discussed. Though, the flickering is still there as it can't be completely avoided.

@nilmerg nilmerg requested a review from lippserd Jul 26, 2019

@lippserd

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

I don't see flickering anymore 👍

@nilmerg nilmerg merged commit b6c89bc into master Jul 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nilmerg nilmerg deleted the feature/collapsible-plugin-output-3566 branch Jul 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.