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
1=== modified file 'lib/lp/registry/browser/sourcepackage.py'
2--- lib/lp/registry/browser/sourcepackage.py 2009-10-16 15:00:55 +0000
3+++ lib/lp/registry/browser/sourcepackage.py 2009-10-27 03:26:11 +0000
4@@ -90,7 +90,15 @@
5 usedfor = ISourcePackage
6 facet = 'overview'
7 links = [
8- 'packaging', 'edit_packaging', 'changelog', 'builds', 'set_upstream']
9+ 'distribution_source_package', 'packaging', 'edit_packaging',
10+ 'changelog', 'builds', 'set_upstream',
11+ ]
12+
13+ def distribution_source_package(self):
14+ target = canonical_url(self.context.distribution_sourcepackage)
15+ text = 'All versions of %s source in %s' % (
16+ self.context.name, self.context.distribution.displayname)
17+ return Link(target, text, icon='package-source')
18
19 def changelog(self):
20 return Link('+changelog', 'View changelog', icon='list')
21
22=== modified file 'lib/lp/registry/templates/sourcepackage-index.pt'
23--- lib/lp/registry/templates/sourcepackage-index.pt 2009-10-22 19:22:20 +0000
24+++ lib/lp/registry/templates/sourcepackage-index.pt 2009-10-27 03:26:11 +0000
25@@ -8,7 +8,12 @@
26 <body>
27 <div metal:fill-slot="main"
28 tal:define="current context/currentrelease">
29- <div class="top-portlet">
30+ <ul class="horizontal">
31+ <li>
32+ <a tal:replace="structure context/menu:overview/distribution_source_package/fmt:link" />
33+ </li>
34+ </ul>
35+ <div class="portlet">
36 <h2>Versions published</h2>
37
38 <p class="warning message" tal:condition="not:current">
39
40=== modified file 'lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt'
41--- lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-09-18 12:18:51 +0000
42+++ lib/lp/soyuz/templates/distroseriessourcepackagerelease-index.pt 2009-10-27 03:26:11 +0000
43@@ -10,14 +10,20 @@
44
45 <div metal:fill-slot="main">
46
47- <div class="top-portlet">
48- <h2>Publishing history</h2>
49-
50- <div id="publishing_history">
51- <tal:block repeat="publishing context/publishing_history"
52- replace="structure publishing/@@+listing-detailed">
53- </tal:block>
54- </div>
55+ <ul class="horizontal">
56+ <li>
57+ <a tal:replace="structure context/sourcepackage/menu:overview/distribution_source_package/fmt:link" />
58+ </li>
59+ </ul>
60+
61+ <div class="portlet">
62+ <h2>Publishing history</h2>
63+
64+ <div id="publishing_history">
65+ <tal:block repeat="publishing context/publishing_history"
66+ replace="structure publishing/@@+listing-detailed">
67+ </tal:block>
68+ </div>
69 </div>
70
71 <div class="yui-g">