← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjohnston/launchpad/publish-ic-link into lp:launchpad

 

Chris Johnston has proposed merging lp:~cjohnston/launchpad/publish-ic-link into lp:launchpad.

Commit message:
Add link below diff to return back to the top of diff (Add comment section) via scrollers.

Requested reviews:
  Celso Providelo (cprov)

For more details, see:
https://code.launchpad.net/~cjohnston/launchpad/publish-ic-link/+merge/220103
-- 
https://code.launchpad.net/~cjohnston/launchpad/publish-ic-link/+merge/220103
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-05-15 04:18:54 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-05-19 22:41:25 +0000
@@ -499,6 +499,22 @@
                 self._showPreviewDiff(previewdiff_id);
             });
         });
+
+        var rc_scroller = Y.one('.review-comment-scroller');
+        if (rc_scroller !== null) {
+            rc_scroller.remove(true);
+        }
+        rc_scroller = Y.Node.create('<a href="" />')
+            .addClass('review-comment-scroller')
+            .set('text', 'Back to top of diff');
+        this.get('srcNode').append(rc_scroller);
+        rc.link_scroller(rc_scroller, '#add-comment-form', function() {
+            if (Y.all('.draft').size() > 0) {
+                Y.one('#add-comment-form input[type=submit]').focus();
+            } else {
+                Y.one('#add-comment-form textarea').focus();
+            }
+        });
     },
 
     /**

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html	2014-05-14 20:03:00 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html	2014-05-19 22:41:25 +0000
@@ -72,7 +72,8 @@
             <li>lp.code.branchmergeproposal.inlinecomments.test</li>
         </ul>
 
-      <div>
+      <div id="add-comment-form">
+        <textarea id="field.comment"></textarea>
         <input id="field.review_type" type="text"></input>
         <input id="field.actions.add" type="submit" value="Save"></input>
       </div>

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-05-15 03:47:40 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-05-19 22:41:25 +0000
@@ -524,6 +524,46 @@
             Y.Assert.areEqual(202, called_pd_id);
         },
 
+        test_diff_nav_publish_scroller: function() {
+            // The Diff Navigator publish comment *scroller* allows
+            // user to got from the bottom of the diff to the review
+            // comment form and is updated according to the existence
+            // of draft inline comments.
+            this.diffnav.render();
+            this.reply_previewdiffs();
+            this.reply_diffcontent();
+            var mockio = this.diffnav.get('lp_client').io_provider;
+            Y.Assert.areEqual(2, mockio.requests.length);
+            // We need to re-instrument the scroller in other to
+            // instantly fire the 'end' event (which runs the code
+            // that matters to us).
+            scroller = Y.one('.review-comment-scroller');
+            scroller.on('click', function() {
+                var rc = Y.lp.code.branchmergeproposal.reviewcomment;
+                rc.window_scroll_anim.fire('end');
+            });
+            scroller.simulate('click');
+            // When there are no drafts, it scrolls to the form and focus
+            // on the comment textarea.
+            Y.Assert.areEqual(
+                'Back to top of diff', scroller.get('text'));
+            Y.Assert.areEqual('field.comment', document.activeElement.id);
+            // Create a draft inline comment and trigger an UI update.
+            module.create_row({'line_number': '2', 'text': 'another'});
+            Y.fire('inlinecomment.UPDATED');
+            // See above ...
+            scroller = Y.one('.review-comment-scroller');
+            scroller.on('click', function() {
+                var rc = Y.lp.code.branchmergeproposal.reviewcomment;
+                rc.window_scroll_anim.fire('end');
+            });
+            scroller.simulate('click');
+            // When there are draft, it scrolls to form and focus on the
+            // submit button.
+            Y.Assert.areEqual(
+               'field.actions.add', document.activeElement.id);
+        },
+
         test_diff_nav_failed_diff_content: function() {
             // An error message is presented when the Diff Navigator
             // fails to fetch the selected diff contents.


References