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

webui: Single Server Details widget #7923

Merged
merged 6 commits into from Dec 21, 2017

Conversation

Projects
None yet
4 participants
@aldemira
Copy link
Contributor

aldemira commented Dec 19, 2017

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@laf

This comment has been minimized.

Copy link
Member

laf commented Dec 20, 2017

You're going to open a can of worms with this :) People will be asking for these all over!

So couple of things, can you download the min version of this library and use that instead. As it's only usable for dashboards then you should include the min.js file within the widget to stop it being needed elsewhere.

We already have raphael included which is the same version. html/js/raphael-min.js so you can drop yours.

Looks ace btw :)

@laf laf added the WebUI label Dec 20, 2017

@aldemira

This comment has been minimized.

Copy link
Contributor Author

aldemira commented Dec 20, 2017

@laf Yeah ok but, I couldn't find the min version. There are old versions of it on different CDNs but I wanted to use the official one.

@laf

This comment has been minimized.

Copy link
Member

laf commented Dec 20, 2017

@laf Yeah ok but, I couldn't find the min version. There are old versions of it on different CDNs but I wanted to use the official one.

Yeah actually can't see it myself. We might need to min it ourselves (as we should with some others). It does raise the question, is this library supported and do we need to pick another one if it's not?

@aldemira

This comment has been minimized.

Copy link
Contributor Author

aldemira commented Dec 20, 2017

I'm open to suggestions. This project hasn't been touched for a year. There's gauge.js which a not very well documented and not as nice as this, but I can give it a try if you want to.

@laf

This comment has been minimized.

Copy link
Member

laf commented Dec 20, 2017

Don't want to cause additional work so let's leave it for now. If we start to use it in anger more then I think we should take a look at the best one.

@laf laf added the Schema label Dec 20, 2017

@aldemira

This comment has been minimized.

Copy link
Contributor Author

aldemira commented Dec 21, 2017

Now I know why there isn't a minified version of the library. I've used a few different sites to compress it, and minified file always breaks the output. I'm attaching the screenshots.
screen shot 2017-12-21 at 09 50 01
screen shot 2017-12-21 at 09 54 07

@laf

This comment has been minimized.

Copy link
Member

laf commented Dec 21, 2017

I've rebased this and also fixed the sql queries to stop the device list being shown to people who don't have access. The widget will also not load now if the user doesn't have access and display an error message.

Will merge once checks complete.

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented Dec 21, 2017

The inspection completed: No new issues

@laf laf merged commit 5d7d99a into librenms:master Dec 21, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@kkrumm1

This comment has been minimized.

Copy link
Member

kkrumm1 commented Dec 22, 2017

Found a small issue :( When you try to use another Single Server Widget in the same dashboard it adds the info to the first Single server widget, not the second server widget.

image

mjclancyau added a commit to mjclancyau/librenms that referenced this pull request Dec 22, 2017

Merge pull request #38 from librenms/master
webui: Added Single server details widget (librenms#7923)
@aldemira

This comment has been minimized.

Copy link
Contributor Author

aldemira commented Dec 22, 2017

Thanks @kkrumm1 ! Just sent another PR to fix this.

@aldemira aldemira deleted the aldemira:serverwidget branch Jan 9, 2018

@lock

This comment has been minimized.

Copy link

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.