diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index a2c2bd248d..54de61c126 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2249,16 +2249,8 @@ var ItemTree = class ItemTree extends LibraryTree { div.classList.remove('tight'); } - if (this._dropRow == index) { - let span; - if (Zotero.DragDrop.currentOrientation != 0) { - span = document.createElement('span'); - span.className = Zotero.DragDrop.currentOrientation < 0 ? 'drop-before' : 'drop-after'; - div.appendChild(span); - } - else { - div.classList.add('drop'); - } + if (this._dropRow == index && Zotero.DragDrop.currentOrientation == 0) { + div.classList.add('drop'); } let { firstColumn } = columns.reduce((acc, column) => { @@ -2272,6 +2264,13 @@ var ItemTree = class ItemTree extends LibraryTree { row.renderRow(div, index, columns, rowData, this._renderCtx); + // After cells -- a leading drop line is treated as the first column and indents the title + if (this._dropRow == index && Zotero.DragDrop.currentOrientation != 0) { + let span = document.createElement('span'); + span.className = Zotero.DragDrop.currentOrientation < 0 ? 'drop-before' : 'drop-after'; + div.appendChild(span); + } + if (!oldDiv) { if (this.props.dragAndDrop && row.isDraggable) { div.setAttribute('draggable', true); diff --git a/scss/components/_virtualized-table.scss b/scss/components/_virtualized-table.scss index c7d8521533..ec17cc30a8 100644 --- a/scss/components/_virtualized-table.scss +++ b/scss/components/_virtualized-table.scss @@ -60,7 +60,8 @@ padding-inline-end: 4px; } - &:first-child { + &:first-child, + &.first-column { --extra-width: var(--first-column-extra-width, 0px); // No padding on the first cell since it's done via twisty and indent padding-inline-start: 0; @@ -114,6 +115,9 @@ width: 100%; box-sizing: border-box; border-radius: 5px; + // Positioning context for span.drop-before / span.drop-after. Without this the + // 20% line is resolved against the table and leftover flow indents a neighbor. + position: relative; &.drop:not(.flashing) { color: var(--material-background) !important; diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index 817b525c38..504b4213b1 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -2188,6 +2188,91 @@ describe("CollectionViewItemTree", function () { }) + describe("#onDragOver()", function () { + function fileDataTransfer() { + var file = getTestDataDirectory(); + file.append('test.png'); + return { + dropEffect: 'copy', + effectAllowed: 'copy', + types: ['application/x-moz-file'], + mozItemCount: 1, + mozGetDataAt: function (type, i) { + if (type == 'application/x-moz-file' && i == 0) { + return file; + } + } + }; + } + + // clientY ratio: first 1/6 of the row is drop-before (-1), last 1/6 is drop-after (1) + function dragOverFile(index, clientY) { + var rowEl = { + classList: { contains: () => false }, + getBoundingClientRect: () => ({ y: 0, height: 100 }) + }; + itemsView.onDragOver({ + preventDefault: () => {}, + stopPropagation: () => {}, + currentTarget: rowEl, + target: rowEl, + clientY, + dataTransfer: fileDataTransfer() + }, index); + } + + function getIndent(node) { + return parseInt(node.querySelector('.cell-indent')?.style.paddingInlineStart || 0); + } + + function assertTopLevelUnindented(node, row) { + assert.equal(row.level, 0); + assert.equal(getIndent(node), 0); + let firstCell = node.querySelector('.first-column') || node.querySelector('.cell'); + assert.ok(firstCell, "row has a first column"); + // span.drop-before/after must not precede the title cell. :first-child first-column + // CSS (padding / --extra-width) otherwise treats the drop line as a cell and shifts + // the title as if the item were becoming a child of the incoming attachment. + assert.isTrue(firstCell.matches(':first-child'), + "first column must remain the row's first child during a between-row file drop"); + } + + afterEach(function () { + itemsView._dropRow = null; + Zotero.DragDrop.currentOrientation = 0; + }); + + it("should not indent a top-level item when a file drop target is a line", async function () { + var item1 = await createDataObject('item', { title: "Top-level A" }); + var item2 = await createDataObject('item', { title: "Top-level B" }); + await waitForItemsLoad(win); + + var index = itemsView.getRowIndexByID(item1.id); + var neighborIndex = itemsView.getRowIndexByID(item2.id); + assert.equal(itemsView.getRow(index).level, 0); + assert.equal(itemsView.getRow(neighborIndex).level, 0); + assert.isTrue(itemsView.canDropCheck(index, -1, fileDataTransfer()), + "Setup: between-row file drops are allowed"); + + for (let { clientY, orient, selector } of [ + { clientY: 5, orient: -1, selector: 'span.drop-before' }, + { clientY: 95, orient: 1, selector: 'span.drop-after' } + ]) { + dragOverFile(index, clientY); + assert.equal(Zotero.DragDrop.currentOrientation, orient); + assert.equal(itemsView._dropRow, index); + + let node = itemsView.tree._renderItem(index); + let neighborNode = itemsView.tree._renderItem(neighborIndex); + + assert.ok(node.querySelector(selector), + `Setup: ${selector} drop line is shown for orientation ${orient}`); + assertTopLevelUnindented(node, itemsView.getRow(index)); + assertTopLevelUnindented(neighborNode, itemsView.getRow(neighborIndex)); + } + }); + }); + describe("#onDrop()", function () { function drop(index, orient, dataTransfer) { Zotero.DragDrop.currentOrientation = orient;