Merge branch 'labels-placement' into 'master'

Move Labels and Milestones as sub tab to Issues/MR

## What does this MR do?

UI/UX change. Moves project labels and milestone tabs under Issues and Merge request tabs. 

## Why was this MR needed?

To solve problem of having to many tabs in top navigation. Since Labels and Milestones depends on Issue/MR feature it belongs to it and can be removed from top nav.

## What are the relevant issue numbers?

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/18368

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - not needed
- [x] API support added - not needed
- [x] Tests
  - [x] Added for this feature/bug - changed existing tests to match new behaviour
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)

cc @jschatz1 @annabeldunstone @skyruler

See merge request !4592

CHANGELOG

@@ -64,6 +64,7 @@ v 8.9.0 (unreleased)
   - Toggling a task list item in a issue/mr description does not creates a Todo for mentions
   - Improved UX of date pickers on issue & milestone forms
   - Cache on the database if a project has an active external issue tracker.
+  - Put project Labels and Milestones pages links under Issues and Merge Requests tabs as subnav
 
 v 8.8.5 (unreleased)
   - Ensure branch cleanup regardless of whether the GitHub import process succeeds

app/views/layouts/nav/_project.html.haml

@@ -33,7 +33,7 @@
         = navbar_icon('activity')
         %span
           Activity
-          
+
     - if project_nav_tab? :files
       = nav_link(controller: %w(tree blob blame edit_tree new_tree find_file commit commits compare repositories tags branches releases network)) do
         = link_to project_files_path(@project), title: 'Code',  class: 'shortcuts-tree' do
@@ -62,15 +62,8 @@
           %span
             Graphs
 
-    - if project_nav_tab? :milestones
-      = nav_link(controller: :milestones) do
-        = link_to namespace_project_milestones_path(@project.namespace, @project), title: 'Milestones' do
-          = navbar_icon('milestones')
-          %span
-            Milestones
-
     - if project_nav_tab? :issues
-      = nav_link(controller: :issues) do
+      = nav_link(controller: [:issues, :labels, :milestones]) do
         = link_to url_for_project_issues(@project, only_path: true), title: 'Issues', class: 'shortcuts-issues' do
           = navbar_icon('issues')
           %span
@@ -86,13 +79,6 @@
             Merge Requests
             %span.badge.count.merge_counter= number_with_delimiter(@project.merge_requests.opened.count)
 
-    - if project_nav_tab? :labels
-      = nav_link(controller: :labels) do
-        = link_to namespace_project_labels_path(@project.namespace, @project), title: 'Labels' do
-          = icon('tags fw')
-          %span
-            Labels
-
     - if project_nav_tab? :wiki
       = nav_link(controller: :wikis) do
         = link_to get_project_wiki_path(@project), title: 'Wiki', class: 'shortcuts-wiki' do

app/views/projects/issues/_head.html.haml

@@ -0,0 +1,25 @@
+%ul.nav-links.sub-nav
+  %div{ class: (container_class) }
+    - if project_nav_tab?(:issues) && !current_controller?(:merge_requests)
+      = nav_link(controller: :issues) do
+        = link_to url_for_project_issues(@project, only_path: true), title: 'Issues' do
+          %span
+            Issues
+
+    - if project_nav_tab?(:merge_requests) && current_controller?(:merge_requests)
+      = nav_link(controller: :merge_requests) do
+        = link_to namespace_project_merge_requests_path(@project.namespace, @project), title: 'Merge Requests' do
+          %span
+            Merge Requests
+
+    - if project_nav_tab? :labels
+      = nav_link(controller: :labels) do
+        = link_to namespace_project_labels_path(@project.namespace, @project), title: 'Labels' do
+          %span
+            Labels
+
+    - if project_nav_tab? :milestones
+      = nav_link(controller: :milestones) do
+        = link_to namespace_project_milestones_path(@project.namespace, @project), title: 'Milestones' do
+          %span
+            Milestones

app/views/projects/issues/index.html.haml

@@ -1,23 +1,26 @@
+- @no_container = true
 - page_title "Issues"
+= render "projects/issues/head"
 
 = content_for :meta_tags do
   - if current_user
     = auto_discovery_link_tag(:atom, namespace_project_issues_url(@project.namespace, @project, :atom, private_token: current_user.private_token), title: "#{@project.name} issues")
 
