Jason Guiditta
2008-May-23 20:03 UTC
[Ovirt-devel] [PATCH] graphs for detail pane of hosts take 2
Sorry, forgot to update before I made that last patch, there would be a cnlfict, this one should better. Reposting notes if you want to ignore the last one: Besides adding the new graphs, I also added a param that can be passed into the js in _graph.rhtml to allow you to specify a css class for the svg container div. This means instead of having to have id-based classes to set height and width (so the svg shows up) we can reuse classes whenever it makes sense (for example, all detail pane graphs should be the same size, etc). Also, for some reason, I did not hit the js error thrown in the background by some of the other graphs. I chalk this up to something being different in the returned json (I am using a different controller) but haven't traked it down quite yet. I'll keep looking, but wanted to get the main patch out in the meantime. -j -------------- next part -------------- A non-text attachment was scrubbed... Name: s2-host-graph-redux.patch Type: text/x-patch Size: 6443 bytes Desc: not available URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20080523/5033aaa7/attachment.bin>
Mohammed Morsi
2008-May-23 21:18 UTC
[Ovirt-devel] [PATCH] graphs for detail pane of hosts take 2
Overall looks good (didn't apply it yet and try it out though). Just had one question. Why in _graph.rhtml do you have this: - svg.graph.noDraw(); + //svg.graph.noDraw().title('',10); eg. you commented out the call to noDraw(). I think this might be a mistake as you didn't comment out the call to reDraw() below. In any case, these both shouldn't be commented out, as they prevent a graph from appearing until is done being generated (I still think we should expirement with removing the noDraw and redraw methods from _graph and putting them at the top and bottom of the site layout, to see if we can get it so all the graphs on a page appear at once in a timely manner). Perhaps thats the reason w/ you weren't getting the js error (in any case we should look into that as well). Other than that, the patch looks good. -Mo Jason Guiditta wrote:> Sorry, forgot to update before I made that last patch, there would be a > cnlfict, this one should better. Reposting notes if you want to ignore > the last one: > > Besides adding the new graphs, I also added a param that can be passed > into the js in _graph.rhtml to allow you to specify a css class for the > svg container div. This means instead of having to have id-based > classes to set height and width (so the svg shows up) we can reuse > classes whenever it makes sense (for example, all detail pane graphs > should be the same size, etc). Also, for some reason, I did not hit the > js error thrown in the background by some of the other graphs. I chalk > this up to something being different in the returned json (I am using a > different controller) but haven't traked it down quite yet. I'll keep > looking, but wanted to get the main patch out in the meantime. > > -j > > ------------------------------------------------------------------------ > > _______________________________________________ > Ovirt-devel mailing list > Ovirt-devel at redhat.com > https://www.redhat.com/mailman/listinfo/ovirt-devel-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20080523/991d5c40/attachment.htm>
Hugh O. Brock
2008-May-23 21:33 UTC
[Ovirt-devel] [PATCH] graphs for detail pane of hosts take 2
On Fri, May 23, 2008 at 04:03:50PM -0400, Jason Guiditta wrote:> Sorry, forgot to update before I made that last patch, there would be a > cnlfict, this one should better. Reposting notes if you want to ignore > the last one: > > Besides adding the new graphs, I also added a param that can be passed > into the js in _graph.rhtml to allow you to specify a css class for the > svg container div. This means instead of having to have id-based > classes to set height and width (so the svg shows up) we can reuse > classes whenever it makes sense (for example, all detail pane graphs > should be the same size, etc). Also, for some reason, I did not hit the > js error thrown in the background by some of the other graphs. I chalk > this up to something being different in the returned json (I am using a > different controller) but haven't traked it down quite yet. I'll keep > looking, but wanted to get the main patch out in the meantime. > > -j> >From ec3e8f5d691b2be2f208137929ae783e7a952bf2 Mon Sep 17 00:00:00 2001 > From: Jason Guiditta <jguiditt at redhat.com> > Date: Fri, 23 May 2008 15:53:21 -0400 > Subject: [PATCH] graphs for host tab, detail pane > > > Signed-off-by: Jason Guiditta <jguiditt at redhat.com> > --- > wui/src/app/controllers/graph_controller.rb | 13 +++++++++- > wui/src/app/controllers/hardware_controller.rb | 2 +- > wui/src/app/views/host/show.rhtml | 30 ++++++++++++++++++++++++ > wui/src/app/views/layouts/_graph.rhtml | 9 ++++--- > wui/src/public/stylesheets/components.css | 15 ++++++++++++ > wui/src/public/stylesheets/layout.css | 3 +- > 6 files changed, 65 insertions(+), 7 deletions(-) > > diff --git a/wui/src/app/controllers/graph_controller.rb b/wui/src/app/controllers/graph_controller.rb > index 0456ae5..08c1d8d 100644 > --- a/wui/src/app/controllers/graph_controller.rb > +++ b/wui/src/app/controllers/graph_controller.rb > @@ -19,7 +19,18 @@ class GraphController < ApplicationController > } > ] > } > - else > + elsif params[:type] == "detail" > + graph_object = { > + :timepoints => ["April 1", "April 2","April 3","April 4","April 5","April 6","April 7"], > + :dataset => [{ > + :name =>'Peak', > + :values => [75.97, 71.80, 68.16, 56.64,95.97, 81.80, 28.16], > + :fill => 'lightblue', > + :stroke => 'blue', > + :strokeWidth => 3 > + }] > + } > + else > graph_object = { > :timepoints => ["April 1", "April 2","April 3","April 4"], > :dataset => [{ > diff --git a/wui/src/app/controllers/hardware_controller.rb b/wui/src/app/controllers/hardware_controller.rb > index ee143bb..265c831 100644 > --- a/wui/src/app/controllers/hardware_controller.rb > +++ b/wui/src/app/controllers/hardware_controller.rb > @@ -144,7 +144,7 @@ class HardwareController < ApplicationController > dates = [ Date::ABBR_MONTHNAMES[today.month] + ' ' + today.day.to_s ] > 1.upto(6){ |x| # TODO get # of days from wui > dte = today - x > - dates.push ( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s ) > + dates.push( Date::ABBR_MONTHNAMES[dte.month] + ' ' + dte.day.to_s ) > } > dates.reverse! # want in ascending order > > diff --git a/wui/src/app/views/host/show.rhtml b/wui/src/app/views/host/show.rhtml > index 1c92e2c..dbc9028 100644 > --- a/wui/src/app/views/host/show.rhtml > +++ b/wui/src/app/views/host/show.rhtml > @@ -33,4 +33,34 @@ > <% end %> > --> > </div> > +<div class="selection_right"> > + <div class="detail-pane-chart-header"> > + <span> > + Load > + </span> > + <span style="position:absolute;right:5px;">Last 7 Days</span> > + </div> > + <%= render :partial => '/layouts/graph', > + :locals => { > + :div_id => 'host_load', > + :css_class => 'detail-pane-chart', > + :chartType => 'line', > + :yGridLines => 'lightgrey', > + :url => (url_for :controller => 'graph', :action => 'graph', :type => 'detail') } %> > + <div class="detail-pane-chart-header"> > + <span> > + RAM > + </span> > + <span style="position:absolute;right:5px;"> > + Last 7 Days > + </span> > + </div> > + <%= render :partial => '/layouts/graph', > + :locals => { > + :div_id => 'host_ram', > + :css_class => 'detail-pane-chart', > + :chartType => 'line', > + :yGridLines => 'lightgrey', > + :url => (url_for :controller => 'graph', :action => 'graph', :type => 'detail') } %> > +</div> > > diff --git a/wui/src/app/views/layouts/_graph.rhtml b/wui/src/app/views/layouts/_graph.rhtml > index f2b095e..92b87ad 100644 > --- a/wui/src/app/views/layouts/_graph.rhtml > +++ b/wui/src/app/views/layouts/_graph.rhtml > @@ -1,9 +1,10 @@ > <script type="text/javascript"> > - $(document).ready(function(){ > + $(document).ready(function(){ > <% dataType = "summary" unless defined? dataType %> > <% chartType = "line" unless defined? chartType %> > <% xGridLines = "null" unless defined? xGridLines %> > <% yGridLines = "null" unless defined? yGridLines %> > + <% css_class = "" unless defined? css_class %> > > $("#<%= div_id %>").svg(); > var svg = svgManager.getSVGFor("#<%= div_id %>"); > @@ -12,7 +13,7 @@ > $.getJSON("<%= url %>", params, > function(response) { > var defs = svg.defs(); > - svg.graph.noDraw(); > + //svg.graph.noDraw().title('',10); > svg.graph.chartFormat('white', 'white').chartType("<%= chartType %>", {explode: [2], explodeDist: 10}) > $(response.dataset).each( > function(){ > @@ -25,11 +26,11 @@ > svg.graph.xAxis.ticks(1, 0).labels(response.timepoints); > svg.graph.yAxis.line('white', 0); > svg.graph.yAxis.ticks(0, 0) > - svg.graph.legend.show(false); > + svg.graph.legend.show(false); > svg.graph.redraw(); > } > ); > }); > </script> > > -<div id="<%= div_id %>"></div> > +<div id="<%= div_id %>" class="<%= css_class %>"></div> > diff --git a/wui/src/public/stylesheets/components.css b/wui/src/public/stylesheets/components.css > index 0e9301b..dc16bab 100644 > --- a/wui/src/public/stylesheets/components.css > +++ b/wui/src/public/stylesheets/components.css > @@ -201,3 +201,18 @@ > float: left; > padding-top: 3px; > } > + > +.detail-pane-chart { > + height: 50px; > + width: 375px; > +} > + > +.detail-pane-chart-header { > + width:252.5px; > + background: #F0F0F0; > + position:relative; > + margin-left:37px; > + margin-bottom:-4px; > + padding: 5px; > + border: 1px solid #CCCCCC; > +} > diff --git a/wui/src/public/stylesheets/layout.css b/wui/src/public/stylesheets/layout.css > index 455b6ca..632f13c 100644 > --- a/wui/src/public/stylesheets/layout.css > +++ b/wui/src/public/stylesheets/layout.css > @@ -251,7 +251,8 @@ textarea:focus, input:focus { > width:48%; > float: right; > border-left: 1px solid #CCCCCC; > - background: #F0F0F0; > + /*background: #F0F0F0;*/ > + height: 148px; > } > > .selection_right_title { > -- > 1.5.4.1Line graphs look like a good start... ACK and committed. --Hugh