-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ui] add support for selecting multiple nodes at once #1227
Changes from 3 commits
c253e7d
6f50f65
aef50d9
acf1bf2
55b16bc
e8a5491
fdfabf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -273,6 +273,7 @@ def __init__(self, undoStack, taskManager, parent=None): | |||||||
self._sortedDFSChunks = QObjectListModel(parent=self) | ||||||||
self._layout = GraphLayout(self) | ||||||||
self._selectedNode = None | ||||||||
self._selectedNodes = QObjectListModel(parent=self) | ||||||||
self._hoveredNode = None | ||||||||
|
||||||||
self.computeStatusChanged.connect(self.updateLockedUndoStack) | ||||||||
|
@@ -499,6 +500,12 @@ def addNewNode(self, nodeType, position=None, **kwargs): | |||||||
position = Position(position.x(), position.y()) | ||||||||
return self.push(commands.AddNodeCommand(self._graph, nodeType, position=position, **kwargs)) | ||||||||
|
||||||||
@Slot(QObject, result=bool) | ||||||||
def nodeSelection(self, node): | ||||||||
""" If the node is part of the selection or not. """ | ||||||||
length = len(self._selectedNodes) > 1 | ||||||||
return length and self._selectedNodes.contains(node) if node else length | ||||||||
|
||||||||
@Slot(Node, QPoint) | ||||||||
def moveNode(self, node, position): | ||||||||
""" | ||||||||
|
@@ -509,26 +516,49 @@ def moveNode(self, node, position): | |||||||
position (QPoint): the target position | ||||||||
""" | ||||||||
if isinstance(position, QPoint): | ||||||||
if self.nodeSelection(node): | ||||||||
self.moveSelectedNodes(position.x() - node.x, position.y() - node.y) | ||||||||
return | ||||||||
position = Position(position.x(), position.y()) | ||||||||
self.push(commands.MoveNodeCommand(self._graph, node, position)) | ||||||||
|
||||||||
def moveSelectedNodes(self, deltaX, deltaY): | ||||||||
with self.groupedGraphModification("Move Selected Nodes"): | ||||||||
for node in self._selectedNodes: | ||||||||
position = Position(node.x + deltaX, node.y + deltaY) | ||||||||
self.push(commands.MoveNodeCommand(self._graph, node, position)) | ||||||||
|
||||||||
@Slot(Node) | ||||||||
def removeNode(self, node): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check on |
||||||||
if self.nodeSelection(node): | ||||||||
self.removeSelectedNodes() | ||||||||
return | ||||||||
if node.locked: | ||||||||
return | ||||||||
self.push(commands.RemoveNodeCommand(self._graph, node)) | ||||||||
|
||||||||
def removeSelectedNodes(self): | ||||||||
with self.groupedGraphModification("Remove Selected Nodes"): | ||||||||
for node in self._selectedNodes: | ||||||||
if not node.locked: | ||||||||
self.push(commands.RemoveNodeCommand(self._graph, node)) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
@Slot(Node) | ||||||||
def removeNodesFrom(self, startNode): | ||||||||
""" | ||||||||
Remove all nodes starting from 'startNode' to graph leaves. | ||||||||
Args: | ||||||||
startNode (Node): the node to start from. | ||||||||
""" | ||||||||
if not startNode: | ||||||||
return | ||||||||
with self.groupedGraphModification("Remove Nodes from {}".format(startNode.name)): | ||||||||
nodes, _ = self._graph.dfsOnDiscover(startNodes=[startNode], reverse=True, dependenciesOnly=True) | ||||||||
# Perform nodes removal from leaves to start node so that edges | ||||||||
# can be re-created in correct order on redo. | ||||||||
for node in reversed(nodes): | ||||||||
self.removeNode(node) | ||||||||
if not node.locked: | ||||||||
self.removeNode(node) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Already checked in the function. |
||||||||
|
||||||||
@Slot(Attribute, Attribute) | ||||||||
def addEdge(self, src, dst): | ||||||||
|
@@ -560,7 +590,7 @@ def resetAttribute(self, attribute): | |||||||
@Slot(Node, bool, result="QVariantList") | ||||||||
def duplicateNode(self, srcNode, duplicateFollowingNodes=False): | ||||||||
""" | ||||||||
Duplicate a node an optionally all the following nodes to graph leaves. | ||||||||
Duplicate a node and optionally all the following nodes to graph leaves. | ||||||||
|
||||||||
Args: | ||||||||
srcNode (Node): node to start the duplication from | ||||||||
|
@@ -569,19 +599,33 @@ def duplicateNode(self, srcNode, duplicateFollowingNodes=False): | |||||||
Returns: | ||||||||
[Nodes]: the list of duplicated nodes | ||||||||
""" | ||||||||
title = "Duplicate Nodes from {}" if duplicateFollowingNodes else "Duplicate {}" | ||||||||
if duplicateFollowingNodes: title = "Duplicate Nodes from {}" | ||||||||
elif self.nodeSelection(srcNode): title = "Duplicate selected nodes" | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not have links with the notion of node selection at this level. The UI use the selection to determine the list of nodes. Then we have the list of nodes and perform actions on it. But the low-level functions should not check the notion of selection again. The function is explicit: |
||||||||
else: title = "Duplicate {}" | ||||||||
# enable updates between duplication and layout to get correct depths during layout | ||||||||
with self.groupedGraphModification(title.format(srcNode.name), disableUpdates=False): | ||||||||
# disable graph updates during duplication | ||||||||
with self.groupedGraphModification("Node duplication", disableUpdates=True): | ||||||||
duplicates = self.push(commands.DuplicateNodeCommand(self._graph, srcNode, duplicateFollowingNodes)) | ||||||||
if self.nodeSelection(srcNode) and not duplicateFollowingNodes: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, no notion of selection here. |
||||||||
command = commands.DuplicateNodeListCommand(self._graph, self._selectedNodes) | ||||||||
else: | ||||||||
command = commands.DuplicateNodeCommand(self._graph, srcNode, duplicateFollowingNodes) | ||||||||
duplicates = self.push(command) | ||||||||
# move nodes below the bounding box formed by the duplicated node(s) | ||||||||
bbox = self._layout.boundingBox(duplicates) | ||||||||
for n in duplicates: | ||||||||
self.moveNode(n, Position(n.x, bbox[3] + self.layout.gridSpacing + n.y)) | ||||||||
|
||||||||
return duplicates | ||||||||
|
||||||||
@Slot(QObject) | ||||||||
def clearData(self, node): | ||||||||
if self.nodeSelection(node): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, no notion of selection here. |
||||||||
for n in self._selectedNodes: | ||||||||
n.clearData() | ||||||||
return | ||||||||
node.clearData() | ||||||||
|
||||||||
@Slot(CompatibilityNode, result=Node) | ||||||||
def upgradeNode(self, node): | ||||||||
""" Upgrade a CompatibilityNode. """ | ||||||||
|
@@ -615,10 +659,33 @@ def appendAttribute(self, attribute, value=QJsonValue()): | |||||||
def removeAttribute(self, attribute): | ||||||||
self.push(commands.ListAttributeRemoveCommand(self._graph, attribute)) | ||||||||
|
||||||||
@Slot(QObject, QObject) | ||||||||
def boxSelect(self, selection, draggable): | ||||||||
x = selection.x() - draggable.x() | ||||||||
y = selection.y() - draggable.y() | ||||||||
otherX = x + selection.width() | ||||||||
otherY = y + selection.height() | ||||||||
x, y, otherX, otherY = [ i / j for i, j in zip([x, y, otherX, otherY], [draggable.scale()] * 4) ] | ||||||||
if x == otherX or y == otherY: | ||||||||
return | ||||||||
for n in self._graph.nodes: | ||||||||
bbox = self._layout.boundingBox([n]) | ||||||||
# evaluate if the selection and node intersect | ||||||||
if not (x > bbox[2] + bbox[0] or otherX < bbox[0] or y > bbox[3] + bbox[1] or otherY < bbox[1]): | ||||||||
if not self._selectedNodes.contains(n): | ||||||||
self._selectedNodes.append(n) | ||||||||
self.selectedNodesChanged.emit() | ||||||||
|
||||||||
def clearNodeSelection(self): | ||||||||
""" Clear node selection. """ | ||||||||
self.selectedNode = None | ||||||||
|
||||||||
@Slot() | ||||||||
def clearNodesSelections(self): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
""" Clear multiple nodes selection. """ | ||||||||
self._selectedNodes.clear() | ||||||||
self.selectedNodesChanged.emit() | ||||||||
|
||||||||
def clearNodeHover(self): | ||||||||
""" Reset currently hovered node to None. """ | ||||||||
self.hoveredNode = None | ||||||||
|
@@ -643,6 +710,10 @@ def clearNodeHover(self): | |||||||
# Currently selected node | ||||||||
selectedNode = makeProperty(QObject, "_selectedNode", selectedNodeChanged, resetOnDestroy=True) | ||||||||
|
||||||||
selectedNodesChanged = Signal() | ||||||||
# Currently selected nodes to drag | ||||||||
selectedNodes = makeProperty(QObject, "_selectedNodes", selectedNodesChanged, resetOnDestroy=True) | ||||||||
|
||||||||
hoveredNodeChanged = Signal() | ||||||||
# Currently hovered node | ||||||||
hoveredNode = makeProperty(QObject, "_hoveredNode", hoveredNodeChanged, resetOnDestroy=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need to have 2 classes to do it with one of multiple nodes and inheritance, etc.
Either we keep
DuplicateNodeListCommand
and always use it (even for a single node) or keepDuplicateNodeCommand
and group the commands for multiple nodes.