launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/mergeproposal-status-icon-589584 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #589584 merge proposal status is clickable but doesn't show hand cursor
  https://bugs.launchpad.net/bugs/589584

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/mergeproposal-status-icon-589584/+merge/44531

Summary
=======

On a merge proposal (for example, this one), there is both an edit icon link and a colored text section indicating the MP's status. Both are clickable and allow you to set the MP status (assuming you have permission to do so) but the mouse cursor only indicates that the edit icon is clickable.

This branch sets an anchor on the text as well, so it properly indicates that it's a control.

Implementation
==============

lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt
------------------------------------------------------------

The span that contained the MP status text is now set only to display if the edit link is not enabled. If the link is enabled, an anchor with similar css class as the span is show instead. The span, new anchor, and the anchor around the edit icon are now inside a tal block defining the link variable, since all three elements require it.

Demo & QA
=========

Create an MP on launchpad.dev; when you hover the mouse over the status text, it should change to the cursor type indicating a clickable control (this is usually a hand). Clicking the link or the edit icon should allow you to set the status.

Lint
====

$ make lint

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt
-- 
https://code.launchpad.net/~jcsackett/launchpad/mergeproposal-status-icon-589584/+merge/44531
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/mergeproposal-status-icon-589584 into lp:launchpad.

=== modified file 'lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt'
--- lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt	2010-03-15 20:28:52 +0000
+++ lib/lp/code/templates/branchmergeproposal-pagelet-summary.pt	2010-12-22 22:50:30 +0000
@@ -17,15 +17,22 @@
     <tr id="summary-row-1-status">
       <th>Status:</th>
       <td id="branchmergeproposal-status-value">
-        <span tal:attributes="class string:value mergestatus${context/queue_status/name}"
+        <tal:merge-status define="link context/menu:context/edit_status">
+        <a tal:condition="link/enabled"
+           tal:attributes="class string:value mergestatus${context/queue_status/name};
+                           href link/url">
+          Work in progress
+        </a>
+        <span tal:condition="not: link/enabled"
+              tal:attributes="class string:value mergestatus${context/queue_status/name}"
               tal:content="context/queue_status/title">
           Work in progress
         </span>
         <a tal:define="link context/menu:context/edit_status"
-           tal:condition="link/enabled"
            tal:attributes="href link/url">
            <img class="editicon" src="/@@/edit" alt="Edit status" />
         </a>
+        </tal:merge-status>
       </td>
     </tr>
     <tal:comment condition="nothing">