From e39db808fb4411f1167dd8eef4580bbbeeec8c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 22 Nov 2018 05:58:25 +0100 Subject: [PATCH] Fix rendering of the sidebar in Files app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a view is rendered it should not be concerned with where it is going to be placed in the document; in general this should be a responsibility of the object using the view. Moreover, when the details view is rendered it should simply prepare a skeleton that includes the root elements provided by the plugins; those elements will be updated by the plugins as needed when a file or a tab is selected. Finally, the details view should not be explicitly rendered. The rendering removes the previous elements, but that is needed only when the details view is in a dirty state, that is, when new plugins were added since the last time that it was rendered. However, that dirty state is internally handled, and the view is automatically rendered again if needed when a file info is set. Due to all that the details view is no longer explicitly rendered when updating it with a different file. Also, as each file list has its own details view, and each details view has its own element, but there can be only one details view/sidebar element in the document, when the file list updates the details view it also replaces the current one in the document with its own details view if needed (that is, if it is not the current one already). Besides that, when the element of a details view is replaced with the element of a different details view the old one should be detached from the document, but never removed. Otherwise the event handlers would not work when that element is attached again later (when changing to a different section in the Files app and then going back to the previous one). Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/detailsview.js | 10 ---------- apps/files/js/filelist.js | 26 +++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/apps/files/js/detailsview.js b/apps/files/js/detailsview.js index bac2a5ebd2..699f884ba2 100644 --- a/apps/files/js/detailsview.js +++ b/apps/files/js/detailsview.js @@ -96,16 +96,6 @@ * Renders this details view */ render: function() { - // remove old instances - var $appSidebar = $('#app-sidebar'); - if ($appSidebar.length === 0) { - this.$el.insertAfter($('#app-content')); - } else { - if ($appSidebar[0] !== this.el) { - $appSidebar.replaceWith(this.$el); - } - } - var templateVars = { closeLabel: t('files', 'Close') }; diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 94fdada937..047837cd9d 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -584,11 +584,35 @@ } this._currentFileModel = model; + + this._replaceDetailsViewElementIfNeeded(); + this._detailsView.setFileInfo(model); - this._detailsView.render(); this._detailsView.$el.scrollTop(0); }, + /** + * Replaces the current details view element with the details view + * element of this file list. + * + * Each file list has its own DetailsView object, and each one has its + * own root element, but there can be just one details view/sidebar + * element in the document. This helper method replaces the current + * details view/sidebar element in the document with the element from + * the DetailsView object of this file list. + */ + _replaceDetailsViewElementIfNeeded: function() { + var $appSidebar = $('#app-sidebar'); + if ($appSidebar.length === 0) { + this._detailsView.$el.insertAfter($('#app-content')); + } else if ($appSidebar[0] !== this._detailsView.el) { + // "replaceWith()" can not be used here, as it removes the old + // element instead of just detaching it. + this._detailsView.$el.insertBefore($appSidebar); + $appSidebar.detach(); + } + }, + /** * Event handler for when the window size changed */