Merge lp:~edwin-grubbs/launchpad/bug-435260-duplicate-links into lp:launchpad

Proposed by Edwin Grubbs
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-435260-duplicate-links
Merge into: lp:launchpad
Diff against target: 232 lines
7 files modified
lib/canonical/launchpad/icing/style-3-0.css (+13/-0)
lib/lp/registry/browser/product.py (+1/-1)
lib/lp/registry/browser/productseries.py (+1/-1)
lib/lp/registry/stories/productseries/xx-productseries-index.txt (+1/-1)
lib/lp/registry/templates/product-packages.pt (+56/-45)
lib/lp/registry/templates/productseries-index.pt (+15/-9)
lib/lp/registry/templates/productseries-milestone-table-row.pt (+1/-1)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-435260-duplicate-links
Reviewer Review Type Date Requested Status
Martin Albisetti (community) ui Approve
Brad Crittenden (community) code Approve
Review via email: mp+13657@code.launchpad.net

Commit message

Eliminate duplicate "Packaging information" links on the $product/+packaging page.

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

Summary
-------

Eliminated duplicate links on the $product/+packages page
pointing to $product/+distributions.

Renamed "Link to other package" to "Link package".

Martin informed me that action links underneeth tables
should now be aligned with the right side of the table.
However, if the table is not displayed because there are
no entries, aligning it to the right looks wrong. I added
the table-actions css class which handles both situations.

I also added the table-actions class to the series index
page to verify that it works well in different page layouts.

Tests
-----

./bin/test -vv -t xx-productseries-index.txt

Demo and Q/A
------------

* Open http://launchpad.dev/firefox/+packages
    * There should be one "Distribution packaging information" links
      instead of multiple "Packaging information" links.
    * The "Link package" link should be right aligned if it is under
      a table.
* Open http://launchpad.net/launchpad/3.1
    * The "Create milestone" and "Create release" links should be
      aligned to the right.
* Open http://launchpad.net/launchpad/trunk
    * The "Create milestone" and "Create release" links should be
      aligned to the left since there is no table above it.

Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Here are screenshots for the UI review.

This screenshot has the duplicate "Packaging information" links replaced with a single "Distribution packaging information" link. The "Link package" link is also right aligned when it appears under a table.
https://chinstrap.canonical.com/~egrubbs/project_packages.png

These screenshots show the "Create milestone" and "Create release" links being left or right aligned depending on whether they appear under a table.
https://chinstrap.canonical.com/~egrubbs/series_with_milestones.png
https://chinstrap.canonical.com/~egrubbs/series_without_milestones.png

Revision history for this message
Martin Albisetti (beuno) wrote :

