Make file actions use icon CSS classes

This makes theming of file actions possible

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;
+}

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
 	 */

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>';

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);

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;

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');

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() {

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'));

core/css/apps.css

@@ -642,6 +642,7 @@ em {
 .popovermenu .menuitem,
 .popovermenu .menuitem>span {
 	cursor: pointer;
+	vertical-align: middle;
 }
 
 .popovermenu .menuitem {
@@ -694,6 +695,7 @@ em {
 	width: 16px;
 	height: 16px;
 	margin-right: 10px;
+	vertical-align: middle;
 }
 
 .popovermenu .menuitem {

core/js/share.js

@@ -106,15 +106,15 @@ OC.Share = _.extend(OC.Share || {}, {
 		}
 		// TODO: iterating over the files might be more efficient
 		for (item in OC.Share.statuses){
-			var image = OC.imagePath('core', 'actions/share');
+			var iconClass = 'icon-share';
 			var data = OC.Share.statuses[item];
 			var hasLink = data.link;
 			// Links override shared in terms of icon display
 			if (hasLink) {
-				image = OC.imagePath('core', 'actions/public');
+				iconClass = 'icon-public';
 			}
 			if (itemType !== 'file' && itemType !== 'folder') {
-				$('a.share[data-item="'+item+'"]').css('background', 'url('+image+') no-repeat center');
+				$('a.share[data-item="'+item+'"] .icon').removeClass('icon-share icon-public').addClass(iconClass);
 			} else {
 				// TODO: ultimately this part should be moved to files_sharing app
 				var file = $fileList.find('tr[data-id="'+item+'"]');
@@ -160,23 +160,24 @@ OC.Share = _.extend(OC.Share || {}, {
 		var shares = false;
 		var link = false;
 		var image = OC.imagePath('core', 'actions/share');
+		var iconClass = '';
 		$.each(OC.Share.itemShares, function(index) {
 			if (OC.Share.itemShares[index]) {
 				if (index == OC.Share.SHARE_TYPE_LINK) {
 					if (OC.Share.itemShares[index] == true) {
 						shares = true;
-						image = OC.imagePath('core', 'actions/public');
+						iconClass = 'icon-public';
 						link = true;
 						return;
 					}
 				} else if (OC.Share.itemShares[index].length > 0) {
 					shares = true;
-					image = OC.imagePath('core', 'actions/share');
+					iconClass = 'icon-share';
 				}
 			}
 		});
 		if (itemType != 'file' && itemType != 'folder') {
-			$('a.share[data-item="'+itemSource+'"]').css('background', 'url('+image+') no-repeat center');
+			$('a.share[data-item="'+itemSource+'"] .icon').removeClass('icon-share icon-public').addClass(iconClass);
 		} else {
 			var $tr = $('tr').filterAttr('data-id', String(itemSource));
 			if ($tr.length > 0) {
@@ -255,12 +256,12 @@ OC.Share = _.extend(OC.Share || {}, {
 	markFileAsShared: function($tr, hasShares, hasLink) {
 		var action = $tr.find('.fileactions .action[data-action="Share"]');
 		var type = $tr.data('type');
-		var img = action.find('img');
+		var icon = action.find('.icon');
 		var message;
 		var recipients;
 		var owner = $tr.attr('data-share-owner');
 		var shareFolderIcon;
-		var image = OC.imagePath('core', 'actions/share');
+		var iconClass = 'icon-share';
 		action.removeClass('shared-style');
 		// update folder icon
 		if (type === 'dir' && (hasShares || hasLink || owner)) {
@@ -299,18 +300,18 @@ OC.Share = _.extend(OC.Share || {}, {
 			else if (recipients) {
 				message = t('core', 'Shared with {recipients}', {recipients: this._formatShareList(recipients.split(", ")).join(", ")}, 0, {escape: false});
 			}
-			action.html('<span> ' + message + '</span>').prepend(img);
+			action.html('<span> ' + message + '</span>').prepend(icon);
 			if (owner || recipients) {
 				action.find('.remoteAddress').tipsy({gravity: 's'});
 			}
 		}
 		else {
-			action.html('<span></span>').prepend(img);
+			action.html('<span></span>').prepend(icon);
 		}
 		if (hasLink) {
-			image = OC.imagePath('core', 'actions/public');
+			iconClass = 'icon-public';
 		}
-		img.attr('src', image);
+		icon.removeClass('icon-share icon-public').addClass(iconClass);
 	},
 	/**
 	 *