Revert "Merge branch 'sy-replace-issuable-sort-dropdown' into 'master'"

This reverts merge request !81430

app/helpers/sorting_helper.rb

@@ -243,51 +243,6 @@ module SortingHelper
     sort_options_hash[sort_value]
   end
 
-  def issuable_sort_options(viewing_issues, viewing_merge_requests)
-    options = [
-      { value: sort_value_priority, text: sort_title_priority, href: page_filter_path(sort: sort_value_priority) },
-      { value: sort_value_created_date, text: sort_title_created_date, href: page_filter_path(sort: sort_value_created_date) },
-      { value: sort_value_recently_updated, text: sort_title_recently_updated, href: page_filter_path(sort: sort_value_recently_updated) },
-      { value: sort_value_milestone, text: sort_title_milestone, href: page_filter_path(sort: sort_value_milestone) }
-    ]
-
-    options.concat([due_date_option]) if viewing_issues
-
-    options.concat([popularity_option, label_priority_option])
-    options.concat([merged_option, closed_option]) if viewing_merge_requests
-    options.concat([relative_position_option]) if viewing_issues
-
-    options.concat([title_option])
-  end
-
-  def due_date_option
-    { value: sort_value_due_date, text: sort_title_due_date, href: page_filter_path(sort: sort_value_due_date) }
-  end
-
-  def popularity_option
-    { value: sort_value_popularity, text: sort_title_popularity, href: page_filter_path(sort: sort_value_popularity) }
-  end
-
-  def label_priority_option
-    { value: sort_value_label_priority, text: sort_title_label_priority, href: page_filter_path(sort: sort_value_label_priority) }
-  end
-
-  def merged_option
-    { value: sort_value_merged_date, text: sort_title_merged_date, href: page_filter_path(sort: sort_value_merged_date) }
-  end
-
-  def closed_option
-    { value: sort_value_closed_date, text: sort_title_closed_date, href: page_filter_path(sort: sort_value_closed_date) }
-  end
-
-  def relative_position_option
-    { value: sort_value_relative_position, text: sort_title_relative_position, href: page_filter_path(sort: sort_value_relative_position) }
-  end
-
-  def title_option
-    { value: sort_value_title, text: sort_title_title, href: page_filter_path(sort: sort_value_title) }
-  end
-
   def sort_direction_icon(sort_value)
     case sort_value
     when sort_value_milestone, sort_value_due_date, sort_value_merged_date, sort_value_closed_date, /_asc\z/

app/views/shared/issuable/_sort_dropdown.html.haml

+- sort_value = @sort
+- sort_title = issuable_sort_option_title(sort_value)
 - viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
 - viewing_merge_requests = controller.controller_name == 'merge_requests'
-- items = issuable_sort_options(viewing_issues, viewing_merge_requests)
-- selected = issuable_sort_option_overrides[@sort] || @sort
 
-.gl-ml-3
+.dropdown.inline.gl-ml-3.issue-sort-dropdown
   .btn-group{ role: 'group' }
-    = gl_redirect_listbox_tag(items, selected, data: { right: true })
-    = issuable_sort_direction_button(@sort)
+    .btn-group{ role: 'group' }
+      %button.dropdown-menu-toggle{ type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'gl-button btn btn-default' }
+        = sort_title
+        = sprite_icon('chevron-down', css_class: "dropdown-menu-toggle-icon gl-top-3")
+      %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable.dropdown-menu-sort
+        %li
+          = sortable_item(sort_title_priority,          page_filter_path(sort: sort_value_priority),          sort_title)
+          = sortable_item(sort_title_created_date,      page_filter_path(sort: sort_value_created_date),      sort_title)
+          = sortable_item(sort_title_recently_updated,  page_filter_path(sort: sort_value_recently_updated),  sort_title)
+          = sortable_item(sort_title_milestone,         page_filter_path(sort: sort_value_milestone),         sort_title)
+          = sortable_item(sort_title_due_date,          page_filter_path(sort: sort_value_due_date),          sort_title) if viewing_issues
+          = sortable_item(sort_title_popularity,        page_filter_path(sort: sort_value_popularity),        sort_title)
+          = sortable_item(sort_title_label_priority,    page_filter_path(sort: sort_value_label_priority),    sort_title)
+          = sortable_item(sort_title_merged_date,       page_filter_path(sort: sort_value_merged_date),       sort_title) if viewing_merge_requests
+          = sortable_item(sort_title_closed_date,       page_filter_path(sort: sort_value_closed_date),       sort_title) if viewing_merge_requests
+          = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues
+          = sortable_item(sort_title_title,             page_filter_path(sort: sort_value_title),             sort_title)
+          = render_if_exists('shared/ee/issuable/sort_dropdown', viewing_issues: viewing_issues, sort_title: sort_title)
+    = issuable_sort_direction_button(sort_value)

