From 27e1afde7f516f658a7e78df0f78a30f8c3fc6da Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 5 Dec 2013 11:09:32 -0500 Subject: [PATCH] Bug fix for dragging past last element in list. STUD-879 --- .../coffee/spec/views/overview_spec.coffee | 140 +++++++++++++----- cms/static/js/views/overview.js | 77 +++++++--- 2 files changed, 160 insertions(+), 57 deletions(-) diff --git a/cms/static/coffee/spec/views/overview_spec.coffee b/cms/static/coffee/spec/views/overview_spec.coffee index cbc08212137a..35fcff4f95bb 100644 --- a/cms/static/coffee/spec/views/overview_spec.coffee +++ b/cms/static/coffee/spec/views/overview_spec.coffee @@ -43,23 +43,36 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base """ appendSetFixtures """ -
    -
  1. -
      -
    1. -
    2. -
    3. -
    -
    2. -
  2. -
      -
    1. -
    -
    3. -
  3. -
      - -
    +
    +
      +
    1. +
        +
      1. +
      +
      2. +
    2. +
        +
      1. +
      2. +
      3. +
      +
      3. +
    3. +
        +
      1. +
      +
      4. +
    4. +
        +
      +
      5. +
    5. +
        +
      1. +
      +
      6. +
    +
    """ spyOn(Overview, 'saveSetSectionScheduleDate').andCallThrough() @@ -78,9 +91,16 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base Overview.overviewDragger.makeDraggable( '.unit', '.unit-drag-handle', - 'ol.sortable-unit-list', + '.sortable-unit-list', 'li.branch, article.subsection-body' ) + + Overview.overviewDragger.makeDraggable( + '.subsection-list', + '.subsection-drag-handle', + '.section-list', + 'section' + ) afterEach -> delete window.analytics @@ -113,7 +133,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base # $('a.delete-section-button').click() # $('a.action-primary').click() # expect(@notificationSpy).toHaveBeenCalled() - + describe "findDestination", -> it "correctly finds the drop target of a drag", -> $ele = $('#unit-1') @@ -123,26 +143,54 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base destination = Overview.overviewDragger.findDestination($ele, 1) expect(destination.ele).toBe($('#unit-2')) expect(destination.attachMethod).toBe('before') - - it "can drag and drop across section boundaries, with special handling for first element", -> + + it "can drag and drop across section boundaries, with special handling for single sibling", -> $ele = $('#unit-1') + $unit4 = $('#unit-4') $ele.offset( - top: $('#unit-4').offset().top + 8 + top: $unit4.offset().top + 8 left: $ele.offset().left ) + # Dragging down, we will insert after. destination = Overview.overviewDragger.findDestination($ele, 1) - expect(destination.ele).toBe($('#unit-4')) - # Dragging down into first element, we have a fudge factor makes it easier to drag at beginning. + expect(destination.ele).toBe($unit4) + expect(destination.attachMethod).toBe('after') + + # Dragging up, we will insert before. + destination = Overview.overviewDragger.findDestination($ele, -1) + expect(destination.ele).toBe($unit4) expect(destination.attachMethod).toBe('before') - # Now past the "fudge factor". + + # If past the end the drop target, will attach after. $ele.offset( - top: $('#unit-4').offset().top + 12 + top: $unit4.offset().top + $unit4.height() + 1 left: $ele.offset().left ) - destination = Overview.overviewDragger.findDestination($ele, 1) - expect(destination.ele).toBe($('#unit-4')) + destination = Overview.overviewDragger.findDestination($ele, 0) + expect(destination.ele).toBe($unit4) expect(destination.attachMethod).toBe('after') - + + $unit0 = $('#unit-0') + # If before the start the drop target, will attach before. + $ele.offset( + top: $unit0.offset().top - 16 + left: $ele.offset().left + ) + destination = Overview.overviewDragger.findDestination($ele, 0) + expect(destination.ele).toBe($unit0) + expect(destination.attachMethod).toBe('before') + + it """can drop before the first element, even if element being dragged is + slightly before the first element""", -> + $ele = $('#subsection-2') + $ele.offset( + top: $('#subsection-0').offset().top - 5 + left: $ele.offset().left + ) + destination = Overview.overviewDragger.findDestination($ele, -1) + expect(destination.ele).toBe($('#subsection-0')) + expect(destination.attachMethod).toBe('before') + it "can drag and drop across section boundaries, with special handling for last element", -> $ele = $('#unit-4') $ele.offset( @@ -161,7 +209,21 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base destination = Overview.overviewDragger.findDestination($ele, -1) expect(destination.ele).toBe($('#unit-3')) expect(destination.attachMethod).toBe('before') - + + it """can drop past the last element, even if element being dragged is + slightly before/taller then the last element""", -> + $ele = $('#subsection-2') + $ele.offset( + # Make the top 1 before the top of the last element in the list. + # This mimics the problem when the element being dropped is taller then then + # the last element in the list. + top: $('#subsection-4').offset().top - 1 + left: $ele.offset().left + ) + destination = Overview.overviewDragger.findDestination($ele, 1) + expect(destination.ele).toBe($('#subsection-4')) + expect(destination.attachMethod).toBe('after') + it "can drag into an empty list", -> $ele = $('#unit-1') $ele.offset( @@ -171,7 +233,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base destination = Overview.overviewDragger.findDestination($ele, 1) expect(destination.ele).toBe($('#subsection-list-3')) expect(destination.attachMethod).toBe('prepend') - + it "reports a null destination on a failed drag", -> $ele = $('#unit-1') $ele.offset( @@ -182,7 +244,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base ele: null attachMethod: "" ) - + it "can drag into a collapsed list", -> $('#subsection-2').addClass('collapsed') $ele = $('#unit-2') @@ -194,7 +256,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base expect(destination.ele).toBe($('#subsection-list-2')) expect(destination.parentList).toBe($('#subsection-2')) expect(destination.attachMethod).toBe('prepend') - + describe "onDragStart", -> it "sets the dragState to its default values", -> expect(Overview.overviewDragger.dragState).toEqual({}) @@ -211,7 +273,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base lastY: 0, dragDirection: 0 ) - + it "collapses expanded elements", -> expect($('#subsection-1')).not.toHaveClass('collapsed') Overview.overviewDragger.onDragStart( @@ -221,7 +283,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base ) expect($('#subsection-1')).toHaveClass('collapsed') expect($('#subsection-1')).toHaveClass('expand-on-drop') - + describe "onDragMove", -> beforeEach -> @scrollSpy = spyOn(window, 'scrollBy').andCallThrough() @@ -239,7 +301,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base ) expect($('#unit-2')).toHaveClass('drop-target drop-target-before') expect($ele).toHaveClass('valid-drop') - + it "does not add CSS class to the drop destination if out of bounds", -> $ele = $('#unit-1') dragY = $ele.offset().top + 10 @@ -252,19 +314,19 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base ) expect($('#unit-2')).not.toHaveClass('drop-target drop-target-before') expect($ele).not.toHaveClass('valid-drop') - + it "scrolls up if necessary", -> Overview.overviewDragger.onDragMove( {element: $('#unit-1')}, '', {clientY: 2} ) expect(@scrollSpy).toHaveBeenCalledWith(0, -10) - + it "scrolls down if necessary", -> Overview.overviewDragger.onDragMove( {element: $('#unit-1')}, '', {clientY: (window.innerHeight - 5)} ) expect(@scrollSpy).toHaveBeenCalledWith(0, 10) - + describe "onDragEnd", -> beforeEach -> @reorderSpy = spyOn(Overview.overviewDragger, 'handleReorder') diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index 9dfb339b0cfa..67691257589e 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -257,28 +257,69 @@ define(["domReady", "jquery", "jquery.ui", "underscore", "gettext", "js/views/fe var siblingHeight = $sibling.height(); var siblingYEnd = siblingY + siblingHeight; + var eleYEnd = eleY + ele.height(); + // Facilitate dropping into the beginning or end of a list // (coming from opposite direction) via a "fudge factor". Math.min is for Jasmine test. var fudge = Math.min(Math.ceil(siblingHeight / 2), 20); - // Dragging up into end of list. - if (j == siblings.length - 1 && yChange < 0 && Math.abs(eleY - siblingYEnd) <= fudge) { - return { - ele: $sibling, - attachMethod: 'after' - }; - } - // Dragging down into beginning of list. - else if (j == 0 && yChange > 0 && Math.abs(eleY - siblingY) <= fudge) { - return { - ele: $sibling, - attachMethod: 'before' - }; + + // Dragging to top or bottom of a list with only one element is tricky + // because the element being dragged may be the same size as the sibling. + if (siblings.length == 1) { + // Element being dragged is within the drop target. Use the direction + // of the drag (yChange) to determine before or after. + if (eleY + fudge >= siblingY && eleYEnd - fudge <= siblingYEnd) { + return { + ele: $sibling, + attachMethod: yChange > 0 ? 'after' : 'before' + }; + } + // Element being dragged is before the drop target. + else if (Math.abs(eleYEnd - siblingY) <= fudge) { + return { + ele: $sibling, + attachMethod: 'before' + }; + } + // Element being dragged is after the drop target. + else if (Math.abs(eleY - siblingYEnd) <= fudge) { + return { + ele: $sibling, + attachMethod: 'after' + }; + } } - else if (eleY >= siblingY && eleY <= siblingYEnd) { - return { - ele: $sibling, - attachMethod: eleY - siblingY <= siblingHeight / 2 ? 'before' : 'after' - }; + else { + // Dragging up into end of list. + if (j == siblings.length - 1 && yChange < 0 && Math.abs(eleY - siblingYEnd) <= fudge) { + return { + ele: $sibling, + attachMethod: 'after' + }; + } + // Dragging up or down into beginning of list. + else if (j == 0 && Math.abs(eleY - siblingY) <= fudge) { + return { + ele: $sibling, + attachMethod: 'before' + }; + } + // Dragging down into end of list. Special handling required because + // the element being dragged may be taller then the element being dragged over + // (if eleY can never be >= siblingY, general case at the end does not work). + else if (j == siblings.length - 1 && yChange > 0 && + Math.abs(eleYEnd - siblingYEnd) <= fudge) { + return { + ele: $sibling, + attachMethod: 'after' + }; + } + else if (eleY >= siblingY && eleY <= siblingYEnd) { + return { + ele: $sibling, + attachMethod: eleY - siblingY <= siblingHeight / 2 ? 'before' : 'after' + }; + } } } }