This is great, thank you Edwin. The only thing I can see I'd like to tweak is to add a   in "Release now" so the link doesn't wrap.

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
2--- lib/canonical/launchpad/icing/style-3-0.css 2009-10-16 20:35:32 +0000
3+++ lib/canonical/launchpad/icing/style-3-0.css 2009-10-20 22:19:17 +0000
4@@ -445,6 +445,19 @@
5 display: inline;
6 padding: 0 1.5em 0 0;
7 }
8+.table-actions:nth-child(2) {
9+ /* text-align only works here because the <li> also has the
10+ style display:inline.
11+ */
12+ text-align: right;
13+ }
14+.table-actions {
15+ margin: 1em 0 0 0;
16+ }
17+.table-actions li {
18+ display: inline;
19+ padding: 0 1.5em 0 0;
20+ }
21 .subordinate {
22 margin-left: 4em;
23 }
24
25=== modified file 'lib/lp/registry/browser/product.py'
26--- lib/lp/registry/browser/product.py 2009-10-16 14:05:31 +0000
27+++ lib/lp/registry/browser/product.py 2009-10-20 22:19:17 +0000
28@@ -388,7 +388,7 @@
29 return Link('+topcontributors', text, icon='info')
30
31 def distributions(self):
32- text = 'Packaging information'
33+ text = 'Distribution packaging information'
34 return Link('+distributions', text, icon='info')
35
36 def packages(self):
37
38=== modified file 'lib/lp/registry/browser/productseries.py'
39--- lib/lp/registry/browser/productseries.py 2009-10-19 15:38:06 +0000
40+++ lib/lp/registry/browser/productseries.py 2009-10-20 22:19:17 +0000
41@@ -199,7 +199,7 @@
42
43 def add_package(self):
44 """Return a link to link this series to a sourcepackage."""
45- text = 'Link to other package'
46+ text = 'Link package'
47 return Link('+addpackage', text, icon='add')
48
49 @enabled_with_permission('launchpad.Edit')
50
51=== modified file 'lib/lp/registry/stories/productseries/xx-productseries-index.txt'
52--- lib/lp/registry/stories/productseries/xx-productseries-index.txt 2009-09-14 15:03:45 +0000
53+++ lib/lp/registry/stories/productseries/xx-productseries-index.txt 2009-10-20 22:19:17 +0000
54@@ -135,7 +135,7 @@
55 >>> driver_browser.getLink('Link to Ubuntu package')
56 <Link ... url='http://launchpad.dev/firefox/trunk/+ubuntupkg'>
57
58- >>> driver_browser.getLink('Link to other package')
59+ >>> driver_browser.getLink('Link package')
60 <Link ... url='http://launchpad.dev/firefox/trunk/+addpackage'>
61
62 There is a section that lists related links to this series
63
64=== modified file 'lib/lp/registry/templates/product-packages.pt'
65--- lib/lp/registry/templates/product-packages.pt 2009-09-15 02:16:19 +0000
66+++ lib/lp/registry/templates/product-packages.pt 2009-10-20 22:19:17 +0000
67@@ -10,60 +10,71 @@
68 <body>
69
70 <div metal:fill-slot="main">
71+ <ul class="horizontal">
72+ <li>
73+ <a tal:replace="structure context/menu:overview/distributions/fmt:link" />
74+ </li>
75+ </ul>
76
77 <tal:series repeat="series context/serieses">
78- <h2>Series &#8220;<span tal:replace="series/name">main</span>&#8221;</h2>
79-
80- <table id="packages" class="listing" tal:condition="series/packagings">
81- <thead>
82- <tr>
83- <th>Distribution</th>
84- <th>Series</th>
85- <th>Source Package</th>
86- <th>Version</th>
87- </tr>
88- </thead>
89- <tbody>
90- <tr tal:repeat="pkging series/packagings">
91- <td>
92- <a tal:replace="structure pkging/distroseries/distribution/fmt:link" />
93- </td>
94-
95- <td>
96- <a tal:attributes="href pkging/distroseries/fmt:url"><tal:version
97- replace="pkging/distroseries/version" />
98- (<tal:name replace="pkging/distroseries/displayname" />)</a>
99- </td>
100-
101- <td>
102- <a
103- tal:attributes="href string:${pkging/distroseries/fmt:url}/+source/${pkging/sourcepackagename/name}"
104- tal:content="pkging/sourcepackagename/name"
105- >Apache</a>
106- </td>
107-
108- <td><span tal:condition="pkging/sourcepackage/currentrelease"
109- tal:replace="pkging/sourcepackage/currentrelease/version"
110- >2.3.4-1</span></td>
111- </tr>
112- </tbody>
113- </table>
114+ <h2 style="margin-top: 2em">
115+ <a tal:attributes="href series/name">
116+ Series &#8220;<span tal:replace="series/name">main</span>&#8221;
117+ </a>
118+ </h2>
119
120 <p tal:condition="not: series/packagings">
121 No packages linked to this series.
122 </p>
123
124- <ul class="horizontal">
125- <li tal:condition="context/required:launchpad.Edit" >
126- <a tal:replace="structure series/menu:overview/add_package/fmt:link" />
127- </li>
128- <li>
129- <a tal:replace="structure context/menu:overview/distributions/fmt:link" />
130- </li>
131- </ul>
132+ <tal:comment condition="nothing">
133+ This DIV is necessary for the table-actions:nth-child stylesheet.
134+ </tal:comment>
135+ <div>
136+ <table id="packages" class="listing" tal:condition="series/packagings">
137+ <thead>
138+ <tr>
139+ <th>Distribution</th>
140+ <th>Series</th>
141+ <th>Source Package</th>
142+ <th>Version</th>
143+ </tr>
144+ </thead>
145+ <tbody>
146+ <tr tal:repeat="pkging series/packagings">
147+ <td>
148+ <a tal:replace="structure pkging/distroseries/distribution/fmt:link" />
149+ </td>
150+
151+ <td>
152+ <a tal:attributes="href pkging/distroseries/fmt:url"><tal:version
153+ replace="pkging/distroseries/version" />
154+ (<tal:name replace="pkging/distroseries/displayname" />)</a>
155+ </td>
156+
157+ <td>
158+ <a
159+ tal:attributes="href string:${pkging/distroseries/fmt:url}/+source/${pkging/sourcepackagename/name}"
160+ tal:content="pkging/sourcepackagename/name"
161+ >Apache</a>
162+ </td>
163+
164+ <td><span tal:condition="pkging/sourcepackage/currentrelease"
165+ tal:replace="pkging/sourcepackage/currentrelease/version"
166+ >2.3.4-1</span></td>
167+ </tr>
168+ </tbody>
169+ </table>
170+
171+ <ul class="table-actions">
172+ <li tal:condition="context/required:launchpad.Edit" >
173+ <a tal:replace="structure series/menu:overview/add_package/fmt:link" />
174+ </li>
175+ </ul>
176+ </div>
177
178 </tal:series>
179
180-</div>
181+</div>
182 </body>
183 </html>
184
185=== modified file 'lib/lp/registry/templates/productseries-index.pt'
186--- lib/lp/registry/templates/productseries-index.pt 2009-09-05 03:17:24 +0000
187+++ lib/lp/registry/templates/productseries-index.pt 2009-10-20 22:19:17 +0000
188@@ -120,16 +120,22 @@
189 <div class="portlet">
190 <h2>Milestones and releases</h2>
191
192- <table tal:replace="structure context/@@+table-releases" />
193+ <tal:comment condition="nothing">
194+ This DIV is necessary for the table-actions:nth-child stylesheet.
195+ </tal:comment>
196+ <div>
197+ <table tal:condition="context/all_milestones"
198+ tal:replace="structure context/@@+table-releases" />
199
200- <ul class="horizontal" style="margin-bottom: 1em;">
201- <li>
202- <a tal:replace="structure overview_menu/create_milestone/fmt:icon-link" />
203- </li>
204- <li>
205- <a tal:replace="structure overview_menu/create_release/fmt:icon-link" />
206- </li>
207- </ul>
208+ <ul class="table-actions" style="margin-bottom: 1em;">
209+ <li>
210+ <a tal:replace="structure overview_menu/create_milestone/fmt:icon-link" />
211+ </li>
212+ <li>
213+ <a tal:replace="structure overview_menu/create_release/fmt:icon-link" />
214+ </li>
215+ </ul>
216+ </div>
217 </div>
218
219 <div class="yui-g">
220
221=== modified file 'lib/lp/registry/templates/productseries-milestone-table-row.pt'
222--- lib/lp/registry/templates/productseries-milestone-table-row.pt 2009-09-22 16:21:12 +0000
223+++ lib/lp/registry/templates/productseries-milestone-table-row.pt 2009-10-20 22:19:17 +0000
224@@ -60,7 +60,7 @@
225 <tal:can-edit
226 define="link view/milestone/menu:overview/create_release"
227 condition="link/enabled">
228- <a style="sprite add"
229+ <a style="white-space: nowrap"
230 tal:attributes="href link/fmt:url;
231 title link/summary;
232 class string:sprite ${link/icon}">Release now</a>