Merge branch '39550-group-labels-redesign' into 'master'

Resolve "Group label list page redesign"

Closes #39550

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

9 files changed, 95 insertions, 37 deletions

diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss
index 32b9894ae04..a6b1bf9b099 100644
--- a/app/assets/stylesheets/framework/modal.scss
+++ b/app/assets/stylesheets/framework/modal.scss
@@ -4,6 +4,11 @@
 
   .page-title {
     margin-top: 0;
+
+    .color-label {
+      font-size: $gl-font-size;
+      padding: $gl-vert-padding $label-padding-modal;
+    }
   }
 }
 
diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss
index 1cc22f5658d..0d21a9f5f77 100644
--- a/app/assets/stylesheets/framework/variables.scss
+++ b/app/assets/stylesheets/framework/variables.scss
@@ -558,6 +558,7 @@ $jq-ui-default-color: #777;
 * Label
 */
 $label-padding: 7px;
+$label-padding-modal: 10px;
 $label-gray-bg: #f8fafc;
 $label-inverse-bg: #333;
 $label-remove-border: rgba(0, 0, 0, .1);
diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss
index e8cd8a4905c..a72e654824e 100644
--- a/app/assets/stylesheets/pages/labels.scss
+++ b/app/assets/stylesheets/pages/labels.scss
@@ -58,13 +58,13 @@
 
     @media (min-width: $screen-sm-min) {
       width: 200px;
+      margin-left: $gl-padding * 2;
       margin-bottom: 0;
     }
 
     .label {
       overflow: hidden;
       text-overflow: ellipsis;
-      vertical-align: middle;
       max-width: 100%;
     }
   }
@@ -79,26 +79,33 @@
       width: 100px;
       margin-left: 10px;
       margin-bottom: 0;
-      vertical-align: middle;
+      vertical-align: top;
     }
   }
 
   .label-description {
     display: block;
     margin-bottom: 10px;
-    margin-left: 50px;
+
+    .description-text {
+      margin-bottom: $gl-padding;
+    }
+
+    a {
+      color: $blue-600;
+    }
 
     @media (min-width: $screen-sm-min) {
       display: inline-block;
-      width: 30%;
+      max-width: 50%;
       margin-left: 10px;
       margin-bottom: 0;
-      vertical-align: middle;
+      vertical-align: top;
     }
   }
 
   .label {
-    padding: 8px 9px 9px;
+    padding: 8px 12px;
     font-size: 14px;
   }
 }