ee/app/helpers/ee/sorting_helper.rb

@@ -37,26 +37,5 @@ module EE
         super
       end
     end
-
-    override :issuable_sort_options
-    def issuable_sort_options(viewing_issues, viewing_merge_requests)
-      options = super
-
-      if viewing_issues && (@project || @group)&.licensed_feature_available?(:issue_weights)
-        options.concat([weight_option])
-      end
-
-      options.concat([blocking_option]) if viewing_issues
-
-      options
-    end
-
-    def weight_option
-      { value: sort_value_weight, text: sort_title_weight, href: page_filter_path(sort: sort_value_weight) }
-    end
-
-    def blocking_option
-      { value: sort_value_blocking_desc, text: sort_title_blocking, href: page_filter_path(sort: sort_value_blocking_desc) }
-    end
   end
 end

ee/app/views/shared/ee/issuable/_sort_dropdown.html.haml

+- allow_weight_sort = viewing_issues && (@project || @group)&.feature_available?(:issue_weights)
+= sortable_item(sort_title_weight, page_filter_path(sort: sort_value_weight), sort_title) if allow_weight_sort
+
+- allow_blocking_sort = viewing_issues
+= sortable_item(sort_title_blocking, page_filter_path(sort: sort_value_blocking_desc), sort_title) if allow_blocking_sort

spec/features/dashboard/issues_filter_spec.rb

@@ -78,14 +78,14 @@ RSpec.describe 'Dashboard Issues filtering', :js do
     end
 
     it 'remembers last sorting value' do
-      pajamas_sort_by(s_('SortOptions|Created date'))
+      sort_by('Created date')
       visit_issues(assignee_username: user.username)
 
       expect(page).to have_button('Created date')
     end
 
     it 'keeps sorting issues after visiting Projects Issues page' do
-      pajamas_sort_by(s_('SortOptions|Created date'))
+      sort_by('Created date')
       visit project_issues_path(project)
 
       expect(page).to have_button('Created date')

spec/features/dashboard/merge_requests_spec.rb

@@ -115,9 +115,6 @@ RSpec.describe 'Dashboard Merge Requests' do
       within("span[aria-label='#{n_("%d merge request", "%d merge requests", 3) % 3}']") do
         expect(page).to have_content('3')
       end
-
-      find('.dashboard-shortcuts-merge_requests').click
-
       expect(find('.js-assigned-mr-count')).to have_content('2')
       expect(find('.js-reviewer-mr-count')).to have_content('1')
     end
@@ -168,16 +165,16 @@ RSpec.describe 'Dashboard Merge Requests' do
       expect(page).to have_content('Please select at least one filter to see results')
     end
 
-    it 'shows sorted merge requests', :js do
-      pajamas_sort_by(s_('SortOptions|Created date'))
+    it 'shows sorted merge requests' do
+      sort_by('Created date')
 
       visit merge_requests_dashboard_path(assignee_username: current_user.username)
 
       expect(find('.issues-filters')).to have_content('Created date')
     end
 
