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
=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2010-01-04 17:15:00 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2010-01-19 20:55:38 +0000
@@ -202,7 +202,7 @@
202 var privacy_text = Y.one('#privacy-text');202 var privacy_text = Y.one('#privacy-text');
203 privacy_link = Y.Node.create(203 privacy_link = Y.Node.create(
204 '<a id="privacy-link" class="sprite edit" title="[edit]">' +204 '<a id="privacy-link" class="sprite edit" title="[edit]">' +
205 '<span class="invisible-link">edit</span></a>');205 '<span class="invisible-link">edit</span>&nbsp;</a>');
206 privacy_link.set('href', privacy_link_url);206 privacy_link.set('href', privacy_link_url);
207 privacy_text.appendChild(privacy_link);207 privacy_text.appendChild(privacy_link);
208 privacy_spinner = Y.Node.create(208 privacy_spinner = Y.Node.create(
209209
=== modified file 'lib/canonical/launchpad/systemhomes.py'
--- lib/canonical/launchpad/systemhomes.py 2009-07-17 00:26:05 +0000
+++ lib/canonical/launchpad/systemhomes.py 2010-01-19 20:55:38 +0000
@@ -159,7 +159,7 @@
159 def latest_bugs(self):159 def latest_bugs(self):
160 user = getUtility(ILaunchBag).user160 user = getUtility(ILaunchBag).user
161 return getUtility(IBugSet).searchAsUser(161 return getUtility(IBugSet).searchAsUser(
162 user=user, orderBy='-datecreated', limit=5)162 user=user, orderBy=['-datecreated', '-id'], limit=5)
163163
164 def default_bug_list(self, user=None):164 def default_bug_list(self, user=None):
165 return getUtility(IBugSet).searchAsUser(user)165 return getUtility(IBugSet).searchAsUser(user)
166166
=== modified file 'lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt'
--- lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt 2009-07-17 17:59:07 +0000
+++ lib/canonical/launchpad/templates/bugs-listing-table-without-navlinks.pt 2010-01-19 20:55:38 +0000
@@ -28,7 +28,7 @@
28 <tbody>28 <tbody>
29 <tr tal:repeat="bugtask context/getBugListingItems">29 <tr tal:repeat="bugtask context/getBugListingItems">
30 <td class="icon right">30 <td class="icon right">
31 <span tal:attributes="class bugtask/image:sprite_css" />31 <span tal:replace="structure bugtask/image:icon" />
32 </td>32 </td>
33 <td33 <td
34 class="amount"34 class="amount"
3535
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2009-11-23 03:10:04 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2010-01-19 20:55:38 +0000
@@ -778,7 +778,7 @@
778 ])778 ])
779779
780 icon_template = (780 icon_template = (
781 '<span alt="%s" title="%s" class="%s" />')781 '<span alt="%s" title="%s" class="%s">&nbsp;</span>')
782782
783 linked_icon_template = (783 linked_icon_template = (
784 '<a href="%s" alt="%s" title="%s" class="%s"></a>')784 '<a href="%s" alt="%s" title="%s" class="%s"></a>')
785785
=== 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'
--- lib/lp/bugs/stories/bugs/xx-front-page-recently-fixed-bugs.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/stories/bugs/xx-front-page-bug-lists.txt 2010-01-19 20:55:38 +0000
@@ -1,16 +1,105 @@
1= Recently Fixed Bugs on the Front Page =1Bug Lists on the Front Page
22===========================
3On the bugs front page there is a list of the most recently fixed bugs,3
4across all products and distributions.4On the bugs front page there is a list of the most recently reported
5and recently fixed bugs, across all products and distributions.
6
7To demonstrate this, a few fixed bugs must be created.
8
9 >>> from zope.component import getUtility
10 >>> from canonical.launchpad.ftests import login, logout
11 >>> import transaction
12 >>> from canonical.launchpad.interfaces import (
13 ... BugTaskImportance, BugTaskStatus, ILaunchBag)
14 >>> login('foo.bar@canonical.com')
15 >>> bigfixer = factory.makeProduct(name='bigfixer')
16 >>> for bug_x in range(1, 11):
17 ... summary = 'Summary for new bug %d' % bug_x
18 ... bug = factory.makeBug(title=summary, product=bigfixer)
19 ... bug.default_bugtask.transitionToStatus(
20 ... BugTaskStatus.FIXRELEASED, getUtility(ILaunchBag).user)
21 ... bug.default_bugtask.transitionToImportance(
22 ... BugTaskImportance.HIGH, getUtility(ILaunchBag).user)
23 >>> transaction.commit()
24 >>> logout()
25
26
27Reported bugs
28-------------
29
30The bugs front page can be consulted now for the list of reported bugs.
531
6 >>> anon_browser.open('http://bugs.launchpad.dev/')32 >>> anon_browser.open('http://bugs.launchpad.dev/')
33 >>> reported_bugs = find_tag_by_id(anon_browser.contents, 'reported-bugs')
34
35These bugs use a color-coded icon, via a sprite class, as an
36indication of the importance assigned to the bug.
37
38 >>> high_bugs = find_tags_by_class(str(reported_bugs), 'sprite bug-high')
39 >>> print high_bugs[0]
40 <span class="sprite bug-high">
41 </span>
42
43The list of recently reported bugs contains up to the last 5 bugs reported
44across Launchpad.
45
46 >>> for tr in reported_bugs('tr'):
47 ... icon_td, summary_td, = tr('td')
48 ... print "%s: %s" % (
49 ... summary_td.b.renderContents().strip(),
50 ... summary_td.a.renderContents())
51 Bigfixer:
52 Summary for new bug 10
53 <BLANKLINE>
54 Bigfixer:
55 Summary for new bug 9
56 <BLANKLINE>
57 Bigfixer:
58 Summary for new bug 8
59 <BLANKLINE>
60 Bigfixer:
61 Summary for new bug 7
62 <BLANKLINE>
63 Bigfixer:
64 Summary for new bug 6
65 <BLANKLINE>
66
67
68Fixed bugs
69----------
70
71The bugs front page also can be consulted for the list of fixed bugs.
72
7 >>> fixed_bugs = find_tag_by_id(anon_browser.contents, 'fixed-bugs')73 >>> fixed_bugs = find_tag_by_id(anon_browser.contents, 'fixed-bugs')
74
75These bugs also use a color-coded icon as an indication of the
76importance assigned to the bug.
77
78 >>> high_bugs = find_tags_by_class(str(fixed_bugs), 'sprite bug-high')
79 >>> print high_bugs[0]
80 <span class="sprite bug-high">
81 </span>
82
83The list of recently fixed bugs contains up to the last 5 bugs fixed
84across Launchpad.
85
8 >>> for tr in fixed_bugs('tr'):86 >>> for tr in fixed_bugs('tr'):
9 ... icon_td, summary_td, = tr('td')87 ... icon_td, summary_td, = tr('td')
10 ... print "%s: %s" % (88 ... print "%s: %s" % (
11 ... summary_td.b.renderContents().strip(),89 ... summary_td.b.renderContents().strip(),
12 ... summary_td.a.renderContents())90 ... summary_td.a.renderContents())
13 Debian: 91 Bigfixer:
14 Printing doesn't work92 Summary for new bug 10
15 <BLANKLINE>93 <BLANKLINE>
1694 Bigfixer:
95 Summary for new bug 9
96 <BLANKLINE>
97 Bigfixer:
98 Summary for new bug 8
99 <BLANKLINE>
100 Bigfixer:
101 Summary for new bug 7
102 <BLANKLINE>
103 Bigfixer:
104 Summary for new bug 6
105 <BLANKLINE>
17106
=== modified file 'lib/lp/bugs/templates/bug-listing-detailed.pt'
--- lib/lp/bugs/templates/bug-listing-detailed.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bug-listing-detailed.pt 2010-01-19 20:55:38 +0000
@@ -1,6 +1,6 @@
1<tr xmlns:tal="http://xml.zope.org/namespaces/tal">1<tr xmlns:tal="http://xml.zope.org/namespaces/tal">
2 <td class="icon left">2 <td class="icon left">
3 <img alt="" src="/@@/bug" />3 <span tal:attributes="class context/default_bugtask/image:sprite_css" />
4 </td>4 </td>
5 <td>5 <td>
6 <div>6 <div>
77
=== modified file 'lib/lp/bugs/templates/bugtarget-bugs.pt'
--- lib/lp/bugs/templates/bugtarget-bugs.pt 2009-12-19 19:12:34 +0000
+++ lib/lp/bugs/templates/bugtarget-bugs.pt 2010-01-19 20:55:38 +0000
@@ -115,7 +115,7 @@
115 <tbody>115 <tbody>
116 <tr tal:repeat="bugtask view/hot_bugtasks">116 <tr tal:repeat="bugtask view/hot_bugtasks">
117 <td class="icon left">117 <td class="icon left">
118 <img alt="" src="/@@/bug" />118 <span tal:replace="structure bugtask/image:icon" />
119 </td>119 </td>
120 <td style="text-align: right">120 <td style="text-align: right">
121 #<span tal:replace="bugtask/bug/id" />121 #<span tal:replace="bugtask/bug/id" />
122122
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2009-12-27 09:08:31 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2010-01-19 20:55:38 +0000
@@ -53,6 +53,7 @@
53 #add-comment-form { max-width: 60em; padding-bottom: 4em; }53 #add-comment-form { max-width: 60em; padding-bottom: 4em; }
54 #add-comment-form .actions {float: right;}54 #add-comment-form .actions {float: right;}
55 .bug-branch-summary dd { font-size: 85% }55 .bug-branch-summary dd { font-size: 85% }
56 a#privacy-link:link:hover, a#privacy-link:visited:hover {text-decoration:none;}
56 </style>57 </style>
57 </metal:block>58 </metal:block>
5859
5960
=== modified file 'lib/lp/bugs/templates/malone-index.pt'
--- lib/lp/bugs/templates/malone-index.pt 2009-09-04 15:06:34 +0000
+++ lib/lp/bugs/templates/malone-index.pt 2010-01-19 20:55:38 +0000
@@ -69,7 +69,7 @@
69 <div class="first yui-u">69 <div class="first yui-u">
70 <div class="portlet">70 <div class="portlet">
71 <h2>Recently reported</h2>71 <h2>Recently reported</h2>
72 <table>72 <table id="reported-bugs">
73 <tbody>73 <tbody>
74 <tr tal:repeat="bug context/latest_bugs"74 <tr tal:repeat="bug context/latest_bugs"
75 tal:replace="structure bug/@@+listing-detailed" />75 tal:replace="structure bug/@@+listing-detailed" />