Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-526608-milestone-ajax-plus-icon into lp:launchpad

  1. bug-526608-milestone-ajax-plus-icon
  2. Merge into devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: 12137
Proposed branch: lp:~edwin-grubbs/launchpad/bug-526608-milestone-ajax-plus-icon
Merge into: lp:launchpad
Diff against target: 78 lines (+28/-30)
2 files modified
lib/lp/bugs/browser/bugtask.py (+14/-0)
lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt (+14/-30)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-526608-milestone-ajax-plus-icon
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
j.c.sackett (community) code* Approve
Review via email: mp+44382@code.launchpad.net

Commit message

[r=jcsackett,sinzui][ui=none][bug=526608] Ajaxify plus sign next to ajaxified link for targeting a bugtask to a milestone.

Description of the change

Summary
-------

When a bugtask has a "Target to Milestone" ajax link, the (+) icon
should also be ajaxified instead of taking you to a new page.

Tests
-----

./bin/test -vv --layer=BugsWindmillLayer

Demo and Q/A
------------

* Open https://bugs.launchpad.dev/firefox/+bug/1
    * Target each bugtask to a milestone using the (+) icon.
    * Remove the milestone from each bugtask.
    * Target each bugtask to a milestone again to ensure that the
      javascript shows and hides all the elements properly.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good to me, assuming the text show/hide tool is tested already and therefor not showing in the diff. My memory tells me it is.

I'm getting Curtis to follow up; if it needs it, I imagine he can provide a UI review as well.

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

The content of the anchors should abut the tags to prevent odd underlining: <a ..><img ../></a>

review: Approve (code)
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> The content of the anchors should abut the tags to prevent odd underlining: <a
> ..><img ../></a>

I ended up changing the addicon into a sprite, since Firefox would apply underlining to the icon since it was in the link with "Target to milestone". I didn't change the editicon into a sprite, since the browsers would hide the <a> unless there was a &nbsp; in it, but then that would be underlined weirdly.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugtask.py'
2--- lib/lp/bugs/browser/bugtask.py 2010-12-21 16:23:11 +0000
3+++ lib/lp/bugs/browser/bugtask.py 2010-12-21 22:34:38 +0000
4@@ -3545,6 +3545,20 @@
5 """
6 return self.context.userCanEditMilestone(self.user)
7
8+ @property
9+ def style_for_add_milestone(self):
10+ if self.context.milestone is None:
11+ return ''
12+ else:
13+ return 'display: none'
14+
15+ @property
16+ def style_for_edit_milestone(self):
17+ if self.context.milestone is None:
18+ return 'display: none'
19+ else:
20+ return ''
21+
22 def js_config(self):
23 """Configuration for the JS widgets on the row, JSON-serialized."""
24 assignee_vocabulary = get_assignee_vocabulary(self.context)
25
26=== modified file 'lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt'
27--- lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2010-11-05 17:13:49 +0000
28+++ lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2010-12-21 22:34:38 +0000
29@@ -152,36 +152,20 @@
30 <div class="milestone-content"
31 tal:condition="view/target_has_milestones"
32 style="width: 100%; float: left">
33- <tal:not-milestoned condition="python: not context.milestone and
34- view.user_can_edit_milestone">
35- <a href="+editstatus" tal:attributes="href editstatus_url">
36- <img class="addicon" src="/@@/add" />
37- </a>
38- <a href="+editstatus" class="nulltext js-action"
39- tal:attributes="href editstatus_url">Target to milestone</a>
40- <a href="+editstatus" class="value" style="display:none"
41- tal:attributes="href editstatus_url" />
42- <a href="+editstatus"
43- tal:attributes="href editstatus_url">
44- <img class="editicon" style="display:none" src="/@@/edit" />
45- </a>
46- </tal:not-milestoned>
47- <tal:milestoned condition="context/milestone">
48- <a href="+editstatus"
49- tal:attributes="href editstatus_url">
50- <img class="addicon" style="display:none" src="/@@/add" />
51- </a>
52- <a href="+editstatus" style="display:none" class="nulltext js-sction"
53- tal:attributes="href editstatus_url">
54- Target to milestone
55- </a>
56- <a tal:attributes="href context/milestone/fmt:url" class="value"
57- tal:content="context/milestone/title" />
58- <a href="+editstatus" tal:condition="view/user_can_edit_milestone"
59- tal:attributes="href editstatus_url">
60- <img class="editicon" src="/@@/edit" />
61- </a>
62- </tal:milestoned>
63+ <a tal:condition="view/user_can_edit_milestone"
64+ class="nulltext addicon js-action sprite add"
65+ tal:attributes="href editstatus_url;
66+ style view/style_for_add_milestone">
67+ Target to milestone
68+ </a>
69+ <a class="value"
70+ tal:attributes="href context/milestone/fmt:url | nothing"
71+ tal:content="context/milestone/title | nothing" />
72+ <a tal:condition="view/user_can_edit_milestone"
73+ tal:attributes="href editstatus_url"
74+ ><img class="editicon" src="/@@/edit"
75+ tal:attributes="style view/style_for_edit_milestone"
76+ /></a>
77 </div>
78 </td>
79