-.top-area
-  = render 'shared/issuable/nav', type: :issues
-  .nav-controls
-    - if current_user
-      = link_to namespace_project_issues_path(@project.namespace, @project, :atom, { private_token: current_user.private_token }), class: 'btn append-right-10' do
-        = icon('rss')
-        %span.icon-label
-          Subscribe
-    = render 'shared/issuable/search_form', path: namespace_project_issues_path(@project.namespace, @project)
-    - if can? current_user, :create_issue, @project
-      = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { assignee_id: @issuable_finder.assignee.try(:id), milestone_id: @issuable_finder.milestones.try(:first).try(:id) }), class: "btn btn-new", title: "New Issue", id: "new_issue_link" do
-        New Issue
+%div{ class: (container_class) }
+  .top-area
+    = render 'shared/issuable/nav', type: :issues
+    .nav-controls
+      - if current_user
+        = link_to namespace_project_issues_path(@project.namespace, @project, :atom, { private_token: current_user.private_token }), class: 'btn append-right-10' do
+          = icon('rss')
+          %span.icon-label
+            Subscribe
+      = render 'shared/issuable/search_form', path: namespace_project_issues_path(@project.namespace, @project)
+      - if can? current_user, :create_issue, @project
+        = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { assignee_id: @issuable_finder.assignee.try(:id), milestone_id: @issuable_finder.milestones.try(:first).try(:id) }), class: "btn btn-new", title: "New Issue", id: "new_issue_link" do
+          New Issue
 
-= render 'shared/issuable/filter', type: :issues
+  = render 'shared/issuable/filter', type: :issues
 
-.issues-holder
-  = render "issues"
+  .issues-holder
+    = render "issues"

app/views/projects/labels/index.html.haml

@@ -1,35 +1,38 @@
+- @no_container = true
 - page_title "Labels"
 - hide_class = ''
+= render "projects/issues/head"
 
-.top-area
-  .nav-text
-    Labels can be applied to issues and merge requests.
-  .nav-controls
-    - if can?(current_user, :admin_label, @project)
-      = link_to new_namespace_project_label_path(@project.namespace, @project), class: "btn btn-new" do
-        New label
+%div{ class: (container_class) }
+  .top-area
+    .nav-text
+      Labels can be applied to issues and merge requests.
+    .nav-controls
+      - if can?(current_user, :admin_label, @project)
+        = link_to new_namespace_project_label_path(@project.namespace, @project), class: "btn btn-new" do
+          New label
 
-.labels
-  - if can?(current_user, :admin_label, @project)
-    -# Only show it in the first page
-    - hide = @project.labels.empty? || (params[:page].present? && params[:page] != '1')
-    .prioritized-labels{ class: ('hide' if hide) }
-      %h5 Prioritized Labels
-      %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) }
-        - if @prioritized_labels.present?
-          = render @prioritized_labels
-        - else
-          %p.empty-message No prioritized labels yet
-  .other-labels
+  .labels
     - if can?(current_user, :admin_label, @project)
-      %h5{ class: ('hide' if hide) } Other Labels
-    - if @labels.present?
-      %ul.content-list.manage-labels-list.js-other-labels
-        = render @labels
-      = paginate @labels, theme: 'gitlab'
-    - else
-      .nothing-here-block
-        - if can?(current_user, :admin_label, @project)
-          Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}.
-        - else
-          No labels created
+      -# Only show it in the first page
+      - hide = @project.labels.empty? || (params[:page].present? && params[:page] != '1')
+      .prioritized-labels{ class: ('hide' if hide) }
+        %h5 Prioritized Labels
+        %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) }
+          - if @prioritized_labels.present?
+            = render @prioritized_labels
+          - else
+            %p.empty-message No prioritized labels yet
+    .other-labels
+      - if can?(current_user, :admin_label, @project)
+        %h5{ class: ('hide' if hide) } Other Labels
+      - if @labels.present?
+        %ul.content-list.manage-labels-list.js-other-labels
+          = render @labels
+        = paginate @labels, theme: 'gitlab'
+      - else
+        .nothing-here-block
+          - if can?(current_user, :admin_label, @project)
+            Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}.
+          - else
+            No labels created

app/views/projects/merge_requests/_head.html.haml

@@ -1,5 +0,0 @@
-.top-tabs
-  = link_to namespace_project_merge_requests_path(@project.namespace, @project), class: "tab #{'active' if current_page?(namespace_project_merge_requests_path(@project.namespace, @project)) }" do
-    %span
-    Merge Requests
-

app/views/projects/merge_requests/index.html.haml

@@ -1,18 +1,20 @@
+- @no_container = true
 - page_title "Merge Requests"
-
+= render "projects/issues/head"
 = render 'projects/last_push'
 
-.top-area
-  = render 'shared/issuable/nav', type: :merge_requests
-  .nav-controls
-    = render 'shared/issuable/search_form', path: namespace_project_merge_requests_path(@project.namespace, @project)
+%div{ class: (container_class) }
+  .top-area
+    = render 'shared/issuable/nav', type: :merge_requests
+    .nav-controls
+      = render 'shared/issuable/search_form', path: namespace_project_merge_requests_path(@project.namespace, @project)
 
