UX: fix jerky UI when creating new posts

- do not scroll screen if post is already on screen
- do not hide/show suggested topics each time you post
- be less aggressive about setting scrollTop in LockOn

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -44,16 +44,12 @@ export default ObjectController.extend(Discourse.SelectedPostsCount, BufferedCon
   }.observes('controllers.search.term', 'controllers.header.visibleDropdown'),
 
   postStreamLoadedAllPostsChanged: function(){
-    // hold back rendering 1 run loop for every transition.
-    var self = this;
+    // in the past we would hold back rendering of suggested topics
+    // 1 run loop, however post optimisations rendering them is fast
+    // holding back rendering just makes for a more jerky UI on initial
+    // transition
     var loaded = this.get('postStream.loadedAllPosts');
-    this.set('loadedAllPosts', false);
-
-    if(loaded){
-      Em.run.next(function(){
-        self.set('loadedAllPosts',true);
-      });
-    }
+    this.set('loadedAllPosts', loaded);
 
   }.observes('postStream', 'postStream.loadedAllPosts'),
 

app/assets/javascripts/discourse/lib/url.js

@@ -14,23 +14,44 @@ Discourse.URL = Ember.Object.createWithMixins({
   /**
     Jumps to a particular post in the stream
   **/
-  jumpToPost: function(postNumber) {
+  jumpToPost: function(postNumber, opts) {
     var holderId = '#post-cloak-' + postNumber;
 
+    var offset = function(){
+
+      var $header = $('header'),
+          $title = $('#topic-title'),
+          windowHeight = $(window).height() - $title.height(),
+          expectedOffset = $title.height() - $header.find('.contents').height() + (windowHeight / 5);
+
+      return $header.outerHeight(true) + ((expectedOffset < 0) ? 0 : expectedOffset);
+    };
+
+
     Em.run.schedule('afterRender', function() {
       if (postNumber === 1) {
         $(window).scrollTop(0);
         return;
       }
 
-      new LockOn(holderId, {offsetCalculator: function() {
-        var $header = $('header'),
-            $title = $('#topic-title'),
-            windowHeight = $(window).height() - $title.height(),
-            expectedOffset = $title.height() - $header.find('.contents').height() + (windowHeight / 5);
+      var lockon = new LockOn(holderId, {offsetCalculator: offset});
+      var holder = $(holderId);
+
+      if(holder.length > 0 && opts && opts.skipIfOnScreen){
+        // if we are on screen just scroll to post
+        var elementTop = lockon.elementTop(),
+            scrollTop = $(window).scrollTop(),
+            windowHeight = $(window).height()-offset(),
+            height = holder.height();
+
+        if (elementTop > scrollTop &&
+            (elementTop + height) < (scrollTop + windowHeight)) {
+          return;
+        }
+      }
+
+      lockon.lock();
 
-        return $header.outerHeight(true) + ((expectedOffset < 0) ? 0 : expectedOffset);
-      }}).lock();
     });
   },
 
@@ -218,7 +239,7 @@ Discourse.URL = Ember.Object.createWithMixins({
           progressController.set('progressPosition', progress);
           self.appEvents.trigger('post:highlight', closest);
         }).then(function() {
-          Discourse.URL.jumpToPost(closest);
+          Discourse.URL.jumpToPost(closest, {skipIfOnScreen: true});
         });
 
         // Abort routing, we have replaced our state.

app/assets/javascripts/discourse/models/post-stream.js.es6

@@ -35,7 +35,15 @@ const PostStream = Ember.Object.extend({
   }.property('stream.@each'),
 
   loadedAllPosts: function() {
-    if (!this.get('hasLoadedData')) { return false; }
+    if (!this.get('hasLoadedData')) {
+      return false;
+    }
+
+    // if we are staging a post assume all is loaded
+    if (this.get('lastPostId') === -1) {
+      return true;
+    }
+
     return !!this.get('posts').findProperty('id', this.get('lastPostId'));
   }.property('hasLoadedData', 'posts.@each.id', 'lastPostId'),
 

vendor/assets/javascripts/lock-on.js

@@ -1,3 +1,21 @@
+// Dear traveller, you are entering a zone where we are at war with the browser
+// the browser is insisting on positioning scrollTop per the location it was in
+// the past, we are insisting on it being where we want it to be
+// The hack is just to keep trying over and over to positiong the scrollbar (up to 1 minute)
+//
+// The root cause is that a "refresh" on a topic page will almost never be at the
+// same position it was in the past, the URL points to the post at the top of the
+// page, so a refresh will try to bring that post into view causing drift
+//
+// Additionally if you loaded multiple batches of posts, on refresh they will not
+// be loaded.
+//
+// This hack leads to a slight jerky experience, however other workarounds are more
+// complex, the 2 options we have are
+//
+// 1. onbeforeunload ensure we are scrolled to the right spot
+// 2. give up on the scrollbar and implement it ourselves (something that will happen)
+
 (function (exports) {
 
   var scrollEvents = "scroll.lock-on touchmove.lock-on mousedown.lock-on wheel.lock-on DOMMouseScroll.lock-on mousewheel.lock-on keyup.lock-on";
@@ -19,11 +37,15 @@
   LockOn.prototype.lock = function() {
     var self = this,
         previousTop = this.elementTop(),
-        startedAt = new Date().getTime()
+        startedAt = new Date().getTime(),
         i = 0;
 
     $(window).scrollTop(previousTop);
 
+    var within = function(threshold,x,y) {
+      return Math.abs(x-y) < threshold;
+    };
+
     var interval = setInterval(function() {
       i = i + 1;
 
@@ -31,20 +53,24 @@
           scrollTop = $(window).scrollTop();
 
       if (typeof(top) === "undefined") {
-        $('body,html').off(scrollEvents)
+        $('body,html').off(scrollEvents);
         clearInterval(interval);
         return;
       }
 
-      if ((top !== previousTop) || (scrollTop !== top)) {
+      if (!within(4, top, previousTop) || !within(4, scrollTop, top)) {
         $(window).scrollTop(top);
+        // animating = true;
+        // $('html,body').animate({scrollTop: parseInt(top,10)+'px'}, 200, 'swing', function(){
+        //   animating = false;
+        // });
         previousTop = top;
       }
 
-      // We commit suicide after 1s just to clean up
+      // We commit suicide after 3s just to clean up
       var nowTime = new Date().getTime();
       if (nowTime - startedAt > 1000) {
-        $('body,html').off(scrollEvents)
+        $('body,html').off(scrollEvents);
         clearInterval(interval);
       }
 
@@ -55,7 +81,7 @@
         $('body,html').off(scrollEvents);
         clearInterval(interval);
       }
-    })
+    });
 
   };