Launchpad itself

Merge lp:~abentley/launchpad/restyle-subscriptions into lp:launchpad

  1. restyle-subscriptions
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Barry Warsaw
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/restyle-subscriptions
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~abentley/launchpad/restyle-subscriptions
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Barry Warsaw (community) code ui* Approve
Review via email: mp+12008@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

= Summary =
Style branch merge proposal subscriptions to match branch subscriptions

== Proposed fix ==
Stop using <dl? and <dt>, start using <ul> and <li> to fix the font weight

Stop using <img src="/@@/yes" /> to remove the checkmark

== Pre-implementation notes ==
Pre-implementation was with Tim

== Implementation details ==
Removed unreachable code for saying that no one was subscribed. Code
was essentially:

<div tal:condition="view/full_subscribers">
<div tal:condition="not view/full_subscribers"> No one </div>
</div>

== Tests ==
None

== Demo and Q/A ==
Go to a branch merge proposal page. Make sure the subscriber list looks
like a branch subscriber list.

= 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/code/templates/branchmergeproposal-pagelet-subscribers.pt
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqylZEACgkQ0F+nu1YWqI3JyQCfaTQ1bULcjV6u565vjMTnbGYv
lbEAn1VNg+asVTp9BM8yAGrhpaRAqof+
=RvZM
-----END PGP SIGNATURE-----

Revision history for this message
Barry Warsaw (barry) wrote :

As I mentioned on irc, I don't much like the

[mail-icon] to all changes:

part. I feel like the word 'email' should be there after the mail icon. Also, the page has double headers.

Neither of these are introduced by your branch though, and you've said that rockstar is working on an overhaul of the page anyway. Your changes look great, thanks for making the subscribers list look like the branch subscribers.

r=me, ui*=me

review: Approve (code ui*)
Revision history for this message
Paul Hummer (rockstar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/templates/branchmergeproposal-pagelet-subscribers.pt'
2--- lib/lp/code/templates/branchmergeproposal-pagelet-subscribers.pt 2009-09-01 22:32:19 +0000
3+++ lib/lp/code/templates/branchmergeproposal-pagelet-subscribers.pt 2009-09-17 19:52:18 +0000
4@@ -19,28 +19,16 @@
5
6 <div id="full-subscribers" tal:condition="view/full_subscribers">
7 <img src="/@@/mail"/> to all changes:
8- <dl>
9- <dt tal:repeat="subscriber view/full_subscribers">
10- <img src="/@@/yes"/>
11- <tal:subscriber replace="structure subscriber/fmt:link"/>
12- </dt>
13- <dt tal:condition="not: view/full_subscribers">
14- <img src="/@@/no"/> No one
15- </dt>
16- </dl>
17+ <ul tal:repeat="subscriber view/full_subscribers">
18+ <li tal:content="structure subscriber/fmt:link"/>
19+ </ul>
20 </div>
21
22 <div id="status-subscribers" tal:condition="view/status_subscribers">
23 <img src="/@@/mail"/> to status/vote changes:
24- <dl>
25- <dt tal:repeat="subscriber view/status_subscribers">
26- <img src="/@@/yes"/>
27- <tal:subscriber replace="structure subscriber/fmt:link"/>
28- </dt>
29- <dt tal:condition="not: view/status_subscribers">
30- <img src="/@@/no"/> No one
31- </dt>
32- </dl>
33+ <ul tal:repeat="subscriber view/status_subscribers" >
34+ <li tal:content="structure subscriber/fmt:link"/>
35+ </ul>
36 </div>
37 </div>
38 </div>