Merge lp:~edwin-grubbs/launchpad/bug-436507-distro-sourcepackage-link into lp:launchpad

Proposed by Edwin Grubbs
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-436507-distro-sourcepackage-link
Merge into: lp:launchpad
Diff against target: 71 lines
3 files modified
lib/lp/registry/browser/sourcepackage.py (+9/-1)
lib/lp/registry/templates/sourcepackage-index.pt (+6/-1)
lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt (+14/-8)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-436507-distro-sourcepackage-link
Reviewer Review Type Date Requested Status
Martin Albisetti (community) ui Approve
Curtis Hovey (community) ui Approve
Henning Eggers (community) code Approve
Review via email: mp+13878@code.launchpad.net

Commit message

Added link from the distroseries' sourcepackage pages to the distro's sourcepackage page.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Revision history for this message
Henning Eggers (henninge) wrote :

Thanks for adding this link. The code looks good to me. I saw you also fixed the indention in the template. Very good!

Henning

review: Approve (code)
Revision history for this message
Curtis Hovey (sinzui) 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.

review: Approve (ui)
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.

Hi Curtis,

This stylesheet overrides the background, which prevents the sprite from showing up. I also don't see any other see-all links with icons, so I assumed that it was intended to just be text.

.see-all a {
    padding-left: 8px;
    background: inherit;
    font-size: 72%;
    text-decoration: underline;
    }

-Edwin

Revision history for this message
Martin Albisetti (beuno) :
review: Approve (ui)
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"

}}}

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thanks Edwin. I think that looks nicer.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py 2009-10-16 15:00:55 +0000
+++ lib/lp/registry/browser/sourcepackage.py 2009-10-27 03:26:11 +0000
@@ -90,7 +90,15 @@
90 usedfor = ISourcePackage90 usedfor = ISourcePackage
91 facet = 'overview'91 facet = 'overview'
92 links = [92 links = [
93 'packaging', 'edit_packaging', 'changelog', 'builds', 'set_upstream']93 'distribution_source_package', 'packaging', 'edit_packaging',
94 'changelog', 'builds', 'set_upstream',
95 ]
96
97 def distribution_source_package(self):
98 target = canonical_url(self.context.distribution_sourcepackage)
99 text = 'All versions of %s source in %s' % (
100 self.context.name, self.context.distribution.displayname)
101 return Link(target, text, icon='package-source')
94102
95 def changelog(self):103 def changelog(self):
96 return Link('+changelog', 'View changelog', icon='list')104 return Link('+changelog', 'View changelog', icon='list')
97105
=== modified file 'lib/lp/registry/templates/sourcepackage-index.pt'
--- lib/lp/registry/templates/sourcepackage-index.pt 2009-10-22 19:22:20 +0000
+++ lib/lp/registry/templates/sourcepackage-index.pt 2009-10-27 03:26:11 +0000
@@ -8,7 +8,12 @@
8 <body>8 <body>
9 <div metal:fill-slot="main"9 <div metal:fill-slot="main"
10 tal:define="current context/currentrelease">10 tal:define="current context/currentrelease">
11 <div class="top-portlet">11 <ul class="horizontal">
12 <li>
13 <a tal:replace="structure context/menu:overview/distribution_source_package/fmt:link" />
14 </li>
15 </ul>
16 <div class="portlet">
12 <h2>Versions published</h2>17 <h2>Versions published</h2>
1318
14 <p class="warning message" tal:condition="not:current">19 <p class="warning message" tal:condition="not:current">
1520
=== modified file 'lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt'
--- lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-09-18 12:18:51 +0000
+++ lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-10-27 03:26:11 +0000
@@ -10,14 +10,20 @@
1010
11<div metal:fill-slot="main">11<div metal:fill-slot="main">
1212
13 <div class="top-portlet">13 <ul class="horizontal">
14 <h2>Publishing history</h2>14 <li>
1515 <a tal:replace="structure context/sourcepackage/menu:overview/distribution_source_package/fmt:link" />
16 <div id="publishing_history">16 </li>
17 <tal:block repeat="publishing context/publishing_history"17 </ul>
18 replace="structure publishing/@@+listing-detailed">18
19 </tal:block>19 <div class="portlet">
20 </div>20 <h2>Publishing history</h2>
21
22 <div id="publishing_history">
23 <tal:block repeat="publishing context/publishing_history"
24 replace="structure publishing/@@+listing-detailed">
25 </tal:block>
26 </div>
21 </div>27 </div>
2228
23 <div class="yui-g">29 <div class="yui-g">