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

Proposed by Björn Tillenius
Status: Merged
Merged at revision: not available
Proposed branch: lp:~bjornt/launchpad/add-comment-submit-button
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~bjornt/launchpad/add-comment-submit-button
Reviewer Review Type Date Requested Status
Martin Albisetti (community) ui Approve
Paul Hummer (community) Approve
Review via email: mp+9702@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Björn Tillenius (bjornt) wrote :

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

Revision history for this message
Björn Tillenius (bjornt) wrote :

The tests failures have been fixed and pushed.

Revision history for this message
Björn Tillenius (bjornt) wrote :

The test failures have been fixed and pushed.

Revision history for this message
Paul Hummer (rockstar) wrote :

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
Revision history for this message
Martin Albisetti (beuno) wrote :

per-fect.

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1569 * @method setup_inline_commenting1569 * @method setup_inline_commenting
1570 */1570 */
1571function setup_inline_commenting() {1571function setup_inline_commenting() {
1572 var save_changes = Y.get('[id="field.actions.save"]');1572 var submit_button = Y.get('[id="field.actions.save"]');
1573 var add_link = Y.Node.create(
1574 '<a href="+addcomment" class="sprite add js-action">' +
1575 'Add comment</a>');
1576 var progress_message = Y.Node.create(1573 var progress_message = Y.Node.create(
1577 '<span class="update-in-progress-message">Saving...</span>');1574 '<span class="update-in-progress-message">Saving...</span>');
1578 var comment_input = Y.get('[id="field.comment"]');1575 var comment_input = Y.get('[id="field.comment"]');
@@ -1582,17 +1579,32 @@
1582 clearProgressUI();1579 clearProgressUI();
1583 };1580 };
1584 error_handler.showError = function (error_msg) {1581 error_handler.showError = function (error_msg) {
1585 display_error(add_link, error_msg);1582 display_error(submit_button, error_msg);
1586 };1583 };
15871584
1588 function clearProgressUI() {1585 function clearProgressUI() {
1589 progress_message.get('parentNode').replaceChild(1586 progress_message.get('parentNode').replaceChild(
1590 add_link, progress_message);1587 submit_button, progress_message);
1591 comment_input.removeAttribute('disabled');1588 comment_input.removeAttribute('disabled');
1592 }1589 }
15931590
1594 save_changes.get('parentNode').replaceChild(add_link, save_changes);1591 function hide_or_show_submit_button(e) {
1595 add_link.on('click', function(e) {1592 if (comment_input.get('value') === '') {
1593 submit_button.set('disabled', true);
1594 }
1595 else {
1596 submit_button.set('disabled', false);
1597 }
1598 };
1599 /* Hook up hide_or_show_submit_button on both keyup and mouseup to
1600 handle both entering text with the keyboard, and pasting with the
1601 mouse. */
1602 comment_input.on('keyup', hide_or_show_submit_button);
1603 comment_input.on('mouseup', hide_or_show_submit_button);
1604 hide_or_show_submit_button(null);
1605 submit_button.addClass('js-action');
1606 submit_button.setStyle('display', 'inline');
1607 submit_button.on('click', function(e) {
1596 e.halt();1608 e.halt();
1597 var comment_text = comment_input.get('value');1609 var comment_text = comment_input.get('value');
1598 /* Don't try to add an empty comment. */1610 /* Don't try to add an empty comment. */
@@ -1626,7 +1638,8 @@
1626 }1638 }
1627 };1639 };
1628 comment_input.set('disabled', 'true');1640 comment_input.set('disabled', 'true');
1629 add_link.get('parentNode').replaceChild(progress_message, add_link);1641 submit_button.get('parentNode').replaceChild(
1642 progress_message, submit_button);
1630 lp_client.named_post(1643 lp_client.named_post(
1631 bug_repr.self_link, 'newMessage', config);1644 bug_repr.self_link, 'newMessage', config);
1632 });1645 });
16331646
=== 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 @@
46 self.addError("Either a comment or attachment "46 self.addError("Either a comment or attachment "
47 "must be provided.")47 "must be provided.")
4848
49 @action(u"Save Changes", name='save')49 @action(u"Post comment", name='save')
50 def save_action(self, action, data):50 def save_action(self, action, data):
51 """Add the comment and/or attachment."""51 """Add the comment and/or attachment."""
5252
5353
=== 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 @@
331 Remember, this bug report is a duplicate.331 Remember, this bug report is a duplicate.
332 Comment here only if you think the duplicate status is wrong.332 Comment here only if you think the duplicate status is wrong.
333 </div>333 </div>
334 <h2>Comment</h2>334 <h2>Add comment</h2>
335 <form action="+addcomment"335 <form action="+addcomment"
336 method="post"336 method="post"
337 enctype="multipart/form-data"337 enctype="multipart/form-data"