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

Beautify port health #9981

Merged
merged 7 commits into from Mar 20, 2019

Conversation

Projects
None yet
2 participants
@amigne
Copy link
Contributor

commented Mar 17, 2019

The presentation of the graphs (display of current and limit values) under Device / Port / Health not as beautiful than the one on Device / Health.

This PR proposes to provide the same "look" on the Device / Port / Health.

This PR also takes the opportunity to do some clean-up in the code / perform minor changes.

Addition of a new helper function "get_unit_for_sensor_class" to get the unit for the different sensor classes.

Capture of the change :
image
image
image

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
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@amigne

This comment has been minimized.

Copy link
Contributor Author

commented Mar 17, 2019

Patch has been successfully tested in my test environment.

@amigne amigne referenced this pull request Mar 17, 2019

Merged

Fixed inconsistent <h3> closed by </div> #9982

1 of 1 task complete

@PipoCanaja PipoCanaja added the WebUI label Mar 17, 2019

@amigne amigne changed the title WIP - Beautify port health Beautify port health Mar 17, 2019

@amigne amigne changed the title Beautify port health WIP - Beautify port health Mar 17, 2019

@amigne amigne changed the title WIP - Beautify port health Beautify port health Mar 17, 2019

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Mar 17, 2019

Hello @amigne
In WebUI PR, do not hesitate to add a screen capture, that will show the exact display you have and other can compare with their setups (different devices, different sensors may give different display)
++

@PipoCanaja
Copy link
Contributor

left a comment

LGTM

@PipoCanaja PipoCanaja merged commit 96797aa into librenms:master Mar 20, 2019

5 of 6 checks passed

codeclimate Code Climate encountered an error attempting to analyze this pull request.
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@amigne amigne deleted the amigne:beautify-port-health branch Mar 20, 2019

funzoneq added a commit to funzoneq/librenms that referenced this pull request Apr 30, 2019

Beautify port health (librenms#9981)
* Removal of the unused  variable
* Replace double quotes by single quotes when possible
* Adding spaces between operands of the concatenation operator
* Rewrite of code echoed to the browser
* Reuse the logic of the Device/Health page for displaying current and limit values
* New helper function to determine the unit for the different sensor classes
* Fixing wrong value for current unit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.