Merge pull request #6094 from nextcloud/sharedby-avatar

Show avatar instead of username for shared files

4 files changed, 68 insertions, 58 deletions

diff --git a/apps/files/css/files.scss b/apps/files/css/files.scss
index c03b1ce6fad..c165063bb85 100644
--- a/apps/files/css/files.scss
+++ b/apps/files/css/files.scss
@@ -555,6 +555,10 @@ a.action > img {
 #fileList a.action.action-share {
 	padding: 17px 14px;
 }
+#fileList a.action.action-share .avatar {
+	display: inline-block;
+	vertical-align: middle;
+}
 
 #fileList a.action.action-menu {
 	padding-top: 17px;
@@ -590,8 +594,12 @@ a.action > img {
 	max-width: 70px;
 	overflow: hidden;
 	text-overflow: ellipsis;
-	vertical-align: bottom;
-	padding-left: 6px;
+	vertical-align: middle;
+	margin-left: 6px;
+}
+
+#fileList .remoteAddress .userDomain {
+	margin-left: 0 !important;
 }
 
 #fileList .favorite-mark.permanent {
diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js
index ea2f427df75..5b0a78c9c64 100644
--- a/apps/files_sharing/tests/js/shareSpec.js
+++ b/apps/files_sharing/tests/js/shareSpec.js
@@ -145,7 +145,7 @@ 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($action.find('>span').text().trim()).toEqual('Shared by User One');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg');
@@ -166,7 +166,7 @@ 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($action.text().trim()).toEqual('Shared with User One Shared with User Two');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 			expect(OC.basename(getImageUrl($tr.find('.filename .thumbnail')))).toEqual('folder-shared.svg');
@@ -273,7 +273,7 @@ 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($action.text().trim()).toEqual('Shared with Group One Shared with Group Two Shared with User One Shared with User Two');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
@@ -306,7 +306,7 @@ 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($action.text().trim()).toEqual('Shared with User One Shared with User Three Shared with User Two');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
@@ -362,7 +362,7 @@ 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($action.find('>span').text().trim()).toEqual('Shared by User One');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
@@ -393,7 +393,7 @@ describe('OCA.Sharing.Util tests', function() {
 
 			expect($tr.attr('data-share-recipients')).not.toBeDefined();
 
-			expect($action.find('>span').text().trim()).toEqual('User One');
+			expect($action.find('>span').text().trim()).toEqual('Shared by User One');
 			expect($action.find('.icon').hasClass('icon-shared')).toEqual(true);
 			expect($action.find('.icon').hasClass('icon-public')).toEqual(false);
 		});
