Use bootstraps own img-responsive class for responsive graphs #2633

Merged
merged 3 commits into from Jan 6, 2016

Projects

None yet

6 participants

@arjitc
Contributor
arjitc commented Dec 15, 2015

Old: http://i.imgur.com/ZKHurbZ.png
When img-responsive is used: http://i.imgur.com/R6sTtiV.png
Automatically resizes as well when I resize the window: http://i.imgur.com/G5QxZN7.png

@arjitc arjitc Use bootstraps own img-responsive class for responsive graphs
9564883
@laf
Member
laf commented Dec 18, 2015

Does this negate the need for #2537?

@arjitc
Contributor
arjitc commented Dec 18, 2015

@laf Yup, does not need the $w and $h values either.

arjitc added some commits Dec 21, 2015
@arjitc arjitc Put graphs correctly in a row
e1e7b83
@arjitc arjitc Add row class
6eec2ca
@Alucardfh

I just tested this PR and it is indeed working very well.

The only downside is that the bootstrap responsive code doesn't upscale image. Which anyway isn't something we want as it would blur graphs. But this means that graphs are still tiny on big resolutions.

So I also tested the changes in the PR on top of the one I have in PR #2537 which looks quite good to me and avoid my ugly workaround to reduce the blur on graphs.

Current master behavior (resolution 2560x1440):

no-img-responsive-noupscale

With this PR :

img-responsive-noupscale

With this PR + PR #2537 ;

img-responsive-upscale

@paulgear
Member
paulgear commented Jan 1, 2016

I've tested this along with PR#2537 and it looks perfect on my browser. Big thanks, @arjitc, and 👍 to merge both this & #2537.

@paulgear paulgear merged commit 170423d into librenms:master Jan 6, 2016

2 checks passed

Auto-Deploy Build finished. No test results found.
Details
Scrutinizer 4 new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment