Commit 2b487edc authored by Jacob Schatz's avatar Jacob Schatz

Merge branch '18334-truncate-award-emoji-users' into 'master'

Truncated long user lists in award emoji tooltips

## What does this MR do?
Truncates award emoji tooltips so that they only show 10 users maximum.

Further users are indicated by appending "and X more."

## Are there points in the code the reviewer needs to double check?
Is 10 too little, should it be raised?

My test cases rely on join() to build the expected output.
This feels a little iffy is it alright?

## Why was this MR needed?
Some issues have a large number of thumbs causing tooltips to be very large.

## What are the relevant issue numbers?
closes #18334, closes #19542

## Screenshots (if relevant)
##### Before
![Screenshot_from_2016-06-20_19-49-12](/uploads/d7a14dd87bb3da2acd7c0818de99852b/Screenshot_from_2016-06-20_19-49-12.png)

##### After
![Screenshot_from_2016-06-20_19-50-58](/uploads/f7f05c44594bfe8cec7dfd48802753a6/Screenshot_from_2016-06-20_19-50-58.png)

Truncation point modified for purposes of screenshot

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- [x] Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4780
parents 4659a64b d548f3ee
...@@ -87,6 +87,8 @@ v 8.11.0 (unreleased) ...@@ -87,6 +87,8 @@ v 8.11.0 (unreleased)
- Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le) - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le)
- Load project invited groups and members eagerly in `ProjectTeam#fetch_members` - Load project invited groups and members eagerly in `ProjectTeam#fetch_members`
- Add pipeline events hook - Add pipeline events hook
- Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison)
- Fix duplicate "me" in award emoji tooltip !5218 (jlogandavison)
- Bump gitlab_git to speedup DiffCollection iterations - Bump gitlab_git to speedup DiffCollection iterations
- Rewrite description of a blocked user in admin settings. (Elias Werberich) - Rewrite description of a blocked user in admin settings. (Elias Werberich)
- Make branches sortable without push permission !5462 (winniehell) - Make branches sortable without push permission !5462 (winniehell)
......
(function() { (function() {
this.AwardsHandler = (function() { this.AwardsHandler = (function() {
const FROM_SENTENCE_REGEX = /(?:, and | and |, )/; //For separating lists produced by ruby's Array#toSentence
function AwardsHandler() { function AwardsHandler() {
this.aliases = gl.emojiAliases(); this.aliases = gl.emojiAliases();
$(document).off('click', '.js-add-award').on('click', '.js-add-award', (function(_this) { $(document).off('click', '.js-add-award').on('click', '.js-add-award', (function(_this) {
...@@ -130,7 +131,7 @@ ...@@ -130,7 +131,7 @@
counter = $emojiButton.find('.js-counter'); counter = $emojiButton.find('.js-counter');
counter.text(parseInt(counter.text()) + 1); counter.text(parseInt(counter.text()) + 1);
$emojiButton.addClass('active'); $emojiButton.addClass('active');
this.addMeToUserList(votesBlock, emoji); this.addYouToUserList(votesBlock, emoji);
return this.animateEmoji($emojiButton); return this.animateEmoji($emojiButton);
} }
} else { } else {
...@@ -176,11 +177,11 @@ ...@@ -176,11 +177,11 @@
counterNumber = parseInt(counter.text(), 10); counterNumber = parseInt(counter.text(), 10);
if (counterNumber > 1) { if (counterNumber > 1) {
counter.text(counterNumber - 1); counter.text(counterNumber - 1);
this.removeMeFromUserList($emojiButton, emoji); this.removeYouFromUserList($emojiButton, emoji);
} else if (emoji === 'thumbsup' || emoji === 'thumbsdown') { } else if (emoji === 'thumbsup' || emoji === 'thumbsdown') {
$emojiButton.tooltip('destroy'); $emojiButton.tooltip('destroy');
counter.text('0'); counter.text('0');
this.removeMeFromUserList($emojiButton, emoji); this.removeYouFromUserList($emojiButton, emoji);
if ($emojiButton.parents('.note').length) { if ($emojiButton.parents('.note').length) {
this.removeEmoji($emojiButton); this.removeEmoji($emojiButton);
} }
...@@ -204,43 +205,48 @@ ...@@ -204,43 +205,48 @@
return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || ''; return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || '';
}; };
AwardsHandler.prototype.removeMeFromUserList = function($emojiButton, emoji) { AwardsHandler.prototype.toSentence = function(list) {
if(list.length <= 2){
return list.join(' and ');
}
else{
return list.slice(0, -1).join(', ') + ', and ' + list[list.length - 1];
}
};
AwardsHandler.prototype.removeYouFromUserList = function($emojiButton, emoji) {
var authors, awardBlock, newAuthors, originalTitle; var authors, awardBlock, newAuthors, originalTitle;
awardBlock = $emojiButton; awardBlock = $emojiButton;
originalTitle = this.getAwardTooltip(awardBlock); originalTitle = this.getAwardTooltip(awardBlock);
authors = originalTitle.split(', '); authors = originalTitle.split(FROM_SENTENCE_REGEX);
authors.splice(authors.indexOf('me'), 1); authors.splice(authors.indexOf('You'), 1);
newAuthors = authors.join(', '); return awardBlock
awardBlock.closest('.js-emoji-btn').removeData('original-title').attr('data-original-title', newAuthors); .closest('.js-emoji-btn')
return this.resetTooltip(awardBlock); .removeData('title')
}; .removeAttr('data-title')
.removeAttr('data-original-title')
AwardsHandler.prototype.addMeToUserList = function(votesBlock, emoji) { .attr('title', this.toSentence(authors))
.tooltip('fixTitle');
};
AwardsHandler.prototype.addYouToUserList = function(votesBlock, emoji) {
var awardBlock, origTitle, users; var awardBlock, origTitle, users;
awardBlock = this.findEmojiIcon(votesBlock, emoji).parent(); awardBlock = this.findEmojiIcon(votesBlock, emoji).parent();
origTitle = this.getAwardTooltip(awardBlock); origTitle = this.getAwardTooltip(awardBlock);
users = []; users = [];
if (origTitle) { if (origTitle) {
users = origTitle.trim().split(', '); users = origTitle.trim().split(FROM_SENTENCE_REGEX);
} }
users.push('me'); users.unshift('You');
awardBlock.attr('title', users.join(', ')); return awardBlock
return this.resetTooltip(awardBlock); .attr('title', this.toSentence(users))
}; .tooltip('fixTitle');
AwardsHandler.prototype.resetTooltip = function(award) {
var cb;
award.tooltip('destroy');
cb = function() {
return award.tooltip();
};
return setTimeout(cb, 200);
}; };
AwardsHandler.prototype.createEmoji_ = function(votesBlock, emoji) { AwardsHandler.prototype.createEmoji_ = function(votesBlock, emoji) {
var $emojiButton, buttonHtml, emojiCssClass; var $emojiButton, buttonHtml, emojiCssClass;
emojiCssClass = this.resolveNameToCssClass(emoji); emojiCssClass = this.resolveNameToCssClass(emoji);
buttonHtml = "<button class='btn award-control js-emoji-btn has-tooltip active' title='me' data-placement='bottom'> <div class='icon emoji-icon " + emojiCssClass + "' data-emoji='" + emoji + "'></div> <span class='award-control-text js-counter'>1</span> </button>"; buttonHtml = "<button class='btn award-control js-emoji-btn has-tooltip active' title='You' data-placement='bottom'> <div class='icon emoji-icon " + emojiCssClass + "' data-emoji='" + emoji + "'></div> <span class='award-control-text js-counter'>1</span> </button>";
$emojiButton = $(buttonHtml); $emojiButton = $(buttonHtml);
$emojiButton.insertBefore(votesBlock.find('.js-award-holder')).find('.emoji-icon').data('emoji', emoji); $emojiButton.insertBefore(votesBlock.find('.js-award-holder')).find('.emoji-icon').data('emoji', emoji);
this.animateEmoji($emojiButton); this.animateEmoji($emojiButton);
......
...@@ -114,9 +114,17 @@ module IssuesHelper ...@@ -114,9 +114,17 @@ module IssuesHelper
end end
def award_user_list(awards, current_user) def award_user_list(awards, current_user)
awards.map do |award| names = awards.map do |award|
award.user == current_user ? 'me' : award.user.name award.user == current_user ? 'You' : award.user.name
end.join(', ') end
# Take first 9 OR current user + first 9
current_user_name = names.delete('You')
names = names.first(9).insert(0, current_user_name).compact
names << "#{awards.size - names.size} more." if awards.size > names.size
names.to_sentence
end end
def award_active_class(awards, current_user) def award_active_class(awards, current_user)
......
...@@ -48,7 +48,7 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps ...@@ -48,7 +48,7 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps
page.within '.awards' do page.within '.awards' do
expect(page).to have_selector '.js-emoji-btn' expect(page).to have_selector '.js-emoji-btn'
expect(page.find('.js-emoji-btn.active .js-counter')).to have_content '1' expect(page.find('.js-emoji-btn.active .js-counter')).to have_content '1'
expect(page).to have_css(".js-emoji-btn.active[data-original-title='me']") expect(page).to have_css(".js-emoji-btn.active[data-original-title='You']")
end end
end end
......
...@@ -62,6 +62,32 @@ describe IssuesHelper do ...@@ -62,6 +62,32 @@ describe IssuesHelper do
it { is_expected.to eq("!1, !2, or !3") } it { is_expected.to eq("!1, !2, or !3") }
end end
describe '#award_user_list' do
let!(:awards) { build_list(:award_emoji, 15) }
it "returns a comma seperated list of 1-9 users" do
expect(award_user_list(awards.first(9), nil)).to eq(awards.first(9).map { |a| a.user.name }.to_sentence)
end
it "displays the current user's name as 'You'" do
expect(award_user_list(awards.first(1), awards[0].user)).to eq('You')
end
it "truncates lists of larger than 9 users" do
expect(award_user_list(awards, nil)).to eq(awards.first(9).map { |a| a.user.name }.join(', ') + ", and 6 more.")
end
it "displays the current user in front of 0-9 other users" do
expect(award_user_list(awards, awards[0].user)).
to eq("You, " + awards[1..9].map { |a| a.user.name }.join(', ') + ", and 5 more.")
end
it "displays the current user in front regardless of position in the list" do
expect(award_user_list(awards, awards[12].user)).
to eq("You, " + awards[0..8].map { |a| a.user.name }.join(', ') + ", and 5 more.")
end
end
describe '#award_active_class' do describe '#award_active_class' do
let!(:upvote) { create(:award_emoji) } let!(:upvote) { create(:award_emoji) }
......
...@@ -143,6 +143,52 @@ ...@@ -143,6 +143,52 @@
return expect($votesBlock.find('[data-emoji=fire]').length).toBe(0); return expect($votesBlock.find('[data-emoji=fire]').length).toBe(0);
}); });
}); });
describe('::addYouToUserList', function() {
it('should prepend "You" to the award tooltip', function() {
var $thumbsUpEmoji, $votesBlock, awardUrl;
awardUrl = awardsHandler.getAwardUrl();
$votesBlock = $('.js-awards-block').eq(0);
$thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
$thumbsUpEmoji.attr('data-title', 'sam, jerry, max, and andy');
awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
$thumbsUpEmoji.tooltip();
return expect($thumbsUpEmoji.data("original-title")).toBe('You, sam, jerry, max, and andy');
});
return it('handles the special case where "You" is not cleanly comma seperated', function() {
var $thumbsUpEmoji, $votesBlock, awardUrl;
awardUrl = awardsHandler.getAwardUrl();
$votesBlock = $('.js-awards-block').eq(0);
$thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
$thumbsUpEmoji.attr('data-title', 'sam');
awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
$thumbsUpEmoji.tooltip();
return expect($thumbsUpEmoji.data("original-title")).toBe('You and sam');
});
});
describe('::removeYouToUserList', function() {
it('removes "You" from the front of the tooltip', function() {
var $thumbsUpEmoji, $votesBlock, awardUrl;
awardUrl = awardsHandler.getAwardUrl();
$votesBlock = $('.js-awards-block').eq(0);
$thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
$thumbsUpEmoji.attr('data-title', 'You, sam, jerry, max, and andy');
$thumbsUpEmoji.addClass('active');
awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
$thumbsUpEmoji.tooltip();
return expect($thumbsUpEmoji.data("original-title")).toBe('sam, jerry, max, and andy');
});
return it('handles the special case where "You" is not cleanly comma seperated', function() {
var $thumbsUpEmoji, $votesBlock, awardUrl;
awardUrl = awardsHandler.getAwardUrl();
$votesBlock = $('.js-awards-block').eq(0);
$thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
$thumbsUpEmoji.attr('data-title', 'You and sam');
$thumbsUpEmoji.addClass('active');
awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
$thumbsUpEmoji.tooltip();
return expect($thumbsUpEmoji.data("original-title")).toBe('sam');
});
});
describe('search', function() { describe('search', function() {
return it('should filter the emoji', function() { return it('should filter the emoji', function() {
$('.js-add-award').eq(0).click(); $('.js-add-award').eq(0).click();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment