Merge branch 'remove-jquery-ui-sortable' into 'master'

Removed jQuery UI sortable

See merge request !8478

app/assets/javascripts/application.js

@@ -37,6 +37,7 @@ require('bootstrap/js/popover');
 require('select2/select2.js');
 window._ = require('underscore');
 window.Dropzone = require('dropzone');
+window.Sortable = require('vendor/Sortable');
 require('mousetrap');
 require('mousetrap/plugins/pause/mousetrap-pause');
 require('./shortcuts');

app/assets/javascripts/boards/boards_bundle.js.es6

@@ -6,7 +6,6 @@ function requireAll(context) { return context.keys().map(context); }
 
 window.Vue = require('vue');
 window.Vue.use(require('vue-resource'));
-window.Sortable = require('vendor/Sortable');
 requireAll(require.context('./models',   true, /^\.\/.*\.(js|es6)$/));
 requireAll(require.context('./stores',   true, /^\.\/.*\.(js|es6)$/));
 requireAll(require.context('./services', true, /^\.\/.*\.(js|es6)$/));

app/assets/javascripts/label_manager.js.es6

 /* eslint-disable comma-dangle, class-methods-use-this, no-underscore-dangle, no-param-reassign, no-unused-vars, consistent-return, func-names, space-before-function-paren, max-len */
 /* global Flash */
