From 9cedca4dc5b82f5fdedd8c5fba6da1885b0512cb Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Fri, 24 May 2013 13:53:25 -0400 Subject: [PATCH] Don't stop the autoscrolls for reply-to. Refactored. The functional change here is that our code to stop autoscrolling on certain events, particularly mousemove, now only runs during system initiated autoscroll events. If the user had been replying to a message, then the feature to stop autoscroll was too aggressive. This patch also starts to put more scrolling-related code into viewport.js, which will hopefully prevent some code duplication and give us a single place to control things like stopping animated scrolls. (imported from commit e7d5946b0ac7fcfda2eff1d0e2b58a78b44ecc1a) --- zephyr/static/js/compose.js | 17 +------------ zephyr/static/js/message_list.js | 9 ++----- zephyr/static/js/viewport.js | 41 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/zephyr/static/js/compose.js b/zephyr/static/js/compose.js index 92c889d69e..88f0e8a80e 100644 --- a/zephyr/static/js/compose.js +++ b/zephyr/static/js/compose.js @@ -44,22 +44,7 @@ function show(tabname, focus_area) { var cover = selected_row.offset().top + selected_row.height() - $("#compose").offset().top; if (cover > 0) { - disable_pointer_movement = true; - // We use $('html, body') because you can't animate window.scrollTop - // on Chrome (http://bugs.jquery.com/ticket/10419). - $('html, body').animate({ - scrollTop: viewport.scrollTop() + cover + 5 - }, { - complete: function () { - // The complete callback is actually called before the - // scrolling has completed, so we try to let scrolling - // finish before allowing pointer movements again or the - // pointer may still move. - setTimeout(function () { - disable_pointer_movement = false; - }, 50); - } - }); + viewport.user_initiated_animate_scroll(cover+5); } }); focus_area.focus(); diff --git a/zephyr/static/js/message_list.js b/zephyr/static/js/message_list.js index 3d4ee26222..c277e56861 100644 --- a/zephyr/static/js/message_list.js +++ b/zephyr/static/js/message_list.js @@ -488,11 +488,7 @@ MessageList.prototype = { } // Ok, we are finally ready to actually scroll. - suppress_scroll_pointer_update = true; // Gets set to false in the scroll handler. - var viewport_offset = viewport.scrollTop(); - // viewport (which is window) doesn't have a scrollTop, so scroll - // the closest concept that does. - $("html, body").animate({scrollTop: viewport_offset + scroll_amount}); + viewport.system_initiated_animate_scroll(scroll_amount); }, clear_trailing_bookend: function MessageList_clear_trailing_bookend() { @@ -601,8 +597,7 @@ MessageList.prototype = { }; $(document).on('message_selected.zephyr hashchange.zephyr mousewheel mousemove', function (event) { - // TODO: Figure out how to limit this animation stop to just the autoscroll - $("html, body").stop(); + viewport.stop_auto_scrolling(); }); }()); /*jslint nomen: false */ diff --git a/zephyr/static/js/viewport.js b/zephyr/static/js/viewport.js index 9c9ed15b56..c04d821f3c 100644 --- a/zephyr/static/js/viewport.js +++ b/zephyr/static/js/viewport.js @@ -4,6 +4,7 @@ var exports = {}; var jwindow; var height; var width; +var in_stoppable_autoscroll = false; exports.scrollTop = function viewport_scrollTop () { return jwindow.scrollTop.apply(jwindow, arguments); @@ -31,6 +32,46 @@ exports.width = function viewport_width() { return width; }; + +exports.stop_auto_scrolling = function() { + if (in_stoppable_autoscroll) { + $("html, body").stop(); + } +}; + +exports.system_initiated_animate_scroll = function (scroll_amount) { + suppress_scroll_pointer_update = true; // Gets set to false in the scroll handler. + var viewport_offset = exports.scrollTop(); + in_stoppable_autoscroll = true; + $("html, body").animate({ + scrollTop: viewport_offset + scroll_amount, + always: function () { + in_stoppable_autoscroll = false; + } + }); +}; + +exports.user_initiated_animate_scroll = function (scroll_amount) { + disable_pointer_movement = true; + in_stoppable_autoscroll = false; // defensive + + // We use $('html, body') because you can't animate window.scrollTop + // on Chrome (http://bugs.jquery.com/ticket/10419). + $('html, body').animate({ + scrollTop: viewport.scrollTop() + scroll_amount + }, { + complete: function () { + // The complete callback is actually called before the + // scrolling has completed, so we try to let scrolling + // finish before allowing pointer movements again or the + // pointer may still move. + setTimeout(function () { + disable_pointer_movement = false; + }, 50); + } + }); +}; + $(function () { jwindow = $(window); // This handler must be placed before all resize handlers in our application