Code review comment for lp:~edwin-grubbs/launchpad/bug-436507-distro-sourcepackage-link

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> Hi Edwin.
>
> The link text is a nice improvement. Without an icon, it looks surprising. I
> expect the link to have class="sprite info". You can land these changes with
> the addition of the icon.

Here is a screenshot of the new link placement as well as the incremental diff.

https://chinstrap.canonical.com/~egrubbs/new_sourcepackage_link.png

{{{
=== modified file 'lib/lp/registry/templates/sourcepackage-index.pt'
--- lib/lp/registry/templates/sourcepackage-index.pt 2009-10-27 03:01:25 +0000
+++ lib/lp/registry/templates/sourcepackage-index.pt 2009-10-27 03:22:15 +0000
@@ -8,13 +8,13 @@
   <body>
     <div metal:fill-slot="main"
         tal:define="current context/currentrelease">
- <div class="top-portlet">
- <h2>
- <span class="see-all">
- <a tal:replace="structure context/menu:overview/distribution_source_package/fmt:link" />
- </span>
- Versions published
- </h2>
+ <ul class="horizontal">
+ <li>
+ <a tal:replace="structure context/menu:overview/distribution_source_package/fmt:link" />
+ </li>
+ </ul>
+ <div class="portlet">
+ <h2>Versions published</h2>

         <p class="warning message" tal:condition="not:current">
           There is no current release of this source package

=== modified file 'lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt'
--- lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-10-23 22:29:57 +0000
+++ lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-10-27 03:22:31 +0000
@@ -10,13 +10,14 @@

 <div metal:fill-slot="main">

- <div class="top-portlet">
- <h2>
- <span class="see-all">
- <a tal:replace="structure context/sourcepackage/menu:overview/distribution_source_package/fmt:link" />
- </span>
- Publishing history
- </h2>
+ <ul class="horizontal">
+ <li>
+ <a tal:replace="structure context/sourcepackage/menu:overview/distribution_source_package/fmt:link" />
+ </li>
+ </ul>
+
+ <div class="portlet">
+ <h2>Publishing history</h2>

     <div id="publishing_history">
       <tal:block repeat="publishing context/publishing_history"

}}}

« Back to merge proposal