+/* global Sortable */
 
 ((global) => {
   class LabelManager {
@@ -9,11 +10,12 @@
       this.otherLabels = otherLabels || $('.js-other-labels');
       this.errorMessage = 'Unable to update label prioritization at this time';
       this.emptyState = document.querySelector('#js-priority-labels-empty-state');
-      this.prioritizedLabels.sortable({
-        items: 'li',
-        placeholder: 'list-placeholder',
-        axis: 'y',
-        update: this.onPrioritySortUpdate.bind(this)
+      this.sortable = Sortable.create(this.prioritizedLabels.get(0), {
+        filter: '.empty-message',
+        forceFallback: true,
+        fallbackClass: 'is-dragging',
+        dataIdAttr: 'data-id',
+        onUpdate: this.onPrioritySortUpdate.bind(this),
       });
       this.bindEvents();
     }
@@ -51,13 +53,13 @@
         $target = this.otherLabels;
         $from = this.prioritizedLabels;
       }
-      if ($from.find('li').length === 1) {
+      $label.detach().appendTo($target);
+      if ($from.find('li').length) {
         $from.find('.empty-message').removeClass('hidden');
       }
-      if (!$target.find('li').length) {
+      if ($target.find('> li:not(.empty-message)').length) {
         $target.find('.empty-message').addClass('hidden');
       }
-      $label.detach().appendTo($target);
       // Return if we are not persisting state
       if (!persistState) {
         return;
@@ -101,8 +103,12 @@
 
     getSortedLabelsIds() {
       const sortedIds = [];
-      this.prioritizedLabels.find('li').each(function() {
-        sortedIds.push($(this).data('id'));
+      this.prioritizedLabels.find('> li').each(function() {
+        const id = $(this).data('id');
+
+        if (id) {
+          sortedIds.push(id);
+        }
       });
       return sortedIds;
     }

app/assets/javascripts/milestone.js

 /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-use-before-define, camelcase, quotes, object-shorthand, no-shadow, no-unused-vars, comma-dangle, no-var, prefer-template, no-underscore-dangle, consistent-return, one-var, one-var-declaration-per-line, default-case, prefer-arrow-callback, max-len */
 /* global Flash */
+/* global Sortable */
 
 (function() {
   this.Milestone = (function() {
@@ -8,11 +9,9 @@
         type: "PUT",
         url: issue_url,
         data: data,
-        success: (function(_this) {
-          return function(_data) {
-            return _this.successCallback(_data, li);
-          };
-        })(this),
+        success: function(_data) {
+          return Milestone.successCallback(_data, li);
+        },
         error: function(data) {
           return new Flash("Issue update failed", 'alert');
         },
@@ -27,11 +26,9 @@
         type: "PUT",
         url: sort_issues_url,
         data: data,
-        success: (function(_this) {
-          return function(_data) {
-            return _this.successCallback(_data);
-          };
-        })(this),
+        success: function(_data) {
+          return Milestone.successCallback(_data);
+        },
         error: function() {
           return new Flash("Issues update failed", 'alert');
         },
@@ -46,11 +43,9 @@
         type: "PUT",
         url: sort_mr_url,
         data: data,
-        success: (function(_this) {
-          return function(_data) {
-            return _this.successCallback(_data);
-          };
-        })(this),
+        success: function(_data) {
+          return Milestone.successCallback(_data);
+        },
         error: function(data) {
           return new Flash("Issue update failed", 'alert');
         },
@@ -63,11 +58,9 @@
         type: "PUT",
         url: merge_request_url,
         data: data,
-        success: (function(_this) {
-          return function(_data) {
-            return _this.successCallback(_data, li);
-          };
-        })(this),
+        success: function(_data) {
+          return Milestone.successCallback(_data, li);
+        },
         error: function(data) {
           return new Flash("Issue update failed", 'alert');
         },
@@ -81,65 +74,30 @@
         img_tag = $('<img/>');
         img_tag.attr('src', data.assignee.avatar_url);
         img_tag.addClass('avatar s16');
-        $(element).find('.assignee-icon').html(img_tag);
+        $(element).find('.assignee-icon img').replaceWith(img_tag);
       } else {
-        $(element).find('.assignee-icon').html('');
+        $(element).find('.assignee-icon').empty();
       }
       return $(element).effect('highlight');
     };
 
     function Milestone() {
       var oldMouseStart;
-      oldMouseStart = $.ui.sortable.prototype._mouseStart;
-      $.ui.sortable.prototype._mouseStart = function(event, overrideHandle, noActivation) {
-        this._trigger("beforeStart", event, this._uiHash());
-        return oldMouseStart.apply(this, [event, overrideHandle, noActivation]);
-      };
       this.bindIssuesSorting();
       this.bindMergeRequestSorting();
       this.bindTabsSwitching();
     }
 
     Milestone.prototype.bindIssuesSorting = function() {
-      return $("#issues-list-unassigned, #issues-list-ongoing, #issues-list-closed").sortable({
-        connectWith: ".issues-sortable-list",
-        dropOnEmpty: true,
-        items: "li:not(.ui-sort-disabled)",
-        beforeStart: function(event, ui) {
-          return $(".issues-sortable-list").css("min-height", ui.item.outerHeight());
-        },
-        stop: function(event, ui) {
-          return $(".issues-sortable-list").css("min-height", "0px");
-        },
-        update: function(event, ui) {
-          var data;
-          // Prevents sorting from container which element has been removed.
-          if ($(this).find(ui.item).length > 0) {
-            data = $(this).sortable("serialize");
-            return Milestone.sortIssues(data);
-          }
-        },
-        receive: function(event, ui) {
-          var data, issue_id, issue_url, new_state;
-          new_state = $(this).data('state');
-          issue_id = ui.item.data('iid');
-          issue_url = ui.item.data('url');
-          data = (function() {
-            switch (new_state) {
-              case 'ongoing':
-                return "issue[assignee_id]=" + gon.current_user_id;
-              case 'unassigned':
-                return "issue[assignee_id]=";
-              case 'closed':
-                return "issue[state_event]=close";
-            }
-          })();
-          if ($(ui.sender).data('state') === "closed") {
-            data += "&issue[state_event]=reopen";
-          }
-          return Milestone.updateIssue(ui.item, issue_url, data);
-        }
-      }).disableSelection();
+      $('#issues-list-unassigned, #issues-list-ongoing, #issues-list-closed').each(function (i, el) {
+        this.createSortable(el, {
+          group: 'issue-list',
+          listEls: $('.issues-sortable-list'),
+          fieldName: 'issue',
+          sortCallback: Milestone.sortIssues,
+          updateCallback: Milestone.updateIssue,
+        });
+      }.bind(this));
     };
 
     Milestone.prototype.bindTabsSwitching = function() {
@@ -154,42 +112,62 @@
     };
 
     Milestone.prototype.bindMergeRequestSorting = function() {
-      return $("#merge_requests-list-unassigned, #merge_requests-list-ongoing, #merge_requests-list-closed").sortable({
-        connectWith: ".merge_requests-sortable-list",
-        dropOnEmpty: true,
-        items: "li:not(.ui-sort-disabled)",
-        beforeStart: function(event, ui) {
-          return $(".merge_requests-sortable-list").css("min-height", ui.item.outerHeight());
+      $("#merge_requests-list-unassigned, #merge_requests-list-ongoing, #merge_requests-list-closed").each(function (i, el) {
+        this.createSortable(el, {
+          group: 'merge-request-list',
+          listEls: $(".merge_requests-sortable-list:not(#merge_requests-list-merged)"),
+          fieldName: 'merge_request',
+          sortCallback: Milestone.sortMergeRequests,
+          updateCallback: Milestone.updateMergeRequest,
+        });
+      }.bind(this));
+    };
+
+    Milestone.prototype.createSortable = function(el, opts) {
+      return Sortable.create(el, {
+        group: opts.group,
+        filter: '.is-disabled',
+        forceFallback: true,
+        onStart: function(e) {
+          opts.listEls.css('min-height', e.item.offsetHeight);
         },
-        stop: function(event, ui) {
-          return $(".merge_requests-sortable-list").css("min-height", "0px");
+        onEnd: function () {
+          opts.listEls.css("min-height", "0px");
         },
-        update: function(event, ui) {
-          var data;
-          data = $(this).sortable("serialize");
-          return Milestone.sortMergeRequests(data);
+        onUpdate: function(e) {
+          var ids = this.toArray(),
+            data;
+
+          if (ids.length) {
+            data = ids.map(function(id) {
+              return 'sortable_' + opts.fieldName + '[]=' + id;
+            }).join('&');
+
+            opts.sortCallback(data);
+          }
         },
-        receive: function(event, ui) {
-          var data, merge_request_id, merge_request_url, new_state;
-          new_state = $(this).data('state');
-          merge_request_id = ui.item.data('iid');
-          merge_request_url = ui.item.data('url');
+        onAdd: function (e) {
+          var data, issuableId, issuableUrl, newState;
+          newState = e.to.dataset.state;
+          issuableUrl = e.item.dataset.url;
           data = (function() {
-            switch (new_state) {
+            switch (newState) {
               case 'ongoing':
-                return "merge_request[assignee_id]=" + gon.current_user_id;
+                return opts.fieldName + '[assignee_id]=' + gon.current_user_id;
               case 'unassigned':
-                return "merge_request[assignee_id]=";
+                return opts.fieldName + '[assignee_id]=';
               case 'closed':
-                return "merge_request[state_event]=close";
+                return opts.fieldName + '[state_event]=close';
             }
           })();
-          if ($(ui.sender).data('state') === "closed") {
-            data += "&merge_request[state_event]=reopen";
+          if (e.from.dataset.state === 'closed') {
+            data += '&' + opts.fieldName + '[state_event]=reopen';
           }
-          return Milestone.updateMergeRequest(ui.item, merge_request_url, data);
+
+          opts.updateCallback(e.item, issuableUrl, data);
+          this.options.onUpdate.call(this, e);
         }
-      }).disableSelection();
+      });
     };
 
     return Milestone;

app/assets/javascripts/boards/test_utils/simulate_drag.js → app/assets/javascripts/test_utils/simulate_drag.js

@@ -50,14 +50,15 @@
     return (
       children[target.index] ||
       children[target.index === 'first' ? 0 : -1] ||
-      children[target.index === 'last' ? children.length - 1 : -1]
+      children[target.index === 'last' ? children.length - 1 : -1] ||
+      el
     );
   }
 
   function getRect(el) {
     var rect = el.getBoundingClientRect();
     var width = rect.right - rect.left;
-    var height = rect.bottom - rect.top;
+    var height = rect.bottom - rect.top + 10;
 
     return {
       x: rect.left,

app/assets/stylesheets/framework/jquery.scss

@@ -70,14 +70,3 @@
     }
   }
 }
-
-.ui-sortable-handle {
-  cursor: move;
-  cursor: -webkit-grab;
-  cursor: -moz-grab;
-
-  &:active {
-    cursor: -webkit-grabbing;
-    cursor: -moz-grabbing;
-  }
-}

app/assets/stylesheets/pages/labels.scss

@@ -116,6 +116,22 @@
 }
 
 .manage-labels-list {
+  > li:not(.empty-message) {
+    background-color: $white-light;
+    cursor: move;
+    cursor: -webkit-grab;
+    cursor: -moz-grab;
+
+    &:active {
+      cursor: -webkit-grabbing;
+      cursor: -moz-grabbing;
+    }
+
+    &.sortable-ghost {
+      opacity: 0.3;
+    }
+  }
+
   .btn-action {
     color: $gl-text-color;
 

app/assets/stylesheets/pages/milestone.scss

@@ -178,3 +178,9 @@
     }
   }
 }
+
+.issuable-row {
+  background-color: $white-light;
+  cursor: -webkit-grab;
+  cursor: grab;
+}

app/views/groups/milestones/show.html.haml

 = render "header_title"
+
+- content_for :page_specific_javascripts do
+  = page_specific_javascript_bundle_tag('simulate_drag') if Rails.env.test?
+
 = render 'shared/milestones/top', milestone: @milestone, group: @group
 = render 'shared/milestones/summary', milestone: @milestone
 = render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true

app/views/projects/boards/_show.html.haml

@@ -4,7 +4,7 @@
 
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('boards')
-  = page_specific_javascript_bundle_tag('boards_test') if Rails.env.test?
+  = page_specific_javascript_bundle_tag('simulate_drag') if Rails.env.test?
 
   %script#js-board-template{ type: "text/x-template" }= render "projects/boards/components/board"
   %script#js-board-list-template{ type: "text/x-template" }= render "projects/boards/components/board_list"

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

@@ -3,6 +3,9 @@
 - hide_class = ''
 = render "projects/issues/head"
 
+- content_for :page_specific_javascripts do
+  = page_specific_javascript_bundle_tag('simulate_drag') if Rails.env.test?
+
 - if @labels.exists? || @prioritized_labels.exists?
   %div{ class: container_class }
     .top-area.adjust

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

@@ -3,6 +3,9 @@
 - page_description @milestone.description
 = render "projects/issues/head"
 
+- content_for :page_specific_javascripts do
+  = page_specific_javascript_bundle_tag('simulate_drag') if Rails.env.test?
+
 %div{ class: container_class }
   .detail-page-header.milestone-page-header
     .status-box{ class: status_box_class(@milestone) }

app/views/shared/milestones/_issuable.html.haml

@@ -5,7 +5,7 @@
 - base_url_args = [project.namespace.becomes(Namespace), project, issuable_type]
 - can_update = can?(current_user, :"update_#{issuable.to_ability_name}", issuable)
 
-%li{ id: dom_id(issuable, 'sortable'),  class: "issuable-row #{'ui-sort-disabled' unless can_update}", 'data-iid' => issuable.iid, 'data-url' => polymorphic_path(issuable) }
+%li{ id: dom_id(issuable, 'sortable'),  class: "issuable-row #{'is-disabled' unless can_update}", 'data-iid' => issuable.iid, 'data-id' => issuable.id, 'data-url' => polymorphic_path(issuable) }
   %span
     - if show_project_name
       %strong #{project.name} ·

changelogs/unreleased/remove-jquery-ui-sortable.yml

+---
+title: Replaced jQuery UI sortable
+merge_request:
+author:

config/webpack.config.js

@@ -17,7 +17,7 @@ var config = {
     application:          './application.js',
     blob_edit:            './blob_edit/blob_edit_bundle.js',
     boards:               './boards/boards_bundle.js',
-    boards_test:          './boards/test_utils/simulate_drag.js',
+    simulate_drag:        './test_utils/simulate_drag.js',
     cycle_analytics:      './cycle_analytics/cycle_analytics_bundle.js',
     commit_pipelines:     './commit/pipelines/pipelines_bundle.js',
     diff_notes:           './diff_notes/diff_notes_bundle.js',

spec/features/boards/boards_spec.rb

@@ -3,6 +3,7 @@ require 'rails_helper'
 describe 'Issue Boards', feature: true, js: true do
   include WaitForAjax
   include WaitForVueResource
+  include DragTo
 
   let(:project) { create(:empty_project, :public) }
   let(:board)   { create(:board, project: project) }
@@ -188,7 +189,7 @@ describe 'Issue Boards', feature: true, js: true do
       end
 
       it 'moves issue to done' do
-        drag_to(list_from_index: 0, list_to_index: 2)
+        drag(list_from_index: 0, list_to_index: 2)
 
         wait_for_board_cards(1, 7)
         wait_for_board_cards(2, 2)
@@ -201,7 +202,7 @@ describe 'Issue Boards', feature: true, js: true do
       end
 
       it 'removes all of the same issue to done' do
-        drag_to(list_from_index: 0, list_to_index: 2)
+        drag(list_from_index: 0, list_to_index: 2)
 
         wait_for_board_cards(1, 7)
         wait_for_board_cards(2, 2)
@@ -215,7 +216,7 @@ describe 'Issue Boards', feature: true, js: true do
 
     context 'lists' do
       it 'changes position of list' do
-        drag_to(list_from_index: 1, list_to_index: 0, selector: '.board-header')
+        drag(list_from_index: 1, list_to_index: 0, selector: '.board-header')
 
         wait_for_board_cards(1, 2)
         wait_for_board_cards(2, 8)
@@ -226,7 +227,7 @@ describe 'Issue Boards', feature: true, js: true do
       end
 
       it 'issue moves between lists' do
-        drag_to(list_from_index: 0, card_index: 1, list_to_index: 1)
+        drag(list_from_index: 0, from_index: 1, list_to_index: 1)
 
         wait_for_board_cards(1, 7)
         wait_for_board_cards(2, 2)
@@ -237,7 +238,7 @@ describe 'Issue Boards', feature: true, js: true do
       end
 
       it 'issue moves between lists' do
-        drag_to(list_from_index: 1, list_to_index: 0)
+        drag(list_from_index: 1, list_to_index: 0)
 
         wait_for_board_cards(1, 9)
         wait_for_board_cards(2, 1)
@@ -248,7 +249,7 @@ describe 'Issue Boards', feature: true, js: true do
       end
 
       it 'issue moves from done' do
-        drag_to(list_from_index: 2, list_to_index: 1)
+        drag(list_from_index: 2, list_to_index: 1)
 
         expect(find('.board:nth-child(2)')).to have_content(issue8.title)
 
@@ -615,14 +616,13 @@ describe 'Issue Boards', feature: true, js: true do
     end
   end
 
-  def drag_to(list_from_index: 0, card_index: 0, to_index: 0, list_to_index: 0, selector: '.board-list')
-    evaluate_script("simulateDrag({scrollable: document.getElementById('board-app'), from: {el: $('#{selector}').eq(#{list_from_index}).get(0), index: #{card_index}}, to: {el: $('.board-list').eq(#{list_to_index}).get(0), index: #{to_index}}});")
-
-    Timeout.timeout(Capybara.default_max_wait_time) do
-      loop until page.evaluate_script('window.SIMULATE_DRAG_ACTIVE').zero?
-    end
-
-    wait_for_vue_resource
+  def drag(selector: '.board-list', list_from_index: 0, from_index: 0, to_index: 0, list_to_index: 0)
+    drag_to(selector: selector,
+            scrollable: '#board-app',
+            list_from_index: list_from_index,
+            from_index: from_index,
+            to_index: to_index,
+            list_to_index: list_to_index)
   end
 
   def wait_for_board_cards(board_number, expected_cards)

spec/features/milestones/milestones_spec.rb

@@ -2,6 +2,7 @@ require 'rails_helper'
 
 describe 'Milestone draggable', feature: true, js: true do
   include WaitForAjax
+  include DragTo
 
   let(:milestone) { create(:milestone, project: project, title: 8.14) }
   let(:project)   { create(:empty_project, :public) }
@@ -75,7 +76,7 @@ describe 'Milestone draggable', feature: true, js: true do
     create(:issue, params.merge(title: 'Foo', project: project, milestone: milestone))
 
     visit namespace_project_milestone_path(project.namespace, project, milestone)
-    issue.drag_to(issue_target)
+    drag_to(selector: '.issues-sortable-list', list_to_index: 1)
 
     wait_for_ajax
   end
@@ -85,7 +86,7 @@ describe 'Milestone draggable', feature: true, js: true do
 
     visit namespace_project_milestone_path(project.namespace, project, milestone)
     page.find("a[href='#tab-merge-requests']").click
-    merge_request.drag_to(merge_request_target)
+    drag_to(selector: '.merge_requests-sortable-list', list_to_index: 1)
 
     wait_for_ajax
   end

spec/features/projects/labels/update_prioritization_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 feature 'Prioritize labels', feature: true do
   include WaitForAjax
+  include DragTo
 
   let(:user)     { create(:user) }
   let(:group)    { create(:group) }
@@ -99,7 +100,7 @@ feature 'Prioritize labels', feature: true do
       expect(page).to have_content 'wontfix'
 
       # Sort labels
-      find("#project_label_#{bug.id}").drag_to find("#group_label_#{feature.id}")
+      drag_to(selector: '.js-prioritized-labels', from_index: 1, to_index: 2)
 
       page.within('.prioritized-labels') do
         expect(first('li')).to have_content('feature')

spec/support/drag_to_helper.rb

+module DragTo
+  def drag_to(list_from_index: 0, from_index: 0, to_index: 0, list_to_index: 0, selector: '', scrollable: 'body')
+    evaluate_script("simulateDrag({scrollable: $('#{scrollable}').get(0), from: {el: $('#{selector}').eq(#{list_from_index}).get(0), index: #{from_index}}, to: {el: $('#{selector}').eq(#{list_to_index}).get(0), index: #{to_index}}});")
+
+    Timeout.timeout(Capybara.default_max_wait_time) do
+      loop until drag_active?
+    end
+  end
+
+  def drag_active?
+    page.evaluate_script('window.SIMULATE_DRAG_ACTIVE').zero?
+  end
+end