diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index 5cfa0015a9..1ce7242183 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -1594,7 +1594,7 @@ class CollectionViewItemTree extends ItemTree { row = 0; orient = -1; } - this._dropRow = null; + this._clearDropIndicator(); Zotero.DragDrop.currentDragSource = null; // Use the effect set in onDragOver(), which the drop event's dropEffect may not reflect // (see LibraryTreeView::setDropEffect()) diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 15968bfdd1..99b86b1e87 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -1149,22 +1149,10 @@ var ItemTree = class ItemTree extends LibraryTree { } componentDidMount() { + super.componentDidMount(); this._initialized = true; this._loadingDeferredResolved = true; this._itemTreeLoadingDeferred.resolve(); - // Create an element where we can create drag images to be displayed next to the cursor while dragging - // since for multiple item drags we need to display all the elements - let elem = this._dragImageContainer = document.createElement("div"); - elem.style.width = "100%"; - elem.style.height = "2000px"; - elem.style.position = "absolute"; - elem.style.top = "-10000px"; - elem.className = "drag-image-container"; - this.domEl.appendChild(elem); - } - - componentWillUnmount() { - this.domEl.removeChild(this._dragImageContainer); } @@ -1258,6 +1246,9 @@ var ItemTree = class ItemTree extends LibraryTree { loading: false, message: null, }) { + // Row indexes are changing, so clear any drop highlight, which is index-based + this._clearDropIndicator(); + // Handle loading/message state if (options.loading) { options.message ||= Zotero.getString('pane.items.loading'); diff --git a/chrome/content/zotero/libraryTree.js b/chrome/content/zotero/libraryTree.js index ee80a5b157..10b28f6ab0 100644 --- a/chrome/content/zotero/libraryTree.js +++ b/chrome/content/zotero/libraryTree.js @@ -66,12 +66,29 @@ var LibraryTree = class LibraryTree extends React.Component { elem.style.top = "-10000px"; elem.className = "drag-image-container"; this.domEl.appendChild(elem); + // Normal mousemove events are suppressed during native drag-and-drop, so a mousemove + // with a drop row still set means the drag ended without a dragleave/drop/dragend + // reaching the tree (e.g., a drag that left the window). Clear the leftover drop + // indicator when mouse movement resumes. + this.domEl.addEventListener('mousemove', this._clearDropIndicator, { passive: true }); } componentWillUnmount() { this.domEl.removeChild(this._dragImageContainer); + this.domEl.removeEventListener('mousemove', this._clearDropIndicator); } + _clearDropIndicator = () => { + let dropRow = this._dropRow; + if (dropRow === null || dropRow === undefined) return; + this._dropRow = null; + // The row may no longer exist if rows were removed, in which case the next full + // render drops it + if (this.tree && dropRow < this.rowCount) { + this.tree.invalidateRow(dropRow); + } + }; + componentDidCatch(error, info) { // Async operations might attempt to update the react components // after window close in tests, which will cause unnecessary crashing diff --git a/test/tests/libraryTreeTest.js b/test/tests/libraryTreeTest.js index 582f43e0bb..e3316cc1d6 100644 --- a/test/tests/libraryTreeTest.js +++ b/test/tests/libraryTreeTest.js @@ -27,6 +27,75 @@ describe("Zotero.LibraryTree", function() { }); }); + describe("#_clearDropIndicator()", function () { + // Simulate a drag pausing over a row, highlighting it as a drop target. The tests then + // send no further drag events, as when a drag ends outside the window without the tree + // receiving a dragleave, drop, or dragend. + function simulateDragOverRow(view, index, draggedItemID) { + var rowEl = { + classList: { contains: () => false }, + getBoundingClientRect: () => ({ y: 0, height: 100 }) + }; + Zotero.DragDrop.currentDragSource = view.collectionTreeRows[0]; + view.onDragOver({ + preventDefault: () => {}, + stopPropagation: () => {}, + currentTarget: rowEl, + target: rowEl, + // Middle of the row, so the drop targets the row itself + clientY: 50, + dataTransfer: { + dropEffect: 'copy', + effectAllowed: 'copyMove', + types: ['zotero/item'], + getData: type => type == 'zotero/item' ? String(draggedItemID) : "" + } + }, index); + Zotero.DragDrop.currentDragSource = null; + } + + it("should clear a leftover drop highlight on mousemove", async function () { + var item = await createDataObject('item'); + var note = new Zotero.Item('note'); + note.setNote('A note'); + await note.saveTx(); + var view = zp.itemsView; + var row = view.getRowIndexByID(item.treeViewID); + + simulateDragOverRow(view, row, note.id); + assert.equal(view._dropRow, row); + assert.ok(view.domEl.querySelector('.row.drop')); + + view.domEl.dispatchEvent(new win.MouseEvent('mousemove', { bubbles: true })); + + assert.isNull(view._dropRow); + assert.notOk(view.domEl.querySelector('.row.drop')); + }); + + it("should clear the drop highlight when the highlighted last row is removed", async function () { + var collection = await createDataObject('collection'); + await select(win, collection); + await createDataObject('item', { collections: [collection.id] }); + await createDataObject('item', { collections: [collection.id] }); + var note = new Zotero.Item('note'); + note.setNote('A note'); + await note.saveTx(); + var view = zp.itemsView; + var lastRow = view.rowCount - 1; + var lastItem = Zotero.Items.get(view.getRow(lastRow).id); + + simulateDragOverRow(view, lastRow, note.id); + assert.equal(view._dropRow, lastRow); + assert.ok(view.domEl.querySelector('.row.drop')); + + await Zotero.Items.trashTx([lastItem.id]); + + assert.equal(view.rowCount, 1); + assert.isNull(view._dropRow); + assert.notOk(view.domEl.querySelector('.row.drop')); + }); + }); + describe("#_removeRow()", function () { it("should remove the last row", async function () { var collection = await createDataObject('collection');