diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2018-11-20 13:18:48 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-11-20 13:18:48 +0100 |
commit | 902ec4a68f0cd3110e5849c3d429ed34ab3e6091 (patch) | |
tree | a651628079a6b257d80fd31d40cd78a777821646 | |
parent | 535f048ce507ca5e5612793133f5d38f35de25b6 (diff) | |
parent | 31e31a6f007957e4769ad2eaa7c9e3ea03ee1503 (diff) |
Merge pull request #1271 from nextcloud/improve-performance-of-the-list-of-chat-messages
Improve performance of the list of chat messages
-rw-r--r-- | css/comments.scss | 8 | ||||
-rw-r--r-- | css/style.scss | 21 | ||||
-rw-r--r-- | js/app.js | 45 | ||||
-rw-r--r-- | js/models/chatmessagecollection.js | 2 | ||||
-rw-r--r-- | js/views/chatview.js | 115 | ||||
-rw-r--r-- | js/views/virtuallist.js | 967 | ||||
-rw-r--r-- | lib/PublicShareAuth/TemplateLoader.php | 1 | ||||
-rw-r--r-- | templates/index-public.php | 1 | ||||
-rw-r--r-- | templates/index.php | 1 | ||||
-rw-r--r-- | tests/acceptance/features/bootstrap/ChatContext.php | 13 |
10 files changed, 1091 insertions, 83 deletions
diff --git a/css/comments.scss b/css/comments.scss index 6eb17f265..4cc5ce955 100644 --- a/css/comments.scss +++ b/css/comments.scss @@ -77,6 +77,14 @@ color: grey; } +/* Internal wrappers used by the virtual list. */ +#commentsTabView .comments .wrapper-background, +#commentsTabView .comments .wrapper { + position: absolute; + left: 0; + right: 0; +} + #commentsTabView .comment { position: relative; margin-bottom: 30px; diff --git a/css/style.scss b/css/style.scss index cfdd86cbb..2d7e62dc1 100644 --- a/css/style.scss +++ b/css/style.scss @@ -225,10 +225,6 @@ flex-direction: column; } -#app-content-wrapper #commentsTabView .comment:first-child { - padding-top: 15px; -} - /* The lateral padding is set for each child instead of for the chat view as a whole to prevent showing the scroll bar padded from the border of the chat view (which could be fixed by using a negative margin and a positive padding @@ -251,6 +247,15 @@ padding-right: 44px; } + +#app-content-wrapper #commentsTabView .comments .wrapper-background, +#app-content-wrapper #commentsTabView .comments .wrapper { + /* Padding is not respected in the comment wrapper due to its absolute + * positioning, so it must be set through its position. */ + left: 44px; + right: 44px; +} + /* Hide all siblings of the chat view when shown as the main view */ #app-content-wrapper #commentsTabView ~ *:not(#video-fullscreen):not([id^=tooltip]) { display: none !important; @@ -1223,6 +1228,14 @@ video { padding-right: 15px; } +#app-sidebar #commentsTabView .comments .wrapper-background, +#app-sidebar #commentsTabView .comments .wrapper { + /* Padding is not respected in the comment wrapper due to its absolute + * positioning, so it must be set through its position. */ + left: 15px; + right: 15px; +} + #app-sidebar #commentsTabView .newCommentForm { /* Make room to show the "Add" button when chat is shown in the sidebar. */ margin-right: 44px; @@ -472,18 +472,16 @@ var flags = this.activeRoom.get('participantFlags') || 0; var inCall = flags & OCA.SpreedMe.app.FLAG_IN_CALL !== 0; if (inCall && this._chatViewInMainView === true) { - this._chatView.saveScrollPosition(); this._chatView.$el.detach(); this._sidebarView.addTab('chat', { label: t('spreed', 'Chat'), icon: 'icon-comment', priority: 100 }, this._chatView); this._sidebarView.selectTab('chat'); - this._chatView.restoreScrollPosition(); + this._chatView.reloadMessageList(); this._chatView.setTooltipContainer(this._chatView.$el); this._chatViewInMainView = false; } else if (!inCall && !this._chatViewInMainView) { - this._chatView.saveScrollPosition(); this._sidebarView.removeTab('chat'); this._chatView.$el.prependTo('#app-content-wrapper'); - this._chatView.restoreScrollPosition(); + this._chatView.reloadMessageList(); this._chatView.setTooltipContainer($('#app')); this._chatView.focusChatInput(); this._chatViewInMainView = true; @@ -671,6 +669,45 @@ this._chatView.restoreScrollPosition(); }.bind(this)); + // Opening or closing the sidebar changes the width of the main + // view, so if the chat view is in the main view it needs to be + // reloaded. + var reloadMessageListOnSidebarVisibilityChange = function() { + if (!this._chatViewInMainView) { + return; + } + + this._chatView.reloadMessageList(); + }.bind(this); + this._chatView.listenTo(this._sidebarView, 'opened', reloadMessageListOnSidebarVisibilityChange); + this._chatView.listenTo(this._sidebarView, 'closed', reloadMessageListOnSidebarVisibilityChange); + + // Resizing the window can change the size of the chat view, both + // when it is in the main view and in the sidebar, so the chat view + // needs to be reloaded. The initial reload is not very heavy, so + // the handler is not debounced for a snappier feel and to reduce + // flickering. + // However, resizing the window below certain width causes the + // navigation bar to be hidden; an explicit handling is needed in + // this case because the app navigation (or, more specifically, its + // Snap object) adds a transition to the app content, so the reload + // needs to be delayed to give the transition time to end and thus + // give the app content time to get its final size. + var reloadMessageListOnWindowResize = function() { + var chatView = this._chatView; + + if ($(window).width() >= 768 || !this._chatViewInMainView) { + chatView.reloadMessageList(); + + return; + } + + setTimeout(function() { + chatView.reloadMessageList(); + }, 300); + }.bind(this); + $(window).resize(reloadMessageListOnWindowResize); + this._messageCollection.listenTo(roomChannel, 'leaveCurrentRoom', function() { this.stopReceivingMessages(); }); diff --git a/js/models/chatmessagecollection.js b/js/models/chatmessagecollection.js index 0410139a1..4d3029473 100644 --- a/js/models/chatmessagecollection.js +++ b/js/models/chatmessagecollection.js @@ -97,7 +97,9 @@ }, _messagesReceived: function(messages) { + this.trigger('add:start'); this.set(messages); + this.trigger('add:end'); }, receiveMessages: function() { diff --git a/js/views/chatview.js b/js/views/chatview.js index bac06c07c..eccf5f858 100644 --- a/js/views/chatview.js +++ b/js/views/chatview.js @@ -89,7 +89,9 @@ initialize: function() { this.listenTo(this.collection, 'reset', this.render); + this.listenTo(this.collection, 'add:start', this._onAddModelStart); this.listenTo(this.collection, 'add', this._onAddModel); + this.listenTo(this.collection, 'add:end', this._onAddModelEnd); this._guestNameEditableTextLabel = new OCA.SpreedMe.Views.EditableTextLabel({ model: this.getOption('guestNameModel'), @@ -244,6 +246,8 @@ this.$el.find('.has-tooltip').tooltip({container: this._tooltipContainer}); this.$container = this.$el.find('ul.comments'); + this._virtualList = new OCA.SpreedMe.Views.VirtualList(this.$container); + if (OC.getCurrentUser().uid) { this.$el.find('.avatar').avatar(OC.getCurrentUser().uid, 32, undefined, false, undefined, OC.getCurrentUser().displayName); } else { @@ -304,56 +308,50 @@ * be able to restore the scroll position when attached again. */ saveScrollPosition: function() { - var self = this; - if (_.isUndefined(this.$container)) { return; } - var containerHeight = this.$container.outerHeight(); - - this._$lastVisibleComment = this.$container.children('.comment').filter(function() { - return self._getCommentTopPosition($(this)) < containerHeight; - }).last(); + this._savedScrollPosition = this.$container.scrollTop(); }, /** * Restores the scroll position of the message list to the last saved * position. * - * When the scroll position is restored the size of the message list may - * have changed (for example, if the chat view was detached from the - * main view and attached to the sidebar); it is not possible to - * guarantee that exactly the same messages that were visible when the - * scroll position was saved will be visible when the scroll position is - * restored. Due to this, restoring the scroll position just ensures - * that the last message that was partially visible when it was saved - * will be fully visible when it is restored. + * Note that the saved scroll position is valid only if the chat view + * was not resized since it was saved; restoring the scroll position + * after the chat view was resized may or may not work as expected. */ restoreScrollPosition: function() { - if (_.isUndefined(this.$container) || _.isUndefined(this._$lastVisibleComment)) { + if (_.isUndefined(this.$container) || _.isUndefined(this._savedScrollPosition)) { return; } - var scrollBottom = this.$container.scrollTop(); - - // When the last visible comment has a next sibling the scroll - // position is based on the top position of that next sibling. - // Basing it on the last visible comment top position and its height - // could cause the next sibling to be shown due to a negative margin - // "pulling it up" over the last visible comment bottom margin. - var $nextSibling = this._$lastVisibleComment.next(); - if ($nextSibling.length > 0) { - // Substract 1px to ensure that it does not scroll into the next - // element (which would cause the next element to be fully shown - // if saving and restoring the scroll position again) due to - // rounding. - scrollBottom += this._getCommentTopPosition($nextSibling) - 1; - } else if (this._$lastVisibleComment.length > 0) { - scrollBottom += this._getCommentTopPosition(this._$lastVisibleComment) + this._getCommentOuterHeight(this._$lastVisibleComment); + this.$container.scrollTop(this._savedScrollPosition); + }, + + /** + * Reloads the message list. + * + * This needs to be called whenever the size of the chat view has + * changed. + * + * When the message list is reloaded its size may have changed (for + * example, if the chat view was detached from the main view and + * attached to the sidebar); it is not possible to guarantee that + * exactly the same messages that were visible before will be visible + * after the message list is reloaded. Due to this, in those cases + * reloading the message list just ensures that the last message that + * was partially visible before will be fully visible after the message + * list is reloaded. + */ + reloadMessageList: function() { + if (!this._virtualList) { + return; } - this.$container.scrollTop(scrollBottom - this.$container.outerHeight()); + this._virtualList.reload(); }, _formatItem: function(commentModel) { @@ -388,18 +386,15 @@ return data; }, - _onAddModel: function(model, collection, options) { - this.$el.find('.emptycontent').toggleClass('hidden', true); + _onAddModelStart: function() { + this._virtualList.appendElementStart(); - var $newestComment = this.$container.children('.comment').last(); - var scrollToNew = $newestComment.length > 0 && this._getCommentTopPosition($newestComment) < this.$container.outerHeight(); + this._scrollToNew = this._virtualList.getLastElement() === this._virtualList.getLastVisibleElement(); + }, + _onAddModel: function(model) { var $el = $(this.commentTemplate(this._formatItem(model))); - if (!_.isUndefined(options.at) && collection.length > 1) { - this.$container.find('li').eq(options.at).before($el); - } else { - this.$container.append($el); - } + this._virtualList.appendElement($el); if (this._modelsHaveSameActor(this._lastAddedMessageModel, model) && this._modelsAreTemporaryNear(this._lastAddedMessageModel, model) && @@ -427,42 +422,16 @@ this._lastAddedMessageModel = model; this._postRenderItem(model, $el); - - if (scrollToNew) { - var newestCommentHiddenHeight = (this._getCommentTopPosition($newestComment) + this._getCommentOuterHeight($newestComment)) - this.$container.outerHeight(); - this.$container.scrollTop(this.$container.scrollTop() + newestCommentHiddenHeight + $el.outerHeight(true)); - } }, - _getCommentTopPosition: function($element) { - // When the margin is positive, jQuery returns the proper top - // position of the element (that is, including the top margin). - // However, when it is negative, jQuery returns where the top - // position of the element would be if there was no margin. Grouped - // messages use a negative top margin to "pull them up" closer to - // the previous message, so in those cases the top position returned - // by jQuery is below the actual top position of the element. - var marginTop = parseInt($element.css('margin-top')); - if (marginTop >= 0) { - return $element.position().top; - } + _onAddModelEnd: function() { + this.$el.find('.emptycontent').toggleClass('hidden', true); - return $element.position().top + marginTop; - }, + this._virtualList.appendElementEnd(); - _getCommentOuterHeight: function($element) { - // When the margin is positive, jQuery returns the proper outer - // height of the element. However, when it is negative, it - // substracts the negative margin from the overall height of the - // element. Grouped messages use a negative top margin to "pull them - // up" closer to the previous message, so in those cases the outer - // height returned by jQuery is smaller than the actual height. - var marginTop = parseInt($element.css('margin-top')); - if (marginTop >= 0) { - return $element.outerHeight(true); + if (this._scrollToNew) { + this._virtualList.scrollTo(this._virtualList.getLastElement()); } - - return $element.outerHeight(true) - marginTop; }, _getDateSeparator: function(timestamp) { diff --git a/js/views/virtuallist.js b/js/views/virtuallist.js new file mode 100644 index 000000000..ec4f1fe29 --- /dev/null +++ b/js/views/virtuallist.js @@ -0,0 +1,967 @@ +/* global _, $ */ + +/** + * + * @copyright Copyright (c) 2018, Daniel Calviño Sánchez (danxuliu@gmail.com) + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +(function(_, $) { + + 'use strict'; + + OCA.SpreedMe = OCA.SpreedMe || {}; + OCA.SpreedMe.Views = OCA.SpreedMe.Views || {}; + + /** + * Virtual list of DOM elements. + * + * The virtual list makes possible to create a list with an "unlimited"* + * number of elements. Despite the browser optimizations there is a limit in + * the number of elements that can be added to a document before the browser + * becomes sluggish when the document is further modified (due to having to + * layout/reflow a high number of elements); the virtual list solves that by + * keeping in the document only those elements that are currently visible, + * and refreshing them as needed when the list is scrolled. + * + * *The actual limit depends, among other things, on the maximum height for + * an element supported by the browser, the available memory to hold the + * elements, and the performance traversing linked lists, although it should + * be high enough for most common uses. + * + * The virtual list receives the container of the list (the element that the + * list elements would have been appended to if the virtual list was not + * used) in its constructor. + * + * The CSS style of the container must have a "visible" or (preferred) an + * "auto" value for its "overflow-y" property. Similarly, the positioning of + * the ".wrapper-background" and ".wrapper" elements child of the container + * must be set to "absolute". + * + * Elements are appended to the virtual list by first notifying the list + * that elements are going to be appended, then appending the elements, and + * finally processing the appended elements. Thus, even if there is only one + * element to add, first "appendElementStart()" must be called, followed by + * one or more calls to "appendElement()" each one with a single element, + * and followed by a final call to "appendElementEnd()". Elements are + * prepended in a similar way using the equivalent methods. + * + * The elements in the list can have different heights, and they can + * partially overlap their previous or next element due to the use of a + * negative top margin, but their top position must not exceed the top + * position of its previous element, and their bottom position must not + * exceed the bottom position of its next element. + * + * It is assumed that the position and size of an element will not change + * once added to the list. Changing the size of the container could change + * the position and size of all the elements, so in that case "reload()" + * needs to be called. + * + * + * + * Internal description: + * --------------------- + * + * Feast your eyes on this glorious ASCII art representation of the virtual + * list: + * + * ············· - List start / Wrapper background start - Top position = 0 + * · · + * · _ _ _ · _ First loaded element + * · · + * · _ _ _ _ _ · _ Wrapper start - Top position ~= scroll position + * :___________: _ + * | ~~~ | | Viewport start / Container top + * | ~~ ||| + * | ~~ ||| + * | ~~~~~ | | Viewport end / Container bottom + * :¯¯¯¯¯¯¯¯¯¯¯: ¯ + * · ¯ ¯ ¯ ¯ ¯ · ¯ Wrapper end + * · · + * · · + * · ¯ ¯ ¯ · ¯ Last loaded element + * · · + * · · + * · · + * · · + * ············· - List end / Wrapper background end + * + * When the children of an element are larger than its parent and the parent + * can not grow any further the parent becomes a viewport for its children: + * it can only show a partial area of the children, but it provides an + * scroll bar to move the viewport up and down. + * + * The virtual list is based on that behaviour. In order to reduce the + * elements in the document, when the virtual list is set for a container, + * only those children of the container that are currently visible in the + * viewport are actually in the document; whenever the container is scrolled + * the elements are added and removed as needed. + * + * Specifically, the visible elements are added to and removed from a direct + * child of the container, a wrapper that only holds the visible elements. + * + * Besides the wrapper, the container has another direct children, a + * background element that simulates the full length of the list; although + * the background is empty its height is set to the height of all the + * elements in the list, so when the list is longer than the container the + * background causes the scroll bar to appear in the container as if it + * contained the real list. + * + * Both the background and the wrapper have an absolute position; this + * absolute position makes possible for the wrapper to move freely over the + * background, and also limits the layout calculations only to the wrapper + * itself when adding and removing the visible elements (although for better + * performance the updates are also done off-line, that is, with the wrapper + * detached from the document so only two reflows, one when it is detached + * and one when it is attached again, are done no matter the number of + * updated elements). + * + * Whenever the container is scrolled the elements are updated in the + * wrapper as needed; the top position of the wrapper is set so its elements + * are at the same distance from the top of the background as they would be + * if all their previous elements were in the document. + * + * In order to know where the elements should be in the full list as well as + * whether they are visible or not their position and size must have been + * calculated before. Thus, when elements are added to the virtual list they + * are briefly added to the document in a temporal wrapper; the position of + * this temporal wrapper is set based on the already added elements, so the + * browser can layout the new elements and their real position and size can + * be cached. + * + * Reloading the list recalculates the position and size of all the + * elements. When the list contains a lot of elements it is not possible to + * recalculate the values for all the elements at once, so they are first + * recalculated for the visible elements and then they are progressively + * recalculated for the rest of elements. During that process it is possible + * to scroll only to the already loaded elements (although eventually all + * the elements will be loaded and it will be p |