From 4f5b072c74dd508bac685b170a74e3d43b865134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:32:15 +0200 Subject: [PATCH 1/3] Fix opening a section again in the Files app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a section is open in the Files app a "show" event is triggered. File list objects handle that event by reloading themselves, but only if the file list was shown at least once. However, the file list objects of plugins are created when the "show" event is triggered for the first time for their section; as the file list objects register their handler for the "show" event when they are created they never handle the first triggered "show" event, as the handler is set while that event is being already handled. Therefore, from the point of view of the handler, the second time that a "show" event was triggered it was seen as if the file list was shown for the first time, and thus it was not reloaded. Now the "shown" property is explicitly set for those file lists that are created while handling a "show" event, which causes them to be reloaded as expected when opening their section again. Note that it is not possible to just reload the file list whenever it is shown; the file list is reloaded also when the directory changes, and this can happen when the web page is initially loaded and the URL is parsed. In that case, if file lists were reloaded when shown for the first time then it could be reloaded twice, one with the default parameters due to the "show" event and another one with the proper parameters once the URL was parsed, and the files that appeard in the list would depend on which response from the server was received the last. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/favoritesplugin.js | 7 ++++++- apps/files/js/filelist.js | 4 ++++ apps/files/js/recentplugin.js | 7 ++++++- apps/files_sharing/js/app.js | 18 +++++++++++++++--- apps/files_trashbin/js/app.js | 6 +++++- apps/systemtags/js/app.js | 7 ++++++- 6 files changed, 42 insertions(+), 7 deletions(-) diff --git a/apps/files/js/favoritesplugin.js b/apps/files/js/favoritesplugin.js index 454a505c7bd68..58a610139958f 100644 --- a/apps/files/js/favoritesplugin.js +++ b/apps/files/js/favoritesplugin.js @@ -67,7 +67,12 @@ return new OCA.Files.FavoritesFileList( $el, { fileActions: fileActions, - scrollContainer: $('#app-content') + scrollContainer: $('#app-content'), + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); }, diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 8b311e57d1bab..0686968b98a3b 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -221,6 +221,10 @@ return; } + if (options.shown) { + this.shown = options.shown; + } + if (options.config) { this._filesConfig = options.config; } else if (!_.isUndefined(OCA.Files) && !_.isUndefined(OCA.Files.App)) { diff --git a/apps/files/js/recentplugin.js b/apps/files/js/recentplugin.js index fcd427b18a2c2..5551aec9d383f 100644 --- a/apps/files/js/recentplugin.js +++ b/apps/files/js/recentplugin.js @@ -67,7 +67,12 @@ return new OCA.Files.RecentFileList( $el, { fileActions: fileActions, - scrollContainer: $('#app-content') + scrollContainer: $('#app-content'), + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); }, diff --git a/apps/files_sharing/js/app.js b/apps/files_sharing/js/app.js index e6c9159eda447..15c5bcb3077f4 100644 --- a/apps/files_sharing/js/app.js +++ b/apps/files_sharing/js/app.js @@ -34,7 +34,11 @@ OCA.Sharing.App = { scrollContainer: $('#app-content'), sharedWithUser: true, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -57,7 +61,11 @@ OCA.Sharing.App = { scrollContainer: $('#app-content'), sharedWithUser: false, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); @@ -80,7 +88,11 @@ OCA.Sharing.App = { scrollContainer: $('#app-content'), linksOnly: true, fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js index fd3d5db32ff9b..c37de63097c4a 100644 --- a/apps/files_trashbin/js/app.js +++ b/apps/files_trashbin/js/app.js @@ -30,7 +30,11 @@ OCA.Trashbin.App = { fileActions: this._createFileActions(), detailsViewEnabled: false, scrollTo: urlParams.scrollto, - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, so + // it should be marked as "shown" like it would have been done + // if handling the event with the file list already created. + shown: true } ); }, diff --git a/apps/systemtags/js/app.js b/apps/systemtags/js/app.js index e027c0be12308..ff0ac159ba888 100644 --- a/apps/systemtags/js/app.js +++ b/apps/systemtags/js/app.js @@ -29,7 +29,12 @@ id: 'systemtags', scrollContainer: $('#app-content'), fileActions: this._createFileActions(), - config: OCA.Files.App.getFilesConfig() + config: OCA.Files.App.getFilesConfig(), + // The file list is created when a "show" event is handled, + // so it should be marked as "shown" like it would have been + // done if handling the event with the file list already + // created. + shown: true } ); From bc7e8cb362e7bff43bc77689b8c040c38015c189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:33:05 +0200 Subject: [PATCH 2/3] Remove event handler no longer needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom handler for "URL changed" events were added to reload the file list whenever the sections for favorites and shares were opened; this was used to fix the problem of not reloading the file lists when opening them for a second time. However, besides that the handlers were not really necessary, and as the root of the bug was fixed in the previous commit those handlers are now removed. The file list for tags uses the handler for a different purpose, though, so that one was kept. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/favoritesfilelist.js | 6 ------ apps/files_sharing/js/sharedfilelist.js | 6 ------ 2 files changed, 12 deletions(-) diff --git a/apps/files/js/favoritesfilelist.js b/apps/files/js/favoritesfilelist.js index 8c9c125d0a173..44adf5d7b5b83 100644 --- a/apps/files/js/favoritesfilelist.js +++ b/apps/files/js/favoritesfilelist.js @@ -94,12 +94,6 @@ $(document).ready(function() { return OCA.Files.FileList.prototype.reloadCallback.call(this, status, result); }, - - _onUrlChanged: function (e) { - if (e && _.isString(e.dir)) { - this.changeDirectory(e.dir, false, true); - } - } }); OCA.Files.FavoritesFileList = FavoritesFileList; diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index bd33035aa9c13..2c4a32c697910 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -393,12 +393,6 @@ // Sort by expected sort comparator return files.sort(this._sortComparator); }, - - _onUrlChanged: function(e) { - if (e && _.isString(e.dir)) { - this.changeDirectory(e.dir, false, true); - } - } }); /** From 3fbb6e70344116ef3b11c1f8114e8613f3172539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 19 Oct 2018 20:34:19 +0200 Subject: [PATCH 3/3] Add acceptance tests for opening a section in the Files app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/app-files.feature | 72 +++++++++++++++++++ .../features/bootstrap/FileListContext.php | 16 +++++ 2 files changed, 88 insertions(+) diff --git a/tests/acceptance/features/app-files.feature b/tests/acceptance/features/app-files.feature index 7ca7a42e9e6f3..7446376751c52 100644 --- a/tests/acceptance/features/app-files.feature +++ b/tests/acceptance/features/app-files.feature @@ -23,6 +23,78 @@ Feature: app-files When I open the details view for "welcome.txt" Then I see that the details view for "All files" section is open + Scenario: show recent files + Given I am logged in + And I create a new folder named "Folder just created" + When I open the "Recent" section + Then I see that the current section is "Recent" + Then I see that the file list contains a file named "Folder just created" + + Scenario: show recent files for a second time + Given I am logged in + And I open the "Recent" section + And I see that the current section is "Recent" + And I open the "All files" section + And I see that the current section is "All files" + And I create a new folder named "Folder just created" + When I open the "Recent" section + Then I see that the current section is "Recent" + Then I see that the file list contains a file named "Folder just created" + + Scenario: show favorites + Given I am logged in + And I mark "welcome.txt" as favorite + When I open the "Favorites" section + Then I see that the current section is "Favorites" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show favorites for a second time + Given I am logged in + And I open the "Favorites" section + And I see that the current section is "Favorites" + And I open the "All files" section + And I see that the current section is "All files" + And I mark "welcome.txt" as favorite + When I open the "Favorites" section + Then I see that the current section is "Favorites" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show shares + Given I am logged in + And I share the link for "welcome.txt" + When I open the "Shared by link" section + Then I see that the current section is "Shared by link" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show shares for a second time + Given I am logged in + And I open the "Shared by link" section + And I see that the current section is "Shared by link" + And I open the "All files" section + And I see that the current section is "All files" + And I share the link for "welcome.txt" + When I open the "Shared by link" section + Then I see that the current section is "Shared by link" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show deleted files + Given I am logged in + And I delete "welcome.txt" + When I open the "Deleted files" section + Then I see that the current section is "Deleted files" + Then I see that the file list contains a file named "welcome.txt" + + Scenario: show deleted files for a second time + Given I am logged in + And I open the "Deleted files" section + And I see that the current section is "Deleted files" + And I open the "All files" section + And I see that the current section is "All files" + And I delete "welcome.txt" + When I open the "Deleted files" section + Then I see that the current section is "Deleted files" + Then I see that the file list contains a file named "welcome.txt" + Scenario: rename a file with the details view open Given I am logged in And I open the details view for "welcome.txt" diff --git a/tests/acceptance/features/bootstrap/FileListContext.php b/tests/acceptance/features/bootstrap/FileListContext.php index bc225e3f9b152..6a39d7a999f74 100644 --- a/tests/acceptance/features/bootstrap/FileListContext.php +++ b/tests/acceptance/features/bootstrap/FileListContext.php @@ -254,6 +254,13 @@ public static function viewFileInFolderMenuItem() { return self::fileActionsMenuItemFor("View in folder"); } + /** + * @return Locator + */ + public static function deleteMenuItem() { + return self::fileActionsMenuItemFor("Delete"); + } + /** * @Given I create a new folder named :folderName */ @@ -322,6 +329,15 @@ public function iViewInFolder($fileName) { $this->actor->find(self::viewFileInFolderMenuItem(), 2)->click(); } + /** + * @When I delete :fileName + */ + public function iDelete($fileName) { + $this->actor->find(self::fileActionsMenuButtonForFile($this->fileListAncestor, $fileName), 10)->click(); + + $this->actor->find(self::deleteMenuItem(), 2)->click(); + } + /** * @Then I see that the file list is eventually loaded */