Merge pull request #14582 from rullzer/avatar_fixes

Avatars in share dialog fixes

Author: MorrisJobke

core/css/share.css

@@ -37,6 +37,15 @@
 	display: none !important;
 }
 
+#dropdown .avatar {
+	margin-right: 2px;
+	display: inline-block;
+	overflow: hidden;
+	vertical-align: middle;
+	width: 32px;
+	height: 32px;
+}
+
 #shareWithList {
 	list-style-type:none;
 	padding:8px;
@@ -68,12 +77,6 @@
 	overflow: hidden;
 	vertical-align: middle;
 }
-#shareWithList .avatar {
-	margin-right: 2px;
-	display: inline-block;
-	overflow: hidden;
-	vertical-align: middle;
-}
 #shareWithList li label{
 	margin-right: 8px;
 }

core/js/core.json

@@ -5,12 +5,14 @@
 		"jquery-ui/ui/jquery-ui.custom.js",
 		"underscore/underscore.js",
 		"moment/min/moment-with-locales.js",
-		"handlebars/handlebars.js"
+		"handlebars/handlebars.js",
+		"blueimp-md5/js/md5.js"
 	],
 	"libraries": [
 		"jquery-showpassword.js",
 		"jquery-tipsy.js",
-		"jquery.avatar.js"
+		"jquery.avatar.js",
+		"placeholder.js"
 	],
 	"modules": [
 		"compatibility.js",

core/js/share.js

@@ -357,20 +357,17 @@ OC.Share={
 		var dropDownEl;
 		var html = '<div id="dropdown" class="drop shareDropDown" data-item-type="'+itemType+'" data-item-source="'+itemSource+'">';
 		if (data !== false && data.reshare !== false && data.reshare.uid_owner !== undefined) {
+			html += '<span class="reshare">';
+			if (oc_config.enable_avatars === true) {
+				html += '<div class="avatar"></div> ';
+			}
+
 			if (data.reshare.share_type == OC.Share.SHARE_TYPE_GROUP) {
-				html += '<span class="reshare">'+t('core', 'Shared with you and the group {group} by {owner}', {group: data.reshare.share_with, owner: data.reshare.displayname_owner});
-				if (oc_config.enable_avatars === true) {
-					html += ' <div id="avatar-share-owner" style="display: inline-block"></div>';
-				}
-				html += '</span>';
+				html += t('core', 'Shared with you and the group {group} by {owner}', {group: data.reshare.share_with, owner: data.reshare.displayname_owner});
 			} else {
-				html += '<span class="reshare">'+t('core', 'Shared with you by {owner}', {owner: data.reshare.displayname_owner});
-				if (oc_config.enable_avatars === true) {
-					html += ' <div id="avatar-share-owner" style="display: inline-block"></div>';
-				}
-				html += '</span>';
+				html += t('core', 'Shared with you by {owner}', {owner: data.reshare.displayname_owner});
 			}
-			html += '<br />';
+			html += '</span><br />';
 			// reduce possible permissions to what the original share allowed
 			possiblePermissions = possiblePermissions & data.reshare.permissions;
 		}
@@ -448,7 +445,7 @@ OC.Share={
 
 			//Get owner avatars
 			if (oc_config.enable_avatars === true && data !== false && data.reshare !== false && data.reshare.uid_owner !== undefined) {
-				$('#avatar-share-owner').avatar(data.reshare.uid_owner, 32);
+				dropDownEl.find(".avatar").avatar(data.reshare.uid_owner, 32);
 			}
 
 			// Reset item shares
@@ -665,11 +662,7 @@ OC.Share={
 			var showCrudsButton;
 			html += '<a href="#" class="unshare"><img class="svg" alt="'+t('core', 'Unshare')+'" title="'+t('core', 'Unshare')+'" src="'+OC.imagePath('core', 'actions/delete')+'"/></a>';
 			if (oc_config.enable_avatars === true) {
-				if (shareType === OC.Share.SHARE_TYPE_USER) {
-					html += '<div data-user="' + escapeHTML(shareWith) + '" class="avatar"></div>';
-				} else {
-					html += '<div class="avatar" style="padding-right: 32px"></div>';
-				}
+				html += '<div class="avatar"></div>';
 			}
 			html += '<span class="username">' + escapeHTML(shareWithDisplayName) + '</span>';
 			var mailNotificationEnabled = $('input:hidden[name=mailNotificationEnabled]').val();
@@ -702,8 +695,13 @@ OC.Share={
 			html += '</div>';
 			html += '</li>';
 			html = $(html).appendTo('#shareWithList');
-			if (oc_config.enable_avatars === true && shareType === OC.Share.SHARE_TYPE_USER) {
-				$('.avatar[data-user="' + escapeHTML(shareWith) + '"]').avatar(escapeHTML(shareWith), 32);
+			if (oc_config.enable_avatars === true) {
+				if (shareType === OC.Share.SHARE_TYPE_USER) {
+					html.find('.avatar').avatar(escapeHTML(shareWith), 32);
+				} else {
+					//Add sharetype to generate different seed if there is a group and use with the same name
+					html.find('.avatar').imageplaceholder(escapeHTML(shareWith) + ' ' + shareType);
+				}
 			}
 			// insert cruds button into last label element
 			var lastLabel = html.find('>label:last');

core/js/tests/specs/shareSpec.js

@@ -28,6 +28,7 @@ describe('OC.Share tests', function() {
 		var autocompleteStub;
 		var oldEnableAvatars;
 		var avatarStub;
+		var placeholderStub;
 
 		beforeEach(function() {
 			$('#testArea').append($('<div id="shareContainer"></div>'));
@@ -60,6 +61,7 @@ describe('OC.Share tests', function() {
 			oldEnableAvatars = oc_config.enable_avatars;
 			oc_config.enable_avatars = false;
 			avatarStub = sinon.stub($.fn, 'avatar');
+			placeholderStub = sinon.stub($.fn, 'imageplaceholder');
 		});
 		afterEach(function() {
 			/* jshint camelcase:false */
@@ -68,6 +70,7 @@ describe('OC.Share tests', function() {
 
 			autocompleteStub.restore();
 			avatarStub.restore();
+			placeholderStub.restore();
 			oc_config.enable_avatars = oldEnableAvatars;
 			$('#dropdown').remove();
 		});
@@ -416,7 +419,12 @@ describe('OC.Share tests', function() {
 		describe('check for avatar', function() {
 			beforeEach(function() {
 				loadItemStub.returns({
-					reshare: [],
+					reshare: {
+						share_type: OC.Share.SHARE_TYPE_USER,
+						uid_owner: 'owner',
+						displayname_owner: 'Owner',
+						permissions: 31
+					},
 					shares: [{
 						id: 100,
 						item_source: 123,
@@ -431,6 +439,14 @@ describe('OC.Share tests', function() {
 						share_type: OC.Share.SHARE_TYPE_GROUP,
 						share_with: 'group',
 						share_with_displayname: 'group'
+					},{
+						id: 102,
+						item_source: 123,
+						permissions: 31,
+						share_type: OC.Share.SHARE_TYPE_REMOTE,
+						share_with: '[email protected]/baz',
+						share_with_displayname: '[email protected]/baz'
+
 					}]
 				});
 			});
@@ -452,21 +468,35 @@ describe('OC.Share tests', function() {
 					oc_config.enable_avatars = false;
 				});
 
-				it('test correct function call', function() {
-					expect(avatarStub.calledOnce).toEqual(true);
-					var args = avatarStub.getCall(0).args;
-
+				it('test correct function calls', function() {
+					expect(avatarStub.calledTwice).toEqual(true);
+					expect(placeholderStub.calledTwice).toEqual(true);
+					expect($('#shareWithList').children().length).toEqual(3);
+					expect($('.avatar').length).toEqual(4);
+				});
 
-					expect($('#shareWithList').children().length).toEqual(2);
+				it('test avatar owner', function() {
+					var args = avatarStub.getCall(0).args;
+					expect(args.length).toEqual(2);
+					expect(args[0]).toEqual('owner');
+				});
 
-					expect($('.avatar[data-user="user1"]').length).toEqual(1);
+				it('test avatar user', function() {
+					var args = avatarStub.getCall(1).args;
 					expect(args.length).toEqual(2);
 					expect(args[0]).toEqual('user1');
 				});
 
-				it('test no avatar for groups', function() {
-					expect($('#shareWithList').children().length).toEqual(2);
-					expect($('#shareWithList li:nth-child(2) .avatar').attr('id')).not.toBeDefined();
+				it('test avatar for groups', function() {
+					var args = placeholderStub.getCall(0).args;
+					expect(args.length).toEqual(1);
+					expect(args[0]).toEqual('group ' + OC.Share.SHARE_TYPE_GROUP);
+				});
+
+				it('test avatar for remotes', function() {
+					var args = placeholderStub.getCall(1).args;
+					expect(args.length).toEqual(1);
+					expect(args[0]).toEqual('[email protected]/baz ' + OC.Share.SHARE_TYPE_REMOTE);
 				});
 			});
 
@@ -484,6 +514,8 @@ describe('OC.Share tests', function() {
 
 				it('no avatar classes', function() {
 					expect($('.avatar').length).toEqual(0);
+					expect(avatarStub.callCount).toEqual(0);
+					expect(placeholderStub.callCount).toEqual(0);
 				});
 			});
 		});