diff --git a/Gemfile.lock b/Gemfile.lock index 11a0da3..57b618a 100644 --- a/Gemfile.lock +++ b/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 diff --git a/app/assets/stylesheets/katalyst/navigation/editor/_item-actions.scss b/app/assets/stylesheets/katalyst/navigation/editor/_item-actions.scss index 88f4dd3..38d1784 100644 --- a/app/assets/stylesheets/katalyst/navigation/editor/_item-actions.scss +++ b/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; diff --git a/app/assets/stylesheets/katalyst/navigation/editor/_new-items.scss b/app/assets/stylesheets/katalyst/navigation/editor/_new-items.scss index 4ea5b15..c941cd0 100644 --- a/app/assets/stylesheets/katalyst/navigation/editor/_new-items.scss +++ b/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 { diff --git a/app/components/katalyst/navigation/editor/table_component.rb b/app/components/katalyst/navigation/editor/table_component.rb index 8020e3e..a1e265d 100644 --- a/app/components/katalyst/navigation/editor/table_component.rb +++ b/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 diff --git a/app/javascript/navigation/editor/list_controller.js b/app/javascript/navigation/editor/list_controller.js index a84a364..70c17d1 100644 --- a/app/javascript/navigation/editor/list_controller.js +++ b/app/javascript/navigation/editor/list_controller.js @@ -1,6 +1,19 @@ 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; @@ -8,51 +21,89 @@ export default class ListController extends Controller { 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']")) + ); +} diff --git a/app/views/katalyst/navigation/items/new.html.erb b/app/views/katalyst/navigation/items/new.html.erb deleted file mode 100644 index fc892b3..0000000 --- a/app/views/katalyst/navigation/items/new.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -<%= turbo_frame_tag "navigation--editor--item-frame" do %> -

New <%= item.model_name.human.downcase %>

- <%= render item.model_name.param_key, item:, path: menu_items_path(item.menu) %> -<% end %> diff --git a/katalyst-navigation.gemspec b/katalyst-navigation.gemspec index 6c45103..1851cea 100644 --- a/katalyst-navigation.gemspec +++ b/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 = ["developers@katalyst.com.au"] diff --git a/spec/dummy/app/assets/stylesheets/application.scss b/spec/dummy/app/assets/stylesheets/application.scss index 306ee92..2d42b99 100644 --- a/spec/dummy/app/assets/stylesheets/application.scss +++ b/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 {