From fdeafef6a08c45f8b45ab9fac303e3bffc3607b0 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 4 May 2016 11:17:53 +0200 Subject: [PATCH] Auto-add fileid in URL for currently displayed folder --- apps/files/js/app.js | 24 ++++++++++++++++--- apps/files/js/fileactions.js | 2 +- apps/files/js/filelist.js | 36 +++++++++++++++++++++-------- apps/files/tests/js/appSpec.js | 20 ++++++++++++++++ apps/files/tests/js/filelistSpec.js | 9 ++++++++ core/js/js.js | 35 ++++++++++++++++++++++++++-- 6 files changed, 111 insertions(+), 15 deletions(-) diff --git a/apps/files/js/app.js b/apps/files/js/app.js index eac080a009..60b5f1816a 100644 --- a/apps/files/js/app.js +++ b/apps/files/js/app.js @@ -174,6 +174,7 @@ // detect when app changed their current directory $('#app-content').delegate('>div', 'changeDirectory', _.bind(this._onDirectoryChanged, this)); + $('#app-content').delegate('>div', 'afterChangeDirectory', _.bind(this._onAfterDirectoryChanged, this)); $('#app-content').delegate('>div', 'changeViewerMode', _.bind(this._onChangeViewerMode, this)); $('#app-navigation').on('itemChanged', _.bind(this._onNavigationChanged, this)); @@ -224,7 +225,16 @@ */ _onDirectoryChanged: function(e) { if (e.dir) { - this._changeUrl(this.navigation.getActiveItem(), e.dir); + this._changeUrl(this.navigation.getActiveItem(), e.dir, e.fileId); + } + }, + + /** + * Event handler for when an app notified that its directory changed + */ + _onAfterDirectoryChanged: function(e) { + if (e.dir && e.fileId) { + this._changeUrl(this.navigation.getActiveItem(), e.dir, e.fileId); } }, @@ -263,12 +273,20 @@ /** * Change the URL to point to the given dir and view */ - _changeUrl: function(view, dir) { + _changeUrl: function(view, dir, fileId) { var params = {dir: dir}; if (view !== 'files') { params.view = view; + } else if (fileId) { + params.fileid = fileId; + } + var currentParams = OC.Util.History.parseUrlQuery(); + if (currentParams.dir === params.dir && currentParams.view === params.view && currentParams.fileid !== params.fileid) { + // if only fileid changed or was added, replace instead of push + OC.Util.History.replaceState(params); + } else { + OC.Util.History.pushState(params); } - OC.Util.History.pushState(params); } }; })(); diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 69e32d500c..c3d4fba9ef 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -619,7 +619,7 @@ this.register('dir', 'Open', OC.PERMISSION_READ, '', function (filename, context) { var dir = context.$file.attr('data-path') || context.fileList.getCurrentDirectory(); - context.fileList.changeDirectory(OC.joinPaths(dir, filename)); + context.fileList.changeDirectory(OC.joinPaths(dir, filename), true, false, parseInt(context.$file.attr('data-id'), 10)); }); this.registerAction({ diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 16ca5e91ed..9395112bce 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -1362,19 +1362,20 @@ return parseInt(this.$el.find('#permissions').val(), 10); }, /** - * @brief Changes the current directory and reload the file list. - * @param targetDir target directory (non URL encoded) - * @param changeUrl false if the URL must not be changed (defaults to true) - * @param {boolean} force set to true to force changing directory + * Changes the current directory and reload the file list. + * @param {string} targetDir target directory (non URL encoded) + * @param {boolean} [changeUrl=true] if the URL must not be changed (defaults to true) + * @param {boolean} [force=false] set to true to force changing directory + * @param {string} [fileId] optional file id, if known, to be appended in the URL */ - changeDirectory: function(targetDir, changeUrl, force) { + changeDirectory: function(targetDir, changeUrl, force, fileId) { var self = this; var currentDir = this.getCurrentDirectory(); targetDir = targetDir || '/'; if (!force && currentDir === targetDir) { return; } - this._setCurrentDir(targetDir, changeUrl); + this._setCurrentDir(targetDir, changeUrl, fileId); this.reload().then(function(success){ if (!success) { self.changeDirectory(currentDir, true); @@ -1389,8 +1390,9 @@ * Sets the current directory name and updates the breadcrumb. * @param targetDir directory to display * @param changeUrl true to also update the URL, false otherwise (default) + * @param {string} [fileId] file id */ - _setCurrentDir: function(targetDir, changeUrl) { + _setCurrentDir: function(targetDir, changeUrl, fileId) { targetDir = targetDir.replace(/\\/g, '/'); var previousDir = this.getCurrentDirectory(), baseDir = OC.basename(targetDir); @@ -1408,10 +1410,14 @@ this.$el.find('#dir').val(targetDir); if (changeUrl !== false) { - this.$el.trigger(jQuery.Event('changeDirectory', { + var params = { dir: targetDir, previousDir: previousDir - })); + }; + if (fileId) { + params.fileId = fileId; + } + this.$el.trigger(jQuery.Event('changeDirectory', params)); } this.breadcrumb.setDirectory(this.getCurrentDirectory()); }, @@ -1557,6 +1563,18 @@ result.sort(this._sortComparator); this.setFiles(result); + + if (this.dirInfo) { + var newFileId = this.dirInfo.id; + // update fileid in URL + var params = { + dir: this.getCurrentDirectory() + }; + if (newFileId) { + params.fileId = newFileId; + } + this.$el.trigger(jQuery.Event('afterChangeDirectory', params)); + } return true; }, diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js index dce513339f..3070d687fc 100644 --- a/apps/files/tests/js/appSpec.js +++ b/apps/files/tests/js/appSpec.js @@ -22,6 +22,7 @@ describe('OCA.Files.App tests', function() { var App = OCA.Files.App; var pushStateStub; + var replaceStateStub; var parseUrlQueryStub; var oldLegacyFileActions; @@ -48,6 +49,7 @@ describe('OCA.Files.App tests', function() { OCA.Files.fileActions = new OCA.Files.FileActions(); pushStateStub = sinon.stub(OC.Util.History, 'pushState'); + replaceStateStub = sinon.stub(OC.Util.History, 'replaceState'); parseUrlQueryStub = sinon.stub(OC.Util.History, 'parseUrlQuery'); parseUrlQueryStub.returns({}); @@ -59,6 +61,7 @@ describe('OCA.Files.App tests', function() { window.FileActions = oldLegacyFileActions; pushStateStub.restore(); + replaceStateStub.restore(); parseUrlQueryStub.restore(); }); @@ -132,6 +135,23 @@ describe('OCA.Files.App tests', function() { expect(pushStateStub.getCall(0).args[0].dir).toEqual('subdir'); expect(pushStateStub.getCall(0).args[0].view).toEqual('other'); }); + it('replaces the state to the URL when fileid is known', function() { + $('#app-content-files').trigger(new $.Event('changeDirectory', {dir: 'subdir'})); + expect(pushStateStub.calledOnce).toEqual(true); + expect(pushStateStub.getCall(0).args[0].dir).toEqual('subdir'); + expect(pushStateStub.getCall(0).args[0].view).not.toBeDefined(); + expect(replaceStateStub.notCalled).toEqual(true); + + parseUrlQueryStub.returns({dir: 'subdir'}); + + $('#app-content-files').trigger(new $.Event('afterChangeDirectory', {dir: 'subdir', fileId: 123})); + + expect(pushStateStub.calledOnce).toEqual(true); + expect(replaceStateStub.calledOnce).toEqual(true); + expect(replaceStateStub.getCall(0).args[0].dir).toEqual('subdir'); + expect(replaceStateStub.getCall(0).args[0].view).not.toBeDefined(); + expect(replaceStateStub.getCall(0).args[0].fileid).toEqual(123); + }); describe('onpopstate', function() { it('sends "urlChanged" event to current app', function() { var handler = sinon.stub(); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index cc3bcd74b4..7e6408128b 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -1358,6 +1358,15 @@ describe('OCA.Files.FileList tests', function() { expect(handler.calledOnce).toEqual(true); expect(handler.getCall(0).args[0].dir).toEqual('/somedir'); }); + it('triggers "afterChangeDirectory" event with fileid after changing directory', function() { + var handler = sinon.stub(); + $('#app-content-files').on('afterChangeDirectory', handler); + fileList.changeDirectory('/somedir'); + deferredList.resolve(200, [testRoot].concat(testFiles)); + expect(handler.calledOnce).toEqual(true); + expect(handler.getCall(0).args[0].dir).toEqual('/somedir'); + expect(handler.getCall(0).args[0].fileId).toEqual(99); + }); it('changes the directory when receiving "urlChanged" event', function() { $('#app-content-files').trigger(new $.Event('urlChanged', {view: 'files', dir: '/somedir'})); expect(fileList.getCurrentDirectory()).toEqual('/somedir'); diff --git a/core/js/js.js b/core/js/js.js index 69ebabdb41..edee72ca3c 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -2050,8 +2050,9 @@ OC.Util.History = { * * @param params to append to the URL, can be either a string * or a map + * @param {boolean} [replace=false] whether to replace instead of pushing */ - pushState: function(params) { + _pushState: function(params, replace) { var strParams; if (typeof(params) === 'string') { strParams = params; @@ -2061,7 +2062,11 @@ OC.Util.History = { } if (window.history.pushState) { var url = location.pathname + '?' + strParams; - window.history.pushState(params, '', url); + if (replace) { + window.history.replaceState(params, '', url); + } else { + window.history.pushState(params, '', url); + } } // use URL hash for IE8 else { @@ -2072,6 +2077,32 @@ OC.Util.History = { } }, + /** + * Push the current URL parameters to the history stack + * and change the visible URL. + * Note: this includes a workaround for IE8/IE9 that uses + * the hash part instead of the search part. + * + * @param params to append to the URL, can be either a string + * or a map + */ + pushState: function(params) { + return this._pushState(params, false); + }, + + /** + * Push the current URL parameters to the history stack + * and change the visible URL. + * Note: this includes a workaround for IE8/IE9 that uses + * the hash part instead of the search part. + * + * @param params to append to the URL, can be either a string + * or a map + */ + replaceState: function(params) { + return this._pushState(params, true); + }, + /** * Add a popstate handler *