Merge lp:~deryck/launchpad/hot-bugs-list-color-icons-439128 into lp:launchpad

Proposed by Deryck Hodge
Status: Merged
Approved by: Deryck Hodge
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~deryck/launchpad/hot-bugs-list-color-icons-439128
Merge into: lp:launchpad
Diff against target: 219 lines (+105/-15)
9 files modified
lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+1/-1)
lib/canonical/launchpad/systemhomes.py (+1/-1)
lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt (+1/-1)
lib/canonical/launchpad/webapp/tales.py (+1/-1)
lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt (+97/-8)
lib/lp/bugs/templates/bug-listing-detailed.pt (+1/-1)
lib/lp/bugs/templates/bugtarget-bugs.pt (+1/-1)
lib/lp/bugs/templates/bugtask-index.pt (+1/-0)
lib/lp/bugs/templates/malone-index.pt (+1/-1)
To merge this branch: bzr merge lp:~deryck/launchpad/hot-bugs-list-color-icons-439128
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+17664@code.launchpad.net

Commit message

Use the correct color icons for hot bug and bugs home page listings.

To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

= Summary =

This branch fixes Bug #439128, hot bugs list not having correct color
icons. I also did a drive-by fix for Bug #144101, color icons missing
from the top-level bugs.lp.net home page.

== Proposed fix ==

The correct fix is to use sprite classes rather than img tags.

== Pre-implementation notes ==

I discussed this with Graham and Tom during our standup call.

== Implementation details ==

The fix is pretty simple, but I spent some time on this branch getting
the test updated to check both bug lists -- recently reported and
recently fixed -- and also checking that sprite classes are used.

I also needed to add a secondary sort order to the latest_bugs method
because the test was unpredictable otherwise. Since 10 bugs are created
via the LaunchpadObjectFactory and all had the same timestamp, I needed
to have a secondary sort on ID to make the test consistent. I don't see
harm in this change otherwise, but welcome some scrutiny of this
particular change.

== Tests ==

./bin/test -cvvt xx-front-page-bug-lists.txt

== Demo and Q/A ==

To demo, visit bugs.lp.net/ANYPROJECT and confirm that the bugs icons
are color coded.

Also, visit bugs.lp.net itself and verify that bug icon use colors.
(Note: recently reported bugs can all be gray if no importance has been
set.)

= 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/bugs/templates/malone-index.pt
  lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt
  lib/canonical/launchpad/systemhomes.py
  lib/lp/bugs/templates/bugtarget-bugs.pt
  lib/lp/bugs/templates/bug-listing-detailed.pt

== Pylint notices ==

lib/canonical/launchpad/systemhomes.py
    51: [F0401] Unable to import 'lazr.restful' (No module named restful)
    52: [F0401] Unable to import 'lazr.restful.interfaces' (No module
named restful)
    378: [E1002, WebServiceApplication.toWADL] Use super on an old style
class

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

Hi Deryck,

Thanks for the branch. It all looks good...except it's a huge regression for webkit-based browsers. We go from always having a grey icon to having NO icon at all.

As Curtis immediately saw, your <span> is wrong in that it is invalid to do a self-closing <span />. So your changes like:

<span tal:attributes="class context/default_bugtask/image:sprite_css" />

need to be:

<span tal:attributes="class context/default_bugtask/image:sprite_css"></span>

Please be a superstar and find the other places in the codebase where this happens. Your demo URL https://bugs.edge.launchpad.net/launchpad/+bugs is currently broken too.

Looking at your new test code I'm a bit confused by:
+ >>> print high_bugs[0]
61 + <span class="sprite bug-high">
62 + </span>

Because when I look at the html for bugs.launchpad.dev I see self-closing spans. Can you figure out why that is happening?

review: Needs Fixing (code)
Revision history for this message
Brad Crittenden (bac) wrote :

Deryck would you like to consider rolling in a fix to bug 393825 at the same time? It seems to be the same class of problem.

Revision history for this message
Deryck Hodge (deryck) wrote :

I've updated the branch to fix both bug 393825 and bug 397457. I fixed the tales formatter for icon to include a nbsp and updated the js generating the privacy icon to also add an nbsp. Some minor style changes were required on the bug page to make sure the privacy icon didn't get a link hover underline.

I'm running tests now to see fallout from these changes.

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

