diff options
author | Tim Zallmann <tzallmann@gitlab.com> | 2017-10-02 11:23:07 +0300 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2017-10-02 11:23:07 +0300 |
commit | 209d7575b63ed889036707eea0183c1ff4bfd67d (patch) | |
tree | d366283eb0e8972e9c9790a7ab7aaf0589dddf8e | |
parent | 147c46cca195f13ef10ec8fc2db160a833121914 (diff) | |
parent | feae8b2e44ee66c572efeab2575234de292ac01e (diff) |
Merge branch '34366-issue-sidebar-don-t-render-participants-in-collapsed-state' into 'master'
Resolve "Issue Sidebar : Don't render participants in collapsed state"
Closes #34366
See merge request gitlab-org/gitlab-ce!14270
-rw-r--r-- | app/assets/javascripts/issuable_context.js | 5 | ||||
-rw-r--r-- | app/assets/javascripts/right_sidebar.js | 44 | ||||
-rw-r--r-- | app/helpers/projects_helper.rb | 7 | ||||
-rw-r--r-- | app/views/shared/issuable/_participants.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/34366-issue-sidebar-don-t-render-participants-in-collapsed-state.yml | 5 | ||||
-rw-r--r-- | spec/helpers/projects_helper_spec.rb | 4 | ||||
-rw-r--r-- | spec/javascripts/issuable_context_spec.js | 34 | ||||
-rw-r--r-- | spec/javascripts/right_sidebar_spec.js | 116 | ||||
-rw-r--r-- | spec/views/shared/issuable/_participants.html.haml.rb | 26 |
9 files changed, 173 insertions, 70 deletions
diff --git a/app/assets/javascripts/issuable_context.js b/app/assets/javascripts/issuable_context.js index 70c364e51fe..1d305f1eb2f 100644 --- a/app/assets/javascripts/issuable_context.js +++ b/app/assets/javascripts/issuable_context.js @@ -67,10 +67,13 @@ const PARTICIPANTS_ROW_COUNT = 7; originalText = $(this).data("original-text"); if (currentText === originalText) { $(this).text(lessText); + + if (gl.lazyLoader) gl.lazyLoader.loadCheck(); } else { $(this).text(originalText); } - return $(".js-participants-hidden").toggle(); + + $(".js-participants-hidden").toggle(); }; return IssuableContext; diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index 0c1ec276baf..a41548bd694 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -29,30 +29,32 @@ import Cookies from 'js-cookie'; $('.dropdown').on('loading.gl.dropdown', this.sidebarDropdownLoading); $('.dropdown').on('loaded.gl.dropdown', this.sidebarDropdownLoaded); - $document.on('click', '.js-sidebar-toggle', function(e, triggered) { - var $allGutterToggleIcons, $this, $thisIcon; - e.preventDefault(); - $this = $(this); - $thisIcon = $this.find('i'); - $allGutterToggleIcons = $('.js-sidebar-toggle i'); - if ($thisIcon.hasClass('fa-angle-double-right')) { - $allGutterToggleIcons.removeClass('fa-angle-double-right').addClass('fa-angle-double-left'); - $('aside.right-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); - $('.page-with-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); - } else { - $allGutterToggleIcons.removeClass('fa-angle-double-left').addClass('fa-angle-double-right'); - $('aside.right-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); - $('.page-with-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); - - if (gl.lazyLoader) gl.lazyLoader.loadCheck(); - } - if (!triggered) { - return Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); - } - }); + $document.on('click', '.js-sidebar-toggle', this.sidebarToggleClicked); return $(document).off('click', '.js-issuable-todo').on('click', '.js-issuable-todo', this.toggleTodo); }; + Sidebar.prototype.sidebarToggleClicked = function (e, triggered) { + var $allGutterToggleIcons, $this, $thisIcon; + e.preventDefault(); + $this = $(this); + $thisIcon = $this.find('i'); + $allGutterToggleIcons = $('.js-sidebar-toggle i'); + if ($thisIcon.hasClass('fa-angle-double-right')) { + $allGutterToggleIcons.removeClass('fa-angle-double-right').addClass('fa-angle-double-left'); + $('aside.right-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); + $('.page-with-sidebar').removeClass('right-sidebar-expanded').addClass('right-sidebar-collapsed'); + } else { + $allGutterToggleIcons.removeClass('fa-angle-double-left').addClass('fa-angle-double-right'); + $('aside.right-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); + $('.page-with-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); + + if (gl.lazyLoader) gl.lazyLoader.loadCheck(); + } + if (!triggered) { + Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); + } + }; + Sidebar.prototype.toggleTodo = function(e) { var $btnText, $this, $todoLoading, ajaxType, url; $this = $(e.currentTarget); diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 21fb17e06d6..4c0cce54527 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -21,11 +21,14 @@ module ProjectsHelper classes = %W[avatar avatar-inline s#{opts[:size]}] classes << opts[:avatar_class] if opts[:avatar_class] - image_tag(avatar_icon(author, opts[:size]), width: opts[:size], class: classes, alt: '') + avatar = avatar_icon(author, opts[:size]) + src = opts[:lazy_load] ? nil : avatar + + image_tag(src, width: opts[:size], class: classes, alt: '', "data-src" => avatar) end def link_to_member(project, author, opts = {}, &block) - default_opts = { avatar: true, name: true, size: 16, author_class: 'author', title: ":name", tooltip: false } + default_opts = { avatar: true, name: true, size: 16, author_class: 'author', title: ":name", tooltip: false, lazy_load: false } opts = default_opts.merge(opts) return "(deleted)" unless author diff --git a/app/views/shared/issuable/_participants.html.haml b/app/views/shared/issuable/_participants.html.haml index 8a71819aa8e..d2b62557e03 100644 --- a/app/views/shared/issuable/_participants.html.haml +++ b/app/views/shared/issuable/_participants.html.haml @@ -11,7 +11,7 @@ .hide-collapsed.participants-list - participants.each do |participant| .participants-author.js-participants-author - = link_to_member(@project, participant, name: false, size: 24) + = link_to_member(@project, participant, name: false, size: 24, lazy_load: true) - if participants_extra > 0 .hide-collapsed.participants-more %a.js-participants-more{ href: "#", data: { original_text: "+ #{participants_size - 7} more", less_text: "- show less" } } diff --git a/changelogs/unreleased/34366-issue-sidebar-don-t-render-participants-in-collapsed-state.yml b/changelogs/unreleased/34366-issue-sidebar-don-t-render-participants-in-collapsed-state.yml new file mode 100644 index 00000000000..d34e685b5f5 --- /dev/null +++ b/changelogs/unreleased/34366-issue-sidebar-don-t-render-participants-in-collapsed-state.yml @@ -0,0 +1,5 @@ +--- +title: Load sidebar participants avatars only when visible +merge_request: 14270 +author: +type: other diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 7ded95d01af..641971485de 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -200,13 +200,13 @@ describe ProjectsHelper do end it 'returns image tag for member avatar' do - expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16"], alt: "" }) + expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16"], alt: "", "data-src" => anything }) helper.link_to_member_avatar(user) end it 'returns image tag with avatar class' do - expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16", "any-avatar-class"], alt: "" }) + expect(helper).to receive(:image_tag).with(expected, { width: 16, class: ["avatar", "avatar-inline", "s16", "any-avatar-class"], alt: "", "data-src" => anything }) helper.link_to_member_avatar(user, avatar_class: "any-avatar-class") end diff --git a/spec/javascripts/issuable_context_spec.js b/spec/javascripts/issuable_context_spec.js new file mode 100644 index 00000000000..bcb2b7b24a0 --- /dev/null +++ b/spec/javascripts/issuable_context_spec.js @@ -0,0 +1,34 @@ +/* global IssuableContext */ +import '~/issuable_context'; +import $ from 'jquery'; + +describe('IssuableContext', () => { + describe('toggleHiddenParticipants', () => { + const event = jasmine.createSpyObj('event', ['preventDefault']); + + beforeEach(() => { + spyOn($.fn, 'data').and.returnValue('data'); + spyOn($.fn, 'text').and.returnValue('data'); + }); + + afterEach(() => { + gl.lazyLoader = undefined; + }); + + it('calls loadCheck if lazyLoader is set', () => { + gl.lazyLoader = jasmine.createSpyObj('lazyLoader', ['loadCheck']); + + IssuableContext.prototype.toggleHiddenParticipants(event); + + expect(gl.lazyLoader.loadCheck).toHaveBeenCalled(); + }); + + it('does not throw if lazyLoader is not defined', () => { + gl.lazyLoader = undefined; + + const toggle = IssuableContext.prototype.toggleHiddenParticipants.bind(null, event); + + expect(toggle).not.toThrow(); + }); + }); +}); diff --git a/spec/javascripts/right_sidebar_spec.js b/spec/javascripts/right_sidebar_spec.js index f2072a6f350..5505f983d71 100644 --- a/spec/javascripts/right_sidebar_spec.js +++ b/spec/javascripts/right_sidebar_spec.js @@ -32,56 +32,86 @@ import '~/right_sidebar'; }; describe('RightSidebar', function() { - var fixtureName = 'issues/open-issue.html.raw'; - preloadFixtures(fixtureName); - loadJSONFixtures('todos/todos.json'); - - beforeEach(function() { - loadFixtures(fixtureName); - this.sidebar = new Sidebar; - $aside = $('.right-sidebar'); - $page = $('.page-with-sidebar'); - $icon = $aside.find('i'); - $toggle = $aside.find('.js-sidebar-toggle'); - return $labelsIcon = $aside.find('.sidebar-collapsed-icon'); - }); - it('should expand/collapse the sidebar when arrow is clicked', function() { - assertSidebarState('expanded'); - $toggle.click(); - assertSidebarState('collapsed'); - $toggle.click(); - assertSidebarState('expanded'); - }); - it('should float over the page and when sidebar icons clicked', function() { - $labelsIcon.click(); - return assertSidebarState('expanded'); - }); - it('should collapse when the icon arrow clicked while it is floating on page', function() { - $labelsIcon.click(); - assertSidebarState('expanded'); - $toggle.click(); - return assertSidebarState('collapsed'); + describe('fixture tests', () => { + var fixtureName = 'issues/open-issue.html.raw'; + preloadFixtures(fixtureName); + loadJSONFixtures('todos/todos.json'); + + beforeEach(function() { + loadFixtures(fixtureName); + this.sidebar = new Sidebar; + $aside = $('.right-sidebar'); + $page = $('.page-with-sidebar'); + $icon = $aside.find('i'); + $toggle = $aside.find('.js-sidebar-toggle'); + return $labelsIcon = $aside.find('.sidebar-collapsed-icon'); + }); + it('should expand/collapse the sidebar when arrow is clicked', function() { + assertSidebarState('expanded'); + $toggle.click(); + assertSidebarState('collapsed'); + $toggle.click(); + assertSidebarState('expanded'); + }); + it('should float over the page and when sidebar icons clicked', function() { + $labelsIcon.click(); + return assertSidebarState('expanded'); + }); + it('should collapse when the icon arrow clicked while it is floating on page', function() { + $labelsIcon.click(); + assertSidebarState('expanded'); + $toggle.click(); + return assertSidebarState('collapsed'); + }); + + it('should broadcast todo:toggle event when add todo clicked', function() { + var todos = getJSONFixture('todos/todos.json'); + spyOn(jQuery, 'ajax').and.callFake(function() { + var d = $.Deferred(); + var response = todos; + d.resolve(response); + return d.promise(); + }); + + var todoToggleSpy = spyOnEvent(document, 'todo:toggle'); + + $('.issuable-sidebar-header .js-issuable-todo').click(); + + expect(todoToggleSpy.calls.count()).toEqual(1); + }); + + it('should not hide collapsed icons', () => { + [].forEach.call(document.querySelectorAll('.sidebar-collapsed-icon'), (el) => { + expect(el.querySelector('.fa, svg').classList.contains('hidden')).toBeFalsy(); + }); + }); }); - it('should broadcast todo:toggle event when add todo clicked', function() { - var todos = getJSONFixture('todos/todos.json'); - spyOn(jQuery, 'ajax').and.callFake(function() { - var d = $.Deferred(); - var response = todos; - d.resolve(response); - return d.promise(); + describe('sidebarToggleClicked', () => { + const event = jasmine.createSpyObj('event', ['preventDefault']); + + beforeEach(() => { + spyOn($.fn, 'hasClass').and.returnValue(false); + }); + + afterEach(() => { + gl.lazyLoader = undefined; }); - var todoToggleSpy = spyOnEvent(document, 'todo:toggle'); + it('calls loadCheck if lazyLoader is set', () => { + gl.lazyLoader = jasmine.createSpyObj('lazyLoader', ['loadCheck']); - $('.issuable-sidebar-header .js-issuable-todo').click(); + Sidebar.prototype.sidebarToggleClicked(event); - expect(todoToggleSpy.calls.count()).toEqual(1); - }); + expect(gl.lazyLoader.loadCheck).toHaveBeenCalled(); + }); + + it('does not throw if lazyLoader is not defined', () => { + gl.lazyLoader = undefined; + + const toggle = Sidebar.prototype.sidebarToggleClicked.bind(null, event); - it('should not hide collapsed icons', () => { - [].forEach.call(document.querySelectorAll('.sidebar-collapsed-icon'), (el) => { - expect(el.querySelector('.fa, svg').classList.contains('hidden')).toBeFalsy(); + expect(toggle).not.toThrow(); }); }); }); diff --git a/spec/views/shared/issuable/_participants.html.haml.rb b/spec/views/shared/issuable/_participants.html.haml.rb new file mode 100644 index 00000000000..51059d4c0d7 --- /dev/null +++ b/spec/views/shared/issuable/_participants.html.haml.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require 'nokogiri' + +describe 'shared/issuable/_participants.html.haml' do + let(:project) { create(:project) } + let(:participants) { create_list(:user, 100) } + + before do + allow(view).to receive_messages(project: project, + participants: participants) + end + + it 'renders lazy loaded avatars' do + render 'shared/issuable/participants' + + html = Nokogiri::HTML(rendered) + + avatars = html.css('.participants-author img') + + avatars.each do |avatar| + expect(avatar[:class]).to include('lazy') + expect(avatar[:src]).to eql(LazyImageTagHelper.placeholder_image) + expect(avatar[:"data-src"]).to match('http://www.gravatar.com/avatar/') + end + end +end |