Make milestones sort dropdown Pajamas friendly

app/helpers/sorting_helper.rb

@@ -134,14 +134,14 @@ module SortingHelper
     )
   end
 
-  def milestone_sort_options_hash
+  def milestones_sort_options_hash
     {
-      sort_value_name             => sort_title_name_asc,
-      sort_value_name_desc        => sort_title_name_desc,
-      sort_value_due_date_later   => sort_title_due_date_later,
       sort_value_due_date_soon    => sort_title_due_date_soon,
+      sort_value_due_date_later   => sort_title_due_date_later,
+      sort_value_start_date_soon  => sort_title_start_date_soon,
       sort_value_start_date_later => sort_title_start_date_later,
-      sort_value_start_date_soon  => sort_title_start_date_soon
+      sort_value_name             => sort_title_name_asc,
+      sort_value_name_desc        => sort_title_name_desc
     }
   end
 

app/views/shared/_milestones_sort_dropdown.html.haml

-.dropdown.inline.gl-ml-3
-  %button.dropdown-menu-toggle{ type: 'button', data: { toggle: 'dropdown' } }
-    %span.light
-    - if @sort.present?
-      = milestone_sort_options_hash[@sort]
-    - else
-      = sort_title_due_date_soon
-    = sprite_icon('chevron-down', css_class: 'dropdown-menu-toggle-icon gl-top-3')
-  %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-sort
-    %li
-      = link_to page_filter_path(sort: sort_value_due_date_soon) do
-        = sort_title_due_date_soon
-      = link_to page_filter_path(sort: sort_value_due_date_later) do
-        = sort_title_due_date_later
-      = link_to page_filter_path(sort: sort_value_start_date_soon) do
-        = sort_title_start_date_soon
-      = link_to page_filter_path(sort: sort_value_start_date_later) do
-        = sort_title_start_date_later
-      = link_to page_filter_path(sort: sort_value_name) do
-        = sort_title_name_asc
-      = link_to page_filter_path(sort: sort_value_name_desc) do
-        = sort_title_name_desc
+- milestones_sort_options = milestones_sort_options_hash.map { |value, text| { value: value, text: text, href: page_filter_path(sort: value) } }
+
+%div{ data: {testid: 'milestone_sort_by_dropdown'} }
+  = gl_redirect_listbox_tag milestones_sort_options, @sort, class: 'gl-ml-3'

spec/features/groups/milestones_sorting_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe 'Milestones sorting', :js do
     sign_in(user)
   end
 
-  it 'visit group milestones and sort by due_date_asc' do
+  it 'visit group milestones and sort by due_date_asc', :js do
     visit group_milestones_path(group)
 
     expect(page).to have_button('Due soon')
@@ -27,13 +27,13 @@ RSpec.describe 'Milestones sorting', :js do
       expect(page.all('ul.content-list > li strong > a').map(&:text)).to eq(['v2.0', 'v2.0', 'v3.0', 'v1.0', 'v1.0'])
     end
 
+    within '[data-testid=milestone_sort_by_dropdown]' do
       click_button 'Due soon'
+      expect(find('.gl-new-dropdown-contents').all('.gl-new-dropdown-item-text-wrapper p').map(&:text)).to eq(['Due soon', 'Due later', 'Start soon', 'Start later', 'Name, ascending', 'Name, descending'])
 
-    expect(find('ul.dropdown-menu-sort li').all('a').map(&:text)).to eq(['Due soon', 'Due later', 'Start soon', 'Start later', 'Name, ascending', 'Name, descending'])
-
-    click_link 'Due later'
-
+      click_button 'Due later'
       expect(page).to have_button('Due later')
+    end
 
     # assert descending sorting
     within '.milestones' do

spec/features/projects/milestones/milestones_sorting_spec.rb

@@ -5,49 +5,55 @@ require 'spec_helper'
 RSpec.describe 'Milestones sorting', :js do
   let(:user)    { create(:user) }
   let(:project) { create(:project, name: 'test', namespace: user.namespace) }
