From 7760f1652190474d1f62b590b6a57799c8ffd68d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 19 Dec 2017 03:06:06 +0100 Subject: [PATCH 1/3] Trigger events before and after a file action is executed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the same way that other elements can know when a FileAction is registered or a default action is set this commit makes possible to be notified before and after a FileAction is executed. This is achieved by wrapping the registered action handler in a custom function that notifies the listeners before and after executing the handler itself. Due to this approach only FileActions registered through "registerAction" trigger the events, although that is not a problem as this is how the actions should be added to the FileActions anyway. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/fileactions.js | 17 +++++- apps/files/tests/js/appSpec.js | 19 +++++-- apps/files/tests/js/fileactionsSpec.js | 73 ++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 6c031ab06d..1b94ee15a6 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -142,7 +142,22 @@ var mime = action.mime; var name = action.name; var actionSpec = { - action: action.actionHandler, + action: function(fileName, context) { + // Actions registered in one FileAction may be executed on a + // different one (for example, due to the "merge" function), + // so the listeners have to be updated on the FileActions + // from the context instead of on the one in which it was + // originally registered. + if (context && context.fileActions) { + context.fileActions._notifyUpdateListeners('beforeTriggerAction', {action: actionSpec, fileName: fileName, context: context}); + } + + action.actionHandler(fileName, context); + + if (context && context.fileActions) { + context.fileActions._notifyUpdateListeners('afterTriggerAction', {action: actionSpec, fileName: fileName, context: context}); + } + }, name: name, displayName: action.displayName, mime: mime, diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js index b9c323e7c1..5728991e19 100644 --- a/apps/files/tests/js/appSpec.js +++ b/apps/files/tests/js/appSpec.js @@ -112,9 +112,22 @@ describe('OCA.Files.App tests', function() { App.initialize(); var actions = App.fileList.fileActions.actions; - expect(actions.all.OverwriteThis.action).toBe(actionStub); - expect(actions.all.LegacyTest.action).toBe(legacyActionStub); - expect(actions.all.RegularTest.action).toBe(actionStub); + var context = { fileActions: sinon.createStubInstance(OCA.Files.FileActions) }; + actions.all.OverwriteThis.action('testFileName', context); + expect(actionStub.calledOnce).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(2); + expect(context.fileActions._notifyUpdateListeners.getCall(0).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(1).calledWith('afterTriggerAction')).toBe(true); + actions.all.LegacyTest.action('testFileName', context); + expect(legacyActionStub.calledOnce).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(4); + expect(context.fileActions._notifyUpdateListeners.getCall(2).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(3).calledWith('afterTriggerAction')).toBe(true); + actions.all.RegularTest.action('testFileName', context); + expect(actionStub.calledTwice).toBe(true); + expect(context.fileActions._notifyUpdateListeners.callCount).toBe(6); + expect(context.fileActions._notifyUpdateListeners.getCall(4).calledWith('beforeTriggerAction')).toBe(true); + expect(context.fileActions._notifyUpdateListeners.getCall(5).calledWith('afterTriggerAction')).toBe(true); // default one still there expect(actions.dir.Open.action).toBeDefined(); }); diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js index 75a1871369..2dc8bb5092 100644 --- a/apps/files/tests/js/fileactionsSpec.js +++ b/apps/files/tests/js/fileactionsSpec.js @@ -299,6 +299,7 @@ describe('OCA.Files.FileActions tests', function() { clock.restore(); }); it('passes context to action handler', function() { + var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners'); $tr.find('.action-test').click(); expect(actionStub.calledOnce).toEqual(true); expect(actionStub.getCall(0).args[0]).toEqual('testName.txt'); @@ -309,6 +310,22 @@ describe('OCA.Files.FileActions tests', function() { expect(context.dir).toEqual('/subdir'); expect(context.fileInfoModel.get('name')).toEqual('testName.txt'); + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'testName.txt', + context: context + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'testName.txt', + context: context + }); + // when data-path is defined actionStub.reset(); $tr.attr('data-path', '/somepath'); @@ -317,6 +334,7 @@ describe('OCA.Files.FileActions tests', function() { expect(context.dir).toEqual('/somepath'); }); it('also triggers action handler when calling triggerAction()', function() { + var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners'); var model = new OCA.Files.FileInfoModel({ id: 1, name: 'Test.txt', @@ -331,7 +349,62 @@ describe('OCA.Files.FileActions tests', function() { expect(actionStub.getCall(0).args[1].fileList).toEqual(fileList); expect(actionStub.getCall(0).args[1].fileActions).toEqual(fileActions); expect(actionStub.getCall(0).args[1].fileInfoModel).toEqual(model); + + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: { + fileActions: fileActions, + fileInfoModel: model, + dir: '/subdir', + fileList: fileList, + $file: fileList.findFileEl('Test.txt') + } + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: { + fileActions: fileActions, + fileInfoModel: model, + dir: '/subdir', + fileList: fileList, + $file: fileList.findFileEl('Test.txt') + } + }); }); + it('triggers listener events when invoked directly', function() { + var context = {fileActions: new OCA.Files.FileActions()} + var notifyUpdateListenersSpy = sinon.spy(context.fileActions, '_notifyUpdateListeners'); + var testAction = fileActions.get('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test']; + + testAction('Test.txt', context); + + expect(actionStub.calledOnce).toEqual(true); + expect(actionStub.getCall(0).args[0]).toEqual('Test.txt'); + expect(actionStub.getCall(0).args[1]).toBe(context); + + expect(notifyUpdateListenersSpy.calledTwice).toEqual(true); + expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true); + expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: context + }); + expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction'); + expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({ + action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'], + fileName: 'Test.txt', + context: context + }); + }), describe('actions menu', function() { it('shows actions menu inside row when clicking the menu trigger', function() { expect($tr.find('td.filename .fileActionsMenu').length).toEqual(0); From 96ed73343eadb8723ca3bc404259d054f42b181d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 21 Dec 2017 00:36:40 +0100 Subject: [PATCH 2/3] Trigger the "Details" action when clicking on an empty file row space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking on an empty space in a file row causes the details view to be shown. As it is a user initiated action on the file list now it is done by triggering the Details action instead of directly calling "_updateDetailsView"; the result is the same in both cases, but using the action is more consistent (clicking on the file name triggers the default action, and clicking on the inline actions triggers those actions) and also makes possible to use the "beforeTriggerAction" and "afterTriggerAction" listeners. Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/filelist.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index fa9819b78b..7735e9357b 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -676,8 +676,25 @@ $(event.target).closest('a').blur(); } } else { - this._updateDetailsView($tr.attr('data-file')); + // Even if there is no Details action the default event + // handler is prevented for consistency (although there + // should always be a Details action); otherwise the link + // would be downloaded by the browser when the user expected + // the details to be shown. event.preventDefault(); + var filename = $tr.attr('data-file'); + var mime = this.fileActions.getCurrentMimeType(); + var type = this.fileActions.getCurrentType(); + var permissions = this.fileActions.getCurrentPermissions(); + var action = this.fileActions.get(mime, type, permissions)['Details']; + if (action) { + action(filename, { + $file: $tr, + fileList: this, + fileActions: this.fileActions, + dir: $tr.attr('data-path') || this.getCurrentDirectory() + }); + } } } }, From c059fbd409b088f65922354149f6d2fd9b5d01b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 21 Dec 2017 02:08:40 +0100 Subject: [PATCH 3/3] Remove internal unused property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/files/js/fileactions.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 1b94ee15a6..2fb7dfba29 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -47,14 +47,6 @@ */ $el: null, - /** - * List of handlers to be notified whenever a register() or - * setDefault() was called. - * - * @member {Function[]} - */ - _updateListeners: {}, - _fileActionTriggerTemplate: null, /** @@ -189,7 +181,6 @@ this.defaults = {}; this.icons = {}; this.currentFile = null; - this._updateListeners = []; }, /** * Sets the default action for a given mime type.