Merge pull request #6709 from nextcloud/show-checkbox-where-the-favourite-icon-is-now

Show checkbox where the favourite icon is now

4 files changed, 98 insertions, 44 deletions

diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index 3da9b06b0d3..0f320c8b3c7 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -706,7 +706,7 @@
 	 * @property {String} mime mime type
 	 * @property {int} permissions permissions
 	 * @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 {(String|OCA.Files.FileActions~iconClassFunction)} 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
 	 */
@@ -746,6 +746,17 @@
 	 */
 
 	/**
+	 * Icon class function for actions.
+	 * The function returns the icon class of the action using
+	 * the given context information.
+	 *
+	 * @callback OCA.Files.FileActions~iconClassFunction
+	 * @param {String} fileName name of the file on which the action must be performed
+	 * @param {OCA.Files.FileActionContext} context action context
+	 * @return {String} icon class
+	 */
+
+	/**
 	 * Action handler function for file actions
 	 *
 	 * @callback OCA.Files.FileActions~actionHandler
diff --git a/apps/files/js/fileactionsmenu.js b/apps/files/js/fileactionsmenu.js
index 45d2bd83049..b8022f13734 100644
--- a/apps/files/js/fileactionsmenu.js
+++ b/apps/files/js/fileactionsmenu.js
@@ -115,6 +115,11 @@
 					item = _.extend({}, item);
 					item.displayName = item.displayName(self._context);
 				}
+				if (_.isFunction(item.iconClass)) {
+					var fileName = self._context.$file.attr('data-file');
+					item = _.extend({}, item);
+					item.iconClass = item.iconClass(fileName, self._context);
+				}
 				return item;
 			});
 			items = items.sort(function(actionA, actionB) {
diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index cc23ac73979..0d45c29b25a 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -332,7 +332,7 @@
 
 			this.$fileList.on('click','td.filename>a.name, td.filesize, td.date', _.bind(this._onClickFile, this));
 
-			this.$fileList.on('change', 'td.filename>.selectCheckBox', _.bind(this._onClickFileCheckbox, this));
+			this.$fileList.on('change', 'td.selection>.selectCheckBox', _.bind(this._onClickFileCheckbox, this));
 			this.$el.on('show', _.bind(this._onShow, this));
 			this.$el.on('urlChanged', _.bind(this._onUrlChanged, this));
 			this.$el.find('.select-all').click(_.bind(this._onClickSelectAll, this));
@@ -593,7 +593,7 @@
 		 * @param {bool} state true to select, false to deselect
 		 */
 		_selectFileEl: function($tr, state, showDetailsView) {
-			var $checkbox = $tr.find('td.filename>.selectCheckBox');
+			var $checkbox = $tr.find('td.selection>.selectCheckBox');
 			var oldData = !!this._selectedFiles[$tr.data('id')];
 			var data;
 			$checkbox.prop('checked', state);
@@ -649,7 +649,7 @@
 				else {
 					this._lastChecked = $tr;
 				}
-				var $checkbox = $tr.find('td.filename>.selectCheckBox');
+				var $checkbox = $tr.find('td.selection>.selectCheckBox');
 				this._selectFileEl($tr, !$checkbox.prop('checked'));
 				this.updateSelectionSummary();
 			} else {
@@ -704,7 +704,7 @@
 		 */
 		_onClickSelectAll: function(e) {
 			var checked = $(e.target).prop('checked');
-			this.$fileList.find('td.filename>.selectCheckBox').prop('checked', checked)
+			this.$fileList.find('td.selection>.selectCheckBox').prop('checked', checked)
 				.closest('tr').toggleClass('selected', checked);
 			this._selectedFiles = {};
 			this._selectionSummary.clear();
@@ -1063,6 +1063,13 @@
 
 			this.$fileList.empty();
 
+			if (this._allowSelection) {
+				// The results table, which has no selection column, checks
+				// whether the main table has a selection column or not in order
+				// to align its contents with those of the main table.
+				this.$el.addClass('has-selection');
+			}
+
 			// clear "Select all" checkbox
 			this.$el.find('.select-all').prop('checked', false);
 
@@ -1192,6 +1199,20 @@
 				path = this.getCurrentDirectory();
 			}
 
+			// selection td
+			if (this._allowSelection) {
+				td = $('<td class="selection"></td>');
+
+				td.append(
+					'<input id="select-' + this.id + '-' + fileData.id +
+					'" type="checkbox" class="selectCheckBox checkbox"/><label for="select-' + this.id + '-' + fileData.id + '">' +
+					'<span class="hidden-visually">' + t('files', 'Select') + '</span>' +
+					'</label>'
+				);
+
+				tr.append(td);
+			}
+
 			// filename td
 			td = $('<td class="filename"></td>');
 
@@ -1203,22 +1224,13 @@
 			else {
 				linkUrl = this.getDownloadUrl(name, path, type === 'dir');
 			}
-			if (this._allowSelection) {
-				td.append(
-					'<input id="select-' + this.id + '-' + fileData.id +
-					'" type="checkbox" class="selectCheckBox checkbox"/><label for="select-' + this.id + '-' + fileData.id + '">' +
-					'<div class="thumbnail" style="background-image:url(' + icon + '); background-size: 32px;"></div>' +
-					'<span class="hidden-visually">' + t('files', 'Select') + '</span>' +
-					'</label>'
-				);
-			} else {
-				td.append('<div class="thumbnail" style="background-image:url(' + icon + '); background-size: 32px;"></div>');
-			}
 			var linkElem = $('<a></a>').attr({
 				"class": "name",
 				"href": linkUrl
 			});
 
+			linkElem.append('<div class="thumbnail-wrapper"><div class="thumbnail" style="background-image:url(' + icon + ');"></div></div>');
+
 			// from here work on the display name
 			name = fileData.displayName || name;
 
@@ -2614,6 +2626,13 @@
 		 */
 		_createSummary: function() {
 			var $tr = $('<tr class="summary"></tr>');
+
+			if (this._allowSelection) {
+				// Dummy column for selection, as all rows must have the same
+				// number of columns.
+				$tr.append('<td></td>');
+			}
+
 			this.$el.find('tfoot').append($tr);
 
 			return new OCA.Files.FileSummary($tr, {config: this._filesConfig});
diff --git a/apps/files/js/tagsplugin.js b/apps/files/js/tagsplugin.js
index 9bd20be4bf8..2286477750c 100644
--- a/apps/files/js/tagsplugin.js
+++ b/apps/files/js/tagsplugin.js
@@ -17,12 +17,12 @@
 		PROPERTY_FAVORITE:	'{' + OC.Files.Client.NS_OWNCLOUD + '}favorite'
 	});
 