-    it 'keeps sorting merge requests after visiting Projects MR page', :js do
-      pajamas_sort_by(s_('SortOptions|Created date'))
+    it 'keeps sorting merge requests after visiting Projects MR page' do
+      sort_by('Created date')
 
       visit project_merge_requests_path(project)
 

spec/features/issuables/sorting_list_spec.rb

@@ -88,14 +88,14 @@ RSpec.describe 'Sort Issuable List' do
       end
     end
 
-    context 'custom sorting', :js do
+    context 'custom sorting' do
       let(:issuable_type) { :merge_request }
 
       it 'supports sorting in asc and desc order' do
         visit_merge_requests_with_state(project, 'open')
 
         click_button('Created date')
-        find('.dropdown-item', text: 'Updated date').click
+        click_link('Updated date')
 
         expect(first_merge_request).to include(last_updated_issuable.title)
         expect(last_merge_request).to include(first_updated_issuable.title)

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -556,7 +556,7 @@ RSpec.describe 'Filter issues', :js do
         sort_toggle = find('.filter-dropdown-container .dropdown')
         sort_toggle.click
 
-        find('.dropdown-item', text: 'Created date').click
+        find('.filter-dropdown-container .dropdown-menu li a', text: 'Created date').click
         wait_for_requests
 
         expect(find('.issues-list .issue:first-of-type .issue-title-text a')).to have_content(new_issue.title)

spec/features/issues/user_sorts_issues_spec.rb

@@ -5,7 +5,6 @@ require "spec_helper"
 RSpec.describe "User sorts issues" do
   include SortingHelper
   include IssueHelpers
-  include Spec::Support::Helpers::Features::SortingHelpers
 
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
@@ -25,22 +24,26 @@ RSpec.describe "User sorts issues" do
     sign_in(user)
   end
 
-  it 'keeps the sort option', :js do
+  it 'keeps the sort option' do
     visit(project_issues_path(project))
 
-    pajamas_sort_by(s_('SortOptions|Milestone'))
+    find('.filter-dropdown-container .dropdown').click
+
+    page.within('ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
 
     visit(issues_dashboard_path(assignee_username: user.username))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(project_issues_path(project))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(issues_group_path(group))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
   end
 
   it 'sorts by popularity', :js do

spec/features/merge_requests/user_lists_merge_requests_spec.rb

@@ -109,7 +109,7 @@ RSpec.describe 'Merge requests > User lists merge requests' do
     expect(count_merge_requests).to eq(4)
   end
 
-  it 'sorts by milestone due date' do
+  it 'sorts by milestone' do
     visit_merge_requests(project, sort: sort_value_milestone)
 
     expect(first_merge_request).to include('fix')
@@ -130,12 +130,12 @@ RSpec.describe 'Merge requests > User lists merge requests' do
     expect(count_merge_requests).to eq(4)
   end
 
-  it 'filters on one label and sorts by milestone due date' do
+  it 'filters on one label and sorts by due date' do
     label = create(:label, project: project)
     create(:label_link, label: label, target: @fix)
 
     visit_merge_requests(project, label_name: [label.name],
-                                  sort: sort_value_milestone)
+                                  sort: sort_value_due_date)
 
     expect(first_merge_request).to include('fix')
     expect(count_merge_requests).to eq(1)
@@ -150,19 +150,19 @@ RSpec.describe 'Merge requests > User lists merge requests' do
       create(:label_link, label: label2, target: @fix)
     end
 
-    it 'sorts by milestone due date' do
+    it 'sorts by due date' do
       visit_merge_requests(project, label_name: [label.name, label2.name],
-                                    sort: sort_value_milestone)
+                                    sort: sort_value_due_date)
 
       expect(first_merge_request).to include('fix')
       expect(count_merge_requests).to eq(1)
     end
 
     context 'filter on assignee and' do
-      it 'sorts by milestone due date' do
+      it 'sorts by due soon' do
         visit_merge_requests(project, label_name: [label.name, label2.name],
                                       assignee_id: user.id,
-                                      sort: sort_value_milestone)
+                                      sort: sort_value_due_date)
 
         expect(first_merge_request).to include('fix')
         expect(count_merge_requests).to eq(1)

