Merge branch '39543-milestone-page-list-redesign' into 'master'

Resolve "Milestone page list redesign"

Closes #39543

See merge request gitlab-org/gitlab-ce!19832

10 files changed, 96 insertions, 87 deletions

diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss
index 326499125fc..218e37602dd 100644
--- a/app/assets/stylesheets/framework/common.scss
+++ b/app/assets/stylesheets/framework/common.scss
@@ -262,12 +262,7 @@ li.note {
 }
 
 .milestone {
-  &.milestone-closed {
-    background: $gray-light;
-  }
-
   .progress {
-    margin-bottom: 0;
     margin-top: 4px;
     box-shadow: none;
     background-color: $border-gray-light;
diff --git a/app/assets/stylesheets/framework/issue_box.scss b/app/assets/stylesheets/framework/issue_box.scss
index 1d247671761..86de88729ee 100644
--- a/app/assets/stylesheets/framework/issue_box.scss
+++ b/app/assets/stylesheets/framework/issue_box.scss
@@ -45,4 +45,9 @@
   &.status-box-upcoming {
     background: $gl-text-color-secondary;
   }
+
+  &.status-box-milestone {
+    color: $gl-text-color;
+    background: $gray-darker;
+  }
 }
diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss
index dba83e56d72..46437ce5841 100644
--- a/app/assets/stylesheets/pages/milestone.scss
+++ b/app/assets/stylesheets/pages/milestone.scss
@@ -3,8 +3,20 @@
 }
 
 .milestones {
+  padding: $gl-padding-8;
+  margin-top: $gl-padding-8;
+  border-radius: $border-radius-default;
+  background-color: $theme-gray-100;
+
   .milestone {
-    padding: 10px 16px;
+    border: 0;
+    padding: $gl-padding-top $gl-padding;
+    border-radius: $border-radius-default;
+    background-color: $white-light;
+
+    &:not(:last-child) {
+      margin-bottom: $gl-padding-4;
+    }
 
     h4 {
       font-weight: $gl-font-weight-bold;
@@ -13,6 +25,24 @@
     .progress {
       width: 100%;
       height: 6px;
+      margin-bottom: $gl-padding-4;
+    }
+
+    .milestone-progress {
+      a {
+        color: $gl-link-color;
+      }
+    }
+
+    .status-box {
+      font-size: $tooltip-font-size;
+      margin-top: 0;
+      margin-right: $gl-padding-4;
+
+      @include media-breakpoint-down(xs) {
+        line-height: unset;
+        padding: $gl-padding-4 $gl-input-padding;
+      }
     }
   }
 }
@@ -229,6 +259,10 @@
   }
 }
 
