launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #16763
[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