-	var TEMPLATE_FAVORITE_ACTION =
-		'<a href="#" ' +
-		'class="action action-favorite {{#isFavorite}}permanent{{/isFavorite}}">' +
+	var TEMPLATE_FAVORITE_MARK =
+		'<div ' +
+		'class="favorite-mark {{#isFavorite}}permanent{{/isFavorite}}">' +
 		'<span class="icon {{iconClass}}" />' +
 		'<span class="hidden-visually">{{altText}}</span>' +
-		'</a>';
+		'</div>';
 
 	/**
 	 * Returns the icon class for the matching state
@@ -42,24 +42,24 @@
 	 */
 	function renderStar(state) {
 		if (!this._template) {
-			this._template = Handlebars.compile(TEMPLATE_FAVORITE_ACTION);
+			this._template = Handlebars.compile(TEMPLATE_FAVORITE_MARK);
 		}
 		return this._template({
 			isFavorite: state,
-			altText: state ? t('files', 'Favorited') : t('files', 'Favorite'),
+			altText: state ? t('files', 'Favorited') : t('files', 'Not favorited'),
 			iconClass: getStarIconClass(state)
 		});
 	}
 
 	/**
-	 * Toggle star icon on action element
+	 * Toggle star icon on favorite mark element
 	 *
-	 * @param {Object} action element
+	 * @param {Object} $favoriteMarkEl favorite mark element
 	 * @param {boolean} state true if starred, false otherwise
 	 */
