launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04892
[Merge] lp:~gmb/launchpad/show-comment-form into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/show-comment-form into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #838825 in Launchpad itself: "Async comment loading doesn't display the comment form"
https://bugs.launchpad.net/launchpad/+bug/838825
For more details, see:
https://code.launchpad.net/~gmb/launchpad/show-comment-form/+merge/74397
This branch fixes bug 838825 by making the add comment form appear once all a bug's comments have finished loading asynchronously.
The Javascript change is simple, and I've refactored the comment form template into a macro so that we're not cluttering up bugtask-index.pt more than necessary. I've also cleaned up some lint as a drive-by fix.
QA Plan
=======
- Make sure that the bugs.batched_comment_loading.enabled flag is set to 'on' on qastaging
- Visit a bug with lots of comments
- Click the "show all comments" link
- All the comments should load
- The comment form should appear at the bottom
--
https://code.launchpad.net/~gmb/launchpad/show-comment-form/+merge/74397
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/show-comment-form into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-09-05 23:14:24 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-09-07 11:33:24 +0000
@@ -1098,11 +1098,12 @@
namespace.load_more_comments = function(batched_comments_url,
comments_container) {
var spinner = Y.Node.create(
- '<img src="/@@/spinner" style="text_align: center; display: none" />');
+ '<img src="/@@/spinner" style="text_align: center; ' +
+ 'display: none" />');
var spinner_span = Y.one('#more-comments-spinner');
spinner_span.setStyle('display', 'inline');
var handlers = {
- success: function(transactionid, response, arguments) {
+ success: function(transactionid, response, args) {
var new_comments_node =
Y.Node.create("<div></div>");
new_comments_node.set(
@@ -1120,9 +1121,17 @@
namespace.load_more_comments(
batched_comments_url, comments_container);
} else {
- // Remove the comments-hidden message to avoid
+ // Remove the comments-hidden messages to avoid
// confusion.
- Y.one('#comments-hidden-message').remove();
+ Y.each(Y.all('.comments-hidden-message'), function(message) {
+ message.remove();
+ });
+ // Show the comment form, if available.
+ var comment_form_container = Y.one(
+ '#add-comment-form-container');
+ if (Y.Lang.isValue(comment_form_container)) {
+ comment_form_container.toggleClass('hidden');
+ }
}
}
};
@@ -1152,7 +1161,7 @@
});
show_comments_link.addClass('js-action');
}
-}
+};
}, "0.1", {"requires": ["base", "oop", "node", "event", "io-base",
=== modified file 'lib/lp/bugs/templates/bugcomment-macros.pt'
--- lib/lp/bugs/templates/bugcomment-macros.pt 2011-08-30 12:29:14 +0000
+++ lib/lp/bugs/templates/bugcomment-macros.pt 2011-09-07 11:33:24 +0000
@@ -51,8 +51,7 @@
xmlns:metal="http://xml.zope.org/namespaces/metal";
metal:define-macro="break">
<div id="comments-container"></div>
- <div class="boardComment" style="border-bottom: 0"
- id="comments-hidden-message">
+ <div class="boardComment comments-hidden-message" style="border-bottom: 0">
<div class="boardCommentDetails">
<table>
<tbody>
@@ -82,4 +81,49 @@
</div>
</div>
+<div
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ metal:define-macro="comment-form">
+ <div tal:define="comment_form nocall:context/@@+addcomment-form;
+ dummy comment_form/initialize" id="add-comment-form">
+ <div tal:condition="context/bug/duplicateof"
+ class="warning message"
+ id="warning-comment-on-duplicate">
+ Remember, this bug report is a duplicate of
+ <a tal:define="duplicateof context/bug/duplicateof"
+ tal:condition="duplicateof/required:launchpad.View"
+ tal:attributes="href duplicateof/fmt:url; title
+ duplicateof/title; style string:margin-right: 4px;
+ id string:duplicate-of-warning-link;"
+ tal:content="string:bug #${duplicateof/id}."
+ >bug #42</a>
+ <span
+ tal:define="duplicateof context/bug/duplicateof"
+ tal:condition="not:duplicateof/required:launchpad.View"
+ tal:replace="string: a private bug." />
+ Comment here only if you think the duplicate status is wrong.
+ </div>
+ <h2>Add comment</h2>
+ <form action="+addcomment"
+ method="post"
+ enctype="multipart/form-data"
+ accept-charset="UTF-8">
+ <tal:comment-input
+ replace="structure comment_form/widgets/comment" />
+ <div class="actions"
+ tal:content="structure
+ comment_form/actions/field.actions.save/render" />
+ </form>
+ <script type="text/javascript">
+ LPS.use('lp.app.comment', function(Y) {
+ var comment = new Y.lp.app.comment.Comment();
+ comment.render();
+ });
+ </script>
+ </div>
+ <tal:attachment-link
+ define="add_attachment_link context_menu/addcomment"
+ replace="structure add_attachment_link/render" />
+ </div>
</tal:root>
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-09-02 01:01:49 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-09-07 11:33:24 +0000
@@ -297,49 +297,8 @@
<tal:comment-list-complete
tal:condition="not:view/visible_comments_truncated_for_display">
<tal:logged-in condition="view/user">
- <div tal:define="comment_form nocall:context/@@+addcomment-form;
- dummy comment_form/initialize"
- id="add-comment-form">
- <div
- tal:condition="context/bug/duplicateof"
- class="warning message"
- id="warning-comment-on-duplicate">
- Remember, this bug report is a duplicate of
- <a
- tal:define="duplicateof context/bug/duplicateof"
- tal:condition="duplicateof/required:launchpad.View"
- tal:attributes="href duplicateof/fmt:url; title
- duplicateof/title; style string:margin-right: 4px;
- id string:duplicate-of-warning-link;"
- tal:content="string:bug #${duplicateof/id}."
- >bug #42</a>
- <span
- tal:define="duplicateof context/bug/duplicateof"
- tal:condition="not:duplicateof/required:launchpad.View"
- tal:replace="string: a private bug." />
- Comment here only if you think the duplicate status is wrong.
- </div>
- <h2>Add comment</h2>
- <form action="+addcomment"
- method="post"
- enctype="multipart/form-data"
- accept-charset="UTF-8">
- <tal:comment-input
- replace="structure comment_form/widgets/comment" />
- <div class="actions"
- tal:content="structure
- comment_form/actions/field.actions.save/render" />
- </form>
- <script type="text/javascript">
- LPS.use('lp.app.comment', function(Y) {
- var comment = new Y.lp.app.comment.Comment();
- comment.render();
- });
- </script>
- </div>
- <tal:attachment-link
- define="add_attachment_link context_menu/addcomment"
- replace="structure add_attachment_link/render" />
+ <metal:comment-form
+ metal:use-macro="context/@@bugcomment-macros/comment-form" />
</tal:logged-in>
<tal:not-logged-in condition="not: view/user">
@@ -351,7 +310,7 @@
</tal:comment-list-complete>
<tal:comment-list-truncated
tal:condition="view/visible_comments_truncated_for_display">
- <div class="informational message">
+ <div class="informational message comments-hidden-message" >
Displaying first <span
tal:replace="view/visible_initial_comments">23</span>
and last <span
@@ -367,6 +326,12 @@
view_all_href">add a comment</a>.
</tal:what-next>
</div>
+ <tal:logged-in condition="view/user">
+ <div id="add-comment-form-container" class="hidden">
+ <metal:comment-form
+ metal:use-macro="context/@@bugcomment-macros/comment-form" />
+ </div>
+ </tal:logged-in>
</tal:comment-list-truncated>
</tal:comments>