diff --git a/core/js/share.js b/core/js/share.js
index 659d719788d..25d59b46fb4 100644
--- a/core/js/share.js
+++ b/core/js/share.js
@@ -203,18 +203,19 @@ OC.Share = _.extend(OC.Share || {}, {
 	 * @param {String} remoteAddress full remote share
 	 * @return {String} HTML code to display
 	 */
-	_formatRemoteShare: function(remoteAddress) {
+	_formatRemoteShare: function(remoteAddress, message) {
 		var parts = this._REMOTE_OWNER_REGEXP.exec(remoteAddress);
 		if (!parts) {
-			// display as is, most likely to be a simple owner name
-			return escapeHTML(remoteAddress);
+			// display avatar of the user
+			var avatar = '<span class="avatar" data-userName="' + escapeHTML(remoteAddress) + '" title="' + message + " " + escapeHTML(remoteAddress) + '"></span>';
+			var hidden = '<span class="hidden-visually">' + message + ' ' + escapeHTML(remoteAddress) + '</span> ';
+			return avatar + hidden;
 		}
 
 		var userName = parts[1];
 		var userDomain = parts[3];
 		var server = parts[4];
-		var dir = parts[6];
-		var tooltip = userName;
+		var tooltip = message + ' ' + userName;
 		if (userDomain) {
 			tooltip += '@' + userDomain;
 		}
@@ -230,7 +231,7 @@ OC.Share = _.extend(OC.Share || {}, {
 		if (userDomain) {
 			html += '<span class="userDomain">@' + escapeHTML(userDomain) + '</span>';
 		}
-		html += '</span>';
+		html += '</span> ';
 		return html;
 	},
 	/**
@@ -243,7 +244,7 @@ OC.Share = _.extend(OC.Share || {}, {
 	_formatShareList: function(recipients) {
 		var _parent = this;
 		return $.map(recipients, function(recipient) {
-			recipient = _parent._formatRemoteShare(recipient);
+			recipient = _parent._formatRemoteShare(recipient, t('core', 'Shared with'));
 			return recipient;
 		});
 	},
@@ -259,8 +260,7 @@ OC.Share = _.extend(OC.Share || {}, {
 		var action = $tr.find('.fileactions .action[data-action="Share"]');
 		var type = $tr.data('type');
 		var icon = action.find('.icon');
-		var message;
-		var recipients;
+		var message, recipients, avatars;
 		var owner = $tr.attr('data-share-owner');
 		var shareFolderIcon;
 		var iconClass = 'icon-shared';
@@ -294,20 +294,23 @@ OC.Share = _.extend(OC.Share || {}, {
 			recipients = $tr.attr('data-share-recipients');
 			action.addClass('shared-style');
 
-			message = t('core', 'Shared');
+			avatars = '<span>' + t('core', 'Shared') + '</span>';
 			// even if reshared, only show "Shared by"
 			if (owner) {
-				message = this._formatRemoteShare(owner);
+				message = t('core', 'Shared by');
+				avatars = this._formatRemoteShare(owner, message);
+			} else if (recipients) {
+				avatars = this._formatShareList(recipients.split(', ')).join('');
 			}
-			else if (recipients) {
-				message = t('core', 'Shared with {recipients}', {recipients: this._formatShareList(recipients.split(", ")).join(", ")}, 0, {escape: false});
-			}
-			action.html('<span> ' + message + '</span>').prepend(icon);
+			action.html(avatars).prepend(icon);
+
 			if (owner || recipients) {
-				action.find('.remoteAddress').tooltip({placement: 'top'});
+				var avatarElement = action.find('.avatar');
+				avatarElement.avatar(avatarElement.data('username'), 32);
+
+				action.find('.icon-shared + span').tooltip({placement: 'top'});
 			}
-		}
-		else {
+		} else {
 			action.html('<span class="hidden-visually">' + t('core', 'Shared') + '</span>').prepend(icon);
 		}
 		if (hasLink) {
diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js
index fbf6eecc8df..70c698c99a2 100644
--- a/core/js/tests/specs/shareSpec.js
+++ b/core/js/tests/specs/shareSpec.js
@@ -47,7 +47,7 @@ describe('OC.Share tests', function() {
 				$file.attr('data-share-owner', input);
 				OC.Share.markFileAsShared($file);
 
-				$action = $file.find('.action-share>span');
+				$action = $file.find('.action-share>span').parent();
 				expect($action.text().trim()).toEqual(output);
 				if (_.isString(title)) {
 					expect($action.find('.remoteAddress').attr('title')).toEqual(title);
@@ -58,41 +58,41 @@ describe('OC.Share tests', function() {
 				tooltipStub.reset();
 			}
 
-			it('displays the local share owner as is', function() {
-				checkOwner('User One', 'User One', null);
+			it('displays the local share owner with "Shared by" prefix', function() {
+				checkOwner('User One', 'Shared by User One', null);
 			});
 			it('displays the user name part of a remote share owner', function() {
 				checkOwner(
 					'User One@someserver.com',
 					'User One@…',
-					'User One@someserver.com'
+					'Shared by User One@someserver.com'
 				);
 				checkOwner(
 					'User One@someserver.com/',
 					'User One@…',
-					'User One@someserver.com'
+					'Shared by User One@someserver.com'
 				);
 				checkOwner(
 					'User One@someserver.com/root/of/owncloud',
 					'User One@…',
-					'User One@someserver.com'
+					'Shared by User One@someserver.com'
 				);
 			});
 			it('displays the user name part with domain of a remote share owner', function() {
 				checkOwner(
 					'User One@example.com@someserver.com',
 					'User One@example.com',
-					'User One@example.com@someserver.com'
+					'Shared by User One@example.com@someserver.com'
 				);
 				checkOwner(
 					'User One@example.com@someserver.com/',
 					'User One@example.com',
-					'User One@example.com@someserver.com'
+					'Shared by User One@example.com@someserver.com'
 				);
 				checkOwner(
 					'User One@example.com@someserver.com/root/of/owncloud',
 					'User One@example.com',
-					'User One@example.com@someserver.com'
+					'Shared by User One@example.com@someserver.com'
 				);
 			});
 		});
@@ -151,14 +151,14 @@ describe('OC.Share tests', function() {
 			});
 		});
 
-		describe('displaying the recipoients', function() {
+		describe('displaying the recipients', function() {
 			function checkRecipients(input, output, title) {
 				var $action;
 
 				$file.attr('data-share-recipients', input);
 				OC.Share.markFileAsShared($file, true);
 
-				$action = $file.find('.action-share>span');
+				$action = $file.find('.action-share>span').parent();
 				expect($action.text().trim()).toEqual(output);
 				if (_.isString(title)) {
 					expect($action.find('.remoteAddress').attr('title')).toEqual(title);
@@ -182,62 +182,61 @@ describe('OC.Share tests', function() {
 			it('displays the user name part of a remote recipient', function() {
 				checkRecipients(
 					'User One@someserver.com',
-					'Shared with User One@…',
-					'User One@someserver.com'
+					'User One@…',
+					'Shared with User One@someserver.com'
 				);
 				checkRecipients(
 					'User One@someserver.com/',
-					'Shared with User One@…',
-					'User One@someserver.com'
+					'User One@…',
+					'Shared with User One@someserver.com'
 				);
 				checkRecipients(
 					'User One@someserver.com/root/of/owncloud',
-					'Shared with User One@…',
-					'User One@someserver.com'
+					'User One@…',
+					'Shared with User One@someserver.com'
 				);
 			});
 			it('displays the user name part with domain of a remote share owner', function() {
 				checkRecipients(
 					'User One@example.com@someserver.com',
-					'Shared with User One@example.com',
-					'User One@example.com@someserver.com'
+					'User One@example.com',
+					'Shared with User One@example.com@someserver.com'
 				);
 				checkRecipients(
 					'User One@example.com@someserver.com/',
-					'Shared with User One@example.com',
-					'User One@example.com@someserver.com'
+					'User One@example.com',
+					'Shared with User One@example.com@someserver.com'
 				);
 				checkRecipients(
 					'User One@example.com@someserver.com/root/of/owncloud',
-					'Shared with User One@example.com',
-					'User One@example.com@someserver.com'
+					'User One@example.com',
+					'Shared with User One@example.com@someserver.com'
 				);
 			});
 			it('display multiple remote recipients', function() {
 				checkRecipients(
 					'One@someserver.com, two@otherserver.com',
-					'Shared with One@…, two@…',
-					['One@someserver.com', 'two@otherserver.com']
+					'One@… two@…',
+					['Shared with One@someserver.com', 'Shared with two@otherserver.com']
 				);
 				checkRecipients(
 					'One@someserver.com/, two@otherserver.com',
-					'Shared with One@…, two@…',
-					['One@someserver.com', 'two@otherserver.com']
+					'One@… two@…',
+					['Shared with One@someserver.com', 'Shared with two@otherserver.com']
 				);
 				checkRecipients(
 					'One@someserver.com/root/of/owncloud, two@otherserver.com',
-					'Shared with One@…, two@…',
-					['One@someserver.com', 'two@otherserver.com']
+					'One@… two@…',
+					['Shared with One@someserver.com', 'Shared with two@otherserver.com']
 				);
 			});
 			it('display mixed recipients', function() {
 				checkRecipients(
 					'One, two@otherserver.com',
-					'Shared with One, two@…',
-					['two@otherserver.com']
+					'Shared with One two@…',
+					['Shared with two@otherserver.com']
 				);
 			});
 		});
 	});
 });
-