From ff71dd07a6db2b9fb60e1b756eac84ef103cabe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 20 Sep 2017 17:19:35 +0200 Subject: [PATCH 1/3] Unify appearance of avatars for undefined and unknown users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When calling the jQuery avatar plugin with a user that did not exist (that is, users for which "/avatar/{user}/{size}" return a JSON response with an empty "displayname" value) "?" on a grey background was shown. However, if the jQuery avatar plugin was called with an undefined JavaScript value then "?" was shown on a bluish background. This commit unifies both cases to use the grey background. The unit tests were also modified to ensure that the grey background is used in both cases. Signed-off-by: Daniel Calviño Sánchez --- core/js/jquery.avatar.js | 1 + core/js/tests/specs/jquery.avatarSpec.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js index 29d019baea..5813a80646 100644 --- a/core/js/jquery.avatar.js +++ b/core/js/jquery.avatar.js @@ -73,6 +73,7 @@ user = this.data('user'); } else { this.imageplaceholder('?'); + this.css('background-color', '#b9b9b9'); return; } } diff --git a/core/js/tests/specs/jquery.avatarSpec.js b/core/js/tests/specs/jquery.avatarSpec.js index dab78500d0..87767c8bc8 100644 --- a/core/js/tests/specs/jquery.avatarSpec.js +++ b/core/js/tests/specs/jquery.avatarSpec.js @@ -62,10 +62,12 @@ describe('jquery.avatar tests', function() { it('undefined user', function() { spyOn($div, 'imageplaceholder'); + spyOn($div, 'css'); $div.avatar(); expect($div.imageplaceholder).toHaveBeenCalledWith('?'); + expect($div.css).toHaveBeenCalledWith('background-color', '#b9b9b9'); }); describe('no avatar', function() { @@ -86,6 +88,7 @@ describe('jquery.avatar tests', function() { it('show placeholder for non existing user', function() { spyOn($div, 'imageplaceholder'); + spyOn($div, 'css'); $div.avatar('foo'); fakeServer.requests[0].respond( @@ -97,6 +100,7 @@ describe('jquery.avatar tests', function() { ); expect($div.imageplaceholder).toHaveBeenCalledWith('foo', '?'); + expect($div.css).toHaveBeenCalledWith('background-color', '#b9b9b9'); }); it('show no placeholder', function() { From ea10a1292a7674149581b4332fb0849cd84cf467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 20 Sep 2017 18:14:00 +0200 Subject: [PATCH 2/3] Use "?" instead of user ID as seed for the image placeholder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seed of the image placeholder is needed to generate the background color of the image, but as the background color is later overriden any seed could be used. When no text is explicitly given the seed is used as text too, so there is no need to pass the user ID and simply using "?" as seed is enough. Signed-off-by: Daniel Calviño Sánchez --- core/js/jquery.avatar.js | 2 +- core/js/tests/specs/jquery.avatarSpec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js index 5813a80646..c8ac70ac69 100644 --- a/core/js/jquery.avatar.js +++ b/core/js/jquery.avatar.js @@ -113,7 +113,7 @@ $div.imageplaceholder(user, result.data.displayname); } else { // User does not exist - $div.imageplaceholder(user, '?'); + $div.imageplaceholder('?'); $div.css('background-color', '#b9b9b9'); } } else { diff --git a/core/js/tests/specs/jquery.avatarSpec.js b/core/js/tests/specs/jquery.avatarSpec.js index 87767c8bc8..d730573669 100644 --- a/core/js/tests/specs/jquery.avatarSpec.js +++ b/core/js/tests/specs/jquery.avatarSpec.js @@ -99,7 +99,7 @@ describe('jquery.avatar tests', function() { }) ); - expect($div.imageplaceholder).toHaveBeenCalledWith('foo', '?'); + expect($div.imageplaceholder).toHaveBeenCalledWith('?'); expect($div.css).toHaveBeenCalledWith('background-color', '#b9b9b9'); }); From 2131e95c6157c5614e3412f5b2fa22395c4ea03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 20 Sep 2017 18:17:08 +0200 Subject: [PATCH 3/3] Extract setting the avatar for the unknown user to its own function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/jquery.avatar.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js index c8ac70ac69..fae100fda2 100644 --- a/core/js/jquery.avatar.js +++ b/core/js/jquery.avatar.js @@ -48,6 +48,11 @@ (function ($) { $.fn.avatar = function(user, size, ie8fix, hidedefault, callback, displayname) { + var setAvatarForUnknownUser = function(target) { + target.imageplaceholder('?'); + target.css('background-color', '#b9b9b9'); + }; + if (typeof(user) !== 'undefined') { user = String(user); } @@ -72,8 +77,7 @@ if (typeof(this.data('user')) !== 'undefined') { user = this.data('user'); } else { - this.imageplaceholder('?'); - this.css('background-color', '#b9b9b9'); + setAvatarForUnknownUser(this); return; } } @@ -113,8 +117,7 @@ $div.imageplaceholder(user, result.data.displayname); } else { // User does not exist - $div.imageplaceholder('?'); - $div.css('background-color', '#b9b9b9'); + setAvatarForUnknownUser($div); } } else { $div.hide();