Merge lp:~benji/launchpad/bug-436247 into lp:launchpad

Proposed by Benji York
Status: Merged
Approved by: Benji York
Approved revision: no longer in the source branch.
Merged at revision: 13351
Proposed branch: lp:~benji/launchpad/bug-436247
Merge into: lp:launchpad
Diff against target: 171 lines (+44/-56)
4 files modified
lib/canonical/launchpad/icing/style-3-0.css (+1/-1)
lib/lp/bugs/javascript/subscribers_list.js (+2/-2)
lib/lp/bugs/templates/bug-portlet-actions.pt (+28/-42)
lib/lp/bugs/templates/bug-portlet-subscription.pt (+13/-11)
To merge this branch: bzr merge lp:~benji/launchpad/bug-436247
Reviewer Review Type Date Requested Status
Steve Kowalik (community) code Approve
Review via email: mp+66336@code.launchpad.net

Commit message

[r=stevenk][bug=436247] change unordered lists to use <ul>

Description of the change

Bug 436247 is about the lists of "things" in portlets should be marked
up with <ul> and <li> instead of <div>. This branch fixes that.

There are before and after screen shots attached (hopefully, if not,
I'll add a comment later pointing to them).

Lint: there is no lint, except for
./lib/canonical/launchpad/icing/style-3-0.css, which I'm ignoring
because fixing it would be a project in itself. Maybe next time.

Tests: there aren't any affected tests.

QA: check to see if pages with portlets (bug pages especially) look
right and lists therein are built with unordered lists.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Hmm, no file attachments for MPs.

Before: http://i.imgur.com/r9dUD.png
After: http://i.imgur.com/kiXbC.png

Revision history for this message
Steve Kowalik (stevenk) wrote :

Benji answered my questions about this branch on IRC.

review: Approve (code)

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 2011-06-28 19:24:39 +0000
3+++ lib/canonical/launchpad/icing/style-3-0.css 2011-07-01 10:36:17 +0000
4@@ -1949,7 +1949,7 @@
5 /* Override sprite style for edit icon on "Mark as duplicate"
6 to make the text not appear on a second line.
7 */
8- display:inline;
9+ display: inline;
10 }
11
12 /* Picker styles */
13
14=== modified file 'lib/lp/bugs/javascript/subscribers_list.js'
15--- lib/lp/bugs/javascript/subscribers_list.js 2011-06-29 13:41:36 +0000
16+++ lib/lp/bugs/javascript/subscribers_list.js 2011-07-01 10:36:17 +0000
17@@ -544,7 +544,7 @@
18 .set('text', subscriber_levels[level]));
19 // Node listing the actual subscribers.
20 node.appendChild(
21- Y.Node.create('<div />')
22+ Y.Node.create('<ul />')
23 .addClass(CSS_CLASSES.list));
24 return node;
25 };
26@@ -689,7 +689,7 @@
27 * @return {Object} Node containing a subscriber link.
28 */
29 SubscribersList.prototype._createSubscriberNode = function(subscriber) {
30- var subscriber_node = Y.Node.create('<div />')
31+ var subscriber_node = Y.Node.create('<li />')
32 .addClass(CSS_CLASSES.subscriber);
33
34 var subscriber_link = Y.Node.create('<a />');
35
36=== modified file 'lib/lp/bugs/templates/bug-portlet-actions.pt'
37--- lib/lp/bugs/templates/bug-portlet-actions.pt 2011-04-12 17:37:13 +0000
38+++ lib/lp/bugs/templates/bug-portlet-actions.pt 2011-07-01 10:36:17 +0000
39@@ -6,8 +6,8 @@
40 class="portlet vertical"
41 tal:define="context_menu context/menu:context"
42 >
43- <div id="duplicate-actions">
44- <span id="mark-duplicate-text"
45+ <ul id="duplicate-actions">
46+ <li id="mark-duplicate-text"
47 tal:define="link context_menu/markduplicate"
48 tal:condition="python: link.enabled and not
49 context.duplicateof"
50@@ -16,17 +16,12 @@
51 <a
52 tal:attributes="href link/path"
53 class="menu-link-mark-dupe">Mark as duplicate</a>
54- </span>
55- <tal:block
56- tal:condition="context/number_of_duplicates"
57- tal:define="number_of_dupes context/number_of_duplicates"
58- >
59- </tal:block>
60+ </li>
61 <tal:block
62 tal:define="duplicateof context/duplicateof"
63 tal:condition="duplicateof"
64 >
65- <a
66+ <li><a
67 tal:define="link context_menu/markduplicate"
68 tal:condition="python: link.enabled"
69 id="change_duplicate_bug"
70@@ -36,42 +31,33 @@
71 <a
72 tal:condition="duplicateof/required:launchpad.View"
73 tal:attributes="href duplicateof/fmt:url; title
74- duplicateof/title; style string:margin-right: 4px;
75- id string:duplicate-of;"
76+ duplicateof/title; style string:margin-right: 4px;
77+ id string:duplicate-of;"
78 tal:content="string:bug #${duplicateof/id}"
79 >bug #42</a>
80 <span
81 tal:condition="not:duplicateof/required:launchpad.View"
82- tal:replace="string:a private bug" />
83- </span>
84+ tal:replace="string:a private bug" /></span></li>
85 </tal:block>
86- </div>
87- <tal:createquestion
88- define="link context_menu/createquestion"
89- condition="link/enabled"
90- replace="structure link/render" />
91-
92- <tal:removequestion
93- define="link context_menu/removequestion"
94- condition="link/enabled"
95- replace="structure link/render" />
96-
97- <div>
98- <tal:addbranch
99- define="link context_menu/addbranch"
100- condition="link/enabled"
101- replace="structure link/render" />
102- </div>
103- <div>
104- <tal:linktocve
105- define="link context_menu/linktocve"
106- condition="link/enabled"
107- replace="structure link/render" />
108- </div>
109- <div>
110- <tal:unlinkcve
111- define="link context_menu/unlinkcve"
112- condition="link/enabled"
113- replace="structure link/render" />
114- </div>
115+ <li
116+ tal:define="link context_menu/createquestion"
117+ tal:condition="link/enabled"
118+ tal:content="structure link/render" />
119+ <li
120+ tal:define="link context_menu/removequestion"
121+ tal:condition="link/enabled"
122+ tal:content="structure link/render" />
123+ <li
124+ tal:define="link context_menu/addbranch"
125+ tal:condition="link/enabled"
126+ tal:content="structure link/render" />
127+ <li
128+ tal:define="link context_menu/linktocve"
129+ tal:condition="link/enabled"
130+ tal:content="structure link/render" />
131+ <li
132+ tal:define="link context_menu/unlinkcve"
133+ tal:condition="link/enabled"
134+ tal:content="structure link/render" />
135+ </ul>
136 </div>
137
138=== modified file 'lib/lp/bugs/templates/bug-portlet-subscription.pt'
139--- lib/lp/bugs/templates/bug-portlet-subscription.pt 2011-06-16 13:50:58 +0000
140+++ lib/lp/bugs/templates/bug-portlet-subscription.pt 2011-07-01 10:36:17 +0000
141@@ -36,17 +36,19 @@
142 tal:content="view/notifications_text/muted" />
143 </div>
144 <div id="sub-unsub-spinner">Subscribing...</div>
145- <tal:show-mute condition="view/user">
146- <div tal:attributes="class python:
147- 'hidden' if not view.user_should_see_mute_link else None"
148- id="mute-link-container">
149- <span tal:replace="structure context_menu/mute_subscription/render"
150- />&nbsp;<a target="help" class="sprite maybe mute-help"
151- href="/+help/subscription-mute.html"
152- >&nbsp;<span class="invisible-link">Mute help</span></a>
153- </div>
154- </tal:show-mute>
155- <div tal:content="structure context_menu/editsubscriptions/render" />
156+ <ul>
157+ <li tal:condition="view/user">
158+ <div tal:attributes="class python:
159+ 'hidden' if not view.user_should_see_mute_link else None"
160+ id="mute-link-container">
161+ <span tal:replace="structure context_menu/mute_subscription/render"
162+ />&nbsp;<a target="help" class="sprite maybe mute-help"
163+ href="/+help/subscription-mute.html"
164+ >&nbsp;<span class="invisible-link">Mute help</span></a>
165+ </div>
166+ </li>
167+ <li tal:content="structure context_menu/editsubscriptions/render" />
168+ </ul>
169 </div>
170 <script type="text/javascript">
171 LPS.use('io-base', 'node',