From 69f35c1c05e26e623d81b8fcdb5e8ffed478e0ab Mon Sep 17 00:00:00 2001
From: Joe <joe.melsha@live.com>
Date: Mon, 2 Feb 2026 18:25:49 -0600
Subject: [PATCH] Enhance PatchVisualizer component for improved height
 management

- Updated `DomLabel` to synchronize element height immediately when parent re-renders, preventing incorrect positioning by the VirtualScroller.
- Refactored `PatchVisualizer` to store height bindings by node ID, ensuring persistent bindings across re-renders and eliminating visual glitches for expanded rows.
- Implemented cleanup logic for unused node height bindings, optimizing memory usage and performance.
---
 .../Components/PatchVisualizer/DomLabel.lua   | 18 +++++++++++--
 .../App/Components/PatchVisualizer/init.lua   | 26 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
index b81fa74d..ba355055 100644
--- a/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
+++ b/plugin/src/App/Components/PatchVisualizer/DomLabel.lua
@@ -297,6 +297,14 @@ function DomLabel:willUnmount()
 end
 
 function DomLabel:didUpdate(prevProps)
+	-- When parent re-renders it creates new bindings starting at 24, but our motor
+	-- may be at expanded height. Sync immediately to prevent VirtualScroller from
+	-- positioning elements incorrectly.
+	if prevProps.setElementHeight ~= self.props.setElementHeight then
+		local currentHeight = self.binding:getValue()
+		self.props.setElementHeight(currentHeight)
+	end
+
 	if
 		prevProps.instance ~= self.props.instance
 		or prevProps.patchType ~= self.props.patchType
@@ -404,15 +412,21 @@ function DomLabel:render()
 					else
 						-- Single click expands the changes (after a delay to check for double click)
 						task.delay(0.3, function()
+							-- Guard against unmounted component or stale click
+							if not self.isMounted then
+								return
+							end
+
 							if os.clock() - (self.lastDoubleClickTime or 0) <= 0.35 then
 								-- This was a double click, so don't expand
 								return
 							end
 
-							if props.changeList then
+							-- Use self.props to get current props, not stale closure
+							if self.props.changeList then
 								self.expanded = not self.expanded
 								local goalHeight = 24
-									+ (if self.expanded then math.clamp(#props.changeList * 24, 24, 24 * 6) else 0)
+									+ (if self.expanded then math.clamp(#self.props.changeList * 24, 24, 24 * 6) else 0)
 								self.motor:setGoal(Flipper.Spring.new(goalHeight, {
 									frequency = 5,
 									dampingRatio = 1,
diff --git a/plugin/src/App/Components/PatchVisualizer/init.lua b/plugin/src/App/Components/PatchVisualizer/init.lua
index c26f9e9c..888d3be2 100644
--- a/plugin/src/App/Components/PatchVisualizer/init.lua
+++ b/plugin/src/App/Components/PatchVisualizer/init.lua
@@ -55,6 +55,11 @@ function PatchVisualizer:init()
 	self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0))
 
 	self.updateEvent = Instance.new("BindableEvent")
+
+	-- Store height bindings by node ID to persist across re-renders.
+	-- Without this, every render creates new bindings starting at 24,
+	-- causing expanded rows to visually glitch when unrelated props change.
+	self.nodeHeightBindings = {}
 end
 
 function PatchVisualizer:willUnmount()
@@ -166,7 +171,15 @@ function PatchVisualizer:render()
 				end
 			end
 
-			local elementHeight, setElementHeight = Roact.createBinding(24)
+			-- Reuse existing bindings for this node, or create new ones
+			local nodeId = node.id
+			if not self.nodeHeightBindings[nodeId] then
+				local binding, setBinding = Roact.createBinding(24)
+				self.nodeHeightBindings[nodeId] = { binding = binding, setBinding = setBinding }
+			end
+			local elementHeight = self.nodeHeightBindings[nodeId].binding
+			local setElementHeight = self.nodeHeightBindings[nodeId].setBinding
+
 			elementHeights[elementIndex] = elementHeight
 			scrollElements[elementIndex] = e(DomLabel, {
 				transparency = self.props.transparency,
@@ -200,6 +213,9 @@ function PatchVisualizer:render()
 			end
 		end
 
+		-- Track which node IDs are used this render
+		local usedNodeIds = {}
+
 		-- Draw all visible nodes in order
 		for i, entry in ipairs(visibleNodes) do
 			local depth = entry.depth
@@ -209,6 +225,14 @@ function PatchVisualizer:render()
 			end
 
 			drawNode(entry.node, depth, i)
+			usedNodeIds[entry.node.id] = true
+		end
+
+		-- Clean up bindings for nodes that no longer exist
+		for nodeId in self.nodeHeightBindings do
+			if not usedNodeIds[nodeId] then
+				self.nodeHeightBindings[nodeId] = nil
+			end
 		end
 	end