<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#ffffff" text="#000000"> 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: <pre wrap="">- svg.graph.noDraw(); + //svg.graph.noDraw().title('',10);</pre> 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: <blockquote cite="mid:1211573030.4053.8.camel@localhost.localdomain" type="cite"> <pre wrap="">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 </pre> <pre wrap=""><pre wrap=""> <hr size="4" width="90%"> _______________________________________________ Ovirt-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Ovirt-devel@redhat.com">Ovirt-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/ovirt-devel">https://www.redhat.com/mailman/listinfo/ovirt-devel</a> </pre></pre> </blockquote> </body> </html>