From 33a30f3875db46d27d5d1834ed955f3a47b7a8b1 Mon Sep 17 00:00:00 2001 From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com> Date: Tue, 21 Nov 2023 23:13:12 +0100 Subject: [PATCH] Remove tree undo and fix drop on root issue --- novelwriter/gui/projtree.py | 92 ++++------------------------- tests/test_gui/test_gui_projtree.py | 52 ++++------------ 2 files changed, 24 insertions(+), 120 deletions(-) diff --git a/novelwriter/gui/projtree.py b/novelwriter/gui/projtree.py index 7324d3e7..fed97b76 100644 --- a/novelwriter/gui/projtree.py +++ b/novelwriter/gui/projtree.py @@ -126,11 +126,6 @@ class GuiProjectView(QWidget): self.keyGoDown.setContext(Qt.ShortcutContext.WidgetShortcut) self.keyGoDown.activated.connect(lambda: self.projTree.moveToLevel(1)) - self.keyUndoMv = QShortcut(self.projTree) - self.keyUndoMv.setKey("Ctrl+Shift+Z") - self.keyUndoMv.setContext(Qt.ShortcutContext.WidgetShortcut) - self.keyUndoMv.activated.connect(lambda: self.projTree.undoLastMove()) - self.keyContext = QShortcut(self.projTree) self.keyContext.setKey("Ctrl+.") self.keyContext.setContext(Qt.ShortcutContext.WidgetShortcut) @@ -320,9 +315,6 @@ class GuiProjectToolBar(QWidget): self.aCollapse = self.mMore.addAction(self.tr("Collapse All")) self.aCollapse.triggered.connect(lambda: self.projTree.setExpandedFromHandle(None, False)) - self.aMoreUndo = self.mMore.addAction(self.tr("Undo Move")) - self.aMoreUndo.triggered.connect(lambda: self.projTree.undoLastMove()) - self.aEmptyTrash = self.mMore.addAction(self.tr("Empty Trash")) self.aEmptyTrash.triggered.connect(lambda: self.projTree.emptyTrash()) @@ -475,7 +467,6 @@ class GuiProjectTree(QTreeWidget): # Internal Variables self._treeMap = {} - self._lastMove = {} self._timeChanged = 0.0 self._popAlert = None @@ -522,8 +513,8 @@ class GuiProjectTree(QTreeWidget): # But don't allow drop on root level # Due to a bug, this stops working somewhere between Qt 5.15.3 # and 5.15.8, so this is also blocked in dropEvent (see #1569) - # trRoot = self.invisibleRootItem() - # trRoot.setFlags(trRoot.flags() ^ Qt.ItemFlag.ItemIsDropEnabled) + trRoot = self.invisibleRootItem() + trRoot.setFlags(trRoot.flags() ^ Qt.ItemFlag.ItemIsDropEnabled) # Cached values self._lblActive = self.tr("Active") @@ -572,7 +563,6 @@ class GuiProjectTree(QTreeWidget): """Clear the GUI content and the related map.""" self.clear() self._treeMap = {} - self._lastMove = {} self._timeChanged = 0.0 return @@ -743,7 +733,6 @@ class GuiProjectTree(QTreeWidget): cItem = pItem.takeChild(tIndex) pItem.insertChild(nIndex, cItem) - self._recordLastMove(cItem, pItem, tIndex) self._alertTreeChange(tHandle, flush=True) self.setCurrentItem(tItem) @@ -932,7 +921,6 @@ class GuiProjectTree(QTreeWidget): trItemT.addChild(trItemC) self._postItemMove(tHandle) - self._recordLastMove(trItemS, trItemP, tIndex) self._alertTreeChange(tHandle, flush=flush) logger.debug("Moved item '%s' to Trash", tHandle) @@ -1093,45 +1081,6 @@ class GuiProjectTree(QTreeWidget): logger.info("%d item(s) added to the project tree", count) return - def undoLastMove(self) -> bool: - """Attempt to undo the last action.""" - srcItem = self._lastMove.get("item", None) - dstItem = self._lastMove.get("parent", None) - dstIndex = self._lastMove.get("index", None) - - srcOK = isinstance(srcItem, QTreeWidgetItem) - dstOk = isinstance(dstItem, QTreeWidgetItem) - if not srcOK or not dstOk or dstIndex is None: - logger.debug("No tree move to undo") - return False - - if srcItem not in self._treeMap.values(): - logger.warning("Source item no longer exists") - return False - - if dstItem not in self._treeMap.values(): - logger.warning("Previous parent item no longer exists") - return False - - dstIndex = min(max(0, dstIndex), dstItem.childCount()) - sHandle = srcItem.data(self.C_DATA, self.D_HANDLE) - dHandle = dstItem.data(self.C_DATA, self.D_HANDLE) - logger.debug("Moving item '%s' back to '%s', index %d", sHandle, dHandle, dstIndex) - - self.propagateCount(sHandle, 0) - parItem = srcItem.parent() - srcIndex = parItem.indexOfChild(srcItem) - movItem = parItem.takeChild(srcIndex) - dstItem.insertChild(dstIndex, movItem) - - self._postItemMove(sHandle) - self._alertTreeChange(sHandle, flush=True) - - self.setCurrentItem(movItem) - self._lastMove = {} - - return True - def getSelectedHandle(self) -> str | None: """Get the currently selected handle. If multiple items are selected, return the first. @@ -1468,30 +1417,25 @@ class GuiProjectTree(QTreeWidget): """Overload the drop item event to ensure the drag and drop action is allowed, and update relevant data. """ - if not self.indexAt(event.pos()).isValid(): - # Make sure nothing can be dropped on invisible root + tItem = self.itemAt(event.pos()) + dropOn = self.dropIndicatorPosition() == QAbstractItemView.DropIndicatorPosition.OnItem + # Make sure nothing can be dropped on invisible root (see #1569) + if not tItem or tItem.parent() is None and not dropOn: logger.error("Invalid drop location") event.ignore() return - mItems: dict[str, tuple[QTreeWidgetItem, QTreeWidgetItem, bool]] = {} + mItems: dict[str, tuple[QTreeWidgetItem, bool]] = {} sItems = self.selectedItems() if sItems and (parent := sItems[0].parent()) and all(x.parent() is parent for x in sItems): for sItem in sItems: - if (pItem := sItem.parent()): - mHandle = str(sItem.data(self.C_DATA, self.D_HANDLE)) - mItems[mHandle] = (sItem, pItem, sItem.isExpanded()) - else: - logger.error("Cannot drag and drop a root item") - event.ignore() - return - - for mHandle in mItems: + mHandle = str(sItem.data(self.C_DATA, self.D_HANDLE)) + mItems[mHandle] = (sItem, sItem.isExpanded()) self.propagateCount(mHandle, 0) super().dropEvent(event) - for mHandle, (sItem, pItem, isExpanded) in mItems.items(): + for mHandle, (sItem, isExpanded) in mItems.items(): self._postItemMove(mHandle) sItem.setExpanded(isExpanded) self._alertTreeChange(mHandle, flush=False) @@ -1530,8 +1474,8 @@ class GuiProjectTree(QTreeWidget): SHARED.project.index.reIndexHandle(mHandle) self.setTreeItemValues(mHandle) - # Trigger dependent updates - self.propagateCount(tHandle, nwItemS.wordCount) + # Update word count + self.propagateCount(tHandle, nwItemS.wordCount, countChildren=True) return @@ -1884,16 +1828,4 @@ class GuiProjectTree(QTreeWidget): return - def _recordLastMove(self, srcItem: QTreeWidgetItem, - parItem: QTreeWidgetItem, parIndex: int) -> None: - """Record the last action so that it can be undone.""" - prevItem = self._lastMove.get("item", None) - if prevItem is None or srcItem != prevItem: - self._lastMove = { - "item": srcItem, - "parent": parItem, - "index": parIndex, - } - return - # END Class GuiProjectTree diff --git a/tests/test_gui/test_gui_projtree.py b/tests/test_gui/test_gui_projtree.py index 0f834d65..c2163081 100644 --- a/tests/test_gui/test_gui_projtree.py +++ b/tests/test_gui/test_gui_projtree.py @@ -251,11 +251,6 @@ def testGuiProjTree_MoveItems(qtbot, monkeypatch, nwGUI, projPath, mockRnd): C.hChapterDir, C.hChapterDoc, C.hSceneDoc, "0000000000010", "0000000000012", "0000000000011", ] - assert projTree.undoLastMove() is True - assert projTree.getTreeFromHandle(C.hChapterDir) == [ - C.hChapterDir, C.hChapterDoc, C.hSceneDoc, - "0000000000010", "0000000000011", "0000000000012", - ] # Root Folder # =========== @@ -988,17 +983,14 @@ def testGuiProjTree_DragAndDrop(qtbot, monkeypatch, caplog, nwGUI: GuiMain, proj mouse = Qt.MouseButton.LeftButton modifier = Qt.KeyboardModifier.NoModifier - # Move an item - # The actual move is blocked, but the undo history should record - # the event, although without an actual move implied - assert projTree._lastMove == {} + projTree.saveTreeOrder() + treeOrder = SHARED.project.tree._order + + # Move an item, but no selection event = QDropEvent(nPos, action, mime, mouse, modifier) projTree.dropEvent(event) - assert projTree._lastMove == { - "item": projTree._getTreeItem(C.hSceneDoc), - "parent": projTree._getTreeItem(C.hChapterDir), - "index": 1, - } + projTree.saveTreeOrder() + assert SHARED.project.tree._order == treeOrder # Invalid location caplog.clear() @@ -1006,23 +998,19 @@ def testGuiProjTree_DragAndDrop(qtbot, monkeypatch, caplog, nwGUI: GuiMain, proj projTree.dropEvent(event) assert event.isAccepted() is False assert "Invalid drop location" in caplog.text - - # No item selected - caplog.clear() - event = QDropEvent(nPos, action, mime, mouse, modifier) - projTree.clearSelection() - projTree.dropEvent(event) - assert event.isAccepted() is False - assert "Invalid drag and drop event" in caplog.text + projTree.saveTreeOrder() + assert SHARED.project.tree._order == treeOrder # Root item selected caplog.clear() event = QDropEvent(nPos, action, mime, mouse, modifier) projTree.clearSelection() - projTree.setSelectedHandle(C.hNovelRoot, True) + projTree._getTreeItem(C.hTitlePage).setSelected(True) # type: ignore + projTree._getTreeItem(C.hNovelRoot).setSelected(True) # type: ignore projTree.dropEvent(event) assert event.isAccepted() is False - assert "Invalid drag and drop event" in caplog.text + projTree.saveTreeOrder() + assert SHARED.project.tree._order == treeOrder # qtbot.stop() @@ -1070,22 +1058,6 @@ def testGuiProjTree_Other(qtbot, monkeypatch, nwGUI: GuiMain, projPath, mockRnd) SHARED.project.tree[nHandle].setParent(C.hInvalid) # type: ignore assert projTree.revealNewTreeItem(nHandle) is False - # Method: undoLastMove - # ==================== - - # Nothing to move - assert projTree.undoLastMove() is False - - projTree._lastMove["item"] = QTreeWidgetItem() - projTree._lastMove["parent"] = QTreeWidgetItem() - projTree._lastMove["index"] = 0 - assert projTree.undoLastMove() is False - - projTree._lastMove["item"] = projTree._treeMap[C.hTitlePage] - projTree._lastMove["parent"] = QTreeWidgetItem() - projTree._lastMove["index"] = 0 - assert projTree.undoLastMove() is False - # Slot: _treeDoubleClick # ======================