Code review comment for lp:~salgado/launchpad/three-o

On Wed, 2009-08-12 at 18:00 +0000, Guilherme Salgado wrote:
> On Wed, 2009-08-12 at 15:33 +0000, Curtis Hovey wrote:
...

> > > <div class="discreet" style="text-align: right"
> > > tal:content="structure context/menu:overview/rdf/fmt:link-icon" />
> >
> > I think this still looks wrong. We can include in in the external links list
> > above or move it into the project group information portlet.
>
> I think I prefer moving it to the -details portlet.
>
> But that doesn't look nice -- see the screenshot attached.

I did not get the image, but this is what I think we can do to land. The
registrant paragraph and link list must follow the DL DIV to ensure the
floating is cleared:

    </dl>
  </div>

  <p id="registrant">
    Registered
    <tal:created replace="context/datecreated/fmt:approximatedate" />
    by <tal:registrant replace="structure context/registrant/fmt:link" />
  </p>

  <ul class="horizontal">
    <li>
      <a tal:replace="structure context/menu:overview/rdf/fmt:link" />
    <li>
  </ul>
</div>

You need to change icon of the rdf to match the sprite:

        return Link('+rdf', text, icon='download-icon')

or maybe duplicate the rule in style.css

    .download-icon {background:url(icon-sprites) 0 -160px no-repeat;}
    .download {background:url(icon-sprites) 0 -160px no-repeat;}

to make the natural implementation work

> > > <ul class="horizontal" style="float: right"
> > > tal:condition="context/menu:overview/new_product/enabled">
> > > <li>
> > > <a tal:replace="structure context/menu:overview/new_product/fmt:link" />
> > > </li>
> > > </ul>
> >
> > We should not be floating content anymore. This list should not render
> > if the link does not render.
>
> I removed the float:right from there, but I don't see what you mean by
> not rendering the list -- the <ul> is conditional on the link being
> enabled, so everything else is too.

You are correct. I was wrong.

> > > <div class="yui-g">
> > > <div class="yui-u first">
> > > <tal:bugs content="structure context/@@+portlet-latestbugs" />
> > > <tal:specs content="structure context/@@+portlet-latestspecs" />
> > > <tal:sprints content="structure context/@@+portlet-coming-sprints" />
> > > </div>
> > > <div class="yui-u">
> > > <tal:questions content="structure context/@@+portlet-latestquestions" />
> > > <tal:contributors content="structure context/@@+portlet-top-contributors" />
> > > </div>
> >
> > I think we may adjust this. The list of project and series and can be very
> > long. We may want questions moved to the left column,
>
> Shall I file a bug about this, wait for us to see what it will look like
> against real data or JF move questions to the left?

I think we should wait. I only suspect a problem and I cannot recommend
an solution without seeing product data.