Merge lp:~bjornt/launchpad/add-comment-submit-button into lp:launchpad

[Björn Tillenius]

[Björn Tillenius (bjornt)]

Replace the 'add comment' link with a submit button to make it more
obvious how to add a comment.

The labels are also renamed, so that the heading is "Add comment" and
the button reads "Post comment." The latter causes test failures of
course, and I intend to fix them, but I don't think it will affect the
review.

As per discussion with Martin, I also made it so that the submit button
is disabled, if there is no text in the comment box.

--
Björn Tillenius | https://launchpad.net/~bjornt

[Björn Tillenius (bjornt)]

The tests failures have been fixed and pushed.

[Björn Tillenius (bjornt)]

The test failures have been fixed and pushed.

[Paul Hummer (rockstar)]

Thanks for the branch! I'm happy with the changes, and I know you've discussed this with beuno, but it might be a good idea to have him sign off on this as well, so I'm going to request his UI review.

review: Approve

[Martin Albisetti (beuno)]

per-fect.

review: Approve (ui)

=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js	2009-08-05 09:33:08 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js	2009-08-05 16:09:16 +0000
@@ -1569,10 +1569,7 @@
  * @method setup_inline_commenting
  */
 function setup_inline_commenting() {
-    var save_changes = Y.get('[id="field.actions.save"]');
-    var add_link = Y.Node.create(
-        '<a href="+addcomment" class="sprite add js-action">' +
-        'Add comment</a>');
+    var submit_button = Y.get('[id="field.actions.save"]');
     var progress_message = Y.Node.create(
         '<span class="update-in-progress-message">Saving...</span>');
     var comment_input = Y.get('[id="field.comment"]');
@@ -1582,17 +1579,32 @@
         clearProgressUI();
     };
     error_handler.showError = function (error_msg) {
-        display_error(add_link, error_msg);
+        display_error(submit_button, error_msg);
     };
 
     function clearProgressUI() {
         progress_message.get('parentNode').replaceChild(
-            add_link, progress_message);
+            submit_button, progress_message);
         comment_input.removeAttribute('disabled');
     }
 
-    save_changes.get('parentNode').replaceChild(add_link, save_changes);
-    add_link.on('click', function(e) {
+    function hide_or_show_submit_button(e) {
+        if (comment_input.get('value') === '') {
+            submit_button.set('disabled', true);
+        }
+        else {
+            submit_button.set('disabled', false);
+        }
+    };
+    /* Hook up hide_or_show_submit_button on both keyup and mouseup to
+       handle both entering text with the keyboard, and pasting with the
+       mouse.  */
+    comment_input.on('keyup', hide_or_show_submit_button);
+    comment_input.on('mouseup', hide_or_show_submit_button);
+    hide_or_show_submit_button(null);
+    submit_button.addClass('js-action');
+    submit_button.setStyle('display', 'inline');
+    submit_button.on('click', function(e) {
         e.halt();
         var comment_text = comment_input.get('value');
         /* Don't try to add an empty comment. */
@@ -1626,7 +1638,8 @@
             }
         };
         comment_input.set('disabled', 'true');
-        add_link.get('parentNode').replaceChild(progress_message, add_link);
+        submit_button.get('parentNode').replaceChild(
+            progress_message, submit_button);
         lp_client.named_post(
             bug_repr.self_link, 'newMessage', config);
     });

=== modified file 'lib/lp/bugs/browser/bugmessage.py'
--- lib/lp/bugs/browser/bugmessage.py	2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/browser/bugmessage.py	2009-08-05 15:05:52 +0000
@@ -46,7 +46,7 @@
                 self.addError("Either a comment or attachment "
                               "must be provided.")
 
-    @action(u"Save Changes", name='save')
+    @action(u"Post comment", name='save')
     def save_action(self, action, data):
         """Add the comment and/or attachment."""
 

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2009-08-04 05:30:51 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2009-08-05 15:05:52 +0000
@@ -331,7 +331,7 @@
                   Remember, this bug report is a duplicate.
                   Comment here only if you think the duplicate status is wrong.
               </div>
-              <h2>Comment</h2>
+              <h2>Add comment</h2>
               <form action="+addcomment"
                     method="post"
                     enctype="multipart/form-data"