diff options
author | Phil Hughes <me@iamphill.com> | 2017-01-06 19:52:18 +0300 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2017-02-06 14:28:16 +0300 |
commit | 653c23b9c23b6af8c57b45e7d5444e1ca6d34d9a (patch) | |
tree | 72318ea3ae9f968d0f3de2be36ea91786a984ffc | |
parent | 572fb0be9b1d45437b7c0ed1000399657f471ec7 (diff) |
Removed jQuery UI sortable
19 files changed, 160 insertions, 135 deletions
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 637fca4d4da..b7079cef444 100644 --- a/app/assets/javascripts/application.js +++ b/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'); diff --git a/app/assets/javascripts/boards/boards_bundle.js.es6 b/app/assets/javascripts/boards/boards_bundle.js.es6 index e3241974e59..2fe8092d29e 100644 --- a/app/assets/javascripts/boards/boards_bundle.js.es6 +++ b/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)$/)); diff --git a/app/assets/javascripts/label_manager.js.es6 b/app/assets/javascripts/label_manager.js.es6 index 2a50b72c8aa..38b2eb9ff14 100644 --- a/app/assets/javascripts/label_manager.js.es6 +++ b/app/assets/javascripts/label_manager.js.es6 @@ -1,5 +1,6 @@ /* 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; } diff --git a/app/assets/javascripts/milestone.js b/app/assets/javascripts/milestone.js index 7ce1259e015..051cb9fe5c5 100644 --- a/app/assets/javascripts/milestone.js +++ b/app/assets/javascripts/milestone.js @@ -1,5 +1,6 @@ /* 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; diff --git a/app/assets/javascripts/boards/test_utils/simulate_drag.js b/app/assets/javascripts/test_utils/simulate_drag.js index f05780167bf..7dba5840c8a 100644 --- a/app/assets/javascripts/boards/test_utils/simulate_drag.js +++ b/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, diff --git a/app/assets/stylesheets/framework/jquery.scss b/app/assets/stylesheets/framework/jquery.scss index 18f2f316f02..8487384aed6 100644 --- a/app/assets/stylesheets/framework/jquery.scss +++ b/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; - } -} diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 21d9b4c54ea..072fad854c4 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/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; diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss index 686b64cdd24..3da1150f89b 100644 --- a/app/assets/stylesheets/pages/milestone.scss +++ b/app/assets/stylesheets/pages/milestone.scss @@ -178,3 +178,9 @@ } } } + +.issuable-row { + background-color: $white-light; + cursor: -webkit-grab; + cursor: grab; +} diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index fb6f0da28f8..e66a8e0a3b3 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -1,4 +1,8 @@ = 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 diff --git a/app/views/projects/boards/_show.html.haml b/app/views/projects/boards/_show.html.haml index f0c76af29dc..1ff2cb3c94e 100644 --- a/app/views/projects/boards/_show.html.haml +++ b/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" diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index 29f861c09c6..8d4a91cb64c 100644 --- a/app/views/projects/labels/index.html.haml +++ b/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 diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index c3a6096aa54..06a31698ee6 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/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) } diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index 28935c8b598..4c7d69d40d5 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/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} · diff --git a/changelogs/unreleased/remove-jquery-ui-sortable.yml b/changelogs/unreleased/remove-jquery-ui-sortable.yml new file mode 100644 index 00000000000..35f47822738 --- /dev/null +++ b/changelogs/unreleased/remove-jquery-ui-sortable.yml @@ -0,0 +1,4 @@ +--- +title: Replaced jQuery UI sortable +merge_request: +author: diff --git a/config/webpack.config.js b/config/webpack.config.js index 7cd92af7d93..52f185e2588 100644 --- a/config/webpack.config.js +++ b/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', diff_notes: './diff_notes/diff_notes_bundle.js', environments: './environments/environments_bundle.js', diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index 34f47daf0e5..7225f38b7e5 100644 --- a/spec/features/boards/boards_spec.rb +++ b/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) diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb index aadd72a9f8e..8de9942c54e 100644 --- a/spec/features/milestones/milestones_spec.rb +++ b/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 diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 97ce9cdfd87..1e900d7e660 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/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') diff --git a/spec/support/drag_to_helper.rb b/spec/support/drag_to_helper.rb new file mode 100644 index 00000000000..0c0659d3ecd --- /dev/null +++ b/spec/support/drag_to_helper.rb @@ -0,0 +1,13 @@ +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 |