Drag and drop on the course outline page

[cahrens]

STUD-1939

@talbs here is the new branch

[talbs]

@cahrens, I have most of the visual styling and rendering back into this branch. There's one outstanding use case I need your help on, dragging an item into am empty parent. I've added the following splint element into the template:

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

But my styling to show this UI splint is looking for the empty parent to have the .drop-target class. From what I can tell from the live DOM, this class isn't being applied. Can you check to see if that's true and if so, make sure that class is added to those parent elements?

[talbs]

@frrrances, most of my rendering work is set. Mind giving this a review from a UI, visual, and FED perspective?

[cahrens]

@talbs I was adding the drop-target class, but it gets added to the ol, and your splint was outside of the ol. I moved it inside (making it an li), and now the drop target is shown. It needs a little styling though.

I rebased on Andy's branch, and performance is MUCH better. Perhaps less styles are being computed now? Let me know if you think it is "zippy" enough-- if not, I can try profiling what is going on.

Update: after using it for longer, it seemed to get slower, at least for units with an icon and a warning message. I wonder if hiding those elements while dragging would help.

[cahrens]

I identified one of the issues with drag and drop performance. If you drag something and release it in a place where it is not a valid drop location, the element goes back to its original location. After that, trying to drag the same element again is incredibly slow (and it is not clear that it ever re-arranges correctly). This is an area that needs investigation.

Repro steps:

1. In a subsection with several units, drag one of them outside the subsection but not into a valid location within a different subsection (the outline of the element you are dragging should NOT be blue).
2. Release the element. It should go back to its original location.
3. Now try dragging the same element again. It will be very slow and basically impossible to relocate until you refresh the page.

[cahrens]

@talbs, I'm noticing that when I drag and am in a "non droppable" area, there is a single pixel on the left-hand side that is not the grey color. Instead, it is the color that the element was in the outline before the drag (yellow for "has warning", blue for "released", etc.).

[talbs]

@cahrens, some status on your feedback...

@talbs I was adding the drop-target class, but it gets added to the ol, and your splint was outside of the ol. I moved it inside (making it an li), and now the drop target is shown. It needs a little styling though.

This should be resolved now - the splint-based indicator should look and act like all other indicators. Thanks for the assist.

---

@talbs, I'm noticing that when I drag and am in a "non droppable" area, there is a single pixel on the left-hand side that is not the grey color. Instead, it is the color that the element was in the outline before the drag (yellow for "has warning", blue for "released", etc.).

Good catch. I've addressed that with a bit style overriding for dragging cases.

[cahrens]

@polesye Please review this PR and investigate the performance issue (bug?) detailed above.

[polesye]

Fix indentation.

[polesye]

I identified one of the issues with drag and drop performance. If you drag something and release it in a place where it is not a valid drop location, the element goes back to its original location. After that, trying to drag the same element again is incredibly slow (and it is not clear that it ever re-arranges correctly). This is an area that needs investigation.

Repro steps:

1. In a subsection with several units, drag one of them outside the subsection but not into a valid location within a different subsection (the outline of the element you are dragging should NOT be blue).
2. Release the element. It should go back to its original location.
3. Now try dragging the same element again. It will be very slow and basically impossible to relocate until you refresh the page.

@cahrens The problem is in the class 'was-dragging' especially in 'transition' property that it uses.
This class name is added only when we drop our element into invalid location and I do not know why it is not removed on dragStart.

[polesye]

The same issue with Sections and subsections:
Class name 'is-collapsible' and @extend %ui-expand-collapse; that uses transition under the hood.

[polesye]

Remove empty lines.

[talbs]

@cahrens The problem is in the class 'was-dragging' especially in 'transition' property that it uses.
This class name is added only when we drop our element into invalid location and I do not know why it is not removed on dragStart.

@polesye, thanks for the help in debugging performance. I've tried removing the CSS transition from the .was-dragging class and there was no difference - the performance issue with units still persisted. @cahrens, can we focus on remove that .was-dragging class on the dragStart event as Anton suggested?

[talbs]

The same issue with Sections and subsections:
Class name 'is-collapsible' and extend %ui-expand-collapse; that uses transition under the hood.

I also tried removing the CSS transition from our expand/collapse placeholder (which is a bit more essential to transitioning the outline UI states) and there was no difference - the performance issue with units still persisted there as well.

I'm hoping the removal of the .was-dragging class helps both of these cases.

[polesye]

@talbs I just double checked with the following diff:

diff --git a/cms/static/sass/elements/_controls.scss b/cms/static/sass/elements/_controls.scss
index a8074eb..15ed230 100644
--- a/cms/static/sass/elements/_controls.scss
+++ b/cms/static/sass/elements/_controls.scss
@@ -305,7 +305,7 @@

 // UI: expand collapse
 %ui-expand-collapse {
-  @include transition(all $tmg-f2 linear 0s);
+  // @include transition(all $tmg-f2 linear 0s);


   // CASE: default (is expanded)
@@ -395,7 +395,7 @@

 // UI: drag state - was dragging
 .was-dragging {
-  @include transition(transform $tmg-f2 ease-in-out 0);
+  // @include transition(transform $tmg-f2 ease-in-out 0);
 }

Everything works well for me.

[talbs]

@polesye, thanks much again for the double check. It looks like my Sass watcher's recompiled sass wasn't busting through the underscore template's cache. I've restarted my app and narrowed it down to the CSS transition on the .was-dragging class. I've removed this from the Sass/CSS.

Can you and @cahrens, give the performance test another try? It was smooth as silk for me (but we do lose the invalid drag recoiling back to its original place in the outline without this line of code - I'm fine with this for performance's sake).

[polesye]

@talbs It also works well with the following changes:

diff --git a/cms/static/js/utils/drag_and_drop.js b/cms/static/js/utils/drag_and_drop.js
index 19a1e00..1c77d71 100644
--- a/cms/static/js/utils/drag_and_drop.js
+++ b/cms/static/js/utils/drag_and_drop.js
@@ -165,6 +165,8 @@ define(["jquery", "jquery.ui", "underscore", "gettext", "js/views/feedback_notif
                     // onDragStart gets called again after the collapse, so we can't just store a variable in the dragState.
                     ele.addClass(this.expandOnDropClass);
                 }
+
+                $(draggie.element).removeClass('was-dragging');
             },

             onDragMove: function (draggie, event, pointer) {
diff --git a/cms/static/sass/elements/_controls.scss b/cms/static/sass/elements/_controls.scss
index 8467bae..93b53f3 100644
--- a/cms/static/sass/elements/_controls.scss
+++ b/cms/static/sass/elements/_controls.scss
@@ -392,6 +392,9 @@
     box-shadow: 0 1px 2px 0 rgba($ui-action-primary-color-focus, 0.50);
   }
 }
+ .was-dragging {
+  @include transition(transform $tmg-f2 ease-in-out 0);
+ }

 // UI: drag target
 .drop-target {

So, as you can see, we can restore transition for the .was-dragging class and do not lose the invalid drag recoiling back to its original place in the outline

[polesye]

Units looks well, but I'm not happy with dragging sections and subsection.
Can we add class .is-collapsible to some internal element?

[talbs]

@polesye, thanks for the JS assist (and adding the transition back in). I've tested things out locally, and things seem to be pretty snappy at the Section, Subsection, and Unit levels.

[cahrens]

Closing this PR. I have created a new branch based off of bulk-publishing. #4605