From 2fc78ddd7d01fe26deb435347bb73429cc2feaab Mon Sep 17 00:00:00 2001
From: lilia <liliakai@gmail.com>
Date: Thu, 12 Nov 2015 10:35:29 -0800
Subject: [PATCH] Fix scroll position jumping when images load

Messages with images or media were causing the scroll position to jump
around when they loaded, because rendering them changed the height of their
elements from 0 to full-height sometime after they were inserted into
the DOM.

Now when rendering attachments, we wait for them to load so they can
render at full height immediately, then warn our parent message list
before and after a potential height change, so the scroll position can
be saved and reset.

// FREEBIE
---
 js/views/attachment_view.js    |  9 +++++----
 js/views/message_list_view.js  | 29 ++++++++++++-----------------
 js/views/message_view.js       | 19 +++++++++++--------
 stylesheets/_conversation.scss | 20 ++++++++++++--------
 stylesheets/_global.scss       | 10 +++++-----
 stylesheets/_variables.scss    |  1 +
 stylesheets/manifest.css       | 27 +++++++++++++++------------
 7 files changed, 61 insertions(+), 54 deletions(-)

diff --git a/js/views/attachment_view.js b/js/views/attachment_view.js
index ece0a6ccb0..1b436777fc 100644
--- a/js/views/attachment_view.js
+++ b/js/views/attachment_view.js
@@ -14,7 +14,7 @@
           'click': 'open'
       },
       update: function() {
-        this.$el.trigger('update');
+        this.trigger('update');
       },
       open: function () {
         window.open(this.dataUrl, '_blank');
@@ -32,10 +32,10 @@
           this.$el.attr('controls', '');
       },
       events: {
-          'loadeddata': 'update'
+          'canplay': 'canplay'
       },
-      update: function() {
-        this.$el.trigger('update');
+      canplay: function() {
+          this.trigger('update');
       },
       render: function() {
           var $el = $('<source>');
@@ -65,6 +65,7 @@
         var view = new View(window.URL.createObjectURL(blob), this.model.contentType);
         view.$el.appendTo(this.$el);
         view.render();
+        view.on('update', this.trigger.bind(this, 'update'));
         return this;
     }
   });
diff --git a/js/views/message_list_view.js b/js/views/message_list_view.js
index 049f8d3f8f..00d0a95555 100644
--- a/js/views/message_list_view.js
+++ b/js/views/message_list_view.js
@@ -10,7 +10,6 @@
         className: 'message-list',
         itemView: Whisper.MessageView,
         events: {
-            'update *': 'scrollToBottomIfNeeded',
             'scroll': 'onScroll',
             'reset-scroll': 'resetScrollPosition'
         },
@@ -27,6 +26,11 @@
             this.scrollPosition = this.$el.scrollTop() + this.$el.outerHeight();
             this.scrollHeight = this.el.scrollHeight;
             this.shouldStickToBottom = this.scrollPosition === this.scrollHeight;
+            if (this.shouldStickToBottom) {
+                this.bottomOffset = 0;
+            } else {
+                this.bottomOffset = this.scrollHeight - this.$el.scrollTop();
+            }
         },
         resetScrollPosition: function() {
             var scrollPosition = this.scrollPosition;
@@ -36,34 +40,25 @@
             this.$el.scrollTop(scrollPosition - this.$el.outerHeight());
         },
         scrollToBottomIfNeeded: function() {
-            if (this.shouldStickToBottom) {
-                this.$el.scrollTop(this.scrollHeight);
-            }
-        },
-        scrollToBottom: function() {
-            // TODO: Avoid scrolling if user has manually scrolled up?
-            this.$el.scrollTop(this.el.scrollHeight);
-            this.measureScrollPosition();
-        },
-        addAll: function() {
-            Whisper.ListView.prototype.addAll.apply(this, arguments); // super()
-            this.scrollToBottom();
+            this.$el.scrollTop(this.el.scrollHeight - this.bottomOffset);
         },
         addOne: function(model) {
             if (this.itemView) {
                 var view = new this.itemView({model: model}).render();
+                this.listenTo(view, 'beforeChangeHeight', this.measureScrollPosition);
+                this.listenTo(view, 'afterChangeHeight', this.scrollToBottomIfNeeded);
                 if (this.collection.indexOf(model) === this.collection.length - 1) {
                     // add to the bottom.
                     this.$el.append(view.el);
-                    this.scrollToBottom();
+                    this.$el.scrollTop(this.el.scrollHeight); // TODO: Avoid scrolling if user has manually scrolled up?
+                    this.measureScrollPosition();
                 } else {
                     // add to the top.
-                    var offset = this.el.scrollHeight - this.$el.scrollTop();
+                    this.measureScrollPosition();
                     this.$el.prepend(view.el);
-                    this.$el.scrollTop(this.el.scrollHeight - offset);
+                    this.scrollToBottomIfNeeded();
                 }
             }
-            this.$el.removeClass('loading');
         },
     });
 })();
