Merge lp:~bac/launchpad/bug-419742 into lp:launchpad

Proposed by Brad Crittenden
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-419742
Merge into: lp:launchpad
Diff against target: 242 lines
4 files modified
lib/canonical/launchpad/icing/style-3-0.css (+4/-0)
lib/lp/registry/stories/product/xx-product-files.txt (+11/-10)
lib/lp/registry/templates/product-files.pt (+57/-32)
lib/lp/registry/templates/productrelease-portlet-data.pt (+6/-8)
To merge this branch: bzr merge lp:~bac/launchpad/bug-419742
Reviewer Review Type Date Requested Status
Martin Albisetti (community) ui Approve
Paul Hummer (community) Approve
Review via email: mp+13807@code.launchpad.net

Commit message

Update the +download page providing more information about releases and re-arranging to better present existing information.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 419742 suggested new information to present on the $product/+download page and
ways to make it more presentable.

== Proposed fix ==

Re-arrange lots of the data:
* Remove the extra information about verification and just present the help link.
* Move the release date into the header.
* Put 'release notes' and 'changelog' into a collapsible field called 'Release
information'.
* Add '(+) Add download file' under the release table.
* Keep the link at the end for each release but add the series name to it for
distinguishing it.[1]
* Make each release a portlet.[2]

[1] beuno had asked that this entire line be removed. I restored it because if a
release has no download files there is no place to add the '(+) Add download file'
menu link. Releases with no files are just skipped in the body of this page. So it
made sense to bring it back and just list all of the releases in one place. We often
get complaints from users that it is too hard to figure out where to go to add
download files I was hesitant to remove one place they may have grown used to using.

[2] I have made each use the 'top-portlet' class and conditionally insert the border.
 This requires less code reorganization than conditionally choosing a class. TAL is
really bad when you need to select attributes conditionally, basically you have to
violate DRY all over the place.

== Pre-implementation notes ==

Lots of discussion with beuno. Unfortunately I have not been able to have a
discussion with mbp but hope to early next week to see if he still has any issues.

== Implementation details ==

As above.

== Tests ==

bin/test -vv -t xx-product-files.txt

== Demo and Q/A ==

https://launchpad.dev/firefox/+download

Also go to any other project and add some releases and download files.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/templates/product-files.pt
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/stories/product/xx-product-files.txt
  lib/lp/registry/templates/productrelease-portlet-data.pt

Revision history for this message
Brad Crittenden (bac) wrote :

Screenshots:
http://people.canonical.com/~bac/download-bazaar.png

Shows a project with a couple of releases within a single series.

http://people.canonical.com/~bac/download-firefox.png

Shows a project with multiple series and releases. Some releases have no download files and are skipped.

Revision history for this message
Paul Hummer (rockstar) :
review: Approve
Revision history for this message
Martin Albisetti (beuno) :
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-20 16:45:23 +0000
3+++ lib/canonical/launchpad/icing/style-3-0.css 2009-10-22 20:20:27 +0000
4@@ -233,6 +233,10 @@
5 .footer .lp-branding {
6 float: left;
7 }
8+.portlet-border {
9+ border-top: 1px solid #EBEBEB;
10+ padding: 1em 0;
11+ }
12 .portlet, .aside {
13 clear: both;
14 border-top: 1px solid #EBEBEB;
15
16=== modified file 'lib/lp/registry/stories/product/xx-product-files.txt'
17--- lib/lp/registry/stories/product/xx-product-files.txt 2009-09-18 15:24:30 +0000
18+++ lib/lp/registry/stories/product/xx-product-files.txt 2009-10-22 20:20:27 +0000
19@@ -33,11 +33,11 @@
20 >>> anon_browser.getLink('All downloads').click()
21 >>> print extract_text(
22 ... find_tag_by_id(anon_browser.contents, 'project-downloads'))
23- 0.9.2 (One (secure) Tree Hill) release, from the trunk series.
24+ 0.9.2 (One (secure) Tree Hill) release from the trunk series released 2004-10-15
25+ Release information
26 Release notes:
27 Security fixes
28 * 250180 - [Windows] Disallow access to insecure shell: protocol.
29- Released: 2004-10-15
30 File Description Downloads
31 firefox-0.9.2.orig.tar.gz (md5) -
32
33@@ -99,7 +99,8 @@
34 Download project files
35 Mozilla Thunderbird...
36 No download files exist for this project...
37- Add download file for release: 0.8
38+ Add download file to the trunk series for release: 0.8
39+
40 >>> tbird_owner.getControl('Delete Files')
41 Traceback (most recent call last):
42 ...
43@@ -114,8 +115,8 @@
44 >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
45 >>> for tag in find_tags_by_class(firefox_owner.contents, 'add-files'):
46 ... print extract_text(tag)
47- Add download file for release: 0.9.2, 0.9.1, 0.9
48- Add download file for release: 1.0.0
49+ Add download file to the trunk series for release: 0.9.2, 0.9.1, 0.9
50+ Add download file to the 1.0 series for release: 1.0.0
51
52 If a project has a series with no releases associated with it, that
53 series should not show up in the list.
54@@ -129,8 +130,8 @@
55 >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
56 >>> for tag in find_tags_by_class(firefox_owner.contents, 'add-files'):
57 ... print extract_text(tag)
58- Add download file for release: 0.9.2, 0.9.1, 0.9
59- Add download file for release: 1.0.0
60+ Add download file to the trunk series for release: 0.9.2, 0.9.1, 0.9
61+ Add download file to the 1.0 series for release: 1.0.0
62
63 If a release is added to the new series the series will appear in
64 the list.
65@@ -150,9 +151,9 @@
66 >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
67 >>> for tag in find_tags_by_class(firefox_owner.contents, 'add-files'):
68 ... print extract_text(tag)
69- Add download file for release: 0.9.2, 0.9.1, 0.9
70- Add download file for release: 3.14159
71- Add download file for release: 1.0.0
72+ Add download file to the trunk series for release: 0.9.2, 0.9.1, 0.9
73+ Add download file to the 3.0 series for release: 3.14159
74+ Add download file to the 1.0 series for release: 1.0.0
75
76
77 == Adding new files ==
78
79=== modified file 'lib/lp/registry/templates/product-files.pt'
80--- lib/lp/registry/templates/product-files.pt 2009-08-31 23:58:26 +0000
81+++ lib/lp/registry/templates/product-files.pt 2009-10-22 20:20:27 +0000
82@@ -17,13 +17,9 @@
83 tal:define="has_edit context/required:launchpad.Edit">
84
85 <p tal:condition="view/has_download_files">
86- After you've downloaded a file, you can verify its authenticity using
87- <tal:hasdownloadfiles condition="view/any_download_files_with_signatures">
88- either </tal:hasdownloadfiles>its MD5 sum
89- <tal:hasdownloadfiles condition="view/any_download_files_with_signatures">
90- or signature</tal:hasdownloadfiles>. (<a
91+ <a
92 href="/+help/verify-downloads.html" target="help">How do I verify a
93- download?</a>)
94+ download?</a>
95 </p>
96 <br />
97
98@@ -45,37 +41,52 @@
99 tal:repeat="series view/sorted_series_list"
100 tal:attributes="id string:series_${series/name}">
101
102+ <div tal:condition="not: repeat/series/start" class="portlet-border" />
103+
104 <tal:seriesfilesexist condition="series/has_release_files">
105 <tal:releases repeat="release series/releases">
106- <div tal:condition="release/files">
107- <h2 style="font-size: 1.2em">
108+ <tal:releasefilesexist condition="release/files">
109+ <div tal:condition="not: repeat/release/start" class="portlet-border" />
110+ <div tal:condition="release/files" class="top-portlet">
111+ <h2 style="font-size: 1.2em; float:left;">
112 <a tal:attributes="href release/fmt:url">
113- <span tal:replace="release/name_with_codename" /> release</a>,
114+ <span tal:replace="release/name_with_codename" /> release</a>
115 from the
116 <a tal:attributes="href series/fmt:url"
117- tal:content="series/name">name</a> series.
118+ tal:content="series/name">name</a> series released
119+ <span
120+ tal:attributes="title release/datereleased/fmt:datetime"
121+ tal:content="release/datereleased/fmt:approximatedate" />
122 </h2>
123
124- <div id="release_notes">
125- <h3>Release notes:</h3>
126- <tal:notes condition="release/release_notes"
127- define="notes release/release_notes/fmt:shorten/800">
128- <div tal:content="structure notes/fmt:text-to-html">
129- Release notes
130- </div>
131- </tal:notes>
132- <div tal:condition="not: release/release_notes">
133- <em>This release does not have release notes.</em>
134- </div>
135- <div>
136- <strong>Released:</strong>
137- <span
138- tal:attributes="title release/datereleased/fmt:datetime"
139- tal:content="release/datereleased/fmt:approximatedate" />
140- </div>
141- </div>
142-
143- <table class="listing">
144+ <div style="clear:both;" />
145+
146+ <div id="release_information"
147+ tal:condition="python: release.release_notes or release.changelog">
148+
149+ <fieldset class="collapsible collapsed" style="padding: 0px;">
150+ <legend>Release information</legend>
151+
152+ <div tal:condition="release/release_notes">
153+ <strong>Release notes:</strong>
154+ <div id="release_notes" style="margin-bottom: 0px;"
155+ tal:define="notes release/release_notes/fmt:shorten/800"
156+ tal:content="structure notes/fmt:text-to-html">
157+ ProductRelease.release_notes
158+ </div>
159+ </div>
160+
161+ <div tal:condition="release/changelog">
162+ <strong>Changelog:</strong>
163+ <div id="changelog" style="margin-bottom: 0px;"
164+ tal:content="structure release/changelog/fmt:obfuscate-email/fmt:text-to-html">
165+ ProductRelease.changelog.
166+ </div>
167+ </div>
168+ </fieldset>
169+ </div> <!-- release information -->
170+
171+ <table class="listing" style="margin-top: 1em;">
172 <thead>
173 <tr>
174 <th width="35%">File</th>
175@@ -105,6 +116,17 @@
176 </tfoot>
177 </table>
178 </div>
179+ <div tal:condition="not: release/total_downloads"
180+ style="margin-top: 2em;">
181+ </div>
182+
183+ <div style="float:right"
184+ tal:define="link release/menu:context/add_file"
185+ tal:condition="link/enabled">
186+ <a tal:replace="structure link/fmt:link" />
187+ </div>
188+ <div style="clear:both" />
189+ </tal:releasefilesexist>
190 </tal:releases>
191 <br />
192
193@@ -112,7 +134,9 @@
194
195 <p tal:condition="has_edit" class="add-files">
196 <tal:releases condition="series/releases">
197- Add download file for release:
198+ Add download file to the
199+ <a tal:attributes="href series/fmt:url"
200+ tal:content="series/name">name</a> series for release:
201 <tal:release repeat="release series/releases">
202 <a tal:attributes="href string:${series/name}/${release/version}/+adddownloadfile"
203 tal:content="release/version"
204@@ -130,4 +154,5 @@
205 </form>
206 </div>
207 </body>
208-</html>
209+</html
210+>
211
212=== modified file 'lib/lp/registry/templates/productrelease-portlet-data.pt'
213--- lib/lp/registry/templates/productrelease-portlet-data.pt 2009-09-11 01:25:30 +0000
214+++ lib/lp/registry/templates/productrelease-portlet-data.pt 2009-10-22 20:20:27 +0000
215@@ -67,10 +67,9 @@
216 </div>
217
218 <div class="top-portlet">
219- <h2>
220- Release notes
221- &nbsp; <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
222- </h2>
223+ <h2 style="float:left">Release notes &nbsp;</h2>
224+ <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
225+ <div style="clear:both" />
226
227 <div id="release-notes"
228 tal:content="structure view/release/release_notes/fmt:text-to-html"
229@@ -82,10 +81,9 @@
230 <em>This release does not have release notes.</em>
231 </p>
232
233- <h2>
234- Changelog
235- &nbsp; <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
236- </h2>
237+ <h2 style="float:left">Changelog &nbsp;</h2>
238+ <a tal:replace="structure view/release/menu:context/edit/fmt:icon" />
239+ <div style="clear:both" />
240
241 <fieldset class="collapsible collapsed" style="padding: 0px;"
242 tal:condition="view/release/changelog">