Merge pull request #12394 from owncloud/files-fileactionsimprovements

Improve FileActions JS to allow for custom rendering

3 files changed, 314 insertions, 128 deletions

diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index 5bf1618b0b8..4334daa7556 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -8,7 +8,6 @@
  *
  */
 
-/* global trashBinApp */
 (function() {
 
 	/**
@@ -109,33 +108,37 @@
 				permissions: permissions,
 				icon: icon,
 				actionHandler: action,
-				displayName: displayName
+				displayName: displayName || name
 			});
 		},
 		/**
 		 * Register action
 		 *
-		 * @param {Object} action action object
-		 * @param {String} action.name identifier of the action
-		 * @param {String} action.displayName display name of the action, defaults
-		 * to the name given in action.name
-		 * @param {String} action.mime mime type
-		 * @param {int} action.permissions permissions
-		 * @param {(Function|String)} action.icon icon path to the icon or function
-		 * that returns it
-		 * @param {OCA.Files.FileActions~actionHandler} action.actionHandler action handler function
+		 * @param {OCA.Files.FileAction} action object
 		 */
 		registerAction: function (action) {
 			var mime = action.mime;
 			var name = action.name;
+			var actionSpec = {
+				action: action.actionHandler,
+				name: name,
+				displayName: action.displayName,
+				mime: mime,
+				icon: action.icon,
+				permissions: action.permissions
+			};
+			if (_.isUndefined(action.displayName)) {
+				actionSpec.displayName = t('files', name);
+			}
+			if (_.isFunction(action.render)) {
+				actionSpec.render = action.render;
+			} else {
+				actionSpec.render = _.bind(this._defaultRenderAction, this);
+			}
 			if (!this.actions[mime]) {
 				this.actions[mime] = {};
 			}
-			this.actions[mime][name] = {
-				action: action.actionHandler,
-				permissions: action.permissions,
-				displayName: action.displayName || t('files', name)
-			};
+			this.actions[mime][name] = actionSpec;
 			this.icons[name] = action.icon;
 			this._notifyUpdateListeners('registerAction', {action: action});
 		},
@@ -213,6 +216,127 @@
 			return actions[name];
 		},
 		/**
+		 * Default function to render actions
+		 *
+		 * @param {OCA.Files.FileAction} actionSpec file action spec
+		 * @param {boolean} isDefault true if the action is a default one,
+		 * false otherwise
+		 * @param {OCA.Files.FileActionContext} context action context
+		 */
+		_defaultRenderAction: function(actionSpec, isDefault, context) {
+			var name = actionSpec.name;
+			if (name === 'Download' || !isDefault) {
+				var $actionLink = this._makeActionLink(actionSpec, context);
+				context.$file.find('a.name>span.fileactions').append($actionLink);
+				return $actionLink;
+			}
+		},
+		/**
+		 * Renders the action link element
+		 *
+		 * @param {OCA.Files.FileAction} actionSpec action object
+		 * @param {OCA.Files.FileActionContext} context action context
+		 */
+		_makeActionLink: function(actionSpec, context) {
+			var img = actionSpec.icon;
+			if (img && img.call) {
+				img = img(context.$file.attr('data-file'));
+			}
+			var html = '<a href="#">';
+			if (img) {
+				html += '<img class="svg" alt="" src="' + img + '" />';
+			}
+			if (actionSpec.displayName) {
+				html += '<span> ' + actionSpec.displayName + '</span></a>';
+			}
+
+			return $(html);
+		},
+		/**
+		 * Custom renderer for the "Rename" action.
+		 * Displays the rename action as an icon behind the file name.
+		 *
+		 * @param {OCA.Files.FileAction} actionSpec file action to render
+		 * @param {boolean} isDefault true if the action is a default action,
+		 * false otherwise
+		 * @param {OCAFiles.FileActionContext} context rendering context
+		 */
+		_renderRenameAction: function(actionSpec, isDefault, context) {
+			var $actionEl = this._makeActionLink(actionSpec, context);
+			var $container = context.$file.find('a.name span.nametext');
+			$container.find('.action-rename').remove();
+			$container.append($actionEl);
+			return $actionEl;
+		},
+		/**
+		 * Custom renderer for the "Delete" action.
+		 * Displays the "Delete" action as a trash icon at the end of
+		 * the table row.
+		 *
+		 * @param {OCA.Files.FileAction} actionSpec file action to render
+		 * @param {boolean} isDefault true if the action is a default action,
+		 * false otherwise
+		 * @param {OCAFiles.FileActionContext} context rendering context
+		 */
+		_renderDeleteAction: function(actionSpec, isDefault, context) {
+			var mountType = context.$file.attr('data-mounttype');
+			var deleteTitle = t('files', 'Delete');
+			if (mountType === 'external-root') {
+				deleteTitle = t('files', 'Disconnect storage');
+			} else if (mountType === 'shared-root') {
+				deleteTitle = t('files', 'Unshare');
+			}
+			var $actionLink = $('<a href="#" original-title="' +
+				escapeHTML(deleteTitle) +
+				'" class="action delete icon-delete" />'
+			);
+			var $container = context.$file.find('td:last');
+			$container.find('.delete').remove();
+			$container.append($actionLink);
+			return $actionLink;
+		},
+		/**
+		 * Renders the action element by calling actionSpec.render() and
+		 * registers the click event to process the action.
+		 *
+		 * @param {OCA.Files.FileAction} actionSpec file action to render
+		 * @param {boolean} isDefault true if the action is a default action,
+		 * false otherwise
+		 * @param {OCAFiles.FileActionContext} context rendering context
+		 */
+		_renderAction: function(actionSpec, isDefault, context) {
+			var $actionEl = actionSpec.render(actionSpec, isDefault, context);
+			if (!$actionEl || !$actionEl.length) {
+				return;
+			}
+			$actionEl.addClass('action action-' + actionSpec.name.toLowerCase());
+			$actionEl.attr('data-action', actionSpec.name);
+			$actionEl.on(
+				'click', {
+					a: null
+				},
+				function(event) {
+					var $file = $(event.target).closest('tr');
+					var currentFile = $file.find('td.filename');
+					var fileName = $file.attr('data-file');
+					event.stopPropagation();
+					event.preventDefault();
+
+					context.fileActions.currentFile = currentFile;
+					// also set on global object for legacy apps
+					window.FileActions.currentFile = currentFile;
+
+					actionSpec.action(
+						fileName,
+						_.extend(context, {
+							dir: $file.attr('data-path') || context.fileList.getCurrentDirectory()
+						})
+					);
+				}
+			);
+			return $actionEl;
+		},
+		/**
 		 * Display file actions for the given element
 		 * @param parent "td" element of the file for which to display actions
 		 * @param triggerEvent if true, triggers the fileActionsReady on the file
@@ -226,107 +350,51 @@
 				return;
 			}
 			this.currentFile = parent;
-			var $tr = parent.closest('tr');
 			var self = this;
-			var actions = this.getActions(this.getCurrentMimeType(), this.getCurrentType(), this.getCurrentPermissions());
-			var file = this.getCurrentFile();
+			var $tr = parent.closest('tr');
+			var actions = this.getActions(
+				this.getCurrentMimeType(),
+				this.getCurrentType(),
+				this.getCurrentPermissions()
+			);
 			var nameLinks;
 			if ($tr.data('renaming')) {
 				return;
 			}
 
-			// recreate fileactions
+			// recreate fileactions container
 			nameLinks = parent.children('a.name');
 			nameLinks.find('.fileactions, .nametext .action').remove();
 			nameLinks.append('<span class="fileactions" />');
-			var defaultAction = this.getDefault(this.getCurrentMimeType(), this.getCurrentType(), this.getCurrentPermissions());
-
-			var actionHandler = function (event) {
-				event.stopPropagation();
-				event.preventDefault();
+			var defaultAction = this.getDefault(
+				this.getCurrentMimeType(),
+				this.getCurrentType(),
+				this.getCurrentPermissions()
+			);
 
-				self.currentFile = event.data.elem;
-				// also set on global object for legacy apps
-				window.FileActions.currentFile = self.currentFile;
-
-				var file = self.getCurrentFile();
-				var $tr = $(this).closest('tr');
-
-				event.data.actionFunc(file, {
-					$file: $tr,
-					fileList: fileList,
-					fileActions: self,
-					dir: $tr.attr('data-path') || fileList.getCurrentDirectory()
-				});
-			};
-
-			var addAction = function (name, action, displayName) {
-
-				if ((name === 'Download' || action !== defaultAction) && name !== 'Delete') {
-
-					var img = self.icons[name],
-						actionText = displayName,
-						actionContainer = 'a.name>span.fileactions';
-
-					if (name === 'Rename') {
-						// rename has only an icon which appears behind
-						// the file name
-						actionText = '';
-						actionContainer = 'a.name span.nametext';
-					}
-					if (img.call) {
-						img = img(file);
-					}
-					var html = '<a href="#" class="action action-' + name.toLowerCase() + '" data-action="' + name + '">';
-					if (img) {
-						html += '<img class ="svg" alt="" src="' + img + '" />';
-					}
-					html += '<span> ' + actionText + '</span></a>';
-
-					var element = $(html);
-					element.data('action', name);
-					element.on('click', {a: null, elem: parent, actionFunc: actions[name].action}, actionHandler);
-					parent.find(actionContainer).append(element);
-				}
-
-			};
-
-			$.each(actions, function (name, action) {
+			$.each(actions, function (name, actionSpec) {
 				if (name !== 'Share') {
-					displayName = action.displayName;
-					ah = action.action;
-
-					addAction(name, ah, displayName);
+					self._renderAction(
+						actionSpec,
+						actionSpec.action === defaultAction, {
+							$file: $tr,
+							fileActions: this,
+							fileList : fileList
+						}
+					);
 				}
 			});
-			if(actions.Share){
-				displayName = t('files', 'Share');
-				addAction('Share', actions.Share, displayName);
-			}
-
-			// remove the existing delete action
-			parent.parent().children().last().find('.action.delete').remove();
-			if (actions['Delete']) {
-				var img = self.icons['Delete'];
-				var html;
-				var mountType = $tr.attr('data-mounttype');
-				var deleteTitle = t('files', 'Delete');
-				if (mountType === 'external-root') {
-					deleteTitle = t('files', 'Disconnect storage');
-				} else if (mountType === 'shared-root') {
-					deleteTitle = t('files', 'Unshare');
-				} else if (fileList.id === 'trashbin') {
-					deleteTitle = t('files', 'Delete permanently');
-				}
-
-				if (img.call) {
-					img = img(file);
-				}
-				html = '<a href="#" original-title="' + escapeHTML(deleteTitle) + '" class="action delete icon-delete" />';
-				var element = $(html);
-				element.data('action', actions['Delete']);
-				element.on('click', {a: null, elem: parent, actionFunc: actions['Delete'].action}, actionHandler);
-				parent.parent().children().last().append(element);
+			// added here to make sure it's always the last action
+			var shareActionSpec = actions.Share;
+			if (shareActionSpec){
+				this._renderAction(
+					shareActionSpec,
+					shareActionSpec.action === defaultAction, {
+						$file: $tr,
+						fileActions: this,
+						fileList: fileList
+					}
+				);
 			}
 
 			if (triggerEvent){
@@ -350,18 +418,34 @@
 		 * Register the actions that are used by default for the files app.
 		 */
 		registerDefaultActions: function() {
-			this.register('all', 'Delete', OC.PERMISSION_DELETE, function () {
-				return OC.imagePath('core', 'actions/delete');
-			}, function (filename, context) {
-				context.fileList.do_delete(filename, context.dir);
-				$('.tipsy').remove();
+			this.registerAction({
+				name: 'Delete',
+				displayName: '',
+				mime: 'all',
+				permissions: OC.PERMISSION_DELETE,
+				icon: function() {
+					return OC.imagePath('core', 'actions/delete');
+				},
+				render: _.bind(this._renderDeleteAction, this),
+				actionHandler: function(fileName, context) {
+					context.fileList.do_delete(fileName, context.dir);
+					$('.tipsy').remove();
+				}
 			});
 
 			// t('files', 'Rename')
-			this.register('all', 'Rename', OC.PERMISSION_UPDATE, function () {
-				return OC.imagePath('core', 'actions/rename');
-			}, function (filename, context) {
-				context.fileList.rename(filename);
+			this.registerAction({
+				name: 'Rename',
+				displayName: '',
+				mime: 'all',
+				permissions: OC.PERMISSION_UPDATE,
+				icon: function() {
+					return OC.imagePath('core', 'actions/rename');
+				},
+				render: _.bind(this._renderRenameAction, this),
+				actionHandler: function (filename, context) {
+					context.fileList.rename(filename);
+				}
 			});
 
 			this.register('dir', 'Open', OC.PERMISSION_READ, '', function (filename, context) {
@@ -389,6 +473,47 @@
 	OCA.Files.FileActions = FileActions;
 
 	/**
+	 * File action attributes.
+	 *
+	 * @todo make this a real class in the future
+	 * @typedef {Object} OCA.Files.FileAction
+	 *
+	 * @property {String} name identifier of the action
+	 * @property {String} displayName display name of the action, defaults
+	 * to the name given in name property
+	 * @property {String} mime mime type
+	 * @property {int} permissions permissions
+	 * @property {(Function|String)} icon icon path to the icon or function
+	 * that returns it
+	 * @property {OCA.Files.FileActions~renderActionFunction} [render] optional rendering function
+	 * @property {OCA.Files.FileActions~actionHandler} actionHandler action handler function
+	 */
+
+	/**
+	 * File action context attributes.
+	 *
+	 * @typedef {Object} OCA.Files.FileActionContext
+	 *
+	 * @property {Object} $file jQuery file row element
+	 * @property {OCA.Files.FileActions} fileActions file actions object
+	 * @property {OCA.Files.FileList} fileList file list object
+	 */
+
+	/**
+	 * Render function for actions.
+	 * The function must render a link element somewhere in the DOM
+	 * and return it. The function should NOT register the event handler
+	 * as this will be done after the link was returned.
+	 *
+	 * @callback OCA.Files.FileActions~renderActionFunction
+	 * @param {OCA.Files.FileAction} actionSpec action definition
+	 * @param {Object} $row row container
+	 * @param {boolean} isDefault true if the action is the default one,
+	 * false otherwise
+	 * @return {Object} jQuery link object
+	 */
+
+	/**
 	 * Action handler function for file actions
 	 *
 	 * @callback OCA.Files.FileActions~actionHandler
diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js
index f5f18a45a75..828aec9b6b9 100644
--- a/apps/files/tests/js/fileactionsSpec.js
+++ b/apps/files/tests/js/fileactionsSpec.js
@@ -193,6 +193,54 @@ describe('OCA.Files.FileActions tests', function() {
 		context = actionStub.getCall(0).args[1];
 		expect(context.dir).toEqual('/somepath');
 	});
+	describe('custom rendering', function() {
+		var $tr;
+		beforeEach(function() {
+			var fileData = {
+				id: 18,
+				type: 'file',
+				name: 'testName.txt',
+				mimetype: 'text/plain',
+				size: '1234',
+				etag: 'a01234c',
+				mtime: '123456'
+			};
+			$tr = fileList.add(fileData);
+		});
+		it('regular function', function() {
+			var actionStub = sinon.stub();
+			FileActions.registerAction({
+				name: 'Test',
+				displayName: '',
+				mime: 'all',
+				permissions: OC.PERMISSION_READ,
+				render: function(actionSpec, isDefault, context) {
+					expect(actionSpec.name).toEqual('Test');
+					expect(actionSpec.displayName).toEqual('');
+					expect(actionSpec.permissions).toEqual(OC.PERMISSION_READ);
+					expect(actionSpec.mime).toEqual('all');
+					expect(isDefault).toEqual(false);
+
+					expect(context.fileList).toEqual(fileList);
+					expect(context.$file[0]).toEqual($tr[0]);
+
+					var $customEl = $('<a href="#"><span>blabli</span><span>blabla</span></a>');
+					$tr.find('td:first').append($customEl);
+					return $customEl;
+				},
+				actionHandler: actionStub
+			});
+			FileActions.display($tr.find('td.filename'), true, fileList);
+
+			var $actionEl = $tr.find('td:first .action-test');
+			expect($actionEl.length).toEqual(1);
+			expect($actionEl.hasClass('action')).toEqual(true);
+
+			$actionEl.click();
+			expect(actionStub.calledOnce).toEqual(true);
+			expect(actionStub.getCall(0).args[0]).toEqual('testName.txt');
+		});
+	});
 	describe('merging', function() {
 		var $tr;
 		beforeEach(function() {
diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js
index a9727542bad..72d9f4a6771 100644
--- a/apps/files_trashbin/js/app.js
+++ b/apps/files_trashbin/js/app.js
@@ -57,21 +57,34 @@ OCA.Trashbin.App = {
 			);
 		}, t('files_trashbin', 'Restore'));
 
-		fileActions.register('all', 'Delete', OC.PERMISSION_READ, function() {
-			return OC.imagePath('core', 'actions/delete');
-		}, function(filename, context) {
-			var fileList = context.fileList;
-			$('.tipsy').remove();
-			var tr = fileList.findFileEl(filename);
-			var deleteAction = tr.children("td.date").children(".action.delete");
-			deleteAction.removeClass('icon-delete').addClass('icon-loading-small');
-			fileList.disableActions();
-			$.post(OC.filePath('files_trashbin', 'ajax', 'delete.php'), {
-					files: JSON.stringify([filename]),
-					dir: fileList.getCurrentDirectory()
-				},
-				_.bind(fileList._removeCallback, fileList)
-			);
+		fileActions.registerAction({
+			name: 'Delete',
+			displayName: '',
+			mime: 'all',
+			permissions: OC.PERMISSION_READ,
+			icon: function() {
+				return OC.imagePath('core', 'actions/delete');
+			},
+			render: function(actionSpec, isDefault, context) {
+				var $actionLink = fileActions._makeActionLink(actionSpec, context);
+				$actionLink.attr('original-title', t('files', 'Delete permanently'));
+				context.$file.find('td:last').append($actionLink);
+				return $actionLink;
+			},
+			actionHandler: function(filename, context) {
+				var fileList = context.fileList;
+				$('.tipsy').remove();
+				var tr = fileList.findFileEl(filename);
+				var deleteAction = tr.children("td.date").children(".action.delete");
+				deleteAction.removeClass('icon-delete').addClass('icon-loading-small');
+				fileList.disableActions();
+				$.post(OC.filePath('files_trashbin', 'ajax', 'delete.php'), {
+						files: JSON.stringify([filename]),
+						dir: fileList.getCurrentDirectory()
+					},
+					_.bind(fileList._removeCallback, fileList)
+				);
+			}
 		});
 		return fileActions;
 	}