From 1a21fa26f6f4ef70157c58329687976fc3f555f7 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Nov 2016 11:11:47 +0000 Subject: Improved ref switcher dropdown performance Closes #18202 --- app/assets/javascripts/project.js | 21 ++++++++++++++++----- app/controllers/projects_controller.rb | 7 +++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 016d999d77e..78ab69206af 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -63,7 +63,8 @@ return $.ajax({ url: $dropdown.data('refs-url'), data: { - ref: $dropdown.data('ref') + ref: $dropdown.data('ref'), + search: term }, dataType: "json" }).done(function(refs) { @@ -72,16 +73,26 @@ }, selectable: true, filterable: true, + filterRemote: true, filterByText: true, fieldName: $dropdown.data('field-name'), renderRow: function(ref) { - var link; + var li = document.createElement('li'); + if (ref.header != null) { - return $('
  • ').addClass('dropdown-header').text(ref.header); + li.className = 'dropdown-header'; + li.textContent = ref.header; } else { - link = $('').attr('href', '#').addClass(ref === selected ? 'is-active' : '').text(ref).attr('data-ref', ref); - return $('
  • ').append(link); + var link = document.createElement('a'); + link.href = '#'; + link.className = ref.name === selected ? 'is-active' : ''; + link.textContent = ref.name; + link.dataset.ref = ref.name; + + li.appendChild(link); } + + return li; }, id: function(obj, $el) { return $el.attr('data-ref'); diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index a8a18b4fa16..fe0670aa246 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -267,12 +267,15 @@ class ProjectsController < Projects::ApplicationController end def refs + branches = BranchesFinder.new(@repository, params).execute + options = { - 'Branches' => @repository.branch_names, + 'Branches' => Kaminari.paginate_array(branches).page(params[:page]).per(100), } unless @repository.tag_count.zero? - options['Tags'] = VersionSorter.rsort(@repository.tag_names) + tags = TagsFinder.new(@repository, params).execute + options['Tags'] = Kaminari.paginate_array(tags).page(params[:page]).per(100) end # If reference is commit id - we should add it to branch/tag selectbox -- cgit v1.2.3 From af02f6ae9d500b0174cae106891b626d1dcae351 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Nov 2016 11:31:00 +0000 Subject: Use cloneNode instead of createElement --- app/assets/javascripts/project.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 78ab69206af..7ac070a9c37 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -54,6 +54,11 @@ }; Project.prototype.initRefSwitcher = function() { + var refListItem = document.createElement('li'), + refLink = document.createElement('a'); + + refLink.href = '#'; + return $('.js-project-refs-dropdown').each(function() { var $dropdown, selected; $dropdown = $(this); @@ -77,21 +82,24 @@ filterByText: true, fieldName: $dropdown.data('field-name'), renderRow: function(ref) { - var li = document.createElement('li'); + var li = refListItem.cloneNode(false); if (ref.header != null) { li.className = 'dropdown-header'; li.textContent = ref.header; } else { - var link = document.createElement('a'); - link.href = '#'; - link.className = ref.name === selected ? 'is-active' : ''; + var link = refLink.cloneNode(false); + + if (ref.name === selected) { + link.className = 'is-active'; + } + link.textContent = ref.name; link.dataset.ref = ref.name; li.appendChild(link); } - + return li; }, id: function(obj, $el) { -- cgit v1.2.3 From ba2089e01711eb3f97770235b86ae9e59862ee8b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 2 Nov 2016 12:15:25 +0000 Subject: Uses take rather than Kaminari --- app/controllers/projects_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index fe0670aa246..6affadfa0a6 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -270,12 +270,12 @@ class ProjectsController < Projects::ApplicationController branches = BranchesFinder.new(@repository, params).execute options = { - 'Branches' => Kaminari.paginate_array(branches).page(params[:page]).per(100), + 'Branches' => branches.take(100), } unless @repository.tag_count.zero? tags = TagsFinder.new(@repository, params).execute - options['Tags'] = Kaminari.paginate_array(tags).page(params[:page]).per(100) + options['Tags'] = tags.take(100) end # If reference is commit id - we should add it to branch/tag selectbox -- cgit v1.2.3 From f9750b4912c6f0e4c7b0b7d213f95223d5d1a593 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Nov 2016 12:16:15 +0000 Subject: Changed how the data is returned - we only care about the branch/tag name --- app/assets/javascripts/project.js | 4 ++-- app/controllers/projects_controller.rb | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 7ac070a9c37..e7db0620848 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -94,8 +94,8 @@ link.className = 'is-active'; } - link.textContent = ref.name; - link.dataset.ref = ref.name; + link.textContent = ref; + link.dataset.ref = ref; li.appendChild(link); } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 6affadfa0a6..d7bc31b0718 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -267,14 +267,15 @@ class ProjectsController < Projects::ApplicationController end def refs - branches = BranchesFinder.new(@repository, params).execute + branches = BranchesFinder.new(@repository, params).execute.map(&:name) options = { 'Branches' => branches.take(100), } unless @repository.tag_count.zero? - tags = TagsFinder.new(@repository, params).execute + tags = TagsFinder.new(@repository, params).execute.map(&:name) + options['Tags'] = tags.take(100) end -- cgit v1.2.3 From 08dd831f02daeeefedf65302a9a9575733943357 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 3 Nov 2016 12:25:08 +0000 Subject: Fixed the active branch selected indicator --- app/assets/javascripts/project.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index e7db0620848..04e18098544 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -90,7 +90,7 @@ } else { var link = refLink.cloneNode(false); - if (ref.name === selected) { + if (ref === selected) { link.className = 'is-active'; } -- cgit v1.2.3 From e95d43dfc5303dcb9e86359049b5f798e8351431 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 7 Nov 2016 16:58:50 +0000 Subject: Fixed up tests --- .../javascripts/protected_branches/protected_branch_dropdown.js.es6 | 2 +- spec/features/projects/ref_switcher_spec.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 index e3f226e9a2a..9b551a1ed8c 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 +++ b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 @@ -48,7 +48,7 @@ class ProtectedBranchDropdown { onClickCreateWildcard() { // Refresh the dropdown's data, which ends up calling `getProtectedBranches` this.$dropdown.data('glDropdown').remote.execute(); - this.$dropdown.data('glDropdown').selectRowAtIndex(0); + this.$dropdown.data('glDropdown').selectRowAtIndex(gon.open_branches.length); } getProtectedBranches(term, callback) { diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb index 472491188c9..38fe2d92885 100644 --- a/spec/features/projects/ref_switcher_spec.rb +++ b/spec/features/projects/ref_switcher_spec.rb @@ -17,14 +17,15 @@ feature 'Ref switcher', feature: true, js: true do page.within '.project-refs-form' do input = find('input[type="search"]') - input.set 'expand' + input.set 'binary' + wait_for_ajax input.native.send_keys :down input.native.send_keys :down input.native.send_keys :enter end - expect(page).to have_title 'expand-collapse-files' + expect(page).to have_title 'binary-encoding' end it "user selects ref with special characters" do -- cgit v1.2.3 From 50d58c14cd2cd1b8fb1bb9e4a7a1091b5af90c04 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 21 Nov 2016 17:45:27 +0000 Subject: Fixed protected branches dropdown --- .../javascripts/protected_branches/protected_branch_dropdown.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 index 9b551a1ed8c..1ab72ee49e4 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 +++ b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 @@ -48,7 +48,7 @@ class ProtectedBranchDropdown { onClickCreateWildcard() { // Refresh the dropdown's data, which ends up calling `getProtectedBranches` this.$dropdown.data('glDropdown').remote.execute(); - this.$dropdown.data('glDropdown').selectRowAtIndex(gon.open_branches.length); + this.$dropdown.data('glDropdown').selectRowAtIndex(); } getProtectedBranches(term, callback) { -- cgit v1.2.3