From cbd066da049c11b79d7139bd6da973bd40c12ae0 Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Thu, 8 Aug 2013 16:24:37 -0400 Subject: [PATCH] Address PR comments. Remove unused variables; some changes to match existing BDD spec for drag/drop (expand-on-hover timeout and dropping with mouse outside of element); bugfix for section expand; some code cleanup; update and re-enable acceptance test. --- .../contentstore/features/course-overview.py | 6 +- .../coffee/spec/views/overview_spec.coffee | 102 +++++++++++++ cms/static/js/views/overview.js | 135 +++++++++--------- 3 files changed, 175 insertions(+), 68 deletions(-) create mode 100644 cms/static/coffee/spec/views/overview_spec.coffee diff --git a/cms/djangoapps/contentstore/features/course-overview.py b/cms/djangoapps/contentstore/features/course-overview.py index 289dbec308..375e3d934e 100644 --- a/cms/djangoapps/contentstore/features/course-overview.py +++ b/cms/djangoapps/contentstore/features/course-overview.py @@ -128,10 +128,10 @@ def change_grading_status(step): @step(u'I reorder subsections') def reorder_subsections(_step): - draggable_css = 'a.drag-handle' + draggable_css = '.subsection-drag-handle' ele = world.css_find(draggable_css).first ele.action_chains.drag_and_drop_by_offset( ele._element, - 30, - 0 + 0, + 10 ).perform() diff --git a/cms/static/coffee/spec/views/overview_spec.coffee b/cms/static/coffee/spec/views/overview_spec.coffee new file mode 100644 index 0000000000..d0e6bb77d3 --- /dev/null +++ b/cms/static/coffee/spec/views/overview_spec.coffee @@ -0,0 +1,102 @@ +describe "Course Overview", -> + + beforeEach -> + _.each ["/static/js/vendor/date.js", "/static/js/vendor/timepicker/jquery.timepicker.js", "/jsi18n/"], (path) -> + appendSetFixtures """ + + """ + + appendSetFixtures """ +
+ + Will Release: 06/12/2013 at 04:00 UTC + + Edit +
+ """ + + appendSetFixtures """ +
+
+

Section Release Date

+
+
+ + +
+
+ + +
+
+

On the date set above, this section – – will be released to students. Any units marked private will only be visible to admins.

+
+
+ SaveCancel +
+
+ """ + + appendSetFixtures """ +
+ +
+ """ + + # appendSetFixtures """ + #
+ #
    + #
  1. + #
  2. + #
  3. + #