-    - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
-    - if merge_project
-      = link_to new_namespace_project_merge_request_path(merge_project.namespace, merge_project), class: "btn btn-new", title: "New Merge Request" do
-        New Merge Request
+      - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+      - if merge_project
+        = link_to new_namespace_project_merge_request_path(merge_project.namespace, merge_project), class: "btn btn-new", title: "New Merge Request" do
+          New Merge Request
 
-= render 'shared/issuable/filter', type: :merge_requests
+  = render 'shared/issuable/filter', type: :merge_requests
 
-.merge-requests-holder
-  = render 'merge_requests'
+  .merge-requests-holder
+    = render 'merge_requests'

app/views/projects/milestones/index.html.haml

@@ -1,19 +1,22 @@
+- @no_container = true
 - page_title "Milestones"
+= render "projects/issues/head"
 
-.top-area
-  = render 'shared/milestones_filter'
+%div{ class: (container_class) }
+  .top-area
+    = render 'shared/milestones_filter'
 
-  .nav-controls
-    - if can?(current_user, :admin_milestone, @project)
-      = link_to new_namespace_project_milestone_path(@project.namespace, @project), class: "btn btn-new", title: "New Milestone" do
-        New Milestone
+    .nav-controls
+      - if can?(current_user, :admin_milestone, @project)
+        = link_to new_namespace_project_milestone_path(@project.namespace, @project), class: "btn btn-new", title: "New Milestone" do
+          New Milestone
 
-.milestones
-  %ul.content-list
-    = render @milestones
+  .milestones
+    %ul.content-list
+      = render @milestones
 
-    - if @milestones.blank?
-      %li
-        .nothing-here-block No milestones to show
+      - if @milestones.blank?
+        %li
+          .nothing-here-block No milestones to show
 
-  = paginate @milestones, theme: "gitlab"
+    = paginate @milestones, theme: "gitlab"

features/project/active_tab.feature

@@ -107,12 +107,16 @@ Feature: Project Active Tab
 
   Scenario: On Project Issues/Milestones
     Given I visit my project's issues page
-    And I click the "Milestones" tab
-    Then the active main tab should be Milestones
+    And I click the "Milestones" sub tab
+    Then the active main tab should be Issues
+    Then the active sub tab should be Milestones
     And no other main tabs should be active
+    And no other sub tabs should be active
 
   Scenario: On Project Issues/Labels
     Given I visit my project's issues page
-    And I click the "Labels" tab
-    Then the active main tab should be Labels
+    And I click the "Labels" sub tab
+    Then the active main tab should be Issues
+    Then the active sub tab should be Labels
     And no other main tabs should be active
+    And no other sub tabs should be active

features/steps/project/active_tab.rb

@@ -77,14 +77,14 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps
 
   # Sub Tabs: Issues
 
-  step 'I click the "Milestones" tab' do
-    page.within('.layout-nav') do
+  step 'I click the "Milestones" sub tab' do
+    page.within('.sub-nav') do
       click_link('Milestones')
     end
   end
 
-  step 'I click the "Labels" tab' do
-    page.within('.layout-nav') do
+  step 'I click the "Labels" sub tab' do
+    page.within('.sub-nav') do
       click_link('Labels')
     end
   end
@@ -93,11 +93,11 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps
     ensure_active_sub_tab('Issues')
   end
 
-  step 'the active main tab should be Milestones' do
-    ensure_active_main_tab('Milestones')
+  step 'the active sub tab should be Milestones' do
+    ensure_active_sub_tab('Milestones')
   end
 
-  step 'the active main tab should be Labels' do
-    ensure_active_main_tab('Labels')
+  step 'the active sub tab should be Labels' do
+    ensure_active_sub_tab('Labels')
   end
 end

features/steps/shared/active_tab.rb

@@ -6,7 +6,7 @@ module SharedActiveTab
   end
 
   def ensure_active_sub_tab(content)
-    expect(find('div.content ul.nav-links li.active')).to have_content(content)
+    expect(find('.sub-nav li.active')).to have_content(content)
   end
 
   def ensure_active_sub_nav(content)
@@ -18,7 +18,7 @@ module SharedActiveTab
   end
 
   step 'no other sub tabs should be active' do
-    expect(page).to have_selector('div.content ul.nav-links li.active', count: 1)
+    expect(page).to have_selector('.sub-nav li.active', count: 1)
   end
 
   step 'no other sub navs should be active' do