Make file actions use icon CSS classes

This makes theming of file actions possible

8 files changed, 125 insertions, 42 deletions

diff --git a/apps/files/css/files.css b/apps/files/css/files.css
index 389b7fae806..3b1bea3fd9a 100644
--- a/apps/files/css/files.css
+++ b/apps/files/css/files.css
@@ -747,9 +747,6 @@ html.ie8 #controls .button.new {
 	white-space: nowrap;
 	overflow: hidden;
 }
-.newFileMenu.popovermenu .menuitem .icon {
-	margin-bottom: -2px;
-}
 .newFileMenu.popovermenu a.menuitem,
 .newFileMenu.popovermenu label.menuitem,
 .newFileMenu.popovermenu .menuitem {
@@ -788,3 +785,12 @@ html.ie8 #controls .button.new {
 	padding: 0;
 }
 
+.app-files .fileactions .action .icon,
+.app-files .actions .button .icon {
+	display: inline-block;
+	vertical-align: middle;
+}
+
+.app-files .actions .button.new .icon {
+	margin-bottom: 2px;
+}
diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index 67125f1570f..1e4f7d1ba8c 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -11,8 +11,13 @@
 (function() {
 
 	var TEMPLATE_FILE_ACTION_TRIGGER =
-		'<a class="action action-{{nameLowerCase}}" href="#" data-action="{{name}}">' +
-		'{{#if icon}}<img class="svg" alt="{{altText}}" src="{{icon}}" />{{/if}}' +
+		'<a class="action action-{{nameLowerCase}} href="#" data-action="{{name}}">' +
+		'{{#if icon}}' +
+			'<img class="svg" alt="{{altText}}" src="{{icon}}" />' +
+		'{{else}}' +
+			'{{#if iconClass}}<span class="icon {{iconClass}}" />{{/if}}' +
+			'{{#unless hasDisplayName}}<span class="hidden-visually">{{altText}}</span>{{/unless}}' +
+		'{{/if}}' +
 		'{{#if displayName}}<span> {{displayName}}</span>{{/if}}' +
 		'</a>';
 
@@ -143,6 +148,7 @@
 				mime: mime,
 				order: action.order || 0,
 				icon: action.icon,
+				iconClass: action.iconClass,
 				permissions: action.permissions,
 				type: action.type || FileActions.TYPE_DROPDOWN,
 				altText: action.altText || ''
@@ -299,10 +305,15 @@
 					nameLowerCase: actionSpec.name.toLowerCase(),
 					displayName: actionSpec.displayName,
 					icon: actionSpec.icon,
+					iconClass: actionSpec.iconClass,
 					altText: actionSpec.altText,
+					hasDisplayName: !!actionSpec.displayName
 				};
 				if (_.isFunction(actionSpec.icon)) {
-					params.icon = actionSpec.icon(context.$file.attr('data-file'));
+					params.icon = actionSpec.icon(context.$file.attr('data-file'), context);
+				}
+				if (_.isFunction(actionSpec.iconClass)) {
+					params.iconClass = actionSpec.iconClass(context.$file.attr('data-file'), context);
 				}
 
 				var $actionLink = this._makeActionLink(params, context);
@@ -363,7 +374,7 @@
 			var $el = this._renderInlineAction({
 				name: 'menu',
 				displayName: '',
-				icon: OC.imagePath('core', 'actions/more'),
+				iconClass: 'icon-more',
 				altText: t('files', 'Actions'),
 				action: this._showMenuClosure
 			}, false, context);
@@ -570,9 +581,7 @@
 				order: -20,
 				mime: 'all',
 				permissions: OC.PERMISSION_READ,
-				icon: function () {
-					return OC.imagePath('core', 'actions/download');
-				},
+				iconClass: 'icon-download',
 				actionHandler: function (filename, context) {
 					var dir = context.dir || context.fileList.getCurrentDirectory();
 					var isDir = context.$file.attr('data-type') === 'dir';
@@ -602,9 +611,7 @@
 				mime: 'all',
 				order: -30,
 				permissions: OC.PERMISSION_UPDATE,
-				icon: function() {
-					return OC.imagePath('core', 'actions/rename');
-				},
+				iconClass: 'icon-rename',
 				actionHandler: function (filename, context) {
 					context.fileList.rename(filename);
 				}
@@ -631,9 +638,7 @@
 				order: 1000,
 				// permission is READ because we show a hint instead if there is no permission
 				permissions: OC.PERMISSION_DELETE,
-				icon: function() {
-					return OC.imagePath('core', 'actions/delete');
-				},
+				iconClass: 'icon-delete',
 				actionHandler: function(fileName, context) {
 					// if there is no permission to delete do nothing
 					if((context.$file.data('permissions') & OC.PERMISSION_DELETE) === 0) {
@@ -682,8 +687,8 @@
 	 * 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 {(Function|String)} icon icon path to the icon or function that returns it (deprecated, use iconClass instead)
+	 * @property {(Function|String)} iconClass class name of the icon (recommended for theming)
 	 * @property {OCA.Files.FileActions~renderActionFunction} [render] optional rendering function
 	 * @property {OCA.Files.FileActions~actionHandler} actionHandler action handler function
 	 */
diff --git a/apps/files/js/fileactionsmenu.js b/apps/files/js/fileactionsmenu.js
index 9e51d8f1f1f..45d2bd83049 100644
--- a/apps/files/js/fileactionsmenu.js
+++ b/apps/files/js/fileactionsmenu.js
@@ -14,7 +14,16 @@
 		'<ul>' +
 		'{{#each items}}' +
 		'<li>' +
-		'<a href="#" class="menuitem action action-{{nameLowerCase}} permanent" data-action="{{name}}">{{#if icon}}<img class="icon" src="{{icon}}"/>{{else}}<span class="no-icon"></span>{{/if}}<span>{{displayName}}</span></a>' +
+		'<a href="#" class="menuitem action action-{{nameLowerCase}} permanent" data-action="{{name}}">' +
+			'{{#if icon}}<img class="icon" src="{{icon}}"/>' +
+			'{{else}}'+
+				'{{#if iconClass}}' +
+				'<span class="icon {{iconClass}}"></span>' +
+				'{{else}}' +
+				'<span class="no-icon"></span>' +
+				'{{/if}}' +
+			'{{/if}}' +
+			'<span>{{displayName}}</span></a>' +
 		'</li>' +
 		'{{/each}}' +
 		'</ul>';
diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 81dfea2114d..95fdac02c2d 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -10,7 +10,10 @@
 
 (function() {
 
-	var TEMPLATE_ADDBUTTON = '<a href="#" class="button new"><img src="{{iconUrl}}" alt="{{addText}}"></img></a>';
+	var TEMPLATE_ADDBUTTON = '<a href="#" class="button new">' +
+		'<span class="icon {{iconClass}}"></span>' +
+		'<span class="hidden-visually">{{addText}}</span>' +
+		'</a>';
 
 	/**
 	 * @class OCA.Files.FileList
@@ -329,7 +332,7 @@
 					displayName: t('files', 'Details'),
 					mime: 'all',
 					order: -50,
-					icon: OC.imagePath('core', 'actions/details'),
+					iconClass: 'icon-details',
 					permissions: OC.PERMISSION_READ,
 					actionHandler: function(fileName, context) {
 						self._updateDetailsView(fileName);
@@ -2715,7 +2718,7 @@
 			}
 			var $newButton = $(this._addButtonTemplate({
 				addText: t('files', 'New'),
-				iconUrl: OC.imagePath('core', 'actions/add')
+				iconClass: 'icon-add'
 			}));
 
 			$actionsContainer.prepend($newButton);
diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js
index 4f4d4d3d197..470f2854f43 100644
--- a/apps/files/tests/js/fileactionsSpec.js
+++ b/apps/files/tests/js/fileactionsSpec.js
@@ -153,7 +153,7 @@ describe('OCA.Files.FileActions tests', function() {
 			expect($tr.find('.action.action-match').length).toEqual(1);
 			expect($tr.find('.action.action-nomatch').length).toEqual(0);
 		});
-		it('display inline icon', function() {
+		it('display inline icon with image path', function() {
 			fileActions.registerAction({
 				name: 'Icon',
 				displayName: 'IconDisplay',
@@ -179,7 +179,7 @@ describe('OCA.Files.FileActions tests', function() {
 			expect($tr.find('.action.action-noicon').length).toEqual(1);
 			expect($tr.find('.action.action-noicon').find('img').length).toEqual(0);
 		});
-		it('display alt text on inline icon', function() {
+		it('display alt text on inline icon with image path', function() {
 			fileActions.registerAction({
 				name: 'IconAltText',
 				displayName: 'IconAltTextDisplay',
@@ -209,6 +209,63 @@ describe('OCA.Files.FileActions tests', function() {
 			expect($tr.find('.action.action-iconnoalttext').find('img').length).toEqual(1);
 			expect($tr.find('.action.action-iconnoalttext').find('img').eq(0).attr('alt')).toEqual('');
 		});
+		it('display inline icon with iconClass', function() {
+			fileActions.registerAction({
+				name: 'Icon',
+				displayName: 'IconDisplay',
+				type: OCA.Files.FileActions.TYPE_INLINE,
+				mime: 'all',
+				iconClass: 'icon-test',
+				permissions: OC.PERMISSION_READ
+			});
+			fileActions.registerAction({
+				name: 'NoIcon',
+				displayName: 'NoIconDisplay',
+				type: OCA.Files.FileActions.TYPE_INLINE,
+				mime: 'all',
+				permissions: OC.PERMISSION_READ
+			});
+
+			fileActions.display($tr.find('td.filename'), true, fileList);
+
+			expect($tr.find('.action.action-icon').length).toEqual(1);
+			expect($tr.find('.action.action-icon').find('.icon').length).toEqual(1);
+			expect($tr.find('.action.action-icon').find('.icon').hasClass('icon-test')).toEqual(true);
+
+			expect($tr.find('.action.action-noicon').length).toEqual(1);
+			expect($tr.find('.action.action-noicon').find('.icon').length).toEqual(0);
+		});
+		it('display alt text on inline icon with iconClass when no display name exists', function() {
+			fileActions.registerAction({
+				name: 'IconAltText',
+				displayName: '',
+				type: OCA.Files.FileActions.TYPE_INLINE,
+				mime: 'all',
+				iconClass: 'icon-alttext',
+				altText: 'alt icon text',
+				permissions: OC.PERMISSION_READ
+			});
+
+			fileActions.registerAction({
+				name: 'IconNoAltText',
+				displayName: 'IconNoAltTextDisplay',
+				type: OCA.Files.FileActions.TYPE_INLINE,
+				mime: 'all',
+				altText: 'useless alt text',
+				iconClass: 'icon-noalttext',
+				permissions: OC.PERMISSION_READ
+			});
+
+			fileActions.display($tr.find('td.filename'), true, fileList);
+
+			expect($tr.find('.action.action-iconalttext').length).toEqual(1);
+			expect($tr.find('.action.action-iconalttext').find('.icon').length).toEqual(1);
+			expect($tr.find('.action.action-iconalttext').find('.hidden-visually').text()).toEqual('alt icon text');
+
+			expect($tr.find('.action.action-iconnoalttext').length).toEqual(1);
+			expect($tr.find('.action.action-iconnoalttext').find('.icon').length).toEqual(1);
+			expect($tr.find('.action.action-iconnoalttext').find('.hidden-visually').length).toEqual(0);
+		});
 	});
 	describe('action handler', function() {
 		var actionStub, $tr, clock;
diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js
index 0a01c5af0ad..2711b2392e9 100644
--- a/apps/files_sharing/js/share.js
+++ b/apps/files_sharing/js/share.js
@@ -131,7 +131,7 @@
 				displayName: '',
 				mime: 'all',
 				permissions: OC.PERMISSION_ALL,
-				icon: OC.imagePath('core', 'actions/share'),
+				iconClass: 'icon-share',
 				type: OCA.Files.FileActions.TYPE_INLINE,
 				actionHandler: function(fileName) {
 					fileList.showDetailsView(fileName, 'shareTabView');
diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js
index fd8e36d71ad..7607ada50ba 100644
--- a/apps/files_sharing/tests/js/shareSpec.js
+++ b/apps/files_sharing/tests/js/shareSpec.js
@@ -97,9 +97,9 @@ describe('OCA.Sharing.Util tests', function() {
 			}]);
 			$tr = fileList.$el.find('tbody tr:first');
 			$action = $tr.find('.action-share');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder.svg');
-			expect($action.find('img').length).toEqual(1);
 		});
 		it('shows simple share text with share icon', function() {
 			var $action, $tr;
@@ -116,9 +116,9 @@ describe('OCA.Sharing.Util tests', function() {
 			$tr = fileList.$el.find('tbody tr:first');
 			$action = $tr.find('.action-share');
 			expect($action.find('>span').text().trim()).toEqual('Shared');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg');
-			expect($action.find('img').length).toEqual(1);
 		});
 		it('shows simple share text with public icon when shared with link', function() {
 			var $action, $tr;
@@ -136,9 +136,9 @@ describe('OCA.Sharing.Util tests', function() {
 			$tr = fileList.$el.find('tbody tr:first');
 			$action = $tr.find('.action-share');
 			expect($action.find('>span').text().trim()).toEqual('Shared');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('public.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(false);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(true);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-public.svg');
-			expect($action.find('img').length).toEqual(1);
 		});
 		it('shows owner name when owner is available', function() {
 			var $action, $tr;
@@ -156,7 +156,8 @@ describe('OCA.Sharing.Util tests', function() {
 			$tr = fileList.$el.find('tbody tr:first');
 			$action = $tr.find('.action-share');
 			expect($action.find('>span').text().trim()).toEqual('User One');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg');
 		});
 		it('shows recipients when recipients are available', function() {
@@ -175,9 +176,9 @@ describe('OCA.Sharing.Util tests', function() {
 			$tr = fileList.$el.find('tbody tr:first');
 			$action = $tr.find('.action-share');
 			expect($action.find('>span').text().trim()).toEqual('Shared with User One, User Two');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg');
-			expect($action.find('img').length).toEqual(1);
 		});
 		it('shows share action when shared with user who has no share permission', function() {
 			var $action, $tr;
@@ -282,7 +283,8 @@ describe('OCA.Sharing.Util tests', function() {
 			expect($tr.attr('data-share-recipients')).toEqual('Group One, Group Two, User One, User Two');
 
 			expect($action.find('>span').text().trim()).toEqual('Shared with Group One, Group Two, User One, User Two');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
 		it('updates share icon after updating shares of a file', function() {
 			var $action, $tr;
@@ -314,7 +316,8 @@ describe('OCA.Sharing.Util tests', function() {
 			expect($tr.attr('data-share-recipients')).toEqual('User One, User Three, User Two');
 
 			expect($action.find('>span').text().trim()).toEqual('Shared with User One, User Three, User Two');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
 		it('removes share icon after removing all shares from a file', function() {
 			var $action, $tr;
@@ -369,7 +372,8 @@ describe('OCA.Sharing.Util tests', function() {
 			expect($tr.attr('data-share-recipients')).toEqual('User Two');
 
 			expect($action.find('>span').text().trim()).toEqual('User One');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
 		it('keep share text after unsharing reshare', function() {
 			var $action, $tr;
@@ -399,7 +403,8 @@ describe('OCA.Sharing.Util tests', function() {
 			expect($tr.attr('data-share-recipients')).not.toBeDefined();
 
 			expect($action.find('>span').text().trim()).toEqual('User One');
-			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect($action.find('.icon').hasClass('icon-share')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
 	});
 	describe('formatRecipients', function() {
diff --git a/apps/files_trashbin/js/app.js b/apps/files_trashbin/js/app.js
index 600a8ce2b03..771ea90bc16 100644
--- a/apps/files_trashbin/js/app.js
+++ b/apps/files_trashbin/js/app.js
@@ -49,7 +49,7 @@ OCA.Trashbin.App = {
 			type: OCA.Files.FileActions.TYPE_INLINE,
 			mime: 'all',
 			permissions: OC.PERMISSION_READ,
-			icon: OC.imagePath('core', 'actions/history'),
+			iconClass: 'icon-history',
 			actionHandler: function(filename, context) {
 				var fileList = context.fileList;
 				var tr = fileList.findFileEl(filename);
@@ -70,9 +70,7 @@ OCA.Trashbin.App = {
 			displayName: t('files', 'Delete'),
 			mime: 'all',
 			permissions: OC.PERMISSION_READ,
-			icon: function() {
-				return OC.imagePath('core', 'actions/delete');
-			},
+			iconClass: 'icon-delete',
 			render: function(actionSpec, isDefault, context) {
 				var $actionLink = fileActions._makeActionLink(actionSpec, context);
 				$actionLink.attr('original-title', t('files_trashbin', 'Delete permanently'));