@@ -116,6 +123,12 @@
 }
 
 .manage-labels-list {
+  @media(min-width: $screen-md-min) {
+    &.content-list li {
+      padding: $gl-padding 0;
+    }
+  }
+
   > li:not(.empty-message):not(.is-not-draggable) {
     background-color: $white-light;
     cursor: move;
@@ -133,8 +146,6 @@
   }
 
   .btn-action {
-    color: $gl-text-color;
-
     .fa {
       font-size: 18px;
       vertical-align: middle;
@@ -155,10 +166,18 @@
       float: right;
     }
   }
+
+  @media (max-width: $screen-xs-max) {
+    .dropdown-menu {
+      min-width: 100%;
+    }
+  }
 }
 
 .draggable-handler {
   display: inline-block;
+  vertical-align: top;
+  margin: 5px 0;
   opacity: 0;
   transition: opacity .3s;
   color: $gray-darkest;
@@ -188,7 +207,7 @@
 
 .toggle-priority {
   display: inline-block;
-  vertical-align: middle;
+  vertical-align: top;
 
   button {
     border-color: transparent;
@@ -255,6 +274,11 @@
 }
 
 .label-subscribe-button {
+  @media(min-width: $screen-md-min) {
+    min-width: 105px;
+    margin-left: $gl-padding;
+  }
+
   .label-subscribe-button-icon {
     &[disabled] {
       opacity: 0.5;
diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb
index dda59262483..f3a9e591c3e 100644
--- a/app/controllers/groups/labels_controller.rb
+++ b/app/controllers/groups/labels_controller.rb
@@ -54,7 +54,7 @@ class Groups::LabelsController < Groups::ApplicationController
 
     respond_to do |format|
       format.html do
-        redirect_to group_labels_path(@group), status: 302, notice: 'Label was removed'
+        redirect_to group_labels_path(@group), status: 302, notice: "#{@label.name} deleted permanently"
       end
       format.js
     end
diff --git a/app/views/shared/_delete_label_modal.html.haml b/app/views/shared/_delete_label_modal.html.haml
new file mode 100644
index 00000000000..01effefc34d
--- /dev/null
+++ b/app/views/shared/_delete_label_modal.html.haml
@@ -0,0 +1,20 @@
+.modal{ id: "modal-delete-label-#{label.id}", tabindex: -1 }
+  .modal-dialog
+    .modal-content
+      .modal-header
+        %button.close{ data: {dismiss: 'modal' } } &times;
+        %h3.page-title Delete #{render_colored_label(label, tooltip: false)} ?
+
+      .modal-body
+        %p
+          %strong= label.name
+          %span will be permanently deleted from #{label.is_a?(ProjectLabel)? label.project.name : label.group.name}. This cannot be undone.
+
+      .modal-footer
+        %a{ href: '#', data: { dismiss: 'modal' }, class: 'btn btn-default' } Cancel
+
+        = link_to 'Delete label',
+          destroy_label_path(label),
+          title: 'Delete',
+          method: :delete,
+          class: 'btn btn-remove'
diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml
index 8e88cecaf9e..c0eebdfaddd 100644
--- a/app/views/shared/_label.html.haml
+++ b/app/views/shared/_label.html.haml
@@ -5,10 +5,10 @@
 - show_label_merge_requests_link = show_label_issuables_link?(label, :merge_requests, project: @project)
 - show_label_issues_link = show_label_issuables_link?(label, :issues, project: @project)
 
-%li{ id: label_css_id, data: { id: label.id } }
+%li.label-list-item{ id: label_css_id, data: { id: label.id } }
   = render "shared/label_row", label: label
 
-  .visible-xs.visible-sm-inline-block.visible-md-inline-block.dropdown
+  .visible-xs.visible-sm-inline-block.dropdown
     %button.btn.btn-default.label-options-toggle{ type: 'button', data: { toggle: "dropdown" } }
       Options
       = icon('caret-down')
@@ -46,14 +46,19 @@
               data: {confirm: 'Remove this label? Are you sure?'},
               class: 'text-danger'
 
-  .pull-right.hidden-xs.hidden-sm.hidden-md
-    - if show_label_merge_requests_link
-      = link_to_label(label, subject: subject, type: :merge_request, css_class: 'btn btn-transparent btn-action btn-link') do
-        view merge requests
-    - if show_label_issues_link
-      = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action btn-link') do
-        view open issues
-
+  .pull-right.hidden-xs.hidden-sm
+    - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_label, label.project.group)
+      = link_to promote_project_label_path(label.project, label), title: "Promote to Group Label", class: 'btn btn-transparent btn-action', data: {confirm: "You are about to promote #{label.title} to a group level. This will make this milestone available to all projects inside #{label.project.group.name}. The existing project label will be merged into the group level. This action cannot be reversed.", toggle: "tooltip"}, method: :post do
+        %span.sr-only Promote to Group
+        = sprite_icon('level-up')
+    - if can?(current_user, :admin_label, label)
+      = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
+        %span.sr-only Edit
+        = sprite_icon('pencil')
+      %span{ data: { toggle: 'modal', target: "#modal-delete-label-#{label.id}" } }
+        = link_to "#", title: "Delete", class: 'btn btn-transparent btn-action remove-row', data: { toggle: "tooltip" } do
+          %span.sr-only Delete
+          = sprite_icon('remove')
     - if current_user
       .label-subscription.inline
         - if can_subscribe_to_label_in_different_levels?(label)
@@ -76,14 +81,4 @@
             %span= label_subscription_toggle_button_text(label, @project)
             = icon('spinner spin', class: 'label-subscribe-button-loading')
 
-    - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_label, label.project.group)
-      = link_to promote_project_label_path(label.project, label), title: "Promote to Group Label", class: 'btn btn-transparent btn-action', data: {confirm: "You are about to promote #{label.title} to a group level. This will make this milestone available to all projects inside #{label.project.group.name}. The existing project label will be merged into the group level. This action cannot be reversed.", toggle: "tooltip"}, method: :post do
-        %span.sr-only Promote to Group
-        = icon('level-up')
-    - if can?(current_user, :admin_label, label)
-      = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
-        %span.sr-only Edit
-        = icon('pencil-square-o')
-      = link_to destroy_label_path(label), title: "Delete", class: 'btn btn-transparent btn-action remove-row', method: :delete, data: {confirm: label_deletion_confirm_text(label), toggle: "tooltip"} do
-        %span.sr-only Delete
-        = icon('trash-o')
+= render 'shared/delete_label_modal', label: label
diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml
index 7f58298c60f..bd4f191502e 100644
--- a/app/views/shared/_label_row.html.haml
+++ b/app/views/shared/_label_row.html.haml
@@ -1,3 +1,7 @@
+- subject = local_assigns[:subject]
+- show_label_issues_link = show_label_issuables_link?(label, :issues, project: @project)
+- show_label_merge_requests_link = show_label_issuables_link?(label, :merge_requests, project: @project)
+
 %span.label-row
   - if can?(current_user, :admin_label, @project)
     .draggable-handler
@@ -13,6 +17,14 @@
   - if defined?(@project) && @project.group.present?
     %span.label-type
       = label.model_name.human.titleize
-  - if label.description
-    %span.label-description
-      = markdown_field(label, :description)
+
+  %span.label-description
+    - if label.description.present?
+      .description-text
+        = markdown_field(label, :description)
+    .hidden-xs.hidden-sm
+      - if show_label_issues_link
+        = link_to_label(label, subject: subject) { 'Issues' }
+      - if show_label_merge_requests_link
+        &middot;
+        = link_to_label(label, subject: subject, type: :merge_request) { 'Merge requests' }
diff --git a/features/steps/project/issues/labels.rb b/features/steps/project/issues/labels.rb
index 196e0fff63a..4df96e081f9 100644
--- a/features/steps/project/issues/labels.rb
+++ b/features/steps/project/issues/labels.rb
@@ -15,8 +15,9 @@ class Spinach::Features::ProjectIssuesLabels < Spinach::FeatureSteps
 
   step 'I delete all labels' do
     page.within '.labels' do
-      page.all('.remove-row').each do
-        accept_confirm { first('.remove-row').click }
+      page.all('.label-list-item').each do
+        first('.remove-row').click
+        first(:link, 'Delete label').click
       end
     end
   end
diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb
index 85bd776932b..ae8b1364ec7 100644
--- a/spec/features/projects/labels/update_prioritization_spec.rb
+++ b/spec/features/projects/labels/update_prioritization_spec.rb
@@ -99,7 +99,7 @@ feature 'Prioritize labels' do
       expect(page).to have_content 'wontfix'
 
       # Sort labels
-      drag_to(selector: '.js-prioritized-labels', from_index: 1, to_index: 2)
+      drag_to(selector: '.label-list-item', from_index: 1, to_index: 2)
 
       page.within('.prioritized-labels') do
         expect(first('li')).to have_content('feature')