diff --git a/js/views/message_view.js b/js/views/message_view.js
index c8c2617bd2..fed6b39e2e 100644
--- a/js/views/message_view.js
+++ b/js/views/message_view.js
@@ -94,16 +94,19 @@
             this.renderSent();
             this.renderDelivered();
             this.renderErrors();
-
-            this.$('.attachments').append(
-                this.model.get('attachments').map(function(attachment) {
-                    return new Whisper.AttachmentView({
-                        model: attachment
-                    }).render().el;
-                })
-            );
+            this.loadAttachments();
 
             return this;
+        },
+        loadAttachments: function() {
+            this.model.get('attachments').forEach(function(attachment) {
+                var view = new Whisper.AttachmentView({ model: attachment }).render();
+                this.listenTo(view, 'update', function() {
+                    this.trigger('beforeChangeHeight');
+                    this.$('.attachments').append(view.el);
+                    this.trigger('afterChangeHeight');
+                });
+            }.bind(this));
         }
     });
 
diff --git a/stylesheets/_conversation.scss b/stylesheets/_conversation.scss
index 746bf068e7..e5c281846e 100644
--- a/stylesheets/_conversation.scss
+++ b/stylesheets/_conversation.scss
@@ -220,17 +220,19 @@
 
 .message-list {
   position: relative;
-  &::before {
-    display: block;
-    margin: $header-height auto;
-    content: " ";
-    height: $header-height;
-    width: $header-height;
-  }
   margin: 0;
-  padding: 1em 0;
+  padding: 2em 0 0;
   overflow-y: auto;
 
+  &:before {
+    display: block;
+    margin: -25px auto 10px;
+    content: " ";
+    height: $loading-height;
+    width: $loading-height;
+    border: solid 3px transparent;
+  }
+
   .timestamp {
     cursor: pointer;
 
@@ -392,10 +394,12 @@
   .attachments {
     img, audio, video {
       max-width: 100%;
+      max-height: 300px;
     }
 
     video {
       background: black;
+      min-height: 300px;
     }
 
     img {
diff --git a/stylesheets/_global.scss b/stylesheets/_global.scss
index ec6f98ae62..0586705c65 100644
--- a/stylesheets/_global.scss
+++ b/stylesheets/_global.scss
@@ -398,13 +398,13 @@ $avatar-size: 44px;
   position: relative;
   &::before {
     display: block;
-    margin: $header-height auto;
+    margin: 0px auto;
     content: " ";
-    height: $header-height;
-    width: $header-height;
-    border-radius: 2 * $header-height;
+    height: $loading-height;
+    width: $loading-height;
+    border-radius: 2 * $loading-height;
     border: solid 3px;
-    border-color: $blue_l $blue_l $grey_l $grey_l;
+    border-color: $blue_l $blue_l $grey_l $grey_l !important;
     animation: rotate 1s linear infinite;
   }
 
diff --git a/stylesheets/_variables.scss b/stylesheets/_variables.scss
index 3b2b40c3ae..36773a1d09 100644
--- a/stylesheets/_variables.scss
+++ b/stylesheets/_variables.scss
@@ -31,3 +31,4 @@ $header-color: $blue;
 $bubble-border-radius: 20px;
 
 $unread-badge-size: 21px;
+$loading-height: 16px;
diff --git a/stylesheets/manifest.css b/stylesheets/manifest.css
index 9d7e22241c..0e92ccad10 100644
--- a/stylesheets/manifest.css
+++ b/stylesheets/manifest.css
@@ -317,13 +317,13 @@ img.emoji {
   position: relative; }
   .loading::before {
     display: block;
-    margin: 36px auto;
+    margin: 0px auto;
     content: " ";
-    height: 36px;
-    width: 36px;
-    border-radius: 72px;
+    height: 16px;
+    width: 16px;
+    border-radius: 32px;
     border: solid 3px;
-    border-color: #a2d2f4 #a2d2f4 #f3f3f3 #f3f3f3;
+    border-color: #a2d2f4 #a2d2f4 #f3f3f3 #f3f3f3 !important;
     animation: rotate 1s linear infinite; }
 @keyframes rotate {
   to {
@@ -668,14 +668,15 @@ input.search {
 .message-list {
   position: relative;
   margin: 0;
-  padding: 1em 0;
+  padding: 2em 0 0;
   overflow-y: auto; }
-  .message-list::before {
+  .message-list:before {
     display: block;
-    margin: 36px auto;
+    margin: -25px auto 10px;
     content: " ";
-    height: 36px;
-    width: 36px; }
+    height: 16px;
+    width: 16px;
+    border: solid 3px transparent; }
   .message-list .timestamp {
     cursor: pointer; }
     .message-list .timestamp:hover {
@@ -800,10 +801,12 @@ input.search {
   .message-list .attachments img,
   .message-list .attachments audio,
   .message-list .attachments video {
-    max-width: 100%; }
+    max-width: 100%;
+    max-height: 300px; }
   .message-detail .attachments video,
   .message-list .attachments video {
-    background: black; }
+    background: black;
+    min-height: 300px; }
   .message-detail .attachments img,
   .message-list .attachments img {
     cursor: pointer; }