diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index c753972d171..b06bad78305 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -1,5 +1,6 @@ (function() { this.AwardsHandler = (function() { + const FROM_SENTENCE_REGEX = /(?:, and | and |, )/; //For separating lists produced by ruby's Array#toSentence function AwardsHandler() { this.aliases = gl.emojiAliases(); $(document).off('click', '.js-add-award').on('click', '.js-add-award', (function(_this) { @@ -204,14 +205,22 @@ return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || ''; }; + 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.removeMeFromUserList = function($emojiButton, emoji) { var authors, awardBlock, newAuthors, originalTitle; awardBlock = $emojiButton; originalTitle = this.getAwardTooltip(awardBlock); - authors = originalTitle.split(', '); + authors = originalTitle.split(FROM_SENTENCE_REGEX); authors.splice(authors.indexOf('me'), 1); - newAuthors = authors.join(', '); - awardBlock.closest('.js-emoji-btn').removeData('original-title').attr('data-original-title', newAuthors); + awardBlock.closest('.js-emoji-btn').removeData('original-title').attr('data-original-title', this.toSentence(authors)); return this.resetTooltip(awardBlock); }; @@ -221,10 +230,10 @@ origTitle = this.getAwardTooltip(awardBlock); users = []; if (origTitle) { - users = origTitle.trim().split(', '); + users = origTitle.trim().split(FROM_SENTENCE_REGEX); } users.unshift('me'); - awardBlock.attr('title', users.join(', ')); + awardBlock.attr('title', this.toSentence(users)); return this.resetTooltip(awardBlock); }; diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index e8081d452c4..e5be8d2404a 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -122,9 +122,9 @@ module IssuesHelper current_user_name = names.delete('me') names = names.first(9).insert(0, current_user_name).compact - names << "and #{awards.size - names.size} more." if awards.size > names.size + names << "#{awards.size - names.size} more." if awards.size > names.size - names.join(', ') + names.to_sentence end def award_active_class(awards, current_user) diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index c4281f8f591..e0e3554819c 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -66,7 +66,7 @@ describe IssuesHelper 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 }.join(', ')) + 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 'me'" do diff --git a/spec/javascripts/awards_handler_spec.js b/spec/javascripts/awards_handler_spec.js index 70191026d0e..7d28a61558a 100644 --- a/spec/javascripts/awards_handler_spec.js +++ b/spec/javascripts/awards_handler_spec.js @@ -144,28 +144,49 @@ }); }); describe('::addMeToUserList', function() { - return it('should prepend "me" to the award tooltip', function() { + it('should prepend "me" 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, andy'); + $thumbsUpEmoji.attr('data-title', 'sam, jerry, max, and andy'); awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false); $thumbsUpEmoji.tooltip(); - return expect($thumbsUpEmoji.data("original-title")).toBe('me, sam, jerry, max, andy'); + return expect($thumbsUpEmoji.data("original-title")).toBe('me, sam, jerry, max, and andy'); + }); + return it('handles the special case where "me" 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('me and sam'); }); }); describe('::removeMeToUserList', function() { - return it('removes "me" from the front of the tooltip', function() { + it('removes "me" 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', 'me, sam, jerry, max, andy'); + $thumbsUpEmoji.attr('data-title', 'me, 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, andy'); + return expect($thumbsUpEmoji.data("original-title")).toBe('sam, jerry, max, and andy'); + }); + return it('handles the special case where "me" 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', 'me and sam'); + $thumbsUpEmoji.addClass('active'); + awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false); + $thumbsUpEmoji.tooltip(); + return expect($thumbsUpEmoji.data("original-title")).toBe('sam'); }); }); describe('search', function() {