Code review comment for lp:~sinzui/launchpad/front-page-footer-0

Revision history for this message
Abel Deuring (adeuring) wrote :

Hi Curtis,

a nice branch. I have just two nitpicks, see below.

> === modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
> --- lib/canonical/launchpad/icing/style-3-0.css.in 2010-06-07 21:48:06 +0000
> +++ lib/canonical/launchpad/icing/style-3-0.css.in 2010-06-10 22:20:48 +0000
> @@ -1785,6 +1785,10 @@
> */
> padding:2px 0 0 18px;
> }
> +.tour {
> + background-image: url(/@@/tour-icon); /* needs-ref: */
> + background-repeat: no-repeat;
> + }

I think "needs-ref" above is worth an XXX.

> === modified file 'lib/lp/app/templates/root-index.pt'
> --- lib/lp/app/templates/root-index.pt 2010-04-29 13:22:29 +0000
> +++ lib/lp/app/templates/root-index.pt 2010-06-10 22:20:48 +0000
> @@ -89,27 +89,42 @@
> tal:condition="not:view/user" tal:content="cache:anonymous">
> <h2><span class="launchpad-gold">Launchpad</span> is a software collaboration platform that provides:</h2>
> <ul tal:define="apphomes view/apphomes">
> - <li><a tal:attributes="href apphomes/bugs"><img src="/@@/bug" alt="" /></a>
> - <a tal:attributes="href apphomes/bugs">Bug tracking</a></li>
> - <li><a tal:attributes="href apphomes/code"><img src="/@@/branch" alt="" /></a>
> - <a tal:attributes="href apphomes/code">Code hosting</a>
> - using <a href="http://bazaar-vcs.org/">Bazaar</a></li>
> - <li><a href="https://help.launchpad.net/Code/Review"><img src="/@@/yes" alt="" /></a>
> - <a href="https://help.launchpad.net/Code/Review">Code reviews</a></li>
> - <li><a tal:attributes="href apphomes/ubuntu"><img src="/@@/ubuntu-icon" alt="" /></a>
> - <a tal:attributes="href apphomes/ubuntu">Ubuntu package building and hosting</a></li>
> - <li><a tal:attributes="href apphomes/translations"><img src="/@@/translation" alt="" /></a>
> - <a tal:attributes="href apphomes/translations">Translations</a></li>
> - <li><a href="https://help.launchpad.net/Teams/MailingLists"><img src="/@@/mail" alt="" /></a>
> - <a href="https://help.launchpad.net/Teams/MailingLists">Mailing lists</a></li>
> - <li><a tal:attributes="href apphomes/answers"><img src="/@@/question" alt="" /></a>
> - <a tal:attributes="href apphomes/answers">Answer tracking and FAQs</a></li>
> - <li><a tal:attributes="href apphomes/blueprints"><img src="/@@/blueprint" alt="" /></a>
> - <a tal:attributes="href apphomes/blueprints">Specification tracking</a></li>
> + <li>
> + <a class="sprite bug"
> + tal:attributes="href apphomes/bugs">Bug tracking</a>
> + </li>
> + <li>
> + <a class="sprite branch"
> + tal:attributes="href apphomes/code">Code hosting</a>
> + using <a href="http://bazaar.canonical.com/">Bazaar</a>
> + </li>
> + <li>
> + <a class="sprite yes"
> + href="https://help.launchpad.net/Code/Review">Code reviews</a>
> + </li>
> + <li>
> + <a class="sprite ubuntu-logo"

The line above has a trailing space.

review: Approve (code)

« Back to merge proposal