Merge lp:~cjohnston/launchpad/IC-usability-ease into lp:launchpad

[Chris Johnston]

Updated style for comment actions across bugs, answers, and MPs. Added new usability links for editing, deleting, and replying to ICs using this new style.

- Add boardCommentFooter to IC's
- Added the ability to 'Edit' 'Delete' 'Reply' to IC's via a link
- Modified the look of boardCommentFooter to lighten the color and make it shorter
- Modified Hide/Unhide comment on bugs and answers to remove duplicated "Comment" in text

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/canonical/launchpad/icing/css/typography.css'
--- lib/canonical/launchpad/icing/css/typography.css	2014-05-14 09:23:56 +0000
+++ lib/canonical/launchpad/icing/css/typography.css	2014-05-28 21:44:50 +0000
@@ -50,15 +50,15 @@
     word-wrap: break-word;
     }
 
-:link, :visited {
+:link, :visited a.js-action {
     /* Links are blue, brighter when clicked, and greyer once visited. */
     color: #03a;
     text-decoration: none;
     }
-:link:hover, :visited:hover {
+:link:hover, :visited:hover a.js-action {
     text-decoration: underline;
     }
-:link:active, :visited:active {
+:link:active, :visited:active a.js-action:active {
     color: #36c;
     }
 a[onclick], .collapsible legend a, a.js-action,
@@ -66,6 +66,7 @@
 a.js-action:active {
     /* Links that don't open separate pages are green. */
     color: #093;
+    cursor: pointer;
     }
 a.help {
     border-bottom: 1px dotted #03a;

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2014-05-21 00:59:56 +0000
+++ lib/canonical/launchpad/icing/style.css	2014-05-28 21:44:50 +0000
@@ -248,10 +248,14 @@
 }
 .boardCommentContent { line-height: 1.4em;}
 .boardCommentFooter {
-  background-color: #eee;
+  background-color: #F8F8F8;
   color: #666;
-  padding: 0.5em 12px;
+  padding: 0.15em 12px;
   border-top: 1px solid #ddd;
+  text-align: right;
+}
+.boardCommentFooter a {
+  margin-left: 1.5em;
 }
 .from-superseded {background-color: #ffffcc;}
 .adminHiddenComment {background-color:gray;}
@@ -569,9 +573,6 @@
 .commentVote {
   float: right;
 }
-.replyLink {
-  text-align: right;
-}
 .codereviewcomment .attachment, #review-diff .attachment,
 div.diff .attachmentBody {
   background-color: #f6f6f6;
@@ -607,22 +608,19 @@
 table.diff .inline-comments > td > div {
   margin: 0 1em 1.5em 1em;
 }
-
 table.diff .inline-comments .yui3-ieditor { padding-right:0!important; }
 table.diff .inline-comments .yui3-ieditor-multiline .yui3-ieditor-btns
     .bg-top-label button.lazr-btn {
   word-wrap: normal;
 }
-
 table.diff .inline-comments .boardComment {
   margin: 1em 0;
 }
-
 table.diff .inline-comments .boardCommentBody {
   word-wrap: break-word;
   font-family: monospace;
+  padding-bottom: 0.5em;
 }
-
 table.diff .inline-comments .yui3-ieditor-multiline .yui3-ieditor-btns
     .bg-top-label {
   background-image: none!important;

=== modified file 'lib/lp/answers/templates/questionmessage-display.pt'
--- lib/lp/answers/templates/questionmessage-display.pt	2011-11-29 20:36:46 +0000
+++ lib/lp/answers/templates/questionmessage-display.pt	2014-05-28 21:44:50 +0000
@@ -60,10 +60,10 @@
     tal:condition="view/canSeeSpamControls"
     class="boardCommentFooter">
     <a tal:attributes="id string:mark-spam-${context/index};"
-       class="js-action sprite edit mark-spam" href="#">
+       class="js-action mark-spam" href="#">
        <tal:not-spam
-         condition="not: context/visible">Unhide comment</tal:not-spam>
-       <tal:spam condition="context/visible">Hide comment</tal:spam>
+         condition="not: context/visible">Unhide</tal:not-spam>
+       <tal:spam condition="context/visible">Hide</tal:spam>
     </a>
   </div>
 </div>

=== modified file 'lib/lp/app/javascript/comment.js'
--- lib/lp/app/javascript/comment.js	2014-05-19 17:06:17 +0000
+++ lib/lp/app/javascript/comment.js	2014-05-28 21:44:50 +0000
@@ -85,8 +85,8 @@
 }, {
     ATTRS: {
         hidden_class: { value: "adminHiddenComment" },
-        hide_text: { value: "Hide comment" },
-        unhide_text: { value: "Unhide comment" },
+        hide_text: { value: "Hide" },
+        unhide_text: { value: "Unhide" },
         comment_list_container: {
             valueFn: function () {
                 return Y.one('#maincontentsub');

=== modified file 'lib/lp/app/javascript/tests/test_hide_comment.html'
--- lib/lp/app/javascript/tests/test_hide_comment.html	2012-10-26 09:54:28 +0000
+++ lib/lp/app/javascript/tests/test_hide_comment.html	2014-05-28 21:44:50 +0000
@@ -51,7 +51,7 @@
               <div class="boardCommentBody"><p>Foo bar baz.</p></div>
               <div class="boardCommentFooter">
                 <a id="mark-spam-0" class="mark-spam js-action sprite edit"
-                  href="#">Hide comment</a>
+                  href="#">Hide</a>
               </div>
           </div>
 
@@ -60,7 +60,7 @@
               <div class="boardCommentBody"><p>Click here for a diploma!</p></div>
               <div class="boardCommentFooter">
                 <a id="mark-spam-1" class="mark-spam js-action sprite edit"
-                  href="#">Unhide comment
+                  href="#">Unhide
                 </a>
               </div>
           </div>

=== modified file 'lib/lp/app/javascript/tests/test_hide_comment.js'
--- lib/lp/app/javascript/tests/test_hide_comment.js	2012-10-26 10:00:20 +0000
+++ lib/lp/app/javascript/tests/test_hide_comment.js	2014-05-28 21:44:50 +0000
@@ -44,8 +44,8 @@
             var comment_node = Y.one('.boardComment');
             link.simulate('click');
             Y.Assert.isTrue(comment_node.hasClass('adminHiddenComment'));
-            Y.Assert.areEqual('Unhide comment', link.get('text'),
-                'Link text should be \'Unhide comment\'');
+            Y.Assert.areEqual('Unhide', link.get('text'),
+                'Link text should be \'Unhide\'');
             Y.Assert.areEqual(
                 'https://launchpad.dev/api/devel/some/comment/',
                 LP.cache.call_data.called_url, 'Call with wrong url.');
@@ -64,8 +64,8 @@
             var comment_node = Y.one('#hidden-comment');
             link.simulate('click');
             Y.Assert.isFalse(comment_node.hasClass('adminHiddenComment'));
-            Y.Assert.areEqual('Hide comment', link.get('text'),
-                'Link text should be \'Hide comment\'');
+            Y.Assert.areEqual('Hide', link.get('text'),
+                'Link text should be \'Hide\'');
             Y.Assert.areEqual(
                 'https://launchpad.dev/api/devel/some/comment/',
                 LP.cache.call_data.called_url, 'Call with wrong url.');

=== modified file 'lib/lp/bugs/templates/bugcomment-box.pt'
--- lib/lp/bugs/templates/bugcomment-box.pt	2012-06-15 16:23:50 +0000
+++ lib/lp/bugs/templates/bugcomment-box.pt	2014-05-28 21:44:50 +0000
@@ -96,11 +96,11 @@
   <div class="boardCommentFooter" tal:condition="comment/show_footer">
     <a tal:attributes="id string:mark-spam-${context/index};"
        tal:condition="view/show_spam_controls"
-       class="js-action sprite edit mark-spam" href="#">
+       class="js-action mark-spam" href="#">
        <tal:not-spam condition="not: context/visible"
-       >Unhide comment</tal:not-spam>
+       >Unhide</tal:not-spam>
        <tal:spam condition="context/visible"
-       >Hide comment</tal:spam>
+       >Hide</tal:spam>
     </a>
     <tal:activity
         define="activity_list comment/activity"

=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-05-26 12:44:57 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-05-28 21:44:50 +0000
@@ -25,17 +25,31 @@
         });
 };
 
+namespace.find_line_number = function(element) {
+    var text_row = element.ancestor('tr', true);
+    if (text_row.hasClass('inline-comments')) {
+        text_row = text_row.previous();
+    }
+    return text_row.one('.line-no').get('text');
+};
+
+namespace.delete_draft = function(draft_div) {
+    var line_number = namespace.find_line_number(draft_div);
+    delete namespace.inlinecomments[line_number];
+    draft_div.remove(true);
+    namespace.cleanup_empty_comment_containers();
+}
+
 namespace.add_doubleclick_handler = function() {
     var handling_request = false;
-    handler = function(e) {
+    var edit_handler = function(e) {
         if (handling_request === true) {
             return;
         }
         handling_request = true;
-        var text_row = e.currentTarget.ancestor('tr', true);
-        var line_number = text_row.one('.line-no').get('text');
         // Retrieve or create a container for the comment editor.
-        var draft_div = Y.one('#comments-' + text_row.get('id') + ' .draft');
+        var line_number = namespace.find_line_number(e.currentTarget);
+        var draft_div = Y.one('#comments-diff-line-' + line_number + ' .draft');
         if (draft_div === null) {
             draft_div = namespace.create_row({'line_number': line_number});
         }
@@ -50,9 +64,7 @@
         widget.editor.on('save', function() {
             namespace.inlinecomments[line_number] = this.get('value');
             if (this.get('value') === '') {
-                delete namespace.inlinecomments[line_number];
-                draft_div.remove(true);
-                namespace.cleanup_empty_comment_containers();
+                namespace.delete_draft(draft_div);
             }
             var config = {
                 on: {
@@ -68,9 +80,11 @@
             namespace.lp_client.named_post(
                 LP.cache.context.self_link, 'saveDraftInlineComment', config);
             handling_request = false;
+            draft_div.one('.boardCommentFooter').show();
         });
         widget.editor.on('cancel', function(e) {
             handling_request = false;
+            draft_div.one('.boardCommentFooter').show();
             // If there's no saved comment to return to, just remove the
             // draft UI.
             if (namespace.inlinecomments[line_number] === undefined) {
@@ -79,14 +93,25 @@
             }
         });
         widget._triggerEdit(e);
+        draft_div.one('.boardCommentFooter').hide();
+    };
+    var delete_handler = function(e) {
+        var line_number = namespace.find_line_number(e.currentTarget);
+        var draft_div = Y.one('#comments-diff-line-' + line_number + ' .draft');
+        namespace.delete_draft(draft_div);
     };
 
     // The editor can be invoked by double-clicking a diff line or
     // clicking the line number. Hovering over a line shows an edit icon
     // near the line number, to hint that it's clickable.
     // XXX: Introduce a separate comment column.
-    Y.one('.diff').delegate('dblclick', handler, 'tr[id^="diff-line"]');
-    Y.one('.diff').delegate('click', handler, 'tr[id^="diff-line"] .line-no');
+    Y.one('.diff').delegate(
+        'click', edit_handler, '.boardCommentFooter a.draft-edit');
+    Y.one('.diff').delegate(
+        'click', delete_handler, '.boardCommentFooter a.draft-delete');
+    Y.one('.diff').delegate('dblclick', edit_handler, 'tr[id^="diff-line"]');
+    Y.one('.diff').delegate(
+        'click', edit_handler, 'tr[id^="diff-line"] .line-no');
     Y.one('.diff').delegate('hover',
         function(e) {
             e.currentTarget.one('.line-no').addClass('active');
@@ -114,6 +139,7 @@
         '<div class="boardComment">' +
         '<div class="boardCommentDetails"></div>' +
         '<div class="boardCommentBody"><div></div></div>' +
+        '<div class="boardCommentFooter"></div>' +
         '</div>');
     if (Y.Lang.isUndefined(comment.person)) {
         // Creates a draft inline comment area.
@@ -122,6 +148,9 @@
         newrow.one('.boardCommentBody div')
             .append('<span class="yui3-editable_text-text"></span>')
             .append('<div class="yui3-editable_text-trigger"></div>');
+        newrow.one('.boardCommentFooter')
+            .append('<a class="js-action draft-edit">Edit</a>')
+            .append('<a class="js-action draft-delete">Delete</a>');
         if (!Y.Lang.isUndefined(comment.text)) {
             namespace.inlinecomments[comment.line_number] = comment.text;
             newrow.one('span').set('text', comment.text);
@@ -133,6 +162,8 @@
         // Wrap resources coming from LP.cache, as they come from
         // the LP API (updates during the page life-span). This way
         // the handling code can be unified.
+        newrow.one('.boardCommentFooter')
+            .append('<a class="js-action draft-edit">Reply</a>');
         if (Y.Lang.isUndefined(comment.person.get)) {
             comment.person = namespace.lp_client.wrap_resource(
                 null, comment.person);

=== modified file 'lib/lp/code/templates/codereviewcomment-body.pt'
--- lib/lp/code/templates/codereviewcomment-body.pt	2012-02-01 15:26:32 +0000
+++ lib/lp/code/templates/codereviewcomment-body.pt	2014-05-28 21:44:50 +0000
@@ -11,14 +11,4 @@
     </div>
   </tal:good-attachments>
 
-  <div class="replyLink" itemprop="replyToUrl"
-    tal:define="link context/menu:context/reply"
-    tal:condition="link/enabled"
-    tal:attributes="url link/fmt:url">
-    <tal:reply
-      tal:replace="structure link/render">
-      Reply
-    </tal:reply>
-  </div>
-
 </tal:root>

=== modified file 'lib/lp/services/comments/templates/comment.pt'
--- lib/lp/services/comments/templates/comment.pt	2012-01-20 19:24:59 +0000
+++ lib/lp/services/comments/templates/comment.pt	2014-05-28 21:44:50 +0000
@@ -17,10 +17,19 @@
          tal:content="structure context/@@+comment-body">
       The comment body
     </div>
-    <div class="boardCommentFooter"
+    <div class="boardBugActivityBody"
          tal:condition="context/has_footer"
          tal:content="structure context/@@+comment-footer">
       Activity or other footer details.
     </div>
+    <div class="boardCommentFooter">
+        <a itemprop="replyToUrl"
+             tal:define="link context/menu:context/reply"
+             tal:condition="link/enabled"
+             tal:attributes="href link/fmt:url"
+             tal:content="link/escapedtext">
+             Reply
+        </a>
+    </div>
   </div>
 </tal:root>