[Ovirt-devel] [PATCH server] First step of detail pane UI cleanup.

[Jason Guiditta]

> > +<!-- FIXME: for the love of pete, are we really using a list just to insert a separator??
> > +     This needs to be cleaned up as soon as somone has time.  It should be a horizontal list
> > +     for the links (tools), with this separator set as a left background on the first item.
> > +-->
> >   
> Does it being a list break anything specifically? I think a list is
> fine, we're not only using it for a separator but we're styling it in
> the css to appear horizontal / have the same effect you described. We'd
> need the same css for the divs if we we're to remove the list plus the
> exception for the first. So not sure if this comment should go in.

Mo, really there are two things here.  If it was indeed a list of links,
that would actually be perfect.  The thing is, while this may have been
the intent, it was not executed that way.  Take a look at any
content_for :action_links (for example
views/resources/quick_summary.rhtml) and you will see not a list but a
bunch of links not contained in li's.  So not only is it presented as a
bunch of links, they are links in this context:
<ul>
  <li>image</li>
  <a href="">blah</a>
  <a href="">blah</a>
</ul>

which is syntactically incorrect, and would fail any validator we threw
it at.

For the second point I am more trying to push the concept of separating
content from layout, which is really just an extension of mvc and
separating presentation from business logic, so we should all feel
pretty comfortable with the idea.  The idea of using an image as a
border is completely fine.  However, it is also completely unrelated to
the content, and therefore belongs in the stylesheet rather than the
rhtml.  In this case if we really wanted to use an image as the
separator, I would likely add a class along the lines of:
.first {
  background: url('../images/list_separator.png') center left no-repeat;
}
There may be some additional setting that need to be dropped in to make
it look completely correct, but I think the idea is clear.  However, as
it is just a gray line, my inclination would be to just use border-left
with a matching color.  One less image to maintain, and preferable css
style-wise.  Anyway, my point here is to improve our markup quality.
While some think it is unimportant, I think it reflects poorly on our
attention to detail to not try to do these things the correct way.  I
would like to see this treated as more craftsmanship than 'just get it
done'.

-j
> 
> 
> Other than that, the patch looks good, the details pane output looks
> just like Jeremy's screenshot.
> 
>     -Mo