Repository: GraphiteEditor/Graphite (language: rust)

Issue:
Clear artboard
<!-- Please reference any relevant issue number below, optionally with a "Closes"/"Resolves"/"Fixes" prefix -->



Discussion / hints:
Note that merge nodes are named "Artboard" as I reserve the name
!build
| 📦 **Build Complete for** ac49479745cde3c23289a54118075b5ccab06b6e |
|-|
| https://5993b57e.graphite.pages.dev |
I would like to see the artboards name/position/downstream connnector/upstream connectors being saved, then the artboard is deleted, a layer node is created,  its name/position are set, then it is reconnecting. All using the same messages as if the user was to do it.

Also, make sure to start and end a transaction at the start and end of the message. Its also important that no mutable network interface functions are called in a message that has a start/end transaction.

Optionally, it would also be nice if the artboards position is then used to set the transform of the new layer, so that the contents stay at the same place. This would require using a monitor node to access the artboard data, where the translation is stored. This could also be used to improve #2028. This would probably be outside the scope of the PR, so you could leave a TODO.
I'll mark this as a draft until those comments are addressed. Thanks for taking the time to review this, @adamgerhant.
> I would like to see the artboards name/position/downstream connnector/upstream connectors being saved, then the artboard is deleted, a layer node is created, its name/position are set, then it is reconnecting. All using the same messages as if the user was to do it.

You want to reduce loops by this right?
You can use however many loops are necessary to achieve the correct behavior
Just checking in on the status of this @moOsama76.
> Just checking in on the status of this @moOsama76.

I was taking a break, will do it this week
I'll close this PR for now, but please feel free to reopen it once you've addressed the code review feedback from Adam.
> I'll close this PR for now, but please feel free to reopen it once you've addressed the code review feedback from Adam.

No problem I am getting back soon I promise :)