+ #
+ # """ + + spyOn(window, 'saveSetSectionScheduleDate').andCallThrough() + # Have to do this here, as it normally gets bound in document.ready() + $('a.save-button').click(saveSetSectionScheduleDate) + $('a.delete-section-button').click(deleteSection) + $(".edit-subsection-publish-settings .start-date").datepicker() + + @notificationSpy = spyOn(CMS.Views.Notification.Mini.prototype, 'show').andCallThrough() + window.analytics = jasmine.createSpyObj('analytics', ['track']) + window.course_location_analytics = jasmine.createSpy() + @xhr = sinon.useFakeXMLHttpRequest() + requests = @requests = [] + @xhr.onCreate = (req) -> requests.push(req) + + afterEach -> + delete window.analytics + delete window.course_location_analytics + @notificationSpy.reset() + + it "should save model when save is clicked", -> + $('a.edit-button').click() + $('a.save-button').click() + expect(saveSetSectionScheduleDate).toHaveBeenCalled() + + it "should show a confirmation on save", -> + $('a.edit-button').click() + $('a.save-button').click() + expect(@notificationSpy).toHaveBeenCalled() + + it "should delete model when delete is clicked", -> + deleteSpy = spyOn(window, '_deleteItem').andCallThrough() + $('a.delete-section-button').click() + $('a.action-primary').click() + expect(deleteSpy).toHaveBeenCalled() + expect(@requests[0].url).toEqual('/delete_item') + + it "should not delete model when cancel is clicked", -> + deleteSpy = spyOn(window, '_deleteItem').andCallThrough() + $('a.delete-section-button').click() + $('a.action-secondary').click() + expect(@requests.length).toEqual(0) + + it "should show a confirmation on delete", -> + $('a.delete-section-button').click() + $('a.action-primary').click() + expect(@notificationSpy).toHaveBeenCalled() diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index 51edfc4417..2dbed015c4 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -1,57 +1,13 @@ $(document).ready(function() { - // Section - makeDraggable( - '.courseware-section', - '.section-drag-handle', - '.courseware-overview', - 'article.courseware-overview' - ); - // Subsection - makeDraggable( - '.id-holder', - '.subsection-drag-handle', - '.subsection-list > ol', - '.courseware-section' - ); - // Unit - makeDraggable( - '.unit', - '.unit-drag-handle', - 'ol.sortable-unit-list', - 'li.branch, article.subsection-body' - ); - - /* - * Make `type` draggable using `handleClass`, able to be dropped - * into `droppableClass`, and with parent type - * `parentLocationSelector`. - */ - function makeDraggable(type, handleClass, droppableClass, parentLocationSelector) { - _.each( - $(type), - function(ele) { - // Remember data necessary to reconstruct the parent-child relationships - $(ele).data('droppable-class', droppableClass); - $(ele).data('parent-location-selector', parentLocationSelector); - $(ele).data('child-selector', type); - var draggable = new Draggabilly(ele, { - handle: handleClass, - axis: 'y' - }); - draggable.on('dragStart', onDragStart); - draggable.on('dragMove', onDragMove); - draggable.on('dragEnd', onDragEnd); - } - ); - } + var droppableClasses = 'drop-target drop-target-prepend drop-target-before drop-target-after'; /* * Determine information about where to drop the currently dragged * element. Returns the element to attach to and the method of * attachment ('before', 'after', or 'prepend'). */ - function findDestination(ele) { + var findDestination = function(ele) { var eleY = ele.offset().top; var containers = $(ele.data('droppable-class')); @@ -90,7 +46,11 @@ $(document).ready(function() { for(var j = 0; j < siblings.length; j++) { var $sibling = $(siblings[j]); var siblingY = $sibling.offset().top; - if(Math.abs(eleY - siblingY) < $sibling.height()) { + // Subtract 1 to be sure that we test if this + // element is actually on top of the sibling, + // rather than next to it. This prevents + // saving when expanding/collapsing a list. + if(Math.abs(eleY - siblingY) < $sibling.height() - 1) { return { ele: $sibling, attachMethod: siblingY > eleY ? 'before' : 'after' @@ -105,12 +65,12 @@ $(document).ready(function() { ele: null, attachMethod: '' }; - } + }; // Information about the current drag. var dragState = {}; - function onDragStart(draggie, event, pointer) { + var onDragStart = function(draggie, event, pointer) { var ele = $(draggie.element); dragState = { // Where we started, in case of a failed drag @@ -122,9 +82,9 @@ $(document).ready(function() { // The list which will be expanded on hover toExpand: null }; - } + }; - function onDragMove(draggie, event, pointer) { + var onDragMove = function(draggie, event, pointer) { var ele = $(draggie.element); var destinationInfo = findDestination(ele); var destinationEle = destinationInfo.ele; @@ -139,29 +99,30 @@ $(document).ready(function() { clearTimeout(dragState.expandTimer); dragState.expandTimer = setTimeout(function() { parentList.removeClass('collapsed'); - }, 1000); + parentList.find('.expand-collapse-icon').removeClass('expand').addClass('collapse'); + }, 400); dragState.toExpand = parentList; } // Clear out the old destination if(dragState.dropDestination) { - dragState.dropDestination.removeClass('drop-target drop-target-prepend' - + ' drop-target-before drop-target-after'); + dragState.dropDestination.removeClass(droppableClasses); } // Mark the new destination if(destinationEle) { destinationEle.addClass('drop-target drop-target-' + destinationInfo.attachMethod); dragState.dropDestination = destinationEle; } - } + }; - function onDragEnd(draggie, event, pointer) { + var onDragEnd = function(draggie, event, pointer) { var ele = $(draggie.element); var destinationInfo = findDestination(ele); var destination = destinationInfo.ele; // If the drag succeeded, rearrange the DOM and send the result. - if(destination) { + if(destination && pointer.x >= ele.offset().left + && pointer.x < ele.offset().left + ele.width()) { // Make sure we don't drop into a collapsed element if(destinationInfo.parentList) { destinationInfo.parentList.removeClass('collapsed'); @@ -179,18 +140,16 @@ $(document).ready(function() { // Clear dragging state in preparation for the next event. if(dragState.dropDestination) { - dragState.dropDestination.removeClass('drop-target drop-target-prepend' - + ' drop-target-before drop-target-after'); + dragState.dropDestination.removeClass(droppableClasses); } clearTimeout(dragState.expandTimer); dragState = {}; - } + }; /* * Find all parent-child changes and save them. */ - function handleReorder(ele) { - var itemID = ele.data('id'); + var handleReorder = function(ele) { var parentSelector = ele.data('parent-location-selector'); var childrenSelector = ele.data('child-selector'); var newParentEle = ele.parents(parentSelector).first(); @@ -213,14 +172,14 @@ $(document).ready(function() { saveItem(newParentEle, childrenSelector, function() { saving.hide(); }); - } + }; /* * Actually save the update to the server. Takes the element * representing the parent item to save, a CSS selector to find * its children, and a success callback. */ - function saveItem(ele, childrenSelector, success) { + var saveItem = function(ele, childrenSelector, success) { // Find all current child IDs. var children = _.map( ele.find(childrenSelector), @@ -239,5 +198,51 @@ $(document).ready(function() { }), success: success }); - } + }; + + /* + * Make `type` draggable using `handleClass`, able to be dropped + * into `droppableClass`, and with parent type + * `parentLocationSelector`. + */ + var makeDraggable = function(type, handleClass, droppableClass, parentLocationSelector) { + _.each( + $(type), + function(ele) { + // Remember data necessary to reconstruct the parent-child relationships + $(ele).data('droppable-class', droppableClass); + $(ele).data('parent-location-selector', parentLocationSelector); + $(ele).data('child-selector', type); + var draggable = new Draggabilly(ele, { + handle: handleClass, + axis: 'y' + }); + draggable.on('dragStart', onDragStart); + draggable.on('dragMove', onDragMove); + draggable.on('dragEnd', onDragEnd); + } + ); + }; + + // Section + makeDraggable( + '.courseware-section', + '.section-drag-handle', + '.courseware-overview', + 'article.courseware-overview' + ); + // Subsection + makeDraggable( + '.id-holder', + '.subsection-drag-handle', + '.subsection-list > ol', + '.courseware-section' + ); + // Unit + makeDraggable( + '.unit', + '.unit-drag-handle', + 'ol.sortable-unit-list', + 'li.branch, article.subsection-body' + ); });