Add persistence to the collapsed state of Properties panel node sections (#4300)

* Add persistence to the collapsed state of Properties panel node sections

* Improve comments

* Fix generator node handling with #4299

editor/src/messages/layout/utility_types/layout_widget.rs

@@ -384,6 +384,8 @@ pub struct WidgetSection {
 	pub pinned: bool,
 	/// Whether this section can be dragged to reorder it within its layer's chain of nodes (true for a layer chain's nodes, but not its terminal layer node).
 	pub draggable: bool,
+	/// Whether this section is expanded to show its contents, as opposed to being collapsed down to just its header.
+	pub expanded: bool,
 	/// The ID of the node whose properties this section displays.
 	pub id: u64,
 	/// The node's properties content, rendered as the section's body when expanded.
@@ -414,13 +416,14 @@ impl LayoutGroup {
 		Self::Table(WidgetTable { rows, unstyled })
 	}
 
-	pub fn section(name: impl Into<String>, description: impl Into<String>, visible: bool, pinned: bool, id: u64, layout: Layout) -> Self {
+	pub fn section(name: impl Into<String>, description: impl Into<String>, visible: bool, pinned: bool, expanded: bool, id: u64, layout: Layout) -> Self {
 		Self::Section(WidgetSection {
 			name: name.into(),
 			description: description.into(),
 			visible,
 			pinned,
 			draggable: false,
+			expanded,
 			id,
 			layout,
 		})
@@ -518,6 +521,7 @@ impl Diffable for LayoutGroup {
 					visible: current_visible,
 					pinned: current_pinned,
 					draggable: current_draggable,
+					expanded: current_expanded,
 					id: current_id,
 					layout: current_layout,
 				}),
@@ -527,6 +531,7 @@ impl Diffable for LayoutGroup {
 					visible: new_visible,
 					pinned: new_pinned,
 					draggable: new_draggable,
+					expanded: new_expanded,
 					id: new_id,
 					layout: new_layout,
 				}),
@@ -539,6 +544,7 @@ impl Diffable for LayoutGroup {
 					|| *current_visible != new_visible
 					|| *current_pinned != new_pinned
 					|| *current_draggable != new_draggable
+					|| *current_expanded != new_expanded
 					|| *current_id != new_id
 				{
 					// Update self to reflect new changes
@@ -547,6 +553,7 @@ impl Diffable for LayoutGroup {
 					*current_visible = new_visible;
 					*current_pinned = new_pinned;
 					*current_draggable = new_draggable;
+					*current_expanded = new_expanded;
 					*current_id = new_id;
 					current_layout.clone_from(&new_layout);
 
@@ -557,6 +564,7 @@ impl Diffable for LayoutGroup {
 						visible: new_visible,
 						pinned: new_pinned,
 						draggable: new_draggable,
+						expanded: new_expanded,
 						id: new_id,
 						layout: new_layout,
 					})

editor/src/messages/portfolio/document/document_message.rs

@@ -216,6 +216,9 @@ pub enum DocumentMessage {
 		tree_path: Vec<NodeId>,
 		recursive: bool,
 	},
+	ToggleNodePropertiesSectionExpanded {
+		node_id: NodeId,
+	},
 	ToggleSelectedVisibility,
 	ToggleSelectedLocked,
 	ToggleGridVisibility,

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -94,6 +94,9 @@ pub struct DocumentMessageHandler {
 	/// Tracks which layer occurrences are collapsed in the Layers panel, keyed by tree path.
 	#[serde(deserialize_with = "deserialize_collapsed_layers", default)]
 	pub collapsed: CollapsedLayers,
+	/// The node IDs whose section is collapsed in the Properties panel.
+	#[serde(default)]
+	pub properties_panel_collapsed_sections: Vec<NodeId>,
 	/// The full Git commit hash of the Graphite repository that was used to build the editor.
 	/// We save this to provide a hint about which version of the editor was used to create the document.
 	pub commit_hash: String,
@@ -173,6 +176,7 @@ impl Default for DocumentMessageHandler {
 			network_interface: default_document_network_interface(),
 			resources: ResourceMessageHandler::default(),
 			collapsed: CollapsedLayers::default(),
+			properties_panel_collapsed_sections: Vec::new(),
 			commit_hash: GRAPHITE_GIT_COMMIT_HASH.to_string(),
 			document_ptz: PTZ::default(),
 			render_mode: RenderMode::default(),
@@ -248,6 +252,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 					document_name: self.name.as_str(),
 					fonts,
 					properties_panel_open,
+					properties_panel_collapsed_sections: &self.properties_panel_collapsed_sections,
 				};
 				self.properties_panel_message_handler.process_message(message, responses, context);
 			}
@@ -271,6 +276,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 						breadcrumb_network_path: &self.breadcrumb_network_path,
 						document_id,
 						collapsed: &mut self.collapsed,
+						properties_panel_collapsed_sections: &mut self.properties_panel_collapsed_sections,
 						ipp,
 						graph_view_overlay_open: self.graph_view_overlay_open,
 						graph_fade_artwork_percentage: self.graph_fade_artwork_percentage,
@@ -1363,6 +1369,14 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 
 				responses.add(NodeGraphMessage::SendGraph);
 			}
+			DocumentMessage::ToggleNodePropertiesSectionExpanded { node_id } => {
+				if let Some(index) = self.properties_panel_collapsed_sections.iter().position(|id| *id == node_id) {
+					self.properties_panel_collapsed_sections.remove(index);
+				} else {
+					self.properties_panel_collapsed_sections.push(node_id);
+				}
+				responses.add(PropertiesPanelMessage::Refresh);
+			}
 			DocumentMessage::ToggleSelectedLocked => responses.add(NodeGraphMessage::ToggleSelectedLocked),
 			DocumentMessage::ToggleSelectedVisibility => {
 				responses.add(NodeGraphMessage::ToggleSelectedVisibility);

editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs

@@ -34,6 +34,8 @@ pub struct NodePropertiesContext<'a> {
 	pub fonts: &'a FontsMessageHandler,
 	pub selection_network_path: &'a [NodeId],
 	pub document_name: &'a str,
+	/// The node IDs whose Properties panel sections the user has collapsed.
+	pub properties_panel_collapsed_sections: &'a [NodeId],
 }
 
 impl NodePropertiesContext<'_> {

editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs

@@ -39,6 +39,7 @@ pub struct NodeGraphMessageContext<'a> {
 	pub breadcrumb_network_path: &'a [NodeId],
 	pub document_id: DocumentId,
 	pub collapsed: &'a mut CollapsedLayers,
+	pub properties_panel_collapsed_sections: &'a mut Vec<NodeId>,
 	pub ipp: &'a InputPreprocessorMessageHandler,
 	pub graph_view_overlay_open: bool,
 	pub graph_fade_artwork_percentage: f64,
@@ -110,6 +111,7 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 			breadcrumb_network_path,
 			document_id,
 			collapsed,
+			properties_panel_collapsed_sections,
 			ipp,
 			graph_view_overlay_open,
 			graph_fade_artwork_percentage,
@@ -188,7 +190,13 @@ impl<'a> MessageHandler<NodeGraphMessage, NodeGraphMessageContext<'a>> for NodeG
 				});
 
 				network_interface.load_structure();
+
+				// Prune the Layers panel collapsed state for any layer tree paths whose nodes no longer exist, so it doesn't accumulate across loads
 				collapsed.0.retain(|path| path.iter().all(|&node_id| network_interface.document_network().nodes.contains_key(&node_id)));
+
+				// Prune the Properties panel node section collapsed state for any nodes (in any nested network) that no longer exist, so it doesn't accumulate across loads
+				let existing_nodes = network_interface.document_network().recursive_nodes().map(|(node_id, ..)| *node_id).collect::<HashSet<_>>();
+				properties_panel_collapsed_sections.retain(|node_id| existing_nodes.contains(node_id));
 			}
 			NodeGraphMessage::SelectedNodesUpdated => {
 				let selected_layers = network_interface.selected_nodes().selected_layers(network_interface.document_metadata()).collect::<Vec<_>>();
@@ -2622,10 +2630,11 @@ impl NodeGraphMessageHandler {
 					.map(|node_id| node_properties::generate_node_properties(node_id, context))
 					.collect::<Vec<_>>();
 
-				// Mark each node in the layer's chain (but not the layer node itself, which is first) as draggable so its section can be reordered within the chain from the Properties panel
+				// Mark each chain node (but not the layer node itself, which is first) draggable so its section can be reordered.
+				// A node without a primary input (e.g. a generator) is left non-draggable.
 				for chain_node_section in node_properties.iter_mut().skip(1) {
 					if let LayoutGroup::Section(section) = chain_node_section {
-						section.draggable = true;
+						section.draggable = context.network_interface.has_primary_input(&NodeId(section.id), context.selection_network_path);
 					}
 				}
 

editor/src/messages/portfolio/document/node_graph/node_properties.rs

@@ -2373,8 +2373,9 @@ pub(crate) fn generate_node_properties(node_id: NodeId, context: &mut NodeProper
 
 	let visible = context.network_interface.is_visible(&node_id, context.selection_network_path);
 	let pinned = context.network_interface.is_pinned(&node_id, context.selection_network_path);
+	let expanded = !context.properties_panel_collapsed_sections.contains(&node_id);
 
-	LayoutGroup::section(name, description, visible, pinned, node_id.0, Layout(layout))
+	LayoutGroup::section(name, description, visible, pinned, expanded, node_id.0, Layout(layout))
 }
 
 /// Resolve the viewport-space orientation of a Fill node's gradient by walking downstream to its owning layer

editor/src/messages/portfolio/document/properties_panel/properties_panel_message_handler.rs

@@ -15,6 +15,7 @@ pub struct PropertiesPanelMessageContext<'a> {
 	pub document_name: &'a str,
 	pub fonts: &'a FontsMessageHandler,
 	pub properties_panel_open: bool,
+	pub properties_panel_collapsed_sections: &'a [NodeId],
 }
 
 #[derive(Debug, Clone, Default, ExtractField)]
@@ -31,6 +32,7 @@ impl MessageHandler<PropertiesPanelMessage, PropertiesPanelMessageContext<'_>> f
 			document_name,
 			fonts,
 			properties_panel_open,
+			properties_panel_collapsed_sections,
 		} = context;
 
 		match message {
@@ -54,6 +56,7 @@ impl MessageHandler<PropertiesPanelMessage, PropertiesPanelMessageContext<'_>> f
 					selection_network_path,
 					document_name,
 					fonts,
+					properties_panel_collapsed_sections,
 				};
 				let layout = Layout(NodeGraphMessageHandler::collate_properties(&mut node_properties_context));
 