+.milestone-range {
+  color: $gl-text-color-tertiary;
+}
+
 @include media-breakpoint-down(xs) {
   .milestone-banner-text,
   .milestone-banner-link {
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index f85dcfe6bfc..594563d1f6f 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -77,7 +77,7 @@ class Projects::MilestonesController < Projects::ApplicationController
   def promote
     promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
 
-    flash[:notice] = "#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, promoted_milestone.iid)}\">group milestone</a>.".html_safe
+    flash[:notice] = "#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, promoted_milestone.iid)}\"><u>group milestone</u></a>.".html_safe
     respond_to do |format|
       format.html do
         redirect_to project_milestones_path(project)
diff --git a/app/views/shared/_milestone_expired.html.haml b/app/views/shared/_milestone_expired.html.haml
index 5e9007aaaac..099e3ac8462 100644
--- a/app/views/shared/_milestone_expired.html.haml
+++ b/app/views/shared/_milestone_expired.html.haml
@@ -1,7 +1,6 @@
 - if milestone.expired? and not milestone.closed?
-  %span.cred (Expired)
+  .status-box.status-box-expired.append-bottom-5 Expired
 - if milestone.upcoming?
-  %span.clgray (Upcoming)
-- if milestone.due_date || milestone.start_date
-  %span
-    = milestone_date_range(milestone)
+  .status-box.status-box-mr-merged.append-bottom-5 Upcoming
+- if milestone.closed?
+  .status-box.status-box-closed.append-bottom-5 Closed
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index 09bbd04c2bf..c559945a9c9 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -1,76 +1,59 @@
 - dashboard = local_assigns[:dashboard]
 - custom_dom_id = dom_id(milestone.try(:milestones) ? milestone.milestones.first : milestone)
+- milestone_type = milestone.group_milestone? ? 'Group Milestone' : 'Project Milestone'
 
 %li{ class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: custom_dom_id }
   .row
     .col-sm-6
-      %strong= link_to truncate(milestone.title, length: 100), milestone_path
-      - if milestone.group_milestone?
-        %span - Group Milestone
-      - else
-        %span - Project Milestone
+      .append-bottom-5
+        %strong= link_to truncate(milestone.title, length: 100), milestone_path
+        - if @group
+          = " - #{milestone_type}"
 
-    .col-sm-6
-      .float-right.light #{milestone.percent_complete(current_user)}% complete
-  .row
-    .col-sm-6
+      - if @project || milestone.is_a?(GlobalMilestone) || milestone.group_milestone?
+        - if milestone.due_date || milestone.start_date
+          .milestone-range.append-bottom-5
+            = milestone_date_range(milestone)
+        %div
+          = render('shared/milestone_expired', milestone: milestone)
+          - if milestone.legacy_group_milestone?
+            .projects
+              - milestone.milestones.each do |milestone|
+                = link_to milestone_path(milestone) do
+                  %span.label-badge.label-badge-blue.d-inline-block.append-bottom-5
+                    = dashboard ? milestone.project.full_name : milestone.project.name
+
+    .col-sm-4.milestone-progress
+      = milestone_progress_bar(milestone)
       = link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path
       &middot;
       = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path
-    .col-sm-6= milestone_progress_bar(milestone)
-  - if milestone.is_a?(GlobalMilestone) || milestone.group_milestone?
-    .row
-      .col-sm-6
-        - if milestone.legacy_group_milestone?
-          .expiration= render('shared/milestone_expired', milestone: milestone)
-          .projects
-            - milestone.milestones.each do |milestone|
-              = link_to milestone_path(milestone) do
-                %span.badge.badge-gray
-                  = dashboard ? milestone.project.full_name : milestone.project.name
-      - if @group
-        .col-sm-6.milestone-actions
+      .float-lg-right.light #{milestone.percent_complete(current_user)}% complete
+    .col-sm-2
+      .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
+        - if @project
+          - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
+            - if @project.group
+              %button.js-promote-project-milestone-button.btn.btn-blank.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'),
+                disabled: true,
+                type: 'button',
+                data: { url: promote_project_milestone_path(milestone.project, milestone),
+                        milestone_title: milestone.title,
+                        group_name: @project.group.name,
+                        target: '#promote-milestone-modal',
+                        container: 'body',
+                        toggle: 'modal' } }
+                = sprite_icon('level-up', size: 14)
+
+            = link_to 'Close Milestone', project_milestone_path(@project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-sm btn-close btn-grouped"
+          - unless milestone.active?
+            = link_to 'Reopen Milestone', project_milestone_path(@project, milestone, {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
+        - if @group
           - if can?(current_user, :admin_milestones, @group)
-            - if milestone.group_milestone?
-              = link_to edit_group_milestone_path(@group, milestone), class: "btn btn-sm btn-grouped" do
-                Edit
-              \
             - if milestone.closed?
               = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen"
             - else
               = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-sm btn-grouped btn-close"
-
-  - if @project
-    .row
-      .col-sm-6
-        = render('shared/milestone_expired', milestone: milestone)
-      .col-sm-6.milestone-actions
-        - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-          = link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-sm btn-grouped" do
-            Edit
-          \
-
-          - if @project.group
-            %button.js-promote-project-milestone-button.btn.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'),
-              disabled: true,
-              type: 'button',
-              data: { url: promote_project_milestone_path(milestone.project, milestone),
-                      milestone_title: milestone.title,
-                      group_name: @project.group.name,
-                      target: '#promote-milestone-modal',
-                      container: 'body',
-                      toggle: 'modal' } }
-              = _('Promote')
-
-          = link_to 'Close Milestone', project_milestone_path(@project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-sm btn-close btn-grouped"
-
-          %button.js-delete-milestone-button.btn.btn-sm.btn-grouped.btn-danger{ data: { toggle: 'modal',
-            target: '#delete-milestone-modal',
-            milestone_id: milestone.id,
-            milestone_title: markdown_field(milestone, :title),
-            milestone_url: project_milestone_path(milestone.project, milestone),
-            milestone_issue_count: milestone.issues.count,
-            milestone_merge_request_count: milestone.merge_requests.count },
-            disabled: true }
-            = _('Delete')
-            = icon('spin spinner', class: 'js-loading-icon hidden' )
+        - if dashboard
+          .status-box.status-box-milestone
+            = milestone_type
diff --git a/changelogs/unreleased/39543-milestone-page-list-redesign.yml b/changelogs/unreleased/39543-milestone-page-list-redesign.yml
new file mode 100644
index 00000000000..dcd73c5eddf
--- /dev/null
+++ b/changelogs/unreleased/39543-milestone-page-list-redesign.yml
@@ -0,0 +1,5 @@
+---
+title: Milestone page list redesign
+merge_request: 19832
+author: Constance Okoghenun
+type: changed
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 02b30f9bc6d..b1d83246238 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -124,7 +124,7 @@ describe Projects::MilestonesController do
       it 'shows group milestone' do
         post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
 
-        expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\">group milestone</a>.")
+        expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\"><u>group milestone</u></a>.")
         expect(response).to redirect_to(project_milestones_path(project))
       end
     end
diff --git a/spec/features/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb
index 20337f1d3b0..2108d763028 100644
--- a/spec/features/groups/milestone_spec.rb
+++ b/spec/features/groups/milestone_spec.rb
@@ -107,19 +107,6 @@ feature 'Group milestones' do
         expect(page).to have_selector("#milestone_#{legacy_milestone.milestones.first.id}", count: 1)
       end
 
-      it 'updates milestone' do
-        page.within(".milestones #milestone_#{active_group_milestone.id}") do
-          click_link('Edit')
-        end
-
-        page.within('.milestone-form') do
-          fill_in 'milestone_title', with: 'new title'
-          click_button('Update milestone')
-        end
-
-        expect(find('#content-body h2')).to have_content('new title')
-      end
-
       it 'shows milestone detail and supports its edit' do
         page.within(".milestones #milestone_#{active_group_milestone.id}") do
           click_link(active_group_milestone.title)
diff --git a/spec/features/milestones/user_deletes_milestone_spec.rb b/spec/features/milestones/user_deletes_milestone_spec.rb
index 414702daba4..9d4a68239d3 100644
--- a/spec/features/milestones/user_deletes_milestone_spec.rb
+++ b/spec/features/milestones/user_deletes_milestone_spec.rb
@@ -13,6 +13,7 @@ describe "User deletes milestone", :js do
   end
 
   it "deletes milestone" do
+    click_link(milestone.title)
     click_button("Delete")
     click_button("Delete milestone")