Safari drag/drop support

Use enter/exit counting instead of related target to identify when
users leave the drag area and cancel the drag. Works around a
safari issue where relatedTarget is not set.

Also refactored list controller to implement esc cancel and add
documentation and type hints.

Fixed some safari styling issues.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    katalyst-navigation (1.5.1)
+    katalyst-navigation (1.5.2)
       katalyst-html-attributes
       katalyst-kpop
       katalyst-tables

app/assets/stylesheets/katalyst/navigation/editor/_item-actions.scss

@@ -38,6 +38,7 @@
 
     &::before {
       @extend %icon;
+      position: static;
       color: white;
       font-size: 1.125rem;
       line-height: 1.125rem;

app/assets/stylesheets/katalyst/navigation/editor/_new-items.scss

@@ -22,7 +22,7 @@
       font-size: 0.8rem;
       overflow: hidden;
       text-overflow: ellipsis;
-      margin-bottom: 0;
+      margin: 0 auto;
     }
 
     &:hover {

app/components/katalyst/navigation/editor/table_component.rb

@@ -11,6 +11,7 @@ class TableComponent < BaseComponent
           dragleave->#{LIST_CONTROLLER}#dragleave
           drop->#{LIST_CONTROLLER}#drop
           dragend->#{LIST_CONTROLLER}#dragend
+          keyup.esc@document->#{LIST_CONTROLLER}#dragend
         ACTIONS
 
         renders_many :items, ->(item) do

app/javascript/navigation/editor/list_controller.js

@@ -1,58 +1,109 @@
 import { Controller } from "@hotwired/stimulus";
 
 export default class ListController extends Controller {
+  connect() {
+    this.enterCount = 0;
+  }
+
+  /**
+   * When the user starts a drag within the list, set the item's dataTransfer
+   * properties to indicate that it's being dragged and update its style.
+   *
+   * We delay setting the dataset property until the next animation frame
+   * so that the style updates can be computed before the drag begins.
+   *
+   * @param event {DragEvent}
+   */
   dragstart(event) {
     if (this.element !== event.target.parentElement) return;
 
     const target = event.target;
     event.dataTransfer.effectAllowed = "move";
 
     // update element style after drag has begun
-    setTimeout(() => (target.dataset.dragging = ""));
+    requestAnimationFrame(() => (target.dataset.dragging = ""));
   }
 
+  /**
+   * When the user drags an item over another item in the last, swap the
+   * dragging item with the item under the cursor.
+   *
+   * As a special case, if the item is dragged over placeholder space at the end
+   * of the list, move the item to the bottom of the list instead. This allows
+   * users to hit the list element more easily when adding new items to an empty
+   * list.
+   *
+   * @param event {DragEvent}
+   */
   dragover(event) {
-    const item = this.dragItem();
+    const item = this.dragItem;
     if (!item) return;
 
-    swap(this.dropTarget(event.target), item);
+    swap(dropTarget(event.target), item);
 
     event.preventDefault();
     return true;
   }
 
+  /**
+   * When the user drags an item into the list, create a placeholder item to
+   * represent the new item. Note that we can't access the drag data
+   * until drop, so we assume that this is our template item for now.
+   *
+   * Users can cancel the drag by dragging the item out of the list or by
+   * pressing escape. Both are handled by `cancelDrag`.
+   *
+   * @param event {DragEvent}
+   */
   dragenter(event) {
     event.preventDefault();
 
-    if (event.dataTransfer.effectAllowed === "copy" && !this.dragItem()) {
+    // Safari doesn't support relatedTarget, so we count enter/leave pairs
+    this.enterCount++;
+
+    if (copyAllowed(event) && !this.dragItem) {
       const item = document.createElement("li");
       item.dataset.dragging = "";
       item.dataset.newItem = "";
-      this.element.prepend(item);
+      this.element.appendChild(item);
     }
   }
 
+  /**
+   * When the user drags the item out of the list, remove the placeholder.
+   * This allows users to cancel the drag by dragging the item out of the list.
+   *
+   * @param event {DragEvent}
+   */
   dragleave(event) {
-    const item = this.dragItem();
-    const related = this.dropTarget(event.relatedTarget);
-
-    // ignore if item is not set or we're moving into a valid drop target
-    if (!item || related) return;
-
-    // remove item if it's a new item
-    if (item.dataset.hasOwnProperty("newItem")) {
-      item.remove();
+    // Safari doesn't support relatedTarget, so we count enter/leave pairs
+    // https://bugs.webkit.org/show_bug.cgi?id=66547
+    this.enterCount--;
+
+    if (
+      this.enterCount <= 0 &&
+      this.dragItem.dataset.hasOwnProperty("newItem")
+    ) {
+      this.cancelDrag(event);
     }
   }
 
+  /**
+   * When the user drops an item into the list, end the drag and reindex the list.
+   *
+   * If the item is a new item, we replace the placeholder with the template
+   * item data from the dataTransfer API.
+   *
+   * @param event {DragEvent}
+   */
   drop(event) {
-    let item = this.dragItem();
+    let item = this.dragItem;
 
     if (!item) return;
 
     event.preventDefault();
     delete item.dataset.dragging;
-    swap(this.dropTarget(event.target), item);
+    swap(dropTarget(event.target), item);
 
     if (item.dataset.hasOwnProperty("newItem")) {
       const placeholder = item;
@@ -61,7 +112,7 @@ export default class ListController extends Controller {
       item = template.content.querySelector("li");
 
       this.element.replaceChild(item, placeholder);
-      setTimeout(() =>
+      requestAnimationFrame(() =>
         item.querySelector("[role='button'][value='edit']").click()
       );
     }
@@ -73,23 +124,28 @@ export default class ListController extends Controller {
     });
   }
 
+  /**
+   * End an in-progress drag. If the item is a new item, remove it, otherwise
+   * reset the item's style and restore its original position in the list.
+   */
   dragend() {
-    const item = this.dragItem();
-    if (!item) return;
+    const item = this.dragItem;
 
-    delete item.dataset.dragging;
-    this.reset();
+    if (!item) {
+    } else if (item.dataset.hasOwnProperty("newItem")) {
+      item.remove();
+    } else {
+      delete item.dataset.dragging;
+      this.reset();
+    }
   }
 
-  dragItem() {
-    return this.element.querySelector("[data-dragging]");
+  get isDragging() {
+    return !!this.dragItem;
   }
 
-  dropTarget(e) {
-    return (
-      e.closest("[data-controller='navigation--editor--list'] > *") ||
-      e.closest("[data-controller='navigation--editor--list']")
-    );
+  get dragItem() {
+    return this.element.querySelector("[data-dragging]");
   }
 
   reindex() {
@@ -101,6 +157,12 @@ export default class ListController extends Controller {
   }
 }
 
+/**
+ * Swaps two list items. If target is a list, the item is appended.
+ *
+ * @param target the target element to swap with
+ * @param item the item the user is dragging
+ */
 function swap(target, item) {
   if (!target) return;
   if (target === item) return;
@@ -118,3 +180,26 @@ function swap(target, item) {
     target.appendChild(item);
   }
 }
+
+/**
+ * Returns true if the event supports copy or copy move.
+ *
+ * Chrome and Firefox use copy, but Safari only supports copyMove.
+ */
+function copyAllowed(event) {
+  return (
+    event.dataTransfer.effectAllowed === "copy" ||
+    event.dataTransfer.effectAllowed === "copyMove"
+  );
+}
+
+/**
+ * Given an event target, return the closest drop target, if any.
+ */
+function dropTarget(e) {
+  return (
+    e &&
+    (e.closest("[data-controller='navigation--editor--list'] > *") ||
+      e.closest("[data-controller='navigation--editor--list']"))
+  );
+}

katalyst-navigation.gemspec

@@ -2,7 +2,7 @@
 
 Gem::Specification.new do |spec|
   spec.name = "katalyst-navigation"
-  spec.version = "1.5.1"
+  spec.version = "1.5.2"
   spec.authors = ["Katalyst Interactive"]
   spec.email = ["[email protected]"]
 

spec/dummy/app/assets/stylesheets/application.scss

@@ -45,9 +45,10 @@ main {
   grid-area: content;
 }
 
-#navigation--editor--item-frame,
+.navigation--editor--new-items,
 .index-table-actions {
   grid-area: sidebar;
+  margin-bottom: auto;
 }
 
 .button-row {