editor/src/messages/portfolio/document/utility_types/network_interface.rs

@@ -352,6 +352,12 @@ impl NodeNetworkInterface {
 		node.inputs.len()
 	}
 
+	/// Whether the node has an exposed input at index 0 to accept the horizontal flow from upstream.
+	/// A node without one (e.g. a generator) can only be the most-upstream node in a chain.
+	pub fn has_primary_input(&self, node_id: &NodeId, network_path: &[NodeId]) -> bool {
+		self.input_from_connector(&InputConnector::node(*node_id, 0), network_path).is_some_and(|input| input.is_exposed())
+	}
+
 	pub fn number_of_outputs(&self, node_id: &NodeId, network_path: &[NodeId]) -> usize {
 		let Some(implementation) = self.implementation(node_id, network_path) else {
 			log::error!("Could not get node {node_id} in number_of_outputs");
@@ -6094,33 +6100,42 @@ impl NodeNetworkInterface {
 			.take_while(|upstream_id| !self.is_layer(upstream_id, network_path))
 			.collect::<Vec<_>>();
 
-		let Some(from) = chain.iter().position(|id| *id == node_id) else {
-			log::error!("Node {node_id} is not part of its layer's chain in reorder_chain_node");
+		// A source node (no primary input) stays pinned at the most-upstream end; only the nodes below it reorder
+		let pinned_source = chain.last().copied().filter(|last| !self.has_primary_input(last, network_path));
+		let reorderable = &chain[..chain.len() - pinned_source.is_some() as usize];
+
+		let Some(from) = reorderable.iter().position(|id| *id == node_id) else {
+			log::error!("Node {node_id} is not a reorderable node in its layer's chain in reorder_chain_node");
 			return;
 		};
 
-		// The drop gap is measured against the chain that still includes the dragged node, so shift it down by one if the node is being removed from before the gap
-		let to = (if insert_index > from { insert_index - 1 } else { insert_index }).min(chain.len() - 1);
+		// The drop gap is measured against the reorderable nodes that still include the dragged node, so shift it down by one if the node is being removed from before the gap
+		let to = (if insert_index > from { insert_index - 1 } else { insert_index }).min(reorderable.len() - 1);
 		if to == from {
 			return;
 		}
 
-		let mut new_order = chain.clone();
+		let mut new_order = reorderable.to_vec();
 		new_order.remove(from);
 		new_order.insert(to, node_id);
 
-		// Preserve whatever feeds the most-upstream chain node (a value, an import, or an upstream layer) so it stays at the top
-		let Some(tail_input) = self.input_from_connector(&InputConnector::node(*chain.last().unwrap(), 0), network_path).cloned() else {
-			log::error!("Could not get the upstream input of the chain in reorder_chain_node");
-			return;
+		// The most-upstream reorderable node connects up to the pinned source, or else whatever fed the top of the chain
+		let tail_input = if let Some(source) = pinned_source {
+			NodeInput::node(source, 0)
+		} else {
+			let Some(input) = self.input_from_connector(&InputConnector::node(*chain.last().unwrap(), 0), network_path).cloned() else {
+				log::error!("Could not get the upstream input of the chain in reorder_chain_node");
+				return;
+			};
+			input
 		};
 
-		// Disconnect the existing internal chain wiring first so the rewiring below can't transiently form a cycle
-		for &chain_node in &chain {
+		// Disconnect first so the rewiring can't transiently form a cycle (the pinned source keeps its wiring)
+		for &chain_node in reorderable {
 			self.disconnect_input(&InputConnector::node(chain_node, 0), network_path);
 		}
 
-		// Rewire in the new order: layer's secondary input -> new_order[0] -> ... -> new_order[last] -> preserved tail input
+		// Rewire in the new order: layer's secondary input -> new_order[0] -> ... -> new_order[last] -> tail input
 		self.set_input(&InputConnector::node(layer, 1), NodeInput::node(new_order[0], 0), network_path);
 		for pair in new_order.windows(2) {
 			self.set_input(&InputConnector::node(pair[0], 0), NodeInput::node(pair[1], 0), network_path);