[Ovirt-devel] [PATCH] graphs for detail pane of hosts take 2

[Mohammed Morsi]

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>