+  let(:milestones_for_sort_by) do
+    {
+      'Due later' => %w[b c a],
+      'Name, ascending' =>  %w[a b c],
+      'Name, descending' => %w[c b a],
+      'Start later' => %w[a c b],
+      'Start soon' => %w[b c a],
+      'Due soon' => %w[a c b]
+    }
+  end
+
+  let(:ordered_milestones) do
+    ['Due soon', 'Due later', 'Start soon', 'Start later', 'Name, ascending', 'Name, descending']
+  end
 
   before do
-    # Milestones
-    create(:milestone,
-      due_date: 10.days.from_now,
-      created_at:  2.hours.ago,
-      title: "aaa", project: project)
-    create(:milestone,
-      due_date: 11.days.from_now,
-      created_at:  1.hour.ago,
-      title: "bbb", project: project)
+    create(:milestone, start_date: 7.days.from_now, due_date: 10.days.from_now, title: "a", project: project)
+    create(:milestone, start_date: 6.days.from_now, due_date: 11.days.from_now, title: "c", project: project)
+    create(:milestone, start_date: 5.days.from_now, due_date: 12.days.from_now, title: "b", project: project)
     sign_in(user)
   end
 
-  it 'visit project milestones and sort by due_date_asc' do
+  it 'visit project milestones and sort by various orders' do
     visit project_milestones_path(project)
 
     expect(page).to have_button('Due soon')
 
-    # assert default sorting
+    # assert default sorting order
     within '.milestones' do
-      expect(page.all('ul.content-list > li').first.text).to include('aaa')
-      expect(page.all('ul.content-list > li').last.text).to include('bbb')
+      expect(page.all('ul.content-list > li strong > a').map(&:text)).to eq(%w[a c b])
     end
 
-    click_button 'Due soon'
-
-    sort_options = find('ul.dropdown-menu-sort li').all('a').collect(&:text)
-
-    expect(sort_options[0]).to eq('Due soon')
-    expect(sort_options[1]).to eq('Due later')
-    expect(sort_options[2]).to eq('Start soon')
-    expect(sort_options[3]).to eq('Start later')
-    expect(sort_options[4]).to eq('Name, ascending')
-    expect(sort_options[5]).to eq('Name, descending')
+    # assert milestones listed for given sort order
+    selected_sort_order = 'Due soon'
+    milestones_for_sort_by.each do |sort_by, expected_milestones|
+      within '[data-testid=milestone_sort_by_dropdown]' do
+        click_button selected_sort_order
+        milestones = find('.gl-new-dropdown-contents').all('.gl-new-dropdown-item-text-wrapper p').map(&:text)
+        expect(milestones).to eq(ordered_milestones)
 
-    click_link 'Due later'
-
-    expect(page).to have_button('Due later')
+        click_button sort_by
+        expect(page).to have_button(sort_by)
+      end
 
       within '.milestones' do
-      expect(page.all('ul.content-list > li').first.text).to include('bbb')
-      expect(page.all('ul.content-list > li').last.text).to include('aaa')
+        expect(page.all('ul.content-list > li strong > a').map(&:text)).to eq(expected_milestones)
+      end
+
+      selected_sort_order = sort_by
     end
   end
 end

spec/views/shared/_milestones_sort_dropdown.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'shared/_milestones_sort_dropdown.html.haml' do
+  describe 'render' do
+    describe 'when a sort option is not selected' do
+      it 'renders a default sort option' do
+        render 'shared/milestones_sort_dropdown'
+
+        expect(rendered).to have_content 'Due soon'
+      end
+    end
+
+    describe 'when a sort option is selected' do
+      before do
+        assign(:sort, 'due_date_desc')
+
+        render 'shared/milestones_sort_dropdown'
+      end
+
+      it 'renders the selected sort option' do
+        expect(rendered).to have_content 'Due later'
+      end
+    end
+  end
+end