Fix patch:
diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs
index 34ebb2acee..75e1d971c7 100644
--- a/editor/src/messages/portfolio/document/document_message_handler.rs
+++ b/editor/src/messages/portfolio/document/document_message_handler.rs
@@ -272,7 +272,6 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 				}
 			}
 			DocumentMessage::ClearArtboards => {
-				responses.add(DocumentMessage::AddTransaction);
 				responses.add(GraphOperationMessage::ClearArtboards);
 			}
 			DocumentMessage::ClearLayersPanel => {
@@ -778,6 +777,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 					responses.add(NodeGraphMessage::SetDisplayName {
 						node_id: layer.to_node(),
 						alias: name,
+						skip_adding_history_step: false,
 					});
 				}
 				if let Some((parent, insert_index)) = parent_and_insert_index {
@@ -822,6 +822,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 					responses.add(NodeGraphMessage::SetDisplayName {
 						node_id: layer.to_node(),
 						alias: name,
+						skip_adding_history_step: false,
 					});
 				}
 				if let Some((parent, insert_index)) = parent_and_insert_index {
diff --git a/editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs b/editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs
index 3852b60301..c23d83522a 100644
--- a/editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs
+++ b/editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs
@@ -1,5 +1,6 @@
 use super::transform_utils;
 use super::utility_types::ModifyInputsContext;
+use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn;
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
 use crate::messages::portfolio::document::utility_types::network_interface::{InputConnector, NodeNetworkInterface, OutputConnector};
 use crate::messages::portfolio::document::utility_types::nodes::CollapsedLayers;
@@ -14,6 +15,13 @@ use graphene_std::vector::convert_usvg_path;
 
 use glam::{DAffine2, DVec2};
 
+#[derive(Debug, Clone)]
+struct ArtboardInfo {
+	input_node: NodeInput,
+	output_nodes: Vec<InputConnector>,
+	merge_node: NodeId,
+}
+
 pub struct GraphOperationMessageData<'a> {
 	pub network_interface: &'a mut NodeNetworkInterface,
 	pub collapsed: &'a mut CollapsedLayers,
@@ -108,7 +116,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
 				network_interface.move_layer_to_stack(artboard_layer, LayerNodeIdentifier::ROOT_PARENT, 0, &[]);
 
 				// If there is a non artboard feeding into the primary input of the artboard, move it to the secondary input
-				let Some(artboard) = network_interface.network(&[]).unwrap().nodes.get(&id) else {
+				let Some(artboard) = network_interface.network(&[]).and_then(|network| network.nodes.get(&id)) else {
 					log::error!("Artboard not created");
 					return;
 				};
@@ -191,13 +199,87 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
 				}
 			}
 			GraphOperationMessage::ClearArtboards => {
+				if network_interface.all_artboards().is_empty() {
+					return;
+				}
+
+				responses.add(DocumentMessage::AddTransaction);
+				responses.add(NodeGraphMessage::DeleteNodes {
+					node_ids: network_interface.all_artboards().iter().map(|layer_node| layer_node.to_node()).collect(),
+					delete_children: false,
+				});
+
+				let mut artboard_data: HashMap<NodeId, ArtboardInfo> = HashMap::new();
+
+				// Go through all artboards and create merge nodes
 				for artboard in network_interface.all_artboards() {
-					responses.add(NodeGraphMessage::DeleteNodes {
-						node_ids: vec![artboard.to_node()],
-						delete_children: false,
+					let node_id = NodeId::new();
+					let Some(document_node) = network_interface.network(&[]).and_then(|network| network.nodes.get(&artboard.to_node())) else {
+						log::error!("Artboard not created");
+						responses.add(DocumentMessage::AbortTransaction);
+						return;
+					};
+
+					artboard_data.insert(
+						artboard.to_node(),
+						ArtboardInfo {
+							input_node: NodeInput::node(document_node.inputs[1].as_node().unwrap_or_default(), 0),
+							output_nodes: network_interface
+								.outward_wires(&[])
+								.and_then(|outward_wires| outward_wires.get(&OutputConnector::node(artboard.to_node(), 0)))
+								.cloned()
+								.unwrap_or_default(),
+							merge_node: node_id,
+						},
+					);
+
+					let mut modify_inputs = ModifyInputsContext::new(network_interface, responses);
+					modify_inputs.create_layer(node_id);
+
+					responses.add(NodeGraphMessage::SetDisplayName {
+						node_id,
+						alias: network_interface.frontend_display_name(&artboard.to_node(), &[]),
+						skip_adding_history_step: true,
+					});
+
+					// Shift node positions in the graph
+					let (x, y) = network_interface.position(&artboard.to_node(), &[]).unwrap_or_default().into();
+					responses.add(NodeGraphMessage::ShiftNodePosition { node_id, x, y });
+				}
+
+				// Go through all artboards and connect them to the merge nodes
+				for artboard in &artboard_data {
+					// Modify downstream connections
+					responses.add(NodeGraphMessage::SetInput {
+						input_connector: InputConnector::node(artboard.1.merge_node, 1),
+						input: NodeInput::node(artboard.1.input_node.as_node().unwrap_or_default(), 0),
+					});
+
+					// Modify upstream connections
+					for outward_wire in &artboard.1.output_nodes {
+						let input = NodeInput::node(artboard_data[artboard.0].merge_node, 0);
+						let input_connector = if let Some(artboard_info) = artboard_data.get(&outward_wire.node_id().unwrap_or_default()) {
+							InputConnector::node(artboard_info.merge_node, outward_wire.input_index())
+						} else {
+							*outward_wire
+						};
+						responses.add(NodeGraphMessage::SetInput { input_connector, input });
+					}
+
+					// Reposition merge nodes
+					let offset = network_interface
+						.document_metadata()
+						.bounding_box_document(LayerNodeIdentifier::new_unchecked(*artboard.0))
+						.map(|p| p[0])
+						.unwrap_or_default();
+					responses.add(GraphOperationMessage::TransformChange {
+						layer: LayerNodeIdentifier::new_unchecked(artboard.1.merge_node),
+						transform: DAffine2::from_translation(offset),
+						transform_in: TransformIn::Local,
+						skip_rerender: false,
 					});
 				}
-				// TODO: Replace deleted artboards with merge nodes
+
 				responses.add(NodeGraphMessage::RunDocumentGraph);
 				responses.add(NodeGraphMessage::SelectedNodesUpdated);
 				responses.add(NodeGraphMessage::SendGraph);
diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs
index ea3fd1321b..1b1591057c 100644
--- a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs
+++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs
@@ -138,6 +138,7 @@ pub enum NodeGraphMessage {
 	SetDisplayName {
 		node_id: NodeId,
 		alias: String,
+		skip_adding_history_step: bool,
 	},
 	SetDisplayNameImpl {
 		node_id: NodeId,
diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs
index 3285f44c2b..48f87ae75c 100644
--- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs
+++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs
@@ -1450,11 +1450,19 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphHandlerData<'a>> for NodeGrap
 				responses.add(NodeGraphMessage::RunDocumentGraph);
 				responses.add(NodeGraphMessage::SendGraph);
 			}
-			NodeGraphMessage::SetDisplayName { node_id, alias } => {
-				responses.add(DocumentMessage::StartTransaction);
+			NodeGraphMessage::SetDisplayName {
+				node_id,
+				alias,
+				skip_adding_history_step,
+			} => {
+				if !skip_adding_history_step {
+					responses.add(DocumentMessage::StartTransaction);
+				}
 				responses.add(NodeGraphMessage::SetDisplayNameImpl { node_id, alias });
-				// Does not add a history step if the name was not changed
-				responses.add(DocumentMessage::EndTransaction);
+				if !skip_adding_history_step {
+					// Does not add a history step if the name was not changed
+					responses.add(DocumentMessage::EndTransaction);
+				}
 				responses.add(DocumentMessage::RenderRulers);
 				responses.add(DocumentMessage::RenderScrollbars);
 				responses.add(NodeGraphMessage::SendGraph);
@@ -2023,6 +2031,7 @@ impl NodeGraphMessageHandler {
 								NodeGraphMessage::SetDisplayName {
 									node_id: layer,
 									alias: text_input.value.clone(),
+									skip_adding_history_step: false,
 								}
 								.into()
 							})
diff --git a/frontend/wasm/src/editor_api.rs b/frontend/wasm/src/editor_api.rs
index 5b14669b8e..ea788735ed 100644
--- a/frontend/wasm/src/editor_api.rs
+++ b/frontend/wasm/src/editor_api.rs
@@ -552,6 +552,7 @@ impl EditorHandle {
 		let message = NodeGraphMessage::SetDisplayName {
 			node_id: layer.to_node(),
 			alias: name,
+			skip_adding_history_step: false,
 		};
 		self.dispatch(message);
 	}


Test patch: