Launchpad itself

Merge lp:~jpds/launchpad/mirror_pages_v3 into lp:launchpad

  1. mirror_pages_v3
  2. Merge into devel
Proposed by Jonathan Davies
Status: Merged
Approved by: Curtis Hovey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jpds/launchpad/mirror_pages_v3
Merge into: lp:launchpad
Diff against target: 177 lines (+88/-56)
2 files modified
lib/lp/registry/browser/tests/distributionmirror-views.txt (+26/-1)
lib/lp/registry/templates/distributionmirror-index.pt (+62/-55)
To merge this branch: bzr merge lp:~jpds/launchpad/mirror_pages_v3
Reviewer Review Type Date Requested Status
Curtis Hovey (community) ui Approve
Graham Binns (community) code Approve
Review via email: mp+18461@code.launchpad.net

Commit message

Updated distribution mirror pages to use v3 styling.

To post a comment you must log in.
Revision history for this message
Jonathan Davies (jpds) wrote :

= Summary =

This branch updates distribution mirror pages to use v3 styling, with (./) buttons to separate options.

Revision history for this message
Graham Binns (gmb) wrote :

Hi Jonathan,

I'm happy with the template changes you've made here - UI review from Curtis pending, of course.

Two things:

 1. This isn't linked to a bug anywhere, so I don't know why these changes are being made. If there's a bug, please link it to the branch. If not, please file one and then link it. We use bugs to track work happening in Launchpad, so it's best if the branches and the bugs are linked.
 2. I said when I reviewed your branch yesterday that you need to write better cover letters. Your cover letter here isn't terribly informative. In future, please try to include:
   - Rationale for the changes in the branch (this can reference a bug if the bug has a lot of discussion or a description of a good fix).
   - The name of the person you had a pre-implementation discussion with about the branch.
   - The results of `make lint` (this helps us make sure that we don't land unused code).

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

Hi Johnathan.

This is a very nice improvement. I think the Organisation field should say "None" when the value is not set. Otherwise, this is good and I can land it for you.

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
--- lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-14 15:22:53 +0000
+++ lib/lp/registry/browser/tests/distributionmirror-views.txt 2010-02-09 21:40:34 +0000
@@ -359,10 +359,35 @@
359 ... archive_mirror, '+index', principal=user)359 ... archive_mirror, '+index', principal=user)
360 >>> content = find_tag_by_id(view.render(), 'maincontent')360 >>> content = find_tag_by_id(view.render(), 'maincontent')
361361
362The page shows the mirror's owner:
363
364 >>> print extract_text(find_tag_by_id(content, 'owner'))
365 Owner:
366 Mark Shuttleworth
367
362The page shows the mirror status368The page shows the mirror status
363369
364 >>> print extract_text(find_tag_by_id(content, 'status'))370 >>> print extract_text(find_tag_by_id(content, 'status'))
365 Official Ubuntu Linux mirror administered by Mark Shuttleworth, ...371 Status:
372 Official
373
374The page shows which country the mirror is in:
375
376 >>> print extract_text(find_tag_by_id(content, 'country'))
377 Country:
378 Antarctica
379
380The page shows which kind of mirror a mirror is:
381
382 >>> print extract_text(find_tag_by_id(content, 'type'))
383 Type:
384 Archive
385
386And which organisation runs a mirror:
387
388 >>> print extract_text(find_tag_by_id(content, 'organisation'))
389 Organisation:
390 None
366391
367The page contains a source list...392The page contains a source list...
368393
369394
=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
--- lib/lp/registry/templates/distributionmirror-index.pt 2009-12-08 15:51:34 +0000
+++ lib/lp/registry/templates/distributionmirror-index.pt 2010-02-09 21:40:34 +0000
@@ -12,70 +12,77 @@
12 <h1 tal:content="view/page_title" />12 <h1 tal:content="view/page_title" />
13 </tal:heading>13 </tal:heading>
1414
15 <tal:main metal:fill-slot="main"15 <tal:registering metal:fill-slot="registering">
16 define="overview_menu context/menu:overview">16 <p>
1717 Registered
18 <div class="top-portlet">18 <span tal:content="context/date_created/fmt:displaydate">
19 <p id="status" class="registered">19 on 2005-01-01</span>
20 <strong>
21 <tal:status replace="context/status/title">Official</tal:status>
22 </strong>
23 <span tal:replace="context/distribution/title" /> mirror administered
24 by
25 <span tal:content="structure context/owner/fmt:link" />, first
26 registered
27 <span tal:content="context/date_created/fmt:displaydate" />
28 </p>20 </p>
21 </tal:registering>
2922
30 <tal:is-admin condition="context/required:launchpad.Edit">23 <tal:main
31 </tal:is-admin>24 metal:fill-slot="main"
25 define="overview_menu context/menu:overview">
26
27 <div class="top-portlet" tal:condition="context/description">
28 <p tal:content="structure context/description/fmt:text-to-html" />
32 </div>29 </div>
33 <h2>Archive information</h2>30
34 <div class="yui-g">31 <div class="yui-g">
35 <div class="yui-u first">32 <div class="portlet">
36 <div class="top-portlet">33 <h2>Archive information</h2>
37 <tal:is-admin condition="context/required:launchpad.Edit">34 <div class="yui-u first">
38 <dl id="whiteboard">35 <div class="two-column-list">
36 <dl id="owner">
37 <dt>Owner:</dt>
38 <dd>
39 <span tal:content="structure context/owner/fmt:link" />
40 <a tal:replace="structure overview_menu/reassign/fmt:icon"/>
41 </dd>
42 </dl>
43
44 <dl id="status">
45 <dt>Status:</dt>
46 <dd>
47 <tal:status replace="context/status/title">Official</tal:status>
48 <a tal:replace="structure overview_menu/review/fmt:icon"/>
49 </dd>
50 </dl>
51 </div>
52
53 <dl id="whiteboard"
54 tal:condition="context/required:launchpad.Edit">
39 <dt>55 <dt>
40 Whiteboard56 Whiteboard
41 <a tal:replace="structure overview_menu/review/fmt:icon"/>57 <a tal:replace="structure overview_menu/review/fmt:icon"/>
42 </dt>58 </dt>
43 <dd tal:content="structure context/whiteboard/fmt:text-to-html" />59 <dd tal:content="structure context/whiteboard/fmt:text-to-html" />
44 </dl>60 </dl>
45 </tal:is-admin>61
46 <dl tal:condition="context/description">62 </div>
47 <dt>Description:</dt>63
48 <dd class="description"64 <div class="yui-u">
49 tal:content="structure context/description/fmt:text-to-html" />65 <div class="two-column-list">
50 </dl>66 <dl id="speed">
51 </div>67 <dt>Speed:</dt>
52 </div>68 <dd tal:content="context/speed/title" />
5369 </dl>
54 <div class="yui-u">70 <dl id="country">
5571 <dt>Country:</dt>
56 <div class="top-portlet">72 <dd tal:content="context/country/name" />
57 <div class="two-column-list">73 </dl>
58 <dl>74 <dl id="type">
59 <dt>Speed:</dt>75 <dt>Type:</dt>
60 <dd tal:content="context/speed/title" />76 <dd tal:content="context/content/title" />
61 </dl>77 </dl>
62 <dl>78 <dl id="organisation">
63 <dt>Country:</dt>79 <dt>Organisation:</dt>
64 <dd tal:content="context/country/name" />80 <dd tal:condition="context/displayname"
65 </dl>81 tal:content="context/displayname" />
66 <dl>82 <dd tal:condition="not: context/displayname">None</dd>
67 <dt>Type:</dt>83 </dl>
68 <dd tal:content="context/content/title" />84 </div>
69 </dl>85 </div>
70 <dl>
71 <dt>Organisation:</dt>
72 <dd tal:content="context/displayname" />
73 </dl>
74
75 </div>
76 </div>
77 </div>
78 </div>
7986
80 <div class="portlet"87 <div class="portlet"
81 id="last-probe"88 id="last-probe"