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
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2010-12-21 16:23:11 +0000
+++ lib/lp/bugs/browser/bugtask.py 2010-12-21 22:34:38 +0000
@@ -3545,6 +3545,20 @@
3545 """3545 """
3546 return self.context.userCanEditMilestone(self.user)3546 return self.context.userCanEditMilestone(self.user)
35473547
3548 @property
3549 def style_for_add_milestone(self):
3550 if self.context.milestone is None:
3551 return ''
3552 else:
3553 return 'display: none'
3554
3555 @property
3556 def style_for_edit_milestone(self):
3557 if self.context.milestone is None:
3558 return 'display: none'
3559 else:
3560 return ''
3561
3548 def js_config(self):3562 def js_config(self):
3549 """Configuration for the JS widgets on the row, JSON-serialized."""3563 """Configuration for the JS widgets on the row, JSON-serialized."""
3550 assignee_vocabulary = get_assignee_vocabulary(self.context)3564 assignee_vocabulary = get_assignee_vocabulary(self.context)
35513565
=== modified file 'lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt'
--- lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2010-11-05 17:13:49 +0000
+++ lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt 2010-12-21 22:34:38 +0000
@@ -152,36 +152,20 @@
152 <div class="milestone-content"152 <div class="milestone-content"
153 tal:condition="view/target_has_milestones"153 tal:condition="view/target_has_milestones"
154 style="width: 100%; float: left">154 style="width: 100%; float: left">
155 <tal:not-milestoned condition="python: not context.milestone and155 <a tal:condition="view/user_can_edit_milestone"
156 view.user_can_edit_milestone">156 class="nulltext addicon js-action sprite add"
157 <a href="+editstatus" tal:attributes="href editstatus_url">157 tal:attributes="href editstatus_url;
158 <img class="addicon" src="/@@/add" />158 style view/style_for_add_milestone">
159 </a>159 Target to milestone
160 <a href="+editstatus" class="nulltext js-action"160 </a>
161 tal:attributes="href editstatus_url">Target to milestone</a>161 <a class="value"
162 <a href="+editstatus" class="value" style="display:none"162 tal:attributes="href context/milestone/fmt:url | nothing"
163 tal:attributes="href editstatus_url" />163 tal:content="context/milestone/title | nothing" />
164 <a href="+editstatus"164 <a tal:condition="view/user_can_edit_milestone"
165 tal:attributes="href editstatus_url">165 tal:attributes="href editstatus_url"
166 <img class="editicon" style="display:none" src="/@@/edit" />166 ><img class="editicon" src="/@@/edit"
167 </a>167 tal:attributes="style view/style_for_edit_milestone"
168 </tal:not-milestoned>168 /></a>
169 <tal:milestoned condition="context/milestone">
170 <a href="+editstatus"
171 tal:attributes="href editstatus_url">
172 <img class="addicon" style="display:none" src="/@@/add" />
173 </a>
174 <a href="+editstatus" style="display:none" class="nulltext js-sction"
175 tal:attributes="href editstatus_url">
176 Target to milestone
177 </a>
178 <a tal:attributes="href context/milestone/fmt:url" class="value"
179 tal:content="context/milestone/title" />
180 <a href="+editstatus" tal:condition="view/user_can_edit_milestone"
181 tal:attributes="href editstatus_url">
182 <img class="editicon" src="/@@/edit" />
183 </a>
184 </tal:milestoned>
185 </div>169 </div>
186 </td>170 </td>
187171