From dfadbe5e45c54a0e76cb712ba8851c72bc83851f Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Tue, 29 Mar 2016 10:52:25 -0400 Subject: [PATCH 01/23] Initial mutli label filter --- app/assets/javascripts/issues.js.coffee | 2 ++ app/assets/javascripts/labels_select.js.coffee | 11 ++++++++++- app/views/shared/issuable/_label_dropdown.html.haml | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 0d9f2094c2a..084ae6e7efd 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -52,7 +52,9 @@ filterResults: (form) => $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') formAction = form.attr('action') + console.log form.find("input[type='hidden'][name='label_names[]']") formData = form.serialize() + console.log 'formData', formData issuesUrl = formAction issuesUrl += ("#{if formAction.indexOf("?") < 0 then '?' else '&'}") issuesUrl += formData diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index bc80980acb7..f864f4fd468 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -234,11 +234,20 @@ class @LabelsSelect label.id hidden: -> + page = $('body').data 'page' + isIssueIndex = page is 'projects:issues:index' + isMRIndex = page is page is 'projects:merge_requests:index' + $selectbox.hide() # display:block overrides the hide-collapse rule $value.removeAttr('style') if $dropdown.hasClass 'js-multiselect' - saveLabelData() + if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) + Issues.filterResults $dropdown.closest('form') + else if $dropdown.hasClass('js-filter-submit') + $dropdown.closest('form').submit() + else + saveLabelData() multiSelect: $dropdown.hasClass 'js-multiselect' clicked: (label) -> diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index f722e61eeac..f57d837c45b 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -2,6 +2,7 @@ = hidden_field_tag(:label_name, params[:label_name]) .dropdown %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} + %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %span.dropdown-toggle-text = h(params[:label_name].presence || "Label") = icon('chevron-down') From 19b9df2d4fe73bb30de1711a15664eedb2e46afa Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Thu, 7 Apr 2016 12:47:32 -0400 Subject: [PATCH 02/23] storing multiple values for comma seperation --- app/assets/javascripts/issues.js.coffee | 6 ++---- app/assets/javascripts/labels_select.js.coffee | 16 +++++++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 084ae6e7efd..dca5bc55eb2 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -49,16 +49,14 @@ Issues.filterResults $("#issue_search_form") , 500) - filterResults: (form) => + filterResults: (form, inputs) => + console.log('form', form) $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') formAction = form.attr('action') - console.log form.find("input[type='hidden'][name='label_names[]']") formData = form.serialize() - console.log 'formData', formData issuesUrl = formAction issuesUrl += ("#{if formAction.indexOf("?") < 0 then '?' else '&'}") issuesUrl += formData - $.ajax type: "GET" url: formAction diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index f864f4fd468..9e66f8ae961 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -243,7 +243,10 @@ class @LabelsSelect $value.removeAttr('style') if $dropdown.hasClass 'js-multiselect' if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) - Issues.filterResults $dropdown.closest('form') + selectedLabels = $dropdown + .closest('form') + .find("input[type='hidden'][name='#{$dropdown.data('field-name')}']") + Issues.filterResults $dropdown.closest('form'), selectedLabels else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else @@ -254,15 +257,18 @@ class @LabelsSelect page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' isMRIndex = page is page is 'projects:merge_requests:index' - + console.log 'clicked' if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) - selectedLabel = label.title - - Issues.filterResults $dropdown.closest('form') + if not $dropdown.hasClass 'js-multiselect' + selectedLabel = label.title + Issues.filterResults $dropdown.closest('form') else if $dropdown.hasClass 'js-filter-submit' + console.log 'clicked else if' $dropdown.closest('form').submit() else + console.log 'clicked else' if $dropdown.hasClass 'js-multiselect' + console.log 'clicked else --> if' return else saveLabelData() From e684480eebe803c21545b3a8ea5a972c54ba7ea4 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Thu, 7 Apr 2016 14:57:21 -0400 Subject: [PATCH 03/23] Proper selecting multiple labels. --- app/assets/javascripts/issues.js.coffee | 44 +++++++++++++++++-- .../javascripts/labels_select.js.coffee | 13 +++--- app/helpers/issuables_helper.rb | 13 ++++++ .../shared/issuable/_label_dropdown.html.haml | 2 +- 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index dca5bc55eb2..40a89d8c2f4 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -49,11 +49,49 @@ Issues.filterResults $("#issue_search_form") , 500) - filterResults: (form, inputs) => - console.log('form', form) + filterResults: (form) => + # Assume for now there is only 1 multi select field + # Find the hidden inputs with square brackets + $multiInputs = form.find('input[name$="[]"]') + if $multiInputs.length + # get the name of one of them + multiInputName = $multiInputs + .first() + .attr('name') + + # get the singular name by + # removing the square brackets from the name + singularName = multiInputName.replace('[]','') + # clone the form so we can mess around with it. + $clonedForm = form.clone() + + # get those inputs from the cloned form + $inputs = $clonedForm + .find("input[name='#{multiInputName}']") + + # make a comma seperated list of labels + commaSeperated = $inputs + .map( -> $(this).val()) + .get() + .join(',') + # append on a hidden input with the comma + # seperated values in it + $clonedForm.append( + $('') + .attr('type','hidden') + .attr('name', singularName) + .val(commaSeperated) + ) + # remove the multi inputs from the + # cloned form so they don't get serialized + $inputs.remove() + # serialize the cloned form + formData = $clonedForm.serialize() + else + formData = form.serialize() + $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') formAction = form.attr('action') - formData = form.serialize() issuesUrl = formAction issuesUrl += ("#{if formAction.indexOf("?") < 0 then '?' else '&'}") issuesUrl += formData diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 9e66f8ae961..9cebc26e668 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -6,7 +6,7 @@ class @LabelsSelect labelUrl = $dropdown.data('labels') issueUpdateURL = $dropdown.data('issueUpdate') selectedLabel = $dropdown.data('selected') - if selectedLabel? + if selectedLabel? and not $dropdown.hasClass 'js-multiselect' selectedLabel = selectedLabel.split(',') newLabelField = $('#new_label_name') newColorField = $('#new_label_color') @@ -246,7 +246,12 @@ class @LabelsSelect selectedLabels = $dropdown .closest('form') .find("input[type='hidden'][name='#{$dropdown.data('field-name')}']") - Issues.filterResults $dropdown.closest('form'), selectedLabels + Issues.filterResults( + $dropdown.closest('form'), + selectedLabels, + $dropdown.data('singularFieldName'), + $dropdown.data('fieldName') + ) else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else @@ -257,18 +262,14 @@ class @LabelsSelect page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' isMRIndex = page is page is 'projects:merge_requests:index' - console.log 'clicked' if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) if not $dropdown.hasClass 'js-multiselect' selectedLabel = label.title Issues.filterResults $dropdown.closest('form') else if $dropdown.hasClass 'js-filter-submit' - console.log 'clicked else if' $dropdown.closest('form').submit() else - console.log 'clicked else' if $dropdown.hasClass 'js-multiselect' - console.log 'clicked else --> if' return else saveLabelData() diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b14b8218d02..d5af0116cf8 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -16,6 +16,19 @@ module IssuablesHelper base_issuable_scope(issuable).where('iid > ?', issuable.iid).last end + def multi_label_name(current_labels, default_label) + if current_labels.presence + if current_labels.include? ',' + labels = current_labels.split(',') + "#{labels[0]} +#{labels.count - 1} more" + else + current_labels + end + else + default_label + end + end + def issuable_json_path(issuable) project = issuable.project diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index f57d837c45b..4ded2d98f37 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -4,7 +4,7 @@ %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %span.dropdown-toggle-text - = h(params[:label_name].presence || "Label") + = h(multi_label_name(params[:label_name], "Label")) = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable .dropdown-page-one From 1617d1e0267f389e040772bfed0dd29e34b25c06 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Fri, 8 Apr 2016 14:23:51 -0400 Subject: [PATCH 04/23] Move functionality to label[] --- app/assets/javascripts/issues.js.coffee | 41 ++----------------------- app/helpers/issuables_helper.rb | 20 ++++++++---- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 40a89d8c2f4..fc9f6301bcc 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -50,45 +50,8 @@ , 500) filterResults: (form) => - # Assume for now there is only 1 multi select field - # Find the hidden inputs with square brackets - $multiInputs = form.find('input[name$="[]"]') - if $multiInputs.length - # get the name of one of them - multiInputName = $multiInputs - .first() - .attr('name') - - # get the singular name by - # removing the square brackets from the name - singularName = multiInputName.replace('[]','') - # clone the form so we can mess around with it. - $clonedForm = form.clone() - - # get those inputs from the cloned form - $inputs = $clonedForm - .find("input[name='#{multiInputName}']") - - # make a comma seperated list of labels - commaSeperated = $inputs - .map( -> $(this).val()) - .get() - .join(',') - # append on a hidden input with the comma - # seperated values in it - $clonedForm.append( - $('') - .attr('type','hidden') - .attr('name', singularName) - .val(commaSeperated) - ) - # remove the multi inputs from the - # cloned form so they don't get serialized - $inputs.remove() - # serialize the cloned form - formData = $clonedForm.serialize() - else - formData = form.serialize() + + formData = form.serialize() $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') formAction = form.attr('action') diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index d5af0116cf8..14e624cb7cf 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -17,15 +17,23 @@ module IssuablesHelper end def multi_label_name(current_labels, default_label) - if current_labels.presence - if current_labels.include? ',' - labels = current_labels.split(',') - "#{labels[0]} +#{labels.count - 1} more" + # current_labels may be a string from before + if current_labels.respond_to?('any?') + if current_labels.any? + if current_labels.count > 1 + "#{current_labels[0]} +#{current_labels.count - 1} more" + else + current_labels[0] + end + else + default_label + end + else + if current_labels.nil? + default_label else current_labels end - else - default_label end end From 42e0625dfb2a791affd592df1f879083702e6f86 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Sat, 9 Apr 2016 00:09:09 -0400 Subject: [PATCH 05/23] Filter by multiple labels with little animation. --- app/assets/javascripts/issues.js.coffee | 36 ++++++++++++++++++- .../javascripts/labels_select.js.coffee | 9 +++-- app/assets/javascripts/lib/animate.js.coffee | 32 ++++++++++++++--- app/controllers/projects/issues_controller.rb | 5 +-- app/views/shared/_label_row.html.haml | 8 ++--- app/views/shared/issuable/_filter.html.haml | 6 ++-- .../shared/issuable/_label_dropdown.html.haml | 4 ++- 7 files changed, 79 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index fc9f6301bcc..320b92d2a60 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -1,5 +1,6 @@ @Issues = init: -> + Issues.initTemplates() Issues.initSearch() Issues.initChecks() @@ -15,6 +16,15 @@ else $(this).html totalIssues - 1 + initTemplates: -> + Issue.labelRow = _.template( + '<% _.each(labels, function(label){ %> + + <%= label.title %> + + <% }); %>' + ) + reload: -> Issues.initChecks() $('#filter_issue_search').val($('#issue_search').val()) @@ -50,7 +60,6 @@ , 500) filterResults: (form) => - formData = form.serialize() $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') @@ -70,6 +79,31 @@ history.replaceState {page: issuesUrl}, document.title, issuesUrl Issues.reload() Issues.updateStateFilters() + $filteredLabels = $('.filtered-labels') + $filteredLabelsSpans = $filteredLabels.find('span') + gl.animate.animateEach( + $filteredLabelsSpans, + 'fadeOutDown', 20, { + cssStart: { + opacity: 1 + }, + cssEnd: { + opacity: 0 + } + }).then( -> + $filteredLabels.html(Issue.labelRow(data)) + $spans = $filteredLabels.find('span') + $spans.css('opacity',0) + return gl.animate.animateEach($spans, 'fadeInUp', 20, { + cssStart: { + opacity: 0 + }, + cssEnd: { + opacity: 1 + } + }) + ) + dataType: "json" checkChanged: -> diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 9cebc26e668..97a813577ed 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -16,6 +16,7 @@ class @LabelsSelect abilityName = $dropdown.data('ability-name') $selectbox = $dropdown.closest('.selectbox') $block = $selectbox.closest('.block') + $form = $dropdown.closest('form') $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon span') $value = $block.find('.value') $loading = $block.find('.block-loading').fadeOut() @@ -171,7 +172,7 @@ class @LabelsSelect .find('a') .each((i) -> setTimeout(=> - glAnimate($(@), 'pulse') + gl.animate.animate($(@), 'pulse') ,200 * i ) ) @@ -201,9 +202,9 @@ class @LabelsSelect renderRow: (label) -> selectedClass = '' - if $selectbox.find("input[type='hidden']\ + if $form.find("input[type='hidden']\ [name='#{$dropdown.data('field-name')}']\ - [value='#{label.id}']").length + [value='#{this.id(label)}']").length selectedClass = 'is-active' color = if label.color? then "" else "" @@ -248,8 +249,6 @@ class @LabelsSelect .find("input[type='hidden'][name='#{$dropdown.data('field-name')}']") Issues.filterResults( $dropdown.closest('form'), - selectedLabels, - $dropdown.data('singularFieldName'), $dropdown.data('fieldName') ) else if $dropdown.hasClass('js-filter-submit') diff --git a/app/assets/javascripts/lib/animate.js.coffee b/app/assets/javascripts/lib/animate.js.coffee index 8f892b5a2b9..64aef4c6d43 100644 --- a/app/assets/javascripts/lib/animate.js.coffee +++ b/app/assets/javascripts/lib/animate.js.coffee @@ -1,13 +1,37 @@ ((w) -> + if not w.gl? then w.gl = {} + if not gl.animate? then gl.animate = {} - w.glAnimate = ($el, animation, done) -> + gl.animate.animate = ($el, animation, options, done) -> + if options?.cssStart? + $el.css(options.cssStart) $el - .removeClass() + .removeClass(animation + ' animated') .addClass(animation + ' animated') .one 'webkitAnimationEnd mozAnimationEnd MSAnimationEnd oanimationend animationend', -> - $(this).removeClass() + $(this).removeClass(animation + ' animated') + if done? + done() + if options?.cssEnd? + $el.css(options.cssEnd) return return - return + gl.animate.animateEach = ($els, animation, time, options, done) -> + dfd = $.Deferred() + $els.each((i) -> + setTimeout(=> + $this = $(@) + gl.animate.animate($this, animation, options, => + if i is $els.length - 1 + dfd.resolve() + if done? + done() + ) + ,time * i + ) + return + ) + return dfd.promise() + return ) window \ No newline at end of file diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index c26cfeccf1d..8ce6772c400 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -33,14 +33,15 @@ class Projects::IssuesController < Projects::ApplicationController end @issues = @issues.page(params[:page]) - @label = @project.labels.find_by(title: params[:label_name]) + @labels = @project.labels.where(title: params[:label_name]) respond_to do |format| format.html format.atom { render layout: false } format.json do render json: { - html: view_to_html_string("projects/issues/_issues") + html: view_to_html_string("projects/issues/_issues"), + labels: @labels } end end diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index b38c5e18efb..f81a04a3c86 100644 --- a/app/views/shared/_label_row.html.haml +++ b/app/views/shared/_label_row.html.haml @@ -1,5 +1,3 @@ -%span.label-row - %span.label-name - = link_to_label(label, tooltip: false) - %span.prepend-left-10 - = markdown(label.description, pipeline: :single_line) +- labels.each do |l| + %span.label-row + = link_to_label(l, tooltip: false) \ No newline at end of file diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 921eaefd79a..02982516de9 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,9 +46,9 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" -- if @label - .gray-content-block.second-block - = render "shared/label_row", label: @label +- if @labels + .gray-content-block.second-block.filtered-labels + = render "shared/label_row", labels: @labels :javascript new UsersSelect(); diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index 4ded2d98f37..6df1bd0ca35 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -1,5 +1,7 @@ - if params[:label_name].present? - = hidden_field_tag(:label_name, params[:label_name]) + - if params[:label_name].respond_to?('any?') + - params[:label_name].each do |label| + = hidden_field_tag "label_name[]", label, id: nil .dropdown %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} From dc13f7c31dee2c0515c36fba2398bc8b843a8108 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 13 Apr 2016 12:05:10 +0200 Subject: [PATCH 06/23] Return unique issues when using multiple labels This ensures that IssuableFinder returns a collection of unique issues, even when filtering issues using multiple labels. --- app/finders/issuable_finder.rb | 4 +++- spec/finders/issues_finder_spec.rb | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index f1df6832bf6..d7c5b0a598c 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -278,7 +278,9 @@ class IssuableFinder end end - items + # When filtering by multiple labels we may end up duplicating issues (if one + # has multiple labels). This ensures we only return unique issues. + items.distinct end def label_names diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index b1648055462..bc607a29751 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -62,6 +62,22 @@ describe IssuesFinder do expect(issues).to eq([issue2]) end + it 'returns unique issues when filtering by multiple labels' do + label2 = create(:label, project: project2) + + create(:label_link, label: label2, target: issue2) + + params = { + scope: 'all', + label_name: [label.title, label2.title].join(','), + state: 'opened' + } + + issues = IssuesFinder.new(user, params).execute + + expect(issues).to eq([issue2]) + end + it 'should filter by no label name' do params = { scope: "all", label_name: Label::None.title, state: 'opened' } issues = IssuesFinder.new(user, params).execute From 22c089c2fd8ebc5c2e1f5768afd3d5accfd6c9bb Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Wed, 13 Apr 2016 17:31:42 -0400 Subject: [PATCH 07/23] Removes dup dropdown --- app/views/shared/issuable/_label_dropdown.html.haml | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index 6df1bd0ca35..3eaa45258f0 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -3,7 +3,6 @@ - params[:label_name].each do |label| = hidden_field_tag "label_name[]", label, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} %span.dropdown-toggle-text = h(multi_label_name(params[:label_name], "Label")) From 6745e58f8f809a6813ac42fcd2ac82729234df1e Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Wed, 13 Apr 2016 18:25:39 -0400 Subject: [PATCH 08/23] Fix issue with labels not showing initially. --- app/assets/javascripts/lib/animate.js.coffee | 2 ++ app/views/shared/issuable/_filter.html.haml | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/lib/animate.js.coffee b/app/assets/javascripts/lib/animate.js.coffee index 64aef4c6d43..ec3b44d6126 100644 --- a/app/assets/javascripts/lib/animate.js.coffee +++ b/app/assets/javascripts/lib/animate.js.coffee @@ -19,6 +19,8 @@ gl.animate.animateEach = ($els, animation, time, options, done) -> dfd = $.Deferred() + if not $els.length + dfd.resolve() $els.each((i) -> setTimeout(=> $this = $(@) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 02982516de9..623ca5ab98f 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,9 +46,9 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" -- if @labels .gray-content-block.second-block.filtered-labels - = render "shared/label_row", labels: @labels + - if @labels + = render "shared/label_row", labels: @labels :javascript new UsersSelect(); From 5f53ca69ace953bf06afe478072836d083f941ca Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 14 Apr 2016 11:16:24 +0100 Subject: [PATCH 09/23] fix failing tests --- app/views/shared/_label_row.html.haml | 8 +++++--- app/views/shared/_labels_row.html.haml | 3 +++ app/views/shared/issuable/_filter.html.haml | 2 +- features/project/issues/filter_labels.feature | 1 + features/steps/project/issues/filter_labels.rb | 4 ++++ 5 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 app/views/shared/_labels_row.html.haml diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index f81a04a3c86..9ce5562e667 100644 --- a/app/views/shared/_label_row.html.haml +++ b/app/views/shared/_label_row.html.haml @@ -1,3 +1,5 @@ -- labels.each do |l| - %span.label-row - = link_to_label(l, tooltip: false) \ No newline at end of file +%span.label-row + %span.label-name + = link_to_label(label, tooltip: false) + %span.prepend-left-10 + = markdown(label.description, pipeline: :single_line) \ No newline at end of file diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml new file mode 100644 index 00000000000..f81a04a3c86 --- /dev/null +++ b/app/views/shared/_labels_row.html.haml @@ -0,0 +1,3 @@ +- labels.each do |l| + %span.label-row + = link_to_label(l, tooltip: false) \ No newline at end of file diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 623ca5ab98f..c14391ada0f 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -48,7 +48,7 @@ .gray-content-block.second-block.filtered-labels - if @labels - = render "shared/label_row", labels: @labels + = render "shared/labels_row", labels: @labels :javascript new UsersSelect(); diff --git a/features/project/issues/filter_labels.feature b/features/project/issues/filter_labels.feature index e07f8053fb7..49d7a3b9af2 100644 --- a/features/project/issues/filter_labels.feature +++ b/features/project/issues/filter_labels.feature @@ -12,6 +12,7 @@ Feature: Project Issues Filter Labels @javascript Scenario: I filter by one label Given I click link "bug" + And I click "dropdown close button" Then I should see "Bugfix1" in issues list And I should see "Bugfix2" in issues list And I should not see "Feature1" in issues list diff --git a/features/steps/project/issues/filter_labels.rb b/features/steps/project/issues/filter_labels.rb index 6d50501a722..d82c6856918 100644 --- a/features/steps/project/issues/filter_labels.rb +++ b/features/steps/project/issues/filter_labels.rb @@ -32,6 +32,10 @@ class Spinach::Features::ProjectIssuesFilterLabels < Spinach::FeatureSteps page.find('.js-label-select').click sleep 0.5 execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") + end + + step 'I click "dropdown close button"' do + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click sleep 2 end From 0c61f052def4c363eea0b3621e540e9af78c7555 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 14 Apr 2016 13:23:46 +0100 Subject: [PATCH 10/23] add some tests for the new feature --- spec/features/issues/filter_by_labels.spec.rb | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 spec/features/issues/filter_by_labels.spec.rb diff --git a/spec/features/issues/filter_by_labels.spec.rb b/spec/features/issues/filter_by_labels.spec.rb new file mode 100644 index 00000000000..7944403f874 --- /dev/null +++ b/spec/features/issues/filter_by_labels.spec.rb @@ -0,0 +1,172 @@ +require 'rails_helper' + +feature 'Issue filtering by Labels', feature: true do + let(:project) { create(:project, :public) } + let!(:user) { create(:user)} + let!(:label) { create(:label, project: project) } + + before do + ['bug', 'feature', 'enhancement'].each do |title| + create(:label, + project: project, + title: title) + end + + issue1 = create(:issue, title: "Bugfix1", project: project) + issue1.labels << project.labels.find_by(title: 'bug') + + issue2 = create(:issue, title: "Bugfix2", project: project) + issue2.labels << project.labels.find_by(title: 'bug') + issue2.labels << project.labels.find_by(title: 'enhancement') + + issue3 = create(:issue, title: "Feature1", project: project) + issue3.labels << project.labels.find_by(title: 'feature') + + project.team << [user, :master] + login_as(user) + + visit namespace_project_issues_path(project.namespace, project) + end + + context 'filter by label bug', js: true do + before do + page.find('.js-label-select').click + sleep 0.5 + execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + end + + it 'should show issue "Bugfix1" and "Bugfix2" in issues list' do + expect(page).to have_content "Bugfix1" + expect(page).to have_content "Bugfix2" + end + + it 'should not show "Feature1" in issues list' do + expect(page).not_to have_content "Feature1" + end + + it 'should show label "bug" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "bug" + end + + it 'should not show label "feature" and "enhancement" in filtered-labels' do + expect(find('.filtered-labels')).not_to have_content "feature" + expect(find('.filtered-labels')).not_to have_content "enhancement" + end + end + + context 'filter by label feature', js: true do + before do + page.find('.js-label-select').click + sleep 0.5 + execute_script("$('.dropdown-menu-labels li:contains(\"feature\") a').click()") + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + end + + it 'should show issue "Feature1" in issues list' do + expect(page).to have_content "Feature1" + end + + it 'should not show "Bugfix1" and "Bugfix2" in issues list' do + expect(page).not_to have_content "Bugfix2" + expect(page).not_to have_content "Bugfix1" + end + + it 'should show label "feature" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "feature" + end + + it 'should not show label "bug" and "enhancement" in filtered-labels' do + expect(find('.filtered-labels')).not_to have_content "bug" + expect(find('.filtered-labels')).not_to have_content "enhancement" + end + end + + context 'filter by label enhancement', js: true do + before do + page.find('.js-label-select').click + sleep 0.5 + execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + end + + it 'should show issue "Bugfix2" in issues list' do + expect(page).to have_content "Bugfix2" + end + + it 'should not show "Feature1" and "Bugfix1" in issues list' do + expect(page).not_to have_content "Feature1" + expect(page).not_to have_content "Bugfix1" + end + + it 'should show label "enhancement" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "enhancement" + end + + it 'should not show label "feature" and "bug" in filtered-labels' do + expect(find('.filtered-labels')).not_to have_content "bug" + expect(find('.filtered-labels')).not_to have_content "feature" + end + end + + context 'filter by label enhancement or feature', js: true do + before do + page.find('.js-label-select').click + sleep 0.5 + execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") + execute_script("$('.dropdown-menu-labels li:contains(\"feature\") a').click()") + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + end + + it 'should show issue "Bugfix2" or "Feature1" in issues list' do + expect(page).to have_content "Bugfix2" + expect(page).to have_content "Feature1" + end + + it 'should not show "Bugfix1" in issues list' do + expect(page).not_to have_content "Bugfix1" + end + + it 'should show label "enhancement" and "feature" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "enhancement" + expect(find('.filtered-labels')).to have_content "feature" + end + + it 'should not show label "bug" in filtered-labels' do + expect(find('.filtered-labels')).not_to have_content "bug" + end + end + + context 'filter by label enhancement or bug in issues list', js: true do + before do + page.find('.js-label-select').click + sleep 0.5 + execute_script("$('.dropdown-menu-labels li:contains(\"enhancement\") a').click()") + execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + end + + it 'should show issue "Bugfix2" or "Bugfix1" in issues list' do + expect(page).to have_content "Bugfix2" + expect(page).to have_content "Bugfix1" + end + + it 'should not show "Feature1"' do + expect(page).not_to have_content "Feature1" + end + + it 'should show label "bug" and "enhancement" in filtered-labels' do + expect(find('.filtered-labels')).to have_content "bug" + expect(find('.filtered-labels')).to have_content "enhancement" + end + + it 'should not show label "feature" in filtered-labels' do + expect(find('.filtered-labels')).not_to have_content "feature" + end + end +end From 0debd9c7d3db445efc89e72d7a1192e5a40d5044 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 14 Apr 2016 15:26:53 +0100 Subject: [PATCH 11/23] fix labels button dropdown not showing how many labels clicked --- app/assets/javascripts/labels_select.js.coffee | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 97a813577ed..e70c3cdd190 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -220,9 +220,19 @@ class @LabelsSelect fields: ['title'] selectable: true - toggleLabel: (selected) -> + toggleLabel: (selected, el) -> + selected_labels = $('.js-label-select').siblings('.dropdown-menu-labels').find('.is-active') + if selected and selected.title? - selected.title + if selected_labels and selected_labels.length > 1 + "#{selected.title} +#{selected_labels.length - 1} more" + else + selected.title + else if not selected and selected_labels.length isnt 0 + if selected_labels.length > 1 + "#{$(selected_labels[0]).text()} +#{selected_labels.length - 1} more" + else if selected_labels.length is 1 + $(selected_labels).text() else defaultLabel fieldName: $dropdown.data('field-name') @@ -238,7 +248,7 @@ class @LabelsSelect page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' isMRIndex = page is page is 'projects:merge_requests:index' - + $selectbox.hide() # display:block overrides the hide-collapse rule $value.removeAttr('style') From d0ad566972dc5c47390df1720cdee8088fda6c73 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 14 Apr 2016 18:52:40 +0100 Subject: [PATCH 12/23] rename the test file --- app/helpers/issuables_helper.rb | 2 +- .../{filter_by_labels.spec.rb => filter_by_labels_spec.rb} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename spec/features/issues/{filter_by_labels.spec.rb => filter_by_labels_spec.rb} (100%) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 14e624cb7cf..b363ed3076c 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -29,7 +29,7 @@ module IssuablesHelper default_label end else - if current_labels.nil? + if current_labels.nil? || current_labels.empty? default_label else current_labels diff --git a/spec/features/issues/filter_by_labels.spec.rb b/spec/features/issues/filter_by_labels_spec.rb similarity index 100% rename from spec/features/issues/filter_by_labels.spec.rb rename to spec/features/issues/filter_by_labels_spec.rb From a0a423fee76d8cbc50cdb2478b05ccb751bc2be8 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Fri, 15 Apr 2016 18:18:31 +0100 Subject: [PATCH 13/23] fix failing tests --- app/assets/javascripts/issues.js.coffee | 27 ++++++++++++++++--- app/helpers/application_helper.rb | 9 ++++++- app/views/shared/issuable/_nav.html.haml | 10 +++---- spec/features/issues/filter_issues_spec.rb | 9 ++++++- .../filter_by_milestone_spec.rb | 6 +++++ 5 files changed, 50 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 320b92d2a60..1148531c068 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -36,19 +36,37 @@ $(".selected_issue").bind "change", Issues.checkChanged + getLabelsQueryString: -> + pageURL = decodeURIComponent(window.location.search.substring(1)) + urlVariables = pageURL.split('&') + labelParams = ( + variables for variables in urlVariables when variables.indexOf('label_name[]') > -1 + ).join('&') + + removeLabelsQueryString: (url) -> + pageURL = decodeURIComponent(url) + urlVariables = pageURL.split('&') + Params = ( + variables for variables in urlVariables when variables.indexOf('label_name[]') is -1 + ).join('&') + # Update state filters if present in page updateStateFilters: -> stateFilters = $('.issues-state-filters') newParams = {} - paramKeys = ['author_id', 'label_name', 'milestone_title', 'assignee_id', 'issue_search'] + paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] for paramKey in paramKeys newParams[paramKey] = gl.utils.getUrlParameter(paramKey) or '' if stateFilters.length stateFilters.find('a').each -> - initialUrl = $(this).attr 'href' - $(this).attr 'href', gl.utils.mergeUrlParams(newParams, initialUrl) + initialUrl = Issues.removeLabelsQueryString($(this).attr 'href') + if Issues.getLabelsQueryString() + newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{Issues.getLabelsQueryString()}" + else + newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) + $(this).attr 'href', newUrl # Make sure we trigger ajax request only after user stop typing initSearch: -> @@ -91,7 +109,8 @@ opacity: 0 } }).then( -> - $filteredLabels.html(Issue.labelRow(data)) + if typeof Issue.labelRow is 'function' + $filteredLabels.html(Issue.labelRow(data)) $spans = $filteredLabels.find('span') $spans.css('opacity',0) return gl.animate.animateEach($spans, 'fadeInUp', 20, { diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 16e5b8ac223..3e0074da394 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -254,11 +254,11 @@ module ApplicationHelper def page_filter_path(options = {}) without = options.delete(:without) + add_label = options.delete(:label) exist_opts = { state: params[:state], scope: params[:scope], - label_name: params[:label_name], milestone_title: params[:milestone_title], assignee_id: params[:assignee_id], author_id: params[:author_id], @@ -275,6 +275,13 @@ module ApplicationHelper path = request.path path << "?#{options.to_param}" + if add_label + if params[:label_name].present? and params[:label_name].respond_to?('any?') + params[:label_name].each do |label| + path << "&label_name[]=#{label}" + end + end + end path end diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml index a6970b7eebb..1d9b09a5ef1 100644 --- a/app/views/shared/issuable/_nav.html.haml +++ b/app/views/shared/issuable/_nav.html.haml @@ -4,22 +4,22 @@ - else - page_context_word = 'issues' %li{class: ("active" if params[:state] == 'opened')} - = link_to page_filter_path(state: 'opened'), title: "Filter by #{page_context_word} that are currently opened." do + = link_to page_filter_path(state: 'opened', label: true), title: "Filter by #{page_context_word} that are currently opened." do #{state_filters_text_for(:opened, @project)} - if defined?(type) && type == :merge_requests %li{class: ("active" if params[:state] == 'merged')} - = link_to page_filter_path(state: 'merged'), title: 'Filter by merge requests that are currently merged.' do + = link_to page_filter_path(state: 'merged', label: true), title: 'Filter by merge requests that are currently merged.' do #{state_filters_text_for(:merged, @project)} %li{class: ("active" if params[:state] == 'closed')} - = link_to page_filter_path(state: 'closed'), title: 'Filter by merge requests that are currently closed and unmerged.' do + = link_to page_filter_path(state: 'closed', label: true), title: 'Filter by merge requests that are currently closed and unmerged.' do #{state_filters_text_for(:closed, @project)} - else %li{class: ("active" if params[:state] == 'closed')} - = link_to page_filter_path(state: 'closed'), title: 'Filter by issues that are currently closed.' do + = link_to page_filter_path(state: 'closed', label: true), title: 'Filter by issues that are currently closed.' do #{state_filters_text_for(:closed, @project)} %li{class: ("active" if params[:state] == 'all')} - = link_to page_filter_path(state: 'all'), title: "Show all #{page_context_word}." do + = link_to page_filter_path(state: 'all', label: true), title: "Show all #{page_context_word}." do #{state_filters_text_for(:all, @project)} diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 69b22232f10..192e3619375 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -84,14 +84,20 @@ describe 'Filter issues', feature: true do it 'should filter by any label' do find('.dropdown-menu-labels a', text: 'Any Label').click + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + page.within '.labels-filter' do expect(page).to have_content 'Any Label' end - expect(find('.js-label-select .dropdown-toggle-text')).to have_content('Label') + expect(find('.js-label-select .dropdown-toggle-text')).to have_content('Any Label') end it 'should filter by no label' do find('.dropdown-menu-labels a', text: 'No Label').click + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click + sleep 2 + page.within '.labels-filter' do expect(page).to have_content 'No Label' end @@ -121,6 +127,7 @@ describe 'Filter issues', feature: true do find('.js-label-select').click find('.dropdown-menu-labels .dropdown-content a', text: label.title).click + page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click sleep 2 end diff --git a/spec/features/merge_requests/filter_by_milestone_spec.rb b/spec/features/merge_requests/filter_by_milestone_spec.rb index c57ab5f3b03..e3ecd60a5f3 100644 --- a/spec/features/merge_requests/filter_by_milestone_spec.rb +++ b/spec/features/merge_requests/filter_by_milestone_spec.rb @@ -2,8 +2,14 @@ require 'rails_helper' feature 'Merge Request filtering by Milestone', feature: true do let(:project) { create(:project, :public) } + let!(:user) { create(:user)} let(:milestone) { create(:milestone, project: project) } + before do + project.team << [user, :master] + login_as(user) + end + scenario 'filters by no Milestone', js: true do create(:merge_request, :with_diffs, source_project: project) create(:merge_request, :simple, source_project: project, milestone: milestone) From ef9f5579d29ac4b72f463fabc6e0ace10078c009 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Sat, 16 Apr 2016 15:56:19 -0400 Subject: [PATCH 14/23] Fix coding style issues. Dashes to camelCase --- app/assets/javascripts/labels_select.js.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index e70c3cdd190..83e3062d222 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -203,7 +203,7 @@ class @LabelsSelect renderRow: (label) -> selectedClass = '' if $form.find("input[type='hidden']\ - [name='#{$dropdown.data('field-name')}']\ + [name='#{$dropdown.data('fieldName')}']\ [value='#{this.id(label)}']").length selectedClass = 'is-active' @@ -256,7 +256,7 @@ class @LabelsSelect if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) selectedLabels = $dropdown .closest('form') - .find("input[type='hidden'][name='#{$dropdown.data('field-name')}']") + .find("input[type='hidden'][name='#{$dropdown.data('fieldName')}']") Issues.filterResults( $dropdown.closest('form'), $dropdown.data('fieldName') From 259970ca1b3118f3eb71751b33a3a53ff4a1fa59 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Sat, 16 Apr 2016 23:30:31 +0100 Subject: [PATCH 15/23] abstract code for removing or getting a param query string from url --- app/assets/javascripts/issues.js.coffee | 20 +++---------------- .../javascripts/lib/url_utility.js.coffee | 16 +++++++++++++++ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 1148531c068..a3d9ce03875 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -36,20 +36,6 @@ $(".selected_issue").bind "change", Issues.checkChanged - getLabelsQueryString: -> - pageURL = decodeURIComponent(window.location.search.substring(1)) - urlVariables = pageURL.split('&') - labelParams = ( - variables for variables in urlVariables when variables.indexOf('label_name[]') > -1 - ).join('&') - - removeLabelsQueryString: (url) -> - pageURL = decodeURIComponent(url) - urlVariables = pageURL.split('&') - Params = ( - variables for variables in urlVariables when variables.indexOf('label_name[]') is -1 - ).join('&') - # Update state filters if present in page updateStateFilters: -> stateFilters = $('.issues-state-filters') @@ -61,9 +47,9 @@ if stateFilters.length stateFilters.find('a').each -> - initialUrl = Issues.removeLabelsQueryString($(this).attr 'href') - if Issues.getLabelsQueryString() - newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{Issues.getLabelsQueryString()}" + initialUrl = gl.utils.removeParamQueryString($(this).attr('href'), 'label_name[]') + if gl.utils.getParamQueryString('label_name[]') + newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{gl.utils.getParamQueryString('label_name[]')}" else newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) $(this).attr 'href', newUrl diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee index abd556e0b4e..c2e3c807e5e 100644 --- a/app/assets/javascripts/lib/url_utility.js.coffee +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -28,4 +28,20 @@ newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" newUrl + # get parameter query string from url. + w.gl.utils.getParamQueryString = (param) -> + pageURL = decodeURIComponent(window.location.search.substring(1)) + urlVariables = pageURL.split('&') + ( + variables for variables in urlVariables when variables.indexOf(param) > -1 + ).join('&') + + # removes parameter query string from url. returns the modified url + w.gl.utils.removeParamQueryString = (url, param) -> + url = decodeURIComponent(url) + urlVariables = url.split('&') + ( + variables for variables in urlVariables when variables.indexOf(param) is -1 + ).join('&') + ) window From 5cefd8ab7655ec6433b3048a7382720c5300dc4c Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Tue, 19 Apr 2016 17:22:55 +0100 Subject: [PATCH 16/23] some refactoring --- app/assets/javascripts/issues.js.coffee | 47 +++++++++++-------- .../javascripts/labels_select.js.coffee | 13 ++--- .../javascripts/lib/url_utility.js.coffee | 16 +++---- .../javascripts/merge_requests.js.coffee | 1 + app/helpers/issuables_helper.rb | 16 +++---- app/views/shared/_labels_row.html.haml | 4 +- 6 files changed, 48 insertions(+), 49 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index a3d9ce03875..f26b4f723eb 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -3,6 +3,7 @@ Issues.initTemplates() Issues.initSearch() Issues.initChecks() + Issues.toggleLabelFilters() $("body").on "ajax:success", ".close_issue, .reopen_issue", -> t = $(this) @@ -20,11 +21,20 @@ Issue.labelRow = _.template( '<% _.each(labels, function(label){ %> - <%= label.title %> + <%= label.title %> <% }); %>' ) + toggleLabelFilters: ()-> + $filteredLabels = $('.filtered-labels') + if $filteredLabels.find('.label-row').length > 0 + #$filteredLabels.show() + $filteredLabels.slideDown().css({'overflow':'visible'}) + else + #$filteredLabels.hide() + $filteredLabels.slideUp().css({'overflow':'visible'}) + reload: -> Issues.initChecks() $('#filter_issue_search').val($('#issue_search').val()) @@ -43,13 +53,15 @@ paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] for paramKey in paramKeys - newParams[paramKey] = gl.utils.getUrlParameter(paramKey) or '' + newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or '' if stateFilters.length stateFilters.find('a').each -> initialUrl = gl.utils.removeParamQueryString($(this).attr('href'), 'label_name[]') - if gl.utils.getParamQueryString('label_name[]') - newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{gl.utils.getParamQueryString('label_name[]')}" + labelNameValues = gl.utils.getParameterValues('label_name[]') + if labelNameValues + labelNameQueryString = ("label_name[]=#{value}" for value in labelNameValues).join('&') + newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{labelNameQueryString}" else newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) $(this).attr 'href', newUrl @@ -84,29 +96,24 @@ Issues.reload() Issues.updateStateFilters() $filteredLabels = $('.filtered-labels') - $filteredLabelsSpans = $filteredLabels.find('span') + $filteredLabelsSpans = $filteredLabels.find('.label-row') gl.animate.animateEach( - $filteredLabelsSpans, - 'fadeOutDown', 20, { - cssStart: { + $filteredLabelsSpans, 'fadeOutDown', 20, + cssStart: opacity: 1 - }, - cssEnd: { + cssEnd: opacity: 0 - } - }).then( -> + ).then( -> if typeof Issue.labelRow is 'function' $filteredLabels.html(Issue.labelRow(data)) - $spans = $filteredLabels.find('span') - $spans.css('opacity',0) - return gl.animate.animateEach($spans, 'fadeInUp', 20, { - cssStart: { + Issues.toggleLabelFilters() + $spans = $filteredLabels.find('.label-row') + $spans.css('opacity', 0) + return gl.animate.animateEach $spans, 'fadeInUp', 20, + cssStart: opacity: 0 - }, - cssEnd: { + cssEnd: opacity: 1 - } - }) ) dataType: "json" diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 83e3062d222..6a89817e647 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -224,7 +224,7 @@ class @LabelsSelect selected_labels = $('.js-label-select').siblings('.dropdown-menu-labels').find('.is-active') if selected and selected.title? - if selected_labels and selected_labels.length > 1 + if selected_labels.length > 1 "#{selected.title} +#{selected_labels.length - 1} more" else selected.title @@ -247,7 +247,7 @@ class @LabelsSelect hidden: -> page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' - isMRIndex = page is page is 'projects:merge_requests:index' + isMRIndex = page is 'projects:merge_requests:index' $selectbox.hide() # display:block overrides the hide-collapse rule @@ -256,11 +256,8 @@ class @LabelsSelect if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) selectedLabels = $dropdown .closest('form') - .find("input[type='hidden'][name='#{$dropdown.data('fieldName')}']") - Issues.filterResults( - $dropdown.closest('form'), - $dropdown.data('fieldName') - ) + .find("input:hidden[name='#{$dropdown.data('fieldName')}']") + Issues.filterResults $dropdown.closest('form') else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else @@ -270,7 +267,7 @@ class @LabelsSelect clicked: (label) -> page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' - isMRIndex = page is page is 'projects:merge_requests:index' + isMRIndex = page is 'projects:merge_requests:index' if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) if not $dropdown.hasClass 'js-multiselect' selectedLabel = label.title diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee index c2e3c807e5e..6a00932c028 100644 --- a/app/assets/javascripts/lib/url_utility.js.coffee +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -3,16 +3,20 @@ w.gl ?= {} w.gl.utils ?= {} - w.gl.utils.getUrlParameter = (sParam) -> + # Returns an array containing the value(s) of the + # of the key passed as an argument + w.gl.utils.getParameterValues = (sParam) -> sPageURL = decodeURIComponent(window.location.search.substring(1)) sURLVariables = sPageURL.split('&') sParameterName = undefined + values = [] i = 0 while i < sURLVariables.length sParameterName = sURLVariables[i].split('=') if sParameterName[0] is sParam - return if sParameterName[1] is undefined then true else sParameterName[1] + values.push(sParameterName[1]) i++ + values # # # @param {Object} params - url keys and value to merge @@ -28,14 +32,6 @@ newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" newUrl - # get parameter query string from url. - w.gl.utils.getParamQueryString = (param) -> - pageURL = decodeURIComponent(window.location.search.substring(1)) - urlVariables = pageURL.split('&') - ( - variables for variables in urlVariables when variables.indexOf(param) > -1 - ).join('&') - # removes parameter query string from url. returns the modified url w.gl.utils.removeParamQueryString = (url, param) -> url = decodeURIComponent(url) diff --git a/app/assets/javascripts/merge_requests.js.coffee b/app/assets/javascripts/merge_requests.js.coffee index b3c73ffce5d..203def58783 100644 --- a/app/assets/javascripts/merge_requests.js.coffee +++ b/app/assets/javascripts/merge_requests.js.coffee @@ -3,6 +3,7 @@ # @MergeRequests = init: -> + $('.filtered-labels').hide() MergeRequests.initSearch() # Make sure we trigger ajax request only after user stop typing diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b363ed3076c..5baa4f53bb9 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -18,22 +18,20 @@ module IssuablesHelper def multi_label_name(current_labels, default_label) # current_labels may be a string from before - if current_labels.respond_to?('any?') - if current_labels.any? - if current_labels.count > 1 - "#{current_labels[0]} +#{current_labels.count - 1} more" - else - current_labels[0] - end + if current_labels.is_a?(Array) + if current_labels.count > 1 + "#{current_labels[0]} +#{current_labels.count - 1} more" else - default_label + current_labels[0] end - else + elsif current_labels.is_a?(String) if current_labels.nil? || current_labels.empty? default_label else current_labels end + else + default_label end end diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml index f81a04a3c86..09dc6d97d10 100644 --- a/app/views/shared/_labels_row.html.haml +++ b/app/views/shared/_labels_row.html.haml @@ -1,3 +1,3 @@ -- labels.each do |l| +- labels.each do |label| %span.label-row - = link_to_label(l, tooltip: false) \ No newline at end of file + = link_to_label(label, tooltip: false) \ No newline at end of file From 2af947931d0c0095d37124b76428f4a1fc6d2545 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Wed, 20 Apr 2016 11:41:10 +0100 Subject: [PATCH 17/23] increase the speed of the labels animation ans aslo refactor some part fo issue.js --- app/assets/javascripts/issues.js.coffee | 47 +++++++++++-------- .../javascripts/labels_select.js.coffee | 13 ++--- .../javascripts/lib/url_utility.js.coffee | 16 +++---- .../javascripts/merge_requests.js.coffee | 1 + app/helpers/issuables_helper.rb | 16 +++---- app/views/shared/_labels_row.html.haml | 4 +- 6 files changed, 48 insertions(+), 49 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index a3d9ce03875..8711c255350 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -3,6 +3,7 @@ Issues.initTemplates() Issues.initSearch() Issues.initChecks() + Issues.toggleLabelFilters() $("body").on "ajax:success", ".close_issue, .reopen_issue", -> t = $(this) @@ -20,11 +21,20 @@ Issue.labelRow = _.template( '<% _.each(labels, function(label){ %> - <%= label.title %> + <%= label.title %> <% }); %>' ) + toggleLabelFilters: ()-> + $filteredLabels = $('.filtered-labels') + if $filteredLabels.find('.label-row').length > 0 + #$filteredLabels.show() + $filteredLabels.slideDown().css({'overflow':'visible'}) + else + #$filteredLabels.hide() + $filteredLabels.slideUp().css({'overflow':'visible'}) + reload: -> Issues.initChecks() $('#filter_issue_search').val($('#issue_search').val()) @@ -43,13 +53,15 @@ paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] for paramKey in paramKeys - newParams[paramKey] = gl.utils.getUrlParameter(paramKey) or '' + newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or '' if stateFilters.length stateFilters.find('a').each -> initialUrl = gl.utils.removeParamQueryString($(this).attr('href'), 'label_name[]') - if gl.utils.getParamQueryString('label_name[]') - newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{gl.utils.getParamQueryString('label_name[]')}" + labelNameValues = gl.utils.getParameterValues('label_name[]') + if labelNameValues + labelNameQueryString = ("label_name[]=#{value}" for value in labelNameValues).join('&') + newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{labelNameQueryString}" else newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) $(this).attr 'href', newUrl @@ -84,29 +96,24 @@ Issues.reload() Issues.updateStateFilters() $filteredLabels = $('.filtered-labels') - $filteredLabelsSpans = $filteredLabels.find('span') + $filteredLabelsSpans = $filteredLabels.find('.label-row') gl.animate.animateEach( - $filteredLabelsSpans, - 'fadeOutDown', 20, { - cssStart: { + $filteredLabelsSpans, 'fadeOutDown', 5, + cssStart: opacity: 1 - }, - cssEnd: { + cssEnd: opacity: 0 - } - }).then( -> + ).then( -> if typeof Issue.labelRow is 'function' $filteredLabels.html(Issue.labelRow(data)) - $spans = $filteredLabels.find('span') - $spans.css('opacity',0) - return gl.animate.animateEach($spans, 'fadeInUp', 20, { - cssStart: { + Issues.toggleLabelFilters() + $spans = $filteredLabels.find('.label-row') + $spans.css('opacity', 0) + return gl.animate.animateEach $spans, 'fadeInUp', 5, + cssStart: opacity: 0 - }, - cssEnd: { + cssEnd: opacity: 1 - } - }) ) dataType: "json" diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 83e3062d222..6a89817e647 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -224,7 +224,7 @@ class @LabelsSelect selected_labels = $('.js-label-select').siblings('.dropdown-menu-labels').find('.is-active') if selected and selected.title? - if selected_labels and selected_labels.length > 1 + if selected_labels.length > 1 "#{selected.title} +#{selected_labels.length - 1} more" else selected.title @@ -247,7 +247,7 @@ class @LabelsSelect hidden: -> page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' - isMRIndex = page is page is 'projects:merge_requests:index' + isMRIndex = page is 'projects:merge_requests:index' $selectbox.hide() # display:block overrides the hide-collapse rule @@ -256,11 +256,8 @@ class @LabelsSelect if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) selectedLabels = $dropdown .closest('form') - .find("input[type='hidden'][name='#{$dropdown.data('fieldName')}']") - Issues.filterResults( - $dropdown.closest('form'), - $dropdown.data('fieldName') - ) + .find("input:hidden[name='#{$dropdown.data('fieldName')}']") + Issues.filterResults $dropdown.closest('form') else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else @@ -270,7 +267,7 @@ class @LabelsSelect clicked: (label) -> page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' - isMRIndex = page is page is 'projects:merge_requests:index' + isMRIndex = page is 'projects:merge_requests:index' if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) if not $dropdown.hasClass 'js-multiselect' selectedLabel = label.title diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee index c2e3c807e5e..6a00932c028 100644 --- a/app/assets/javascripts/lib/url_utility.js.coffee +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -3,16 +3,20 @@ w.gl ?= {} w.gl.utils ?= {} - w.gl.utils.getUrlParameter = (sParam) -> + # Returns an array containing the value(s) of the + # of the key passed as an argument + w.gl.utils.getParameterValues = (sParam) -> sPageURL = decodeURIComponent(window.location.search.substring(1)) sURLVariables = sPageURL.split('&') sParameterName = undefined + values = [] i = 0 while i < sURLVariables.length sParameterName = sURLVariables[i].split('=') if sParameterName[0] is sParam - return if sParameterName[1] is undefined then true else sParameterName[1] + values.push(sParameterName[1]) i++ + values # # # @param {Object} params - url keys and value to merge @@ -28,14 +32,6 @@ newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" newUrl - # get parameter query string from url. - w.gl.utils.getParamQueryString = (param) -> - pageURL = decodeURIComponent(window.location.search.substring(1)) - urlVariables = pageURL.split('&') - ( - variables for variables in urlVariables when variables.indexOf(param) > -1 - ).join('&') - # removes parameter query string from url. returns the modified url w.gl.utils.removeParamQueryString = (url, param) -> url = decodeURIComponent(url) diff --git a/app/assets/javascripts/merge_requests.js.coffee b/app/assets/javascripts/merge_requests.js.coffee index b3c73ffce5d..203def58783 100644 --- a/app/assets/javascripts/merge_requests.js.coffee +++ b/app/assets/javascripts/merge_requests.js.coffee @@ -3,6 +3,7 @@ # @MergeRequests = init: -> + $('.filtered-labels').hide() MergeRequests.initSearch() # Make sure we trigger ajax request only after user stop typing diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b363ed3076c..5baa4f53bb9 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -18,22 +18,20 @@ module IssuablesHelper def multi_label_name(current_labels, default_label) # current_labels may be a string from before - if current_labels.respond_to?('any?') - if current_labels.any? - if current_labels.count > 1 - "#{current_labels[0]} +#{current_labels.count - 1} more" - else - current_labels[0] - end + if current_labels.is_a?(Array) + if current_labels.count > 1 + "#{current_labels[0]} +#{current_labels.count - 1} more" else - default_label + current_labels[0] end - else + elsif current_labels.is_a?(String) if current_labels.nil? || current_labels.empty? default_label else current_labels end + else + default_label end end diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml index f81a04a3c86..09dc6d97d10 100644 --- a/app/views/shared/_labels_row.html.haml +++ b/app/views/shared/_labels_row.html.haml @@ -1,3 +1,3 @@ -- labels.each do |l| +- labels.each do |label| %span.label-row - = link_to_label(l, tooltip: false) \ No newline at end of file + = link_to_label(label, tooltip: false) \ No newline at end of file From 1a7ccd0f0aacb33be3d8d3456bdedd8f08b7cc96 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 11:47:35 +0100 Subject: [PATCH 18/23] Removed animation from labels filter --- app/assets/javascripts/issues.js.coffee | 30 +++++---------------- app/views/shared/_labels_row.html.haml | 2 +- app/views/shared/issuable/_filter.html.haml | 2 +- 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index f26b4f723eb..8719d22a5bb 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -29,11 +29,9 @@ toggleLabelFilters: ()-> $filteredLabels = $('.filtered-labels') if $filteredLabels.find('.label-row').length > 0 - #$filteredLabels.show() - $filteredLabels.slideDown().css({'overflow':'visible'}) + $filteredLabels.removeClass('hidden') else - #$filteredLabels.hide() - $filteredLabels.slideUp().css({'overflow':'visible'}) + $filteredLabels.addClass('hidden') reload: -> Issues.initChecks() @@ -96,25 +94,11 @@ Issues.reload() Issues.updateStateFilters() $filteredLabels = $('.filtered-labels') - $filteredLabelsSpans = $filteredLabels.find('.label-row') - gl.animate.animateEach( - $filteredLabelsSpans, 'fadeOutDown', 20, - cssStart: - opacity: 1 - cssEnd: - opacity: 0 - ).then( -> - if typeof Issue.labelRow is 'function' - $filteredLabels.html(Issue.labelRow(data)) - Issues.toggleLabelFilters() - $spans = $filteredLabels.find('.label-row') - $spans.css('opacity', 0) - return gl.animate.animateEach $spans, 'fadeInUp', 20, - cssStart: - opacity: 0 - cssEnd: - opacity: 1 - ) + + if typeof Issue.labelRow is 'function' + $filteredLabels.html(Issue.labelRow(data)) + + Issues.toggleLabelFilters() dataType: "json" diff --git a/app/views/shared/_labels_row.html.haml b/app/views/shared/_labels_row.html.haml index 09dc6d97d10..dc89e36419c 100644 --- a/app/views/shared/_labels_row.html.haml +++ b/app/views/shared/_labels_row.html.haml @@ -1,3 +1,3 @@ - labels.each do |label| %span.label-row - = link_to_label(label, tooltip: false) \ No newline at end of file + = link_to_label(label, tooltip: false) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index c14391ada0f..9dba3f522db 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,7 +46,7 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" - .gray-content-block.second-block.filtered-labels + .gray-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels) } - if @labels = render "shared/labels_row", labels: @labels From a845252983e4a0346c17e1b058c388f4c23a547d Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Wed, 20 Apr 2016 13:55:19 +0100 Subject: [PATCH 19/23] get the multi filter labels feature to work on merge request, also escape characters in the templates to prevent xss attack --- app/assets/javascripts/dispatcher.js.coffee | 1 + app/assets/javascripts/issues.js.coffee | 2 +- app/assets/javascripts/merge_requests.js.coffee | 1 - app/controllers/projects/merge_requests_controller.rb | 5 +++-- app/views/shared/issuable/_filter.html.haml | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 70fd6f50e9c..ffc5dc602e2 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -58,6 +58,7 @@ class Dispatcher when 'projects:merge_requests:index' shortcut_handler = new ShortcutsNavigation() MergeRequests.init() + Issues.init() when 'dashboard:activity' new Activities() when 'dashboard:projects:starred' diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 8719d22a5bb..afd1ebd0a22 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -21,7 +21,7 @@ Issue.labelRow = _.template( '<% _.each(labels, function(label){ %> - <%= label.title %> + <%= _.escape(label.title) %> <% }); %>' ) diff --git a/app/assets/javascripts/merge_requests.js.coffee b/app/assets/javascripts/merge_requests.js.coffee index 203def58783..b3c73ffce5d 100644 --- a/app/assets/javascripts/merge_requests.js.coffee +++ b/app/assets/javascripts/merge_requests.js.coffee @@ -3,7 +3,6 @@ # @MergeRequests = init: -> - $('.filtered-labels').hide() MergeRequests.initSearch() # Make sure we trigger ajax request only after user stop typing diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3e0cfc6aa65..e33fb4ddf13 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -38,13 +38,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_requests = @merge_requests.page(params[:page]) @merge_requests = @merge_requests.preload(:target_project) - @label = @project.labels.find_by(title: params[:label_name]) + @labels = @project.labels.where(title: params[:label_name]) respond_to do |format| format.html format.json do render json: { - html: view_to_html_string("projects/merge_requests/_merge_requests") + html: view_to_html_string("projects/merge_requests/_merge_requests"), + labels: @labels } end end diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 9dba3f522db..c14391ada0f 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,7 +46,7 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" - .gray-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels) } + .gray-content-block.second-block.filtered-labels - if @labels = render "shared/labels_row", labels: @labels From 75626d5f0134770065a18c73223bdd798866fa5b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 17:00:12 +0100 Subject: [PATCH 20/23] Label text color comes from JSON Created issuable singleton to house the filtering --- app/assets/javascripts/dispatcher.js.coffee | 4 +- app/assets/javascripts/issuable.js.coffee | 84 +++++++++++++++++++ app/assets/javascripts/issues.js.coffee | 82 +----------------- .../javascripts/labels_select.js.coffee | 4 +- .../javascripts/merge_requests.js.coffee | 35 -------- .../javascripts/milestone_select.js.coffee | 2 +- app/assets/javascripts/users_select.js.coffee | 2 +- app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/models/label.rb | 4 + app/views/shared/issuable/_filter.html.haml | 4 +- 11 files changed, 99 insertions(+), 126 deletions(-) create mode 100644 app/assets/javascripts/issuable.js.coffee delete mode 100644 app/assets/javascripts/merge_requests.js.coffee diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index ffc5dc602e2..f3ae146691b 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -17,6 +17,7 @@ class Dispatcher switch page when 'projects:issues:index' Issues.init() + Issuable.init() shortcut_handler = new ShortcutsNavigation() when 'projects:issues:show' new Issue() @@ -57,8 +58,7 @@ class Dispatcher new ZenMode() when 'projects:merge_requests:index' shortcut_handler = new ShortcutsNavigation() - MergeRequests.init() - Issues.init() + Issuable.init() when 'dashboard:activity' new Activities() when 'dashboard:projects:starred' diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee new file mode 100644 index 00000000000..afffed63ac5 --- /dev/null +++ b/app/assets/javascripts/issuable.js.coffee @@ -0,0 +1,84 @@ +@Issuable = + init: -> + Issuable.initTemplates() + Issuable.initSearch() + + initTemplates: -> + Issuable.labelRow = _.template( + '<% _.each(labels, function(label){ %> + + <%= _.escape(label.title) %> + + <% }); %>' + ) + + initSearch: -> + @timer = null + $('#issue_search') + .off 'keyup' + .on 'keyup', -> + clearTimeout(@timer) + @timer = setTimeout( -> + Issuable.filterResults $('#issue_search_form') + , 500) + + toggleLabelFilters: -> + $filteredLabels = $('.filtered-labels') + if $filteredLabels.find('.label-row').length > 0 + $filteredLabels.removeClass('hidden') + else + $filteredLabels.addClass('hidden') + + filterResults: (form) => + formData = form.serialize() + + $('.issues-holder, .merge-requests-holder').css('opacity', '0.5') + formAction = form.attr('action') + issuesUrl = formAction + issuesUrl += ("#{if formAction.indexOf('?') < 0 then '?' else '&'}") + issuesUrl += formData + $.ajax + type: 'GET' + url: formAction + data: formData + complete: -> + $('.issues-holder, .merge-requests-holder').css('opacity', '1.0') + success: (data) -> + $('.issues-holder, .merge-requests-holder').html(data.html) + # Change url so if user reload a page - search results are saved + history.replaceState {page: issuesUrl}, document.title, issuesUrl + Issuable.reload() + Issuable.updateStateFilters() + $filteredLabels = $('.filtered-labels') + + if typeof Issuable.labelRow is 'function' + $filteredLabels.html(Issuable.labelRow(data)) + + Issuable.toggleLabelFilters() + + dataType: "json" + + reload: -> + if Issues.created + Issues.initChecks() + + $('#filter_issue_search').val($('#issue_search').val()) + + updateStateFilters: -> + stateFilters = $('.issues-state-filters') + newParams = {} + paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] + + for paramKey in paramKeys + newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or '' + + if stateFilters.length + stateFilters.find('a').each -> + initialUrl = gl.utils.removeParamQueryString($(this).attr('href'), 'label_name[]') + labelNameValues = gl.utils.getParameterValues('label_name[]') + if labelNameValues + labelNameQueryString = ("label_name[]=#{value}" for value in labelNameValues).join('&') + newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{labelNameQueryString}" + else + newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) + $(this).attr 'href', newUrl diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index afd1ebd0a22..3330e6c68ad 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -1,9 +1,7 @@ @Issues = init: -> - Issues.initTemplates() - Issues.initSearch() + Issues.created = true Issues.initChecks() - Issues.toggleLabelFilters() $("body").on "ajax:success", ".close_issue, .reopen_issue", -> t = $(this) @@ -17,26 +15,6 @@ else $(this).html totalIssues - 1 - initTemplates: -> - Issue.labelRow = _.template( - '<% _.each(labels, function(label){ %> - - <%= _.escape(label.title) %> - - <% }); %>' - ) - - toggleLabelFilters: ()-> - $filteredLabels = $('.filtered-labels') - if $filteredLabels.find('.label-row').length > 0 - $filteredLabels.removeClass('hidden') - else - $filteredLabels.addClass('hidden') - - reload: -> - Issues.initChecks() - $('#filter_issue_search').val($('#issue_search').val()) - initChecks: -> $(".check_all_issues").click -> $(".selected_issue").prop("checked", @checked) @@ -44,64 +22,6 @@ $(".selected_issue").bind "change", Issues.checkChanged - # Update state filters if present in page - updateStateFilters: -> - stateFilters = $('.issues-state-filters') - newParams = {} - paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] - - for paramKey in paramKeys - newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or '' - - if stateFilters.length - stateFilters.find('a').each -> - initialUrl = gl.utils.removeParamQueryString($(this).attr('href'), 'label_name[]') - labelNameValues = gl.utils.getParameterValues('label_name[]') - if labelNameValues - labelNameQueryString = ("label_name[]=#{value}" for value in labelNameValues).join('&') - newUrl = "#{gl.utils.mergeUrlParams(newParams, initialUrl)}&#{labelNameQueryString}" - else - newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) - $(this).attr 'href', newUrl - - # Make sure we trigger ajax request only after user stop typing - initSearch: -> - @timer = null - $("#issue_search").keyup -> - clearTimeout(@timer) - @timer = setTimeout( -> - Issues.filterResults $("#issue_search_form") - , 500) - - filterResults: (form) => - formData = form.serialize() - - $('.issues-holder, .merge-requests-holder').css("opacity", '0.5') - formAction = form.attr('action') - issuesUrl = formAction - issuesUrl += ("#{if formAction.indexOf("?") < 0 then '?' else '&'}") - issuesUrl += formData - $.ajax - type: "GET" - url: formAction - data: formData - complete: -> - $('.issues-holder, .merge-requests-holder').css("opacity", '1.0') - success: (data) -> - $('.issues-holder, .merge-requests-holder').html(data.html) - # Change url so if user reload a page - search results are saved - history.replaceState {page: issuesUrl}, document.title, issuesUrl - Issues.reload() - Issues.updateStateFilters() - $filteredLabels = $('.filtered-labels') - - if typeof Issue.labelRow is 'function' - $filteredLabels.html(Issue.labelRow(data)) - - Issues.toggleLabelFilters() - - dataType: "json" - checkChanged: -> checked_issues = $(".selected_issue:checked") if checked_issues.length > 0 diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 6a89817e647..0b39efa8d78 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -257,7 +257,7 @@ class @LabelsSelect selectedLabels = $dropdown .closest('form') .find("input:hidden[name='#{$dropdown.data('fieldName')}']") - Issues.filterResults $dropdown.closest('form') + Issuable.filterResults $dropdown.closest('form') else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else @@ -271,7 +271,7 @@ class @LabelsSelect if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) if not $dropdown.hasClass 'js-multiselect' selectedLabel = label.title - Issues.filterResults $dropdown.closest('form') + Issuable.filterResults $dropdown.closest('form') else if $dropdown.hasClass 'js-filter-submit' $dropdown.closest('form').submit() else diff --git a/app/assets/javascripts/merge_requests.js.coffee b/app/assets/javascripts/merge_requests.js.coffee deleted file mode 100644 index b3c73ffce5d..00000000000 --- a/app/assets/javascripts/merge_requests.js.coffee +++ /dev/null @@ -1,35 +0,0 @@ -# -# * Filter merge requests -# -@MergeRequests = - init: -> - MergeRequests.initSearch() - - # Make sure we trigger ajax request only after user stop typing - initSearch: -> - @timer = null - $("#issue_search").keyup -> - clearTimeout(@timer) - @timer = setTimeout(MergeRequests.filterResults, 500) - - filterResults: => - form = $("#issue_search_form") - search = $("#issue_search").val() - $('.merge-requests-holder').css("opacity", '0.5') - issues_url = form.attr('action') + '?' + form.serialize() - - $.ajax - type: "GET" - url: form.attr('action') - data: form.serialize() - complete: -> - $('.merge-requests-holder').css("opacity", '1.0') - success: (data) -> - $('.merge-requests-holder').html(data.html) - # Change url so if user reload a page - search results are saved - history.replaceState {page: issues_url}, document.title, issues_url - MergeRequests.reload() - dataType: "json" - - reload: -> - $('#filter_issue_search').val($('#issue_search').val()) diff --git a/app/assets/javascripts/milestone_select.js.coffee b/app/assets/javascripts/milestone_select.js.coffee index 6bd4e885a03..ee37c6eec4c 100644 --- a/app/assets/javascripts/milestone_select.js.coffee +++ b/app/assets/javascripts/milestone_select.js.coffee @@ -97,7 +97,7 @@ class @MilestoneSelect selectedMilestone = selected.name else selectedMilestone = '' - Issues.filterResults $dropdown.closest('form') + Issuable.filterResults $dropdown.closest('form') else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee index eee9b6e690e..f55d8462ac4 100644 --- a/app/assets/javascripts/users_select.js.coffee +++ b/app/assets/javascripts/users_select.js.coffee @@ -157,7 +157,7 @@ class @UsersSelect if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex) selectedId = user.id - Issues.filterResults $dropdown.closest('form') + Issuable.filterResults $dropdown.closest('form') else if $dropdown.hasClass 'js-filter-submit' $dropdown.closest('form').submit() else diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 8ce6772c400..89e43164926 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -41,7 +41,7 @@ class Projects::IssuesController < Projects::ApplicationController format.json do render json: { html: view_to_html_string("projects/issues/_issues"), - labels: @labels + labels: @labels.as_json(methods: :text_color) } end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e33fb4ddf13..1388ea9d66c 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -45,7 +45,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController format.json do render json: { html: view_to_html_string("projects/merge_requests/_merge_requests"), - labels: @labels + labels: @labels.as_json(methods: :text_color) } end end diff --git a/app/models/label.rb b/app/models/label.rb index 55c01cae762..60bdce32952 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -113,6 +113,10 @@ class Label < ActiveRecord::Base template end + def text_color + LabelsHelper::text_color_for_bg(self.color) + end + private def label_format_reference(format = :id) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index c14391ada0f..f828717250b 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,8 +46,8 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" - .gray-content-block.second-block.filtered-labels - - if @labels + .gray-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) } + - if @labels.any? = render "shared/labels_row", labels: @labels :javascript From 439a72803b32be97253ce689659251530afbca21 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 17:23:10 +0100 Subject: [PATCH 21/23] Any label & no label out weigh other labels - these two will clear previously selected labels Fixed issue with no label not working correctly --- app/assets/javascripts/gl_dropdown.js.coffee | 8 ++++---- app/assets/javascripts/labels_select.js.coffee | 15 +++++++++------ app/finders/issuable_finder.rb | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 2dc37257e22..e37b3fb5707 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -389,13 +389,13 @@ class GitLabDropdown else selectedObject else - if !value? - field.remove() - - if not @options.multiSelect + if not @options.multiSelect or el.hasClass('dropdown-clear-active') @dropdown.find(".#{ACTIVE_CLASS}").removeClass ACTIVE_CLASS @dropdown.parent().find("input[name='#{fieldName}']").remove() + if !value? + field.remove() + # Toggle active class for the tick mark el.addClass ACTIVE_CLASS diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 0b39efa8d78..ab1a57f7d30 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -201,16 +201,21 @@ class @LabelsSelect callback data renderRow: (label) -> - selectedClass = '' + removesAll = label.id is 0 or label.id is undefined + + selectedClass = [] if $form.find("input[type='hidden']\ [name='#{$dropdown.data('fieldName')}']\ [value='#{this.id(label)}']").length - selectedClass = 'is-active' + selectedClass.push 'is-active' + + if $dropdown.hasClass('js-multiselect') and removesAll + selectedClass.push 'dropdown-clear-active' color = if label.color? then "" else "" "
  • - + #{color} #{label.title} @@ -237,9 +242,7 @@ class @LabelsSelect defaultLabel fieldName: $dropdown.data('field-name') id: (label) -> - if label.isAny? - '' - else if $dropdown.hasClass "js-filter-submit" + if $dropdown.hasClass("js-filter-submit") and not label.isAny? label.title else label.id diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index d7c5b0a598c..5eb1d3f5aac 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -117,7 +117,7 @@ class IssuableFinder end def filter_by_no_label? - labels? && params[:label_name] == Label::None.title + labels? && params[:label_name].include?(Label::None.title) end def labels From ab93ea04aa6b81d518033af7c042d2c70c1e9828 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 17:52:55 +0100 Subject: [PATCH 22/23] Correctly checks for undefined in coffeescript :see_no_evil: --- app/assets/javascripts/labels_select.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index ab1a57f7d30..8e39b470257 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -201,7 +201,7 @@ class @LabelsSelect callback data renderRow: (label) -> - removesAll = label.id is 0 or label.id is undefined + removesAll = label.id is 0 or not label.id? selectedClass = [] if $form.find("input[type='hidden']\ From 490f77318b8d54be30558c3ef8914335ed83549b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 19:32:13 +0100 Subject: [PATCH 23/23] Fixed tests because of nil array --- app/views/shared/issuable/_filter.html.haml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index f828717250b..ade0a56b2e7 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -46,9 +46,10 @@ .filter-item.inline = button_tag "Update issues", class: "btn update_selected_issues btn-save" - .gray-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) } - - if @labels.any? - = render "shared/labels_row", labels: @labels + - if !@labels.nil? + .gray-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) } + - if @labels.any? + = render "shared/labels_row", labels: @labels :javascript new UsersSelect();