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

[Edwin Grubbs]

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

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.

[j.c.sackett (jcsackett)]

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*)

[Curtis Hovey (sinzui)]

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

review: Approve (code)

[Edwin Grubbs (edwin-grubbs)]

> 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.

=== 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 @@
         """
         return self.context.userCanEditMilestone(self.user)
 
+    @property
+    def style_for_add_milestone(self):
+        if self.context.milestone is None:
+            return ''
+        else:
+            return 'display: none'
+
+    @property
+    def style_for_edit_milestone(self):
+        if self.context.milestone is None:
+            return 'display: none'
+        else:
+            return ''
+
     def js_config(self):
         """Configuration for the JS widgets on the row, JSON-serialized."""
         assignee_vocabulary = get_assignee_vocabulary(self.context)

=== 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 @@
       <div class="milestone-content"
            tal:condition="view/target_has_milestones"
            style="width: 100%; float: left">
-        <tal:not-milestoned condition="python: not context.milestone and
-                                               view.user_can_edit_milestone">
-          <a href="+editstatus" tal:attributes="href editstatus_url">
-            <img class="addicon" src="/@@/add" />
-          </a>
-          <a href="+editstatus" class="nulltext js-action"
-             tal:attributes="href editstatus_url">Target to milestone</a>
-          <a href="+editstatus" class="value" style="display:none"
-             tal:attributes="href editstatus_url" />
-          <a href="+editstatus"
-             tal:attributes="href editstatus_url">
-            <img class="editicon" style="display:none" src="/@@/edit" />
-          </a>
-        </tal:not-milestoned>
-        <tal:milestoned condition="context/milestone">
-          <a href="+editstatus"
-             tal:attributes="href editstatus_url">
-            <img class="addicon" style="display:none" src="/@@/add" />
-          </a>
-          <a href="+editstatus" style="display:none" class="nulltext js-sction"
-             tal:attributes="href editstatus_url">
-            Target to milestone
-          </a>
-          <a tal:attributes="href context/milestone/fmt:url" class="value"
-             tal:content="context/milestone/title" />
-          <a href="+editstatus" tal:condition="view/user_can_edit_milestone"
-             tal:attributes="href editstatus_url">
-            <img class="editicon" src="/@@/edit" />
-          </a>
-        </tal:milestoned>
+        <a tal:condition="view/user_can_edit_milestone"
+           class="nulltext addicon js-action sprite add"
+           tal:attributes="href editstatus_url;
+                           style view/style_for_add_milestone">
+          Target to milestone
+        </a>
+        <a class="value"
+           tal:attributes="href context/milestone/fmt:url | nothing"
+           tal:content="context/milestone/title | nothing" />
+        <a tal:condition="view/user_can_edit_milestone"
+           tal:attributes="href editstatus_url"
+          ><img class="editicon" src="/@@/edit"
+                tal:attributes="style view/style_for_edit_milestone"
+          /></a>
       </div>
     </td>