From de744a6c5550306c45f57dd00e3ad4a86ded5125 Mon Sep 17 00:00:00 2001 From: lilia Date: Tue, 4 Oct 2016 16:24:00 +0900 Subject: [PATCH] Fix messages being inserted in the wrong place Usually new elements are inserted in a predictable order relative to the sort order of the models/collection, but it's not garaunteed. This fixes up message element insertion to handle the general case where elements can be added in any order and must be displayed in correct order as determined by the collection's sort function. In the worst case, we'll have to iterate over the entire list of elements to find the right spot, but in practice most of the time we can short circuit based on the index of the model or by looking for the predecessor or successor of the element in question. --- js/views/message_list_view.js | 29 +++++++++++++++++++++++++---- js/views/message_view.js | 6 ++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/js/views/message_list_view.js b/js/views/message_list_view.js index e7d2cde6db..fdcb6f37ab 100644 --- a/js/views/message_list_view.js +++ b/js/views/message_list_view.js @@ -51,15 +51,36 @@ this.listenTo(view, 'beforeChangeHeight', this.measureScrollPosition); this.listenTo(view, 'afterChangeHeight', this.scrollToBottomIfNeeded); } - if (this.collection.indexOf(model) === this.collection.length - 1) { + var index = this.collection.indexOf(model); + if (index === this.collection.length - 1) { // add to the bottom. this.$el.append(view.el); this.$el.scrollTop(this.el.scrollHeight); // TODO: Avoid scrolling if user has manually scrolled up? this.measureScrollPosition(); - } else { - // add to the top. - this.measureScrollPosition(); + } else if (index === 0) { this.$el.prepend(view.el); + } else { + // insert + this.measureScrollPosition(); + + var next = this.$('#' + this.collection.at(index + 1).id); + var prev = this.$('#' + this.collection.at(index - 1).id); + if (next.length > 0) { + view.$el.insertBefore(next); + } else if (prev.length > 0) { + view.$el.insertAfter(prev); + } else { + // scan for the right spot + var elements = this.$el.children(); + for (var i in elements) { + var m = this.collection.get(elements[i].id); + var m_index = this.collection.indexOf(m); + if (m_index > index) { + view.$el.insertBefore(elements[i]); + break; + } + } + } this.scrollToBottomIfNeeded(); } }, diff --git a/js/views/message_view.js b/js/views/message_view.js index e9299bb2dd..d1e1273c82 100644 --- a/js/views/message_view.js +++ b/js/views/message_view.js @@ -47,6 +47,9 @@ tagName: 'li', className: 'expirationTimerUpdate advisory', templateName: 'expirationTimerUpdate', + id: function() { + return this.model.id; + }, initialize: function() { this.conversation = this.model.getContact(); this.listenTo(this.conversation, 'change', this.render); @@ -67,6 +70,9 @@ Whisper.MessageView = Whisper.View.extend({ tagName: 'li', templateName: 'message', + id: function() { + return this.model.id; + }, initialize: function() { this.listenTo(this.model, 'change:errors', this.onErrorsChanged); this.listenTo(this.model, 'change:body', this.render);