Merge pull request #22461 from owncloud/fix_6773

Allow theming of actions

8 files changed, 121 insertions, 39 deletions

diff --git a/apps/files/css/files.css b/apps/files/css/files.css
index 389b7fae806..b65af1fa0f3 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,14 @@ html.ie8 #controls .button.new {
 	padding: 0;
 }
 
+#filestable .filename .action .icon,
+#filestable .selectedActions a .icon,
+#controls .actions .button .icon {
+	display: inline-block;
+	vertical-align: middle;
+	background-size: 16px 16px;
+}
+
+.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..05ff2f0cbfa 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -12,7 +12,12 @@
 
 	var TEMPLATE_FILE_ACTION_TRIGGER =
 		'<a class="action action-{{nameLowerCase}}" href="#" data-action="{{name}}">' +
-		'{{#if icon}}<img class="svg" alt="{{altText}}" src="{{icon}}" />{{/if}}' +
+		'{{#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/js/tagsplugin.js b/apps/files/js/tagsplugin.js
index 81b22e34cc2..3c0a231d003 100644
--- a/apps/files/js/tagsplugin.js
+++ b/apps/files/js/tagsplugin.js
@@ -15,17 +15,18 @@
 	var TEMPLATE_FAVORITE_ACTION =
 		'<a href="#" ' +
 		'class="action action-favorite {{#isFavorite}}permanent{{/isFavorite}}">' +
-		'<img class="svg" alt="{{altText}}" src="{{imgFile}}" />' +
+		'<span class="icon {{iconClass}}" />' +
+		'<span class="hidden-visually">{{altText}}</span>' +
 		'</a>';
 
 	/**
-	 * Returns the path to the star image
+	 * Returns the icon class for the matching state
 	 *
 	 * @param {boolean} state true if starred, false otherwise
-	 * @return {string} path to star image
+	 * @return {string} icon class for star image
 	 */
-	function getStarImage(state) {
-		return OC.imagePath('core', state ? 'actions/starred' : 'actions/star');
+	function getStarIconClass(state) {
+		return state ? 'icon-starred' : 'icon-star';
 	}
 
 	/**
@@ -41,7 +42,7 @@
 		return this._template({
 			isFavorite: state,
 			altText: state ? t('files', 'Favorited') : t('files', 'Favorite'),
-			imgFile: getStarImage(state)
+			iconClass: getStarIconClass(state)
 		});
 	}
 
@@ -52,8 +53,7 @@
 	 * @param {boolean} state true if starred, false otherwise
 	 */
 	function toggleStar($actionEl, state) {
-		$actionEl.find('img').attr('src', getStarImage(state));
-		$actionEl.hide().show(0); //force Safari to redraw element on src change
+		$actionEl.removeClass('icon-star icon-starred').addClass(getStarIconClass(state));
 		$actionEl.toggleClass('permanent', state);
 	}
 
diff --git a/apps/files/templates/list.php b/apps/files/templates/list.php
index ea3e6c61d4a..29769bcd825 100644
--- a/apps/files/templates/list.php
+++ b/apps/files/templates/list.php
@@ -52,9 +52,8 @@
 					<a class="name sort columntitle" data-sort="name"><span><?php p($l->t( 'Name' )); ?></span><span class="sort-indicator"></span></a>
 					<span id="selectedActionsList" class="selectedActions">
 						<a href="" class="download">
-							<img class="svg" alt=""
-								 src="<?php print_unescaped(image_path("core", "actions/download.svg")); ?>" />
-							<?php p($l->t('Download'))?>
+							<span class="icon icon-download"></span>
+							<span><?php p($l->t('Download'))?></span>
 						</a>
 					</span>
 				</div>
@@ -65,9 +64,8 @@
 			<th id="headerDate" class="hidden column-mtime">
 				<a id="modified" class="columntitle" data-sort="mtime"><span><?php p($l->t( 'Modified' )); ?></span><span class="sort-indicator"></span></a>
 					<span class="selectedActions"><a href="" class="delete-selected">
-						<?php p($l->t('Delete'))?>
-						<img class="svg" alt=""
-							 src="<?php print_unescaped(image_path("core", "actions/delete.svg")); ?>" />
+						<span><?php p($l->t('Delete'))?></span>
+						<span class="icon icon-delete"></span>
 					</a></span>
 			</th>
 		</tr>
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/tests/js/tagspluginspec.js b/apps/files/tests/js/tagspluginspec.js
index 533aa63362c..a4efc08aa53 100644
--- a/apps/files/tests/js/tagspluginspec.js
+++ b/apps/files/tests/js/tagspluginspec.js
@@ -99,7 +99,8 @@ describe('OCA.Files.TagsPlugin tests', function() {
 			expect($tr.attr('data-favorite')).toEqual('true');
 			expect($tr.attr('data-tags').split('|')).toEqual(['tag1', 'tag2', 'tag3', OC.TAG_FAVORITE]);
 			expect(fileList.files[0].tags).toEqual(['tag1', 'tag2', 'tag3', OC.TAG_FAVORITE]);
-			expect($action.find('img').attr('src')).toEqual(OC.imagePath('core', 'actions/starred'));
+			expect($action.find('.icon').hasClass('icon-star')).toEqual(false);
+			expect($action.find('.icon').hasClass('icon-starred')).toEqual(true);
 
 			$action.click();
 
@@ -119,7 +120,8 @@ describe('OCA.Files.TagsPlugin tests', function() {
 			expect($tr.attr('data-favorite')).toBeFalsy();
 			expect($tr.attr('data-tags').split('|')).toEqual(['tag1', 'tag2', 'tag3']);
 			expect(fileList.files[0].tags).toEqual(['tag1', 'tag2', 'tag3']);
-			expect($action.find('img').attr('src')).toEqual(OC.imagePath('core', 'actions/star'));
+			expect($action.find('.icon').hasClass('icon-star')).toEqual(true);
+			expect($action.find('.icon').hasClass('icon-starred')).toEqual(false);
 		});
 	});
 	describe('elementToFile', function() {