-	function toggleStar($actionEl, state) {
-		$actionEl.removeClass('icon-star icon-starred').addClass(getStarIconClass(state));
-		$actionEl.toggleClass('permanent', state);
+	function toggleStar($favoriteMarkEl, state) {
+		$favoriteMarkEl.removeClass('icon-star icon-starred').addClass(getStarIconClass(state));
+		$favoriteMarkEl.toggleClass('permanent', state);
 	}
 
 	OCA.Files = OCA.Files || {};
@@ -67,8 +67,9 @@
 	/**
 	 * @namespace OCA.Files.TagsPlugin
 	 *
-	 * Extends the file actions and file list to include a favorite action icon
-	 * and addition "data-tags" and "data-favorite" attributes.
+	 * Extends the file actions and file list to include a favorite mark icon
+	 * and a favorite action in the file actions menu; it also adds "data-tags"
+	 * and "data-favorite" attributes to file elements.
 	 */
 	OCA.Files.TagsPlugin = {
 		name: 'Tags',
@@ -84,22 +85,38 @@
 
 		_extendFileActions: function(fileActions) {
 			var self = this;
-			// register "star" action
+
 			fileActions.registerAction({
 				name: 'Favorite',
-				displayName: t('files', 'Favorite'),
+				displayName: function(context) {
+					var $file = context.$file;
+					var isFavorite = $file.data('favorite') === true;
+
+					if (isFavorite) {
+						return t('files', 'Remove from favorites');
+					}
+
+					// As it is currently not possible to provide a context for
+					// the i18n strings "Add to favorites" was used instead of
+					// "Favorite" to remove the ambiguity between verb and noun
+					// when it is translated.
+					return t('files', 'Add to favorites');
+				},
 				mime: 'all',
+				order: -23,
 				permissions: OC.PERMISSION_READ,
-				type: OCA.Files.FileActions.TYPE_INLINE,
-				render: function(actionSpec, isDefault, context) {
+				iconClass: function(fileName, context) {
 					var $file = context.$file;
 					var isFavorite = $file.data('favorite') === true;
-					var $icon = $(renderStar(isFavorite));
-					$file.find('td:first>.favorite').replaceWith($icon);
-					return $icon;
+
+					if (isFavorite) {
+						return 'icon-starred';
+					}
+
+					return 'icon-star';
 				},
 				actionHandler: function(fileName, context) {
-					var $actionEl = context.$file.find('.action-favorite');
+					var $favoriteMarkEl = context.$file.find('.favorite-mark');
 					var $file = context.$file;
 					var fileInfo = context.fileList.files[$file.index()];
 					var dir = context.dir || context.fileList.getCurrentDirectory();
@@ -118,14 +135,14 @@
 					}
 
 					// pre-toggle the star
-					toggleStar($actionEl, !isFavorite);
+					toggleStar($favoriteMarkEl, !isFavorite);
 
 					context.fileInfoModel.trigger('busy', context.fileInfoModel, true);
 
 					self.applyFileTags(
 						dir + '/' + fileName,
 						tags,
-						$actionEl,
+						$favoriteMarkEl,
 						isFavorite
 					).then(function(result) {
 						context.fileInfoModel.trigger('busy', context.fileInfoModel, false);
@@ -145,17 +162,19 @@
 
 		_extendFileList: function(fileList) {
 			// extend row prototype
-			fileList.$el.addClass('has-favorites');
 			var oldCreateRow = fileList._createRow;
 			fileList._createRow = function(fileData) {
 				var $tr = oldCreateRow.apply(this, arguments);
+				var isFavorite = false;
 				if (fileData.tags) {
 					$tr.attr('data-tags', fileData.tags.join('|'));
 					if (fileData.tags.indexOf(OC.TAG_FAVORITE) >= 0) {
 						$tr.attr('data-favorite', true);
+						isFavorite = true;
 					}
 				}
-				$tr.find('td:first').prepend('<div class="favorite"></div>');
+				var $icon = $(renderStar(isFavorite));
+				$tr.find('td.filename .thumbnail').append($icon);
 				return $tr;
 			};
 			var oldElementToFile = fileList.elementToFile;
@@ -215,10 +234,10 @@
 		 *
 		 * @param {String} fileName path to the file or folder to tag
 		 * @param {Array.<String>} tagNames array of tag names
-		 * @param {Object} $actionEl element
+		 * @param {Object} $favoriteMarkEl favorite mark element
 		 * @param {boolean} isFavorite Was the item favorited before
 		 */
-		applyFileTags: function(fileName, tagNames, $actionEl, isFavorite) {
+		applyFileTags: function(fileName, tagNames, $favoriteMarkEl, isFavorite) {
 			var encodedPath = OC.encodePath(fileName);
 			while (encodedPath[0] === '/') {
 				encodedPath = encodedPath.substr(1);
@@ -238,7 +257,7 @@
 					message = ': ' + response.responseJSON.message;
 				}
 				OC.Notification.show(t('files', 'An error occurred while trying to update the tags' + message), {type: 'error'});
-				toggleStar($actionEl, isFavorite);
+				toggleStar($favoriteMarkEl, isFavorite);
 			});
 		}
 	};