The new changes look good Deryck and work well with webkit. Thanks for the fix.

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/javascript/bugs/bugtask-index.js'
2--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2010-01-04 17:15:00 +0000
3+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2010-01-19 20:55:38 +0000
4@@ -202,7 +202,7 @@
5 var privacy_text = Y.one('#privacy-text');
6 privacy_link = Y.Node.create(
7 '<a id="privacy-link" class="sprite edit" title="[edit]">' +
8- '<span class="invisible-link">edit</span></a>');
9+ '<span class="invisible-link">edit</span>&nbsp;</a>');
10 privacy_link.set('href', privacy_link_url);
11 privacy_text.appendChild(privacy_link);
12 privacy_spinner = Y.Node.create(
13
14=== modified file 'lib/canonical/launchpad/systemhomes.py'
15--- lib/canonical/launchpad/systemhomes.py 2009-07-17 00:26:05 +0000
16+++ lib/canonical/launchpad/systemhomes.py 2010-01-19 20:55:38 +0000
17@@ -159,7 +159,7 @@
18 def latest_bugs(self):
19 user = getUtility(ILaunchBag).user
20 return getUtility(IBugSet).searchAsUser(
21- user=user, orderBy='-datecreated', limit=5)
22+ user=user, orderBy=['-datecreated', '-id'], limit=5)
23
24 def default_bug_list(self, user=None):
25 return getUtility(IBugSet).searchAsUser(user)
26
27=== modified file 'lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt'
28--- lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt 2009-07-17 17:59:07 +0000
29+++ lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt 2010-01-19 20:55:38 +0000
30@@ -28,7 +28,7 @@
31 <tbody>
32 <tr tal:repeat="bugtask context/getBugListingItems">
33 <td class="icon right">
34- <span tal:attributes="class bugtask/image:sprite_css" />
35+ <span tal:replace="structure bugtask/image:icon" />
36 </td>
37 <td
38 class="amount"
39
40=== modified file 'lib/canonical/launchpad/webapp/tales.py'
41--- lib/canonical/launchpad/webapp/tales.py 2009-11-23 03:10:04 +0000
42+++ lib/canonical/launchpad/webapp/tales.py 2010-01-19 20:55:38 +0000
43@@ -778,7 +778,7 @@
44 ])
45
46 icon_template = (
47- '<span alt="%s" title="%s" class="%s" />')
48+ '<span alt="%s" title="%s" class="%s">&nbsp;</span>')
49
50 linked_icon_template = (
51 '<a href="%s" alt="%s" title="%s" class="%s"></a>')
52
53=== renamed file 'lib/lp/bugs/stories/bugs/xx-front-page-recently-fixed-bugs.txt' => 'lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt'
54--- lib/lp/bugs/stories/bugs/xx-front-page-recently-fixed-bugs.txt 2009-06-12 16:36:02 +0000
55+++ lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt 2010-01-19 20:55:38 +0000
56@@ -1,16 +1,105 @@
57-= Recently Fixed Bugs on the Front Page =
58-
59-On the bugs front page there is a list of the most recently fixed bugs,
60-across all products and distributions.
61+Bug Lists on the Front Page
62+===========================
63+
64+On the bugs front page there is a list of the most recently reported
65+and recently fixed bugs, across all products and distributions.
66+
67+To demonstrate this, a few fixed bugs must be created.
68+
69+ >>> from zope.component import getUtility
70+ >>> from canonical.launchpad.ftests import login, logout
71+ >>> import transaction
72+ >>> from canonical.launchpad.interfaces import (
73+ ... BugTaskImportance, BugTaskStatus, ILaunchBag)
74+ >>> login('foo.bar@canonical.com')
75+ >>> bigfixer = factory.makeProduct(name='bigfixer')
76+ >>> for bug_x in range(1, 11):
77+ ... summary = 'Summary for new bug %d' % bug_x
78+ ... bug = factory.makeBug(title=summary, product=bigfixer)
79+ ... bug.default_bugtask.transitionToStatus(
80+ ... BugTaskStatus.FIXRELEASED, getUtility(ILaunchBag).user)
81+ ... bug.default_bugtask.transitionToImportance(
82+ ... BugTaskImportance.HIGH, getUtility(ILaunchBag).user)
83+ >>> transaction.commit()
84+ >>> logout()
85+
86+
87+Reported bugs
88+-------------
89+
90+The bugs front page can be consulted now for the list of reported bugs.
91
92 >>> anon_browser.open('http://bugs.launchpad.dev/')
93+ >>> reported_bugs = find_tag_by_id(anon_browser.contents, 'reported-bugs')
94+
95+These bugs use a color-coded icon, via a sprite class, as an
96+indication of the importance assigned to the bug.
97+
98+ >>> high_bugs = find_tags_by_class(str(reported_bugs), 'sprite bug-high')
99+ >>> print high_bugs[0]
100+ <span class="sprite bug-high">
101+ </span>
102+
103+The list of recently reported bugs contains up to the last 5 bugs reported
104+across Launchpad.
105+
106+ >>> for tr in reported_bugs('tr'):
107+ ... icon_td, summary_td, = tr('td')
108+ ... print "%s: %s" % (
109+ ... summary_td.b.renderContents().strip(),
110+ ... summary_td.a.renderContents())
111+ Bigfixer:
112+ Summary for new bug 10
113+ <BLANKLINE>
114+ Bigfixer:
115+ Summary for new bug 9
116+ <BLANKLINE>
117+ Bigfixer:
118+ Summary for new bug 8
119+ <BLANKLINE>
120+ Bigfixer:
121+ Summary for new bug 7
122+ <BLANKLINE>
123+ Bigfixer:
124+ Summary for new bug 6
125+ <BLANKLINE>
126+
127+
128+Fixed bugs
129+----------
130+
131+The bugs front page also can be consulted for the list of fixed bugs.
132+
133 >>> fixed_bugs = find_tag_by_id(anon_browser.contents, 'fixed-bugs')
134+
135+These bugs also use a color-coded icon as an indication of the
136+importance assigned to the bug.
137+
138+ >>> high_bugs = find_tags_by_class(str(fixed_bugs), 'sprite bug-high')
139+ >>> print high_bugs[0]
140+ <span class="sprite bug-high">
141+ </span>
142+
143+The list of recently fixed bugs contains up to the last 5 bugs fixed
144+across Launchpad.
145+
146 >>> for tr in fixed_bugs('tr'):
147 ... icon_td, summary_td, = tr('td')
148 ... print "%s: %s" % (
149 ... summary_td.b.renderContents().strip(),
150 ... summary_td.a.renderContents())
151- Debian:
152- Printing doesn't work
153- <BLANKLINE>
154-
155+ Bigfixer:
156+ Summary for new bug 10
157+ <BLANKLINE>
158+ Bigfixer:
159+ Summary for new bug 9
160+ <BLANKLINE>
161+ Bigfixer:
162+ Summary for new bug 8
163+ <BLANKLINE>
164+ Bigfixer:
165+ Summary for new bug 7
166+ <BLANKLINE>
167+ Bigfixer:
168+ Summary for new bug 6
169+ <BLANKLINE>
170
171=== modified file 'lib/lp/bugs/templates/bug-listing-detailed.pt'
172--- lib/lp/bugs/templates/bug-listing-detailed.pt 2009-07-17 17:59:07 +0000
173+++ lib/lp/bugs/templates/bug-listing-detailed.pt 2010-01-19 20:55:38 +0000
174@@ -1,6 +1,6 @@
175 <tr xmlns:tal="http://xml.zope.org/namespaces/tal">
176 <td class="icon left">
177- <img alt="" src="/@@/bug" />
178+ <span tal:attributes="class context/default_bugtask/image:sprite_css" />
179 </td>
180 <td>
181 <div>
182
183=== modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
184--- lib/lp/bugs/templates/bugtarget-bugs.pt 2009-12-19 19:12:34 +0000
185+++ lib/lp/bugs/templates/bugtarget-bugs.pt 2010-01-19 20:55:38 +0000
186@@ -115,7 +115,7 @@
187 <tbody>
188 <tr tal:repeat="bugtask view/hot_bugtasks">
189 <td class="icon left">
190- <img alt="" src="/@@/bug" />
191+ <span tal:replace="structure bugtask/image:icon" />
192 </td>
193 <td style="text-align: right">
194 #<span tal:replace="bugtask/bug/id" />
195
196=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
197--- lib/lp/bugs/templates/bugtask-index.pt 2009-12-27 09:08:31 +0000
198+++ lib/lp/bugs/templates/bugtask-index.pt 2010-01-19 20:55:38 +0000
199@@ -53,6 +53,7 @@
200 #add-comment-form { max-width: 60em; padding-bottom: 4em; }
201 #add-comment-form .actions {float: right;}
202 .bug-branch-summary dd { font-size: 85% }
203+ a#privacy-link:link:hover, a#privacy-link:visited:hover {text-decoration:none;}
204 </style>
205 </metal:block>
206
207
208=== modified file 'lib/lp/bugs/templates/malone-index.pt'
209--- lib/lp/bugs/templates/malone-index.pt 2009-09-04 15:06:34 +0000
210+++ lib/lp/bugs/templates/malone-index.pt 2010-01-19 20:55:38 +0000
211@@ -69,7 +69,7 @@
212 <div class="first yui-u">
213 <div class="portlet">
214 <h2>Recently reported</h2>
215- <table>
216+ <table id="reported-bugs">
217 <tbody>
218 <tr tal:repeat="bug context/latest_bugs"
219 tal:replace="structure bug/@@+listing-detailed" />