From dd0c5773241795d5a04991b1138f9e97e3e08c36 Mon Sep 17 00:00:00 2001
From: Johannes Loher <johannes.loher@fg4f.de>
Date: Sun, 25 Jul 2021 02:40:28 +0200
Subject: [PATCH] Fix drag & drop of items

---
 src/module/actor/sheets/actor-sheet.ts        | 60 +++++++++++--------
 .../actor/components/control-button-group.hbs |  2 +-
 .../actor/components/effect-list-entry.hbs    |  2 +-
 .../actor/components/item-list-entry.hbs      |  2 +-
 4 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/src/module/actor/sheets/actor-sheet.ts b/src/module/actor/sheets/actor-sheet.ts
index 72d75a81..99e8829f 100644
--- a/src/module/actor/sheets/actor-sheet.ts
+++ b/src/module/actor/sheets/actor-sheet.ts
@@ -147,7 +147,9 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating click event
      */
     protected onEditItem(event: JQuery.ClickEvent): void {
-        const id = $(event.currentTarget).parents(".item").data("id");
+        const id = $(event.currentTarget)
+            .parents(embeddedDocumentListEntryProperties.Item.selector)
+            .data(embeddedDocumentListEntryProperties.Item.idDataAttribute);
         const item = this.actor.items.get(id);
         enforce(item, getGame().i18n.format("DS4.ErrorActorDoesNotHaveItem", { id, actor: this.actor.name }));
         enforce(item.sheet);
@@ -160,8 +162,8 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating click event
      */
     protected onDeleteItem(event: JQuery.ClickEvent): void {
-        const li = $(event.currentTarget).parents(".item");
-        this.actor.deleteEmbeddedDocuments("Item", [li.data("id")]);
+        const li = $(event.currentTarget).parents(embeddedDocumentListEntryProperties.Item.selector);
+        this.actor.deleteEmbeddedDocuments("Item", [li.data(embeddedDocumentListEntryProperties.Item.idDataAttribute)]);
         li.slideUp(200, () => this.render(false));
     }
 
@@ -172,7 +174,7 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating change event
      */
     protected onChangeItem(event: JQuery.ChangeEvent): void {
-        return this.onChangeEmbeddedDocument(event, ".item", "Item");
+        return this.onChangeEmbeddedDocument(event, "Item");
     }
 
     /**
@@ -214,7 +216,9 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating click event
      */
     protected onEditEffect(event: JQuery.ClickEvent): void {
-        const id = $(event.currentTarget).parents(".effect").data("id");
+        const id = $(event.currentTarget)
+            .parents(embeddedDocumentListEntryProperties.ActiveEffect.selector)
+            .data(embeddedDocumentListEntryProperties.ActiveEffect.idDataAttribute);
         const effect = this.actor.effects.get(id);
         enforce(effect, getGame().i18n.format("DS4.ErrorActorDoesNotHaveEffect", { id, actor: this.actor.name }));
         effect.sheet.render(true);
@@ -226,8 +230,8 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating click event
      */
     protected onDeleteEffect(event: JQuery.ClickEvent): void {
-        const li = $(event.currentTarget).parents(".effect");
-        const id = li.data("id");
+        const li = $(event.currentTarget).parents(embeddedDocumentListEntryProperties.ActiveEffect.selector);
+        const id = li.data(embeddedDocumentListEntryProperties.ActiveEffect.idDataAttribute);
         this.actor.deleteEmbeddedDocuments("ActiveEffect", [id]);
         li.slideUp(200, () => this.render(false));
     }
@@ -239,7 +243,7 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * @param event - The originating change event
      */
     protected onChangeEffect(event: JQuery.ChangeEvent): void {
-        return this.onChangeEmbeddedDocument(event, ".effect", "ActiveEffect");
+        return this.onChangeEmbeddedDocument(event, "ActiveEffect");
     }
 
     /**
@@ -247,45 +251,37 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      * on the `data-property` attribute of the {@link HTMLInputElement} that has been changed and its new value.
      *
      * @param event - The originating change event
-     * @param documentSelector - The selector for the closest parent of the changed {@link HTMLInputElement}, which
-     *                           contains the `data-id` attribute providing  the `id` of the embedded document to be
-     *                           changed.
      * @param documentName - The name of the embedded document to be changed.
      */
-    protected onChangeEmbeddedDocument(
-        event: JQuery.ChangeEvent,
-        documentSelector: string,
-        documentName: "Item" | "ActiveEffect",
-    ): void {
+    protected onChangeEmbeddedDocument(event: JQuery.ChangeEvent, documentName: "Item" | "ActiveEffect"): void {
         event.preventDefault();
         const element = $(event.currentTarget).get(0);
         enforce(element instanceof HTMLInputElement);
         if (element.disabled) return;
 
-        const effectElement = element.closest(documentSelector);
+        const effectElement = element.closest(embeddedDocumentListEntryProperties[documentName].selector);
         enforce(effectElement instanceof HTMLElement);
-        const id = effectElement.dataset["id"];
+        const id = effectElement.dataset[embeddedDocumentListEntryProperties[documentName].idDataAttribute];
         const property = element.dataset["property"];
-        const inverted = Boolean(element.dataset["inverted"]);
         enforce(property !== undefined, TypeError("HTML element does not provide 'data-property' attribute"));
 
-        const newValue = this.parseValue(element, inverted);
+        const newValue = this.parseValue(element);
         this.actor.updateEmbeddedDocuments(documentName, [{ _id: id, [property]: newValue }]);
     }
 
     /**
      * Parses the value of the given {@link HTMLInputElement} depending on the element's type
      * The value is parsed to:
-     * - checkbox: `boolean`
+     * - checkbox: `boolean`, if the attribute `data-inverted` is set to a truthy value, the parsed value is inverted
      * - text input: `string`
      * - number: `number`
      *
      * @param element - The input element to parse the value from
-     * @param inverted - Whether or not the value should be inverted
      */
-    protected parseValue(element: HTMLInputElement, inverted = false): boolean | string | number {
+    protected parseValue(element: HTMLInputElement): boolean | string | number {
         switch (element.type) {
             case "checkbox": {
+                const inverted = Boolean(element.dataset["inverted"]);
                 const value: boolean = element.checked;
                 return inverted ? !value : value;
             }
@@ -311,7 +307,9 @@ export class DS4ActorSheet extends ActorSheet<ActorSheet.Options, DS4ActorSheetD
      */
     protected onRollItem(event: JQuery.ClickEvent): void {
         event.preventDefault();
-        const id = $(event.currentTarget).parents(".item").data("id");
+        const id = $(event.currentTarget)
+            .parents(embeddedDocumentListEntryProperties.Item.selector)
+            .data(embeddedDocumentListEntryProperties.Item.idDataAttribute);
         const item = this.actor.items.get(id);
         enforce(item, getGame().i18n.format("DS4.ErrorActorDoesNotHaveItem", { id, actor: this.actor.name }));
         item.roll().catch((e) => notifications.error(e, { log: true }));
@@ -379,3 +377,17 @@ interface EnrichedActiveEffectDataSource extends ActiveEffectDataSource {
     icon: string;
     sourceName: string;
 }
+
+/**
+ * This object contains information about specific properties embedded document list entries for each different type.
+ */
+const embeddedDocumentListEntryProperties = Object.freeze({
+    ActiveEffect: {
+        selector: ".effect",
+        idDataAttribute: "effectId",
+    },
+    Item: {
+        selector: ".item",
+        idDataAttribute: "itemId",
+    },
+});
diff --git a/src/templates/sheets/actor/components/control-button-group.hbs b/src/templates/sheets/actor/components/control-button-group.hbs
index 89713898..dffef3e0 100644
--- a/src/templates/sheets/actor/components/control-button-group.hbs
+++ b/src/templates/sheets/actor/components/control-button-group.hbs
@@ -7,7 +7,7 @@ SPDX-License-Identifier: MIT
 
 {{!--
 !-- Render a group of an "edit" and a "delete" button.
-!-- The current item is defined by the data-id attribute of the parent li element.
+!-- The current item is defined by the a corresponding id attribute of the parent li element.
 !-- @param documentType: The type of document that is controlled by this button group, item or effect
 !-- @param editTitle: The title to use for the edit link element (will be localized)
 !-- @param deleteTitle: The title to use for the delete link element (will be localized)
diff --git a/src/templates/sheets/actor/components/effect-list-entry.hbs b/src/templates/sheets/actor/components/effect-list-entry.hbs
index 4fbb0a95..f9c9d908 100644
--- a/src/templates/sheets/actor/components/effect-list-entry.hbs
+++ b/src/templates/sheets/actor/components/effect-list-entry.hbs
@@ -9,7 +9,7 @@ SPDX-License-Identifier: MIT
 !-- Render an effect list entry row.
 !-- @param effectData: The data of the item.
 --}}
-<li class="ds4-embedded-document-list__row effect" data-id="{{effectData._id}}">
+<li class="ds4-embedded-document-list__row effect" data-effect-id="{{effectData._id}}">
     {{!-- enabled --}}
     <input class="ds4-embedded-document-list__editable ds4-embedded-document-list__editable--checkbox change-effect" type="checkbox" {{checked
         (ne effectData.disabled true)}} data-dtype="Boolean" data-property="disabled" data-inverted="true"
diff --git a/src/templates/sheets/actor/components/item-list-entry.hbs b/src/templates/sheets/actor/components/item-list-entry.hbs
index 29a6d00a..8cfd3a44 100644
--- a/src/templates/sheets/actor/components/item-list-entry.hbs
+++ b/src/templates/sheets/actor/components/item-list-entry.hbs
@@ -15,7 +15,7 @@ SPDX-License-Identifier: MIT
 !-- @param hideDescription: A flag to disable the description column.
 !-- @param @partial-block: Custom column headers can be passed using the partial block.
 --}}
-<li class="ds4-embedded-document-list__row item" data-id="{{itemData._id}}">
+<li class="ds4-embedded-document-list__row item" data-item-id="{{itemData._id}}">
     {{!-- equipped --}}
     {{#if isEquipable}}
     <input class="ds4-embedded-document-list__editable ds4-embedded-document-list__editable--checkbox change-item" type="checkbox" {{checked