Merge pull request #611 from edx/peter-fogg/draggabilly

Rework drag/drop on overview and subsection pages.

CHANGELOG.rst

@@ -22,6 +22,8 @@ Studio: Switched to loading Javascript using require.js
 
 Studio: Better feedback during the course import process
 
+Studio: Improve drag and drop on the course overview and subsection views.
+
 LMS: Add split testing functionality for internal use.
 
 CMS: Add edit_course_tabs management command, providing a primitive

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,
+        25
     ).perform()

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1484,7 +1484,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         resp = self.client.get(reverse('course_index', kwargs=data))
         self.assertContains(
             resp,
-            '<article class="courseware-overview" data-course-id="i4x://MITx/999/course/Robot_Super_Course">',
+            '<article class="courseware-overview" data-id="i4x://MITx/999/course/Robot_Super_Course">',
             status_code=200,
             html=True
         )

cms/static/coffee/spec/main.coffee

 requirejs.config({
-   paths: {
+    paths: {
         "gettext": "xmodule_js/common_static/js/test/i18n",
         "mustache": "xmodule_js/common_static/js/vendor/mustache",
         "codemirror": "xmodule_js/common_static/js/vendor/CodeMirror/codemirror",
@@ -32,9 +32,11 @@ requirejs.config({
         "squire": "xmodule_js/common_static/js/vendor/Squire",
         "jasmine-stealth": "xmodule_js/common_static/js/vendor/jasmine-stealth",
         "jasmine.async": "xmodule_js/common_static/js/vendor/jasmine.async",
+        "draggabilly": "xmodule_js/common_static/js/vendor/draggabilly.pkgd",
+        "domReady": "xmodule_js/common_static/js/vendor/domReady",
 
         "coffee/src/ajax_prefix": "xmodule_js/common_static/coffee/src/ajax_prefix"
-    },
+    }
     shim: {
         "gettext": {
             exports: "gettext"
@@ -145,6 +147,7 @@ define([
     "coffee/spec/views/section_spec",
     "coffee/spec/views/course_info_spec", "coffee/spec/views/feedback_spec",
     "coffee/spec/views/metadata_edit_spec", "coffee/spec/views/module_edit_spec",
+    "coffee/spec/views/overview_spec",
     "coffee/spec/views/textbook_spec", "coffee/spec/views/upload_spec",
 
     # these tests are run separate in the cms-squire suite, due to process

cms/static/coffee/spec/views/module_edit_spec.coffee

@@ -5,7 +5,6 @@ define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) ->
         @stubModule = jasmine.createSpy("Module")
         @stubModule.id = 'stub-id'
 
-
         setFixtures """
         <li class="component" id="stub-id">
           <div class="component-editor">
@@ -19,7 +18,7 @@ define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) ->
             <a href="#" class="edit-button"><span class="edit-icon white"></span>Edit</a>
             <a href="#" class="delete-button"><span class="delete-icon white"></span>Delete</a>
           </div>
-          <a href="#" class="drag-handle"></a>
+          <span class="drag-handle"></span>
           <section class="xmodule_display xmodule_stub" data-type="StubModule">
             <div id="stub-module-content"/>
           </section>

cms/static/js/base.js

@@ -863,5 +863,8 @@ function saveSetSectionScheduleDate(e) {
         saving.hide();
     });
 }
+    // Add to window object for unit test (overview_spec).
+    window.saveSetSectionScheduleDate = saveSetSectionScheduleDate;
+    window.deleteSection = deleteSection;
 
 }); // end require()

cms/static/js/hesitate.js

-/*
- * Create a HesitateEvent and assign it as the event to execute:
- *   $(el).on('mouseEnter', CMS.HesitateEvent( expand, 'mouseLeave').trigger);
- *   It calls the executeOnTimeOut function with the event.currentTarget after the configurable timeout IFF the cancelSelector event
- *   did not occur on the event.currentTarget.
- *
- *   More specifically, when trigger is called (triggered by the event you bound it to), it starts a timer
- *   which the cancelSelector event will cancel or if the timer finished, it executes the executeOnTimeOut function
- *   passing it the original event (whose currentTarget s/b the specific ele). It never accumulates events; however, it doesn't hurt for your
- *   code to minimize invocations of trigger by binding to mouseEnter v mouseOver and such.
- *
- *   NOTE: if something outside of this wants to cancel the event, invoke cachedhesitation.untrigger(null | anything);
- */
-
-define(["jquery"], function($) {
-    var HesitateEvent = function(executeOnTimeOut, cancelSelector, onlyOnce) {
-        this.executeOnTimeOut = executeOnTimeOut;
-        this.cancelSelector = cancelSelector;
-        this.timeoutEventId = null;
-        this.originalEvent = null;
-        this.onlyOnce = (onlyOnce === true);
-    };
-
-    HesitateEvent.DURATION = 800;
-
-    HesitateEvent.prototype.trigger = function(event) {
-        if (event.data.timeoutEventId == null) {
-            event.data.timeoutEventId = window.setTimeout(
-                    function() { event.data.fireEvent(event); },
-                    HesitateEvent.DURATION);
-            event.data.originalEvent = event;
-            $(event.data.originalEvent.delegateTarget).on(event.data.cancelSelector, event.data, event.data.untrigger);
-        }
-    };
-
-    HesitateEvent.prototype.fireEvent = function(event) {
-        event.data.timeoutEventId = null;
-        $(event.data.originalEvent.delegateTarget).off(event.data.cancelSelector, event.data.untrigger);
-        if (event.data.onlyOnce) $(event.data.originalEvent.delegateTarget).off(event.data.originalEvent.type, event.data.trigger);
-        event.data.executeOnTimeOut(event.data.originalEvent);
-    };
-
-    HesitateEvent.prototype.untrigger = function(event) {
-        if (event.data.timeoutEventId) {
-            window.clearTimeout(event.data.timeoutEventId);
-            $(event.data.originalEvent.delegateTarget).off(event.data.cancelSelector, event.data.untrigger);
-        }
-        event.data.timeoutEventId = null;
-    };
-
-    return HesitateEvent;
-});

cms/static/js_test.yml

@@ -51,6 +51,10 @@ lib_paths:
     - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js
     - xmodule_js/src/xmodule.js
     - xmodule_js/common_static/js/test/i18n.js
+    - xmodule_js/common_static/js/vendor/draggabilly.pkgd.js
+    - xmodule_js/common_static/js/vendor/date.js
+    - xmodule_js/common_static/js/vendor/domReady.js
+    - xmodule_js/common_static/js/vendor/jquery.smooth-scroll.min.js
 
 # Paths to source JavaScript files
 src_paths:

cms/static/sass/_base.scss

@@ -528,9 +528,9 @@ p, ul, ol, dl {
 .new-subsection-item,
 .new-policy-item {
   @include grey-button;
-  margin: 5px 8px;
-  padding: 3px 10px 4px 10px;
-  font-size: 10px;
+  @include font-size(10);
+  margin: ($baseline/2);
+  padding: 3px ($baseline/2) 4px ($baseline/2);
 
   .new-folder-icon,
   .new-policy-icon,

cms/static/sass/_shame.scss

@@ -3,7 +3,7 @@
 // ====================
 
 // view - dashboard
-body.dashboard {
+.view-dashboard {
 
   // elements - authorship controls
   .wrapper-authorshiprights {
@@ -22,6 +22,35 @@ body.dashboard {
   }
 }
 
+// ====================
+
+.view-unit {
+
+  .unit-location .draggable-drop-indicator {
+    display: none; //needed to not show DnD UI (UI is shared across both views)
+  }
+}
+
+// ====================
+
+// needed to override ui-window styling for dragging state (outline selectors get too specific)
+.courseware-section.is-dragging {
+    box-shadow: 0 1px 2px 0 $shadow-d1 !important;
+    border: 1px solid $gray-d3 !important;
+}
+
+.courseware-section.is-dragging.valid-drop {
+  border-color: $blue-s1 !important;
+  box-shadow: 0 1px 2px 0 $blue-t2 !important;
+}
+
+// ====================
+
+// needed for poorly scoped margin rules on all content elements
+.branch .sortable-unit-list {
+  margin-bottom: 0;
+}
+
 
 // yes we have no boldness today - need to fix the resets
 body strong,
@@ -29,12 +58,13 @@ body b {
   font-weight: 700;
 }
 
-// known things to do (paint the fence, sand the floor, wax on/off)
 // ====================
 
 
-// known things to do (paint the fence, sand the floor, wax on/off):
+/* known things to do (paint the fence, sand the floor, wax on/off):
+
+* centralize and move form styling into forms.scss - cms/static/sass/views/_textbooks.scss and cms/static/sass/views/_settings.scss
+* move dialogue styles into cms/static/sass/elements/_modal.scss
+* use the @include placeholder Bourbon mixin (http://bourbon.io/docs/#placeholder) for any placeholder styling
 
-// * centralize and move form styling into forms.scss - cms/static/sass/views/_textbooks.scss and cms/static/sass/views/_settings.scss
-// * move dialogue styles into cms/static/sass/elements/_modal.scss
-// * use the @include placeholder Bourbon mixin (http://bourbon.io/docs/#placeholder) for any placeholder styling
+*/

cms/static/sass/_variables.scss

@@ -173,7 +173,8 @@ $tmg-f3: 0.125s;
 // ====================
 
 // specific UI
-$notification-height: ($baseline*10);
+$ui-notification-height: ($baseline*10);
+$ui-update-color: $blue-l4;
 
 // ====================
 

cms/static/sass/assets/_anims.scss

@@ -140,22 +140,22 @@
   }
 
   90% {
-    @include transform(translateY(-($notification-height)));
+    @include transform(translateY(-($ui-notification-height)));
   }
 
   100% {
-    @include transform(translateY(-($notification-height*0.99)));
+    @include transform(translateY(-($ui-notification-height*0.99)));
   }
 }
 
 // notifications slide down
 @include keyframes(notificationSlideDown) {
   0% {
-    @include transform(translateY(-($notification-height*0.99)));
+    @include transform(translateY(-($ui-notification-height*0.99)));
   }
 
   10% {
-    @include transform(translateY(-($notification-height)));
+    @include transform(translateY(-($ui-notification-height)));
   }
 
   100% {
@@ -211,3 +211,39 @@
 %anim-bounceOut {
   @include animation(bounceOut $tmg-f1 ease-in-out 1);
 }
+
+
+// ====================
+
+
+// flash
+@include keyframes(flash) {
+  0%, 100% {
+    opacity: 1.0;
+  }
+
+  50% {
+    opacity: 0.0;
+  }
+}
+
+// canned animation - use if you want out of the box/non-customized anim
+%anim-flash {
+  @include animation(flash $tmg-f1 ease-in-out 1);
+}
+
+// flash - double
+@include keyframes(flashDouble) {
+0%, 50%, 100% {
+  opacity: 1.0;
+}
+
+25%, 75% {
+    opacity: 0.0;
+  }
+  }
+
+// canned animation - use if you want out of the box/non-customized anim
+%anim-flashDouble {
+  @include animation(flashDouble $tmg-f1 ease-in-out 1);
+}
\ No newline at end of file

cms/static/sass/elements/_controls.scss

@@ -200,3 +200,83 @@
 %view-live-button {
   @extend %t-action4;
 }
+
+// ====================
+
+// UI: drag handles
+.drag-handle {
+
+  &:hover, &:focus {
+    cursor: move;
+  }
+}
+
+// UI: elem is draggable
+.is-draggable {
+  @include transition(border-color $tmg-f2 ease-in-out 0, box-shadow $tmg-f2 ease-in-out 0);
+  position: relative;
+
+  .draggable-drop-indicator {
+    @extend %ui-depth3;
+    @include transition(opacity $tmg-f2 linear 0s);
+    @include size(100%, auto);
+    position: absolute;
+    border-top: 1px solid $blue-l1;
+    opacity: 0.0;
+
+    *[class^="icon-caret"] {
+      @extend %t-icon5;
+      position: absolute;
+      top: -12px;
+      left: -($baseline/4);
+      color: $blue-s1;
+    }
+  }
+
+  .draggable-drop-indicator-before {
+    top: -($baseline/2);
+  }
+
+  .draggable-drop-indicator-after {
+    bottom: -($baseline/2);
+  }
+}
+
+// UI: drag state - is dragging
+.is-dragging {
+  @extend %ui-depth4;
+  left: -($baseline/4);
+  box-shadow: 0 1px 2px 0 $shadow-d1;
+  cursor: move;
+  opacity: 0.65;
+  border: 1px solid $gray-d3;
+
+  // UI: condition - valid drop
+  &.valid-drop {
+    border-color: $blue-s1;
+    box-shadow: 0 1px 2px 0 $blue-t2;
+  }
+}
+
+// UI: drag state - was dragging
+.was-dragging {
+  @include transition(transform $tmg-f2 ease-in-out 0);
+}
+
+// UI: drag target
+.drop-target {
+
+  &.drop-target-before {
+
+    > .draggable-drop-indicator-before {
+      opacity: 1.0;
+    }
+  }
+
+  &.drop-target-after {
+
+    > .draggable-drop-indicator-after {
+      opacity: 1.0;
+    }
+  }
+}

cms/static/sass/elements/_system-feedback.scss

@@ -712,7 +712,7 @@
 
   // notification showing/hiding
   .wrapper-notification {
-    bottom: -($notification-height);
+    bottom: -($ui-notification-height);
 
     // varying animations
     &.is-shown {

cms/static/sass/views/_subsection.scss

@@ -400,4 +400,21 @@
       }
     }
   }
+
+  // UI: DnD - specific elems/cases - units
+  .courseware-unit {
+
+    .draggable-drop-indicator-before {
+      top: 0;
+    }
+
+    .draggable-drop-indicator-after {
+      bottom: 0;
+    }
+  }
+
+  // UI: DnD - specific elems/cases - empty parents initial drop indicator
+  .draggable-drop-indicator-initial {
+    display: none;
+  }
 }

cms/static/sass/views/_unit.scss

@@ -415,6 +415,19 @@ body.course.unit,.view-unit {
         margin-left: 0;
       }
     }
+
+    // UI: DnD - specific elems/cases - unit
+    .courseware-unit {
+
+      // STATE: was dropped
+      &.was-dropped {
+
+        > .section-item {
+          background-color: $ui-update-color !important; // nasty, but needed for specificity
+        }
+      }
+    }
+
     // ====================
 
     // Component Editing

cms/templates/base.html

@@ -65,7 +65,8 @@ var require = {
         "jquery.tinymce": "js/vendor/tiny_mce/jquery.tinymce",
         "mathjax": "https://edx-static.s3.amazonaws.com/mathjax-MathJax-727332c/MathJax.js?config=TeX-MML-AM_HTMLorMML-full",
         "xmodule": "/xmodule/xmodule",
-        "utility": "js/src/utility"
+        "utility": "js/src/utility",
+        "draggabilly": "js/vendor/draggabilly.pkgd"
     },
     shim: {
         "gettext": {

cms/templates/component.html

@@ -31,6 +31,6 @@
   <a href="#" class="edit-button standard"><span class="edit-icon"></span>${_("Edit")}</a>
   <a href="#" class="delete-button standard"><span class="delete-icon"></span>${_("Delete")}</a>
 </div>
-<a data-tooltip='${_("Drag to reorder")}' href="#" class="drag-handle"></a>
+<span data-tooltip='${_("Drag to reorder")}' class="drag-handle"></span>
 ${preview}
 

cms/templates/overview.html

@@ -82,7 +82,7 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
         </div>
         <div class="item-actions">
           <a href="#" data-tooltip="${_('Delete this section')}" class="delete-button delete-section-button"><span class="delete-icon"></span></a>
-          <a href="#" data-tooltip="${_('Drag to re-order')}" class="drag-handle"></a>
+          <span data-tooltip="${_('Drag to re-order')}" class="drag-handle"></span>
         </div>
       </header>
     </section>
@@ -138,9 +138,12 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
 
   <div class="main-wrapper">
     <div class="inner-wrapper">
-      <article class="courseware-overview" data-course-id="${context_course.location.url()}">
+      <article class="courseware-overview" data-id="${context_course.location.url()}">
         % for section in sections:
-        <section class="courseware-section branch" data-id="${section.location}">
+        <section class="courseware-section branch is-draggable" data-id="${section.location}" data-parent-id="${context_course.location.url()}">
+
+          <%include file="widgets/_ui-dnd-indicator-before.html" />
+
           <header>
           <a href="#" data-tooltip="${_('Expand/collapse this section')}" class="expand-collapse-icon collapse"></a>
 
@@ -169,7 +172,7 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
 
             <div class="item-actions">
               <a href="#" data-tooltip="${_('Delete this section')}" class="delete-button delete-section-button"><span class="delete-icon"></span></a>
-              <a href="#" data-tooltip="${_('Drag to reorder')}" class="drag-handle"></a>
+              <span data-tooltip="${_('Drag to reorder')}" class="drag-handle section-drag-handle"></span>
             </div>
           </header>
           <div class="subsection-list">
@@ -178,9 +181,12 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
                 <span class="new-folder-icon"></span>${_("New Subsection")}
               </a>
             </div>
-            <ol data-section-id="${section.location.url()}">
+            <ol class="sortable-subsection-list" data-id="${section.location.url()}">
               % for subsection in section.get_children():
-              <li class="branch collapsed id-holder" data-id="${subsection.location}">
+              <li class="courseware-subsection branch collapsed id-holder is-draggable" data-id="${subsection.location}" data-parent-id="${section.location.url()}">
+
+                <%include file="widgets/_ui-dnd-indicator-before.html" />
+
                 <div class="section-item">
                   <div class="details">
                     <a href="#" data-tooltip="${_('Expand/collapse this subsection')}" class="expand-collapse-icon expand"></a>
@@ -195,14 +201,21 @@ require(["domReady!", "jquery", "js/models/location", "js/models/section", "js/v
 
                   <div class="item-actions">
                     <a href="#" data-tooltip="${_('Delete this subsection')}" class="delete-button delete-subsection-button"><span class="delete-icon"></span></a>
-                    <a href="#" data-tooltip="${_('Drag to reorder')}" class="drag-handle"></a>
+                    <span data-tooltip="${_('Drag to reorder')}" class="drag-handle subsection-drag-handle"></span>
                   </div>
                 </div>
                 ${units.enum_units(subsection)}
+
+                <%include file="widgets/_ui-dnd-indicator-after.html" />
               </li>
               % endfor
+              <li class="ui-splint ui-splint-indicator">
+                <%include file="widgets/_ui-dnd-indicator-initial.html" />
+              </li>
             </ol>
           </div>
+
+          <%include file="widgets/_ui-dnd-indicator-after.html" />
         </section>
         % endfor
       </article>

cms/templates/static-pages.html

@@ -18,21 +18,21 @@
           <a href="#" class="page-name">${_("Course Info")}</a>
           <div class="item-actions">
             <a href="#" class="edit-button wip"><span class="delete-icon"></span></a>
-            <a href="#" class="drag-handle wip"></a>
+            <span class="drag-handle wip"></span>
           </div>
         </li>
         <li class="static-page-item">
           <a href="#" class="page-name">${_("Textbook")}</a>
           <div class="item-actions">
             <a href="#" class="edit-button wip"><span class="delete-icon"></span></a>
-            <a href="#" class="drag-handle wip"></a>
+            <span class="drag-handle wip"></span>
           </div>
         </li>
         <li class="static-page-item">
           <a href="#" class="page-name">${_("Syllabus")}</a>
           <div class="item-actions">
             <a href="#" class="edit-button wip"><span class="delete-icon"></span></a>
-            <a href="#" class="drag-handle wip"></a>
+            <span class="drag-handle wip"></span>
           </div>
         </li>
       </ul>

cms/templates/widgets/_ui-dnd-indicator-after.html

+<span class="draggable-drop-indicator draggable-drop-indicator-after"><i class="icon-caret-right"></i></span>

cms/templates/widgets/_ui-dnd-indicator-before.html

+<span class="draggable-drop-indicator draggable-drop-indicator-before"><i class="icon-caret-right"></i></span>

cms/templates/widgets/_ui-dnd-indicator-initial.html

+<span class="draggable-drop-indicator draggable-drop-indicator-initial"><i class="icon-caret-right"></i></span>

cms/templates/widgets/units.html

@@ -5,13 +5,16 @@
 This def will enumerate through a passed in subsection and list all of the units
 -->
 <%def name="enum_units(subsection, actions=True, selected=None, sortable=True, subsection_units=None)">
-<ol ${'class="sortable-unit-list"' if sortable else ''} data-subsection-id="${subsection.location}">
+<ol ${'class="sortable-unit-list"' if sortable else ''}>
   <%
     if subsection_units is None:
       subsection_units = subsection.get_children()
   %>
   % for unit in subsection_units:
-  <li class="leaf unit" data-id="${unit.location}">
+  <li class="courseware-unit leaf unit is-draggable" data-id="${unit.location}" data-parent-id="${subsection.location.url()}">
+
+    <%include file="_ui-dnd-indicator-before.html" />
+
     <%
       unit_state = compute_unit_state(unit)
       if unit.location == selected:
@@ -27,13 +30,17 @@ This def will enumerate through a passed in subsection and list all of the units
       % if actions:
       <div class="item-actions">
 	<a href="#" data-tooltip="Delete this unit" class="delete-button" data-id="${unit.location}"><span class="delete-icon"></span></a>
-	<a href="#" data-tooltip="Drag to sort" class="drag-handle"></a>
+	<span data-tooltip="Drag to sort" class="drag-handle unit-drag-handle"></span>
       </div>
       % endif
     </div>
+
+    <%include file="_ui-dnd-indicator-after.html" />
   </li>
   % endfor
   <li>
+    <%include file="_ui-dnd-indicator-initial.html" />
+
     <a href="#" class="new-unit-item" data-category="${new_unit_category}" data-parent="${subsection.location}">
       <span class="new-unit-icon"></span>New Unit
     </a>

common/static/sass/_mixins-inherited.scss

@@ -358,11 +358,7 @@
   background: $lightGrey;
 
   .branch {
-    margin-bottom: 10px;
-
-    &.collapsed {
-      margin-bottom: 0;
-    }
+    margin-bottom: 0;
   }
 
   .branch > .section-item {
@@ -370,6 +366,7 @@
   }
 
   .section-item {
+    @include transition(background $tmg-avg ease-in-out 0);
     position: relative;
     display: block;
     padding: 6px 8px 8px 16px;
@@ -377,7 +374,7 @@
     font-size: 13px;
 
     &:hover {
-      background: #fffcf1;
+      background: $orange-l4;
 
       .item-actions {
         display: block;
@@ -385,7 +382,7 @@
     }
 
     &.editing {
-      background: #fffcf1;
+      background: $orange-l4;
     }
 
     .draft-item:after,