spec/features/merge_requests/user_sorts_merge_requests_spec.rb

@@ -2,9 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe 'User sorts merge requests', :js do
+RSpec.describe 'User sorts merge requests' do
   include CookieHelper
-  include Spec::Support::Helpers::Features::SortingHelpers
 
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let!(:merge_request2) do
@@ -23,19 +22,23 @@ RSpec.describe 'User sorts merge requests', :js do
   end
 
   it 'keeps the sort option' do
-    pajamas_sort_by(s_('SortOptions|Milestone'))
+    find('.filter-dropdown-container .dropdown').click
+
+    page.within('ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
 
     visit(merge_requests_dashboard_path(assignee_username: user.username))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(project_merge_requests_path(project))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(merge_requests_group_path(group))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
   end
 
   it 'fallbacks to issuable_sort cookie key when remembering the sorting option' do
@@ -43,13 +46,17 @@ RSpec.describe 'User sorts merge requests', :js do
 
     visit(merge_requests_dashboard_path(assignee_username: user.username))
 
-    expect(find('.filter-dropdown-container button.dropdown-toggle')).to have_content('Milestone')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
   end
 
   it 'separates remember sorting with issues' do
     create(:issue, project: project)
 
-    pajamas_sort_by(s_('SortOptions|Milestone'))
+    find('.filter-dropdown-container .dropdown').click
+
+    page.within('ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
 
     visit(project_issues_path(project))
 
@@ -66,7 +73,11 @@ RSpec.describe 'User sorts merge requests', :js do
     end
 
     it 'sorts by popularity' do
-      pajamas_sort_by(s_('SortOptions|Popularity'))
+      find('.filter-dropdown-container .dropdown').click
+
+      page.within('ul.dropdown-menu.dropdown-menu-right li') do
+        click_link('Popularity')
+      end
 
       page.within('.mr-list') do
         page.within('li.merge-request:nth-child(1)') do

spec/features/user_sorts_things_spec.rb

@@ -6,7 +6,7 @@ require "spec_helper"
 # to check if the sorting option set by user is being kept persisted while going through pages.
 # The `it`s are named here by convention `starting point -> some pages -> final point`.
 # All those specs are moved out to this spec intentionally to keep them all in one place.
-RSpec.describe "User sorts things", :js do
+RSpec.describe "User sorts things" do
   include Spec::Support::Helpers::Features::SortingHelpers
   include DashboardHelper
 
@@ -21,11 +21,11 @@ RSpec.describe "User sorts things", :js do
   end
 
   it "issues -> project home page -> issues" do
-    sort_option = s_('SortOptions|Updated date')
+    sort_option = 'Updated date'
 
     visit(project_issues_path(project))
 
-    pajamas_sort_by(sort_option)
+    sort_by(sort_option)
 
     visit(project_path(project))
     visit(project_issues_path(project))
@@ -34,11 +34,11 @@ RSpec.describe "User sorts things", :js do
   end
 
   it "merge requests -> dashboard merge requests" do
-    sort_option = s_('SortOptions|Updated date')
+    sort_option = 'Updated date'
 
     visit(project_merge_requests_path(project))
 
-    pajamas_sort_by(sort_option)
+    sort_by(sort_option)
 
     visit(assigned_mrs_dashboard_path)
 

spec/support/helpers/features/sorting_helpers.rb

@@ -21,14 +21,6 @@ module Spec
               click_link(value)
             end
           end
-
-          # pajamas_sort_by is used to sort new pajamas dropdowns. When
-          # all of the dropdowns are converted, pajamas_sort_by can be renamed to sort_by
-          # https://gitlab.com/groups/gitlab-org/-/epics/7551
-          def pajamas_sort_by(value)
-            find('.filter-dropdown-container .dropdown').click
-            find('.dropdown-item', text: value).click
-          end
         end
       end
     end