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.
This commit is contained in:
Joe 2026-02-02 18:25:49 -06:00
parent 2379f5bce0
commit 69f35c1c05
2 changed files with 41 additions and 3 deletions

View file

@ -297,6 +297,14 @@ function DomLabel:willUnmount()
end end
function DomLabel:didUpdate(prevProps) 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 if
prevProps.instance ~= self.props.instance prevProps.instance ~= self.props.instance
or prevProps.patchType ~= self.props.patchType or prevProps.patchType ~= self.props.patchType
@ -404,15 +412,21 @@ function DomLabel:render()
else else
-- Single click expands the changes (after a delay to check for double click) -- Single click expands the changes (after a delay to check for double click)
task.delay(0.3, function() 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 if os.clock() - (self.lastDoubleClickTime or 0) <= 0.35 then
-- This was a double click, so don't expand -- This was a double click, so don't expand
return return
end 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 self.expanded = not self.expanded
local goalHeight = 24 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, { self.motor:setGoal(Flipper.Spring.new(goalHeight, {
frequency = 5, frequency = 5,
dampingRatio = 1, dampingRatio = 1,

View file

@ -55,6 +55,11 @@ function PatchVisualizer:init()
self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0))
self.updateEvent = Instance.new("BindableEvent") 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 end
function PatchVisualizer:willUnmount() function PatchVisualizer:willUnmount()
@ -166,7 +171,15 @@ function PatchVisualizer:render()
end end
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 elementHeights[elementIndex] = elementHeight
scrollElements[elementIndex] = e(DomLabel, { scrollElements[elementIndex] = e(DomLabel, {
transparency = self.props.transparency, transparency = self.props.transparency,
@ -200,6 +213,9 @@ function PatchVisualizer:render()
end end
end end
-- Track which node IDs are used this render
local usedNodeIds = {}
-- Draw all visible nodes in order -- Draw all visible nodes in order
for i, entry in ipairs(visibleNodes) do for i, entry in ipairs(visibleNodes) do
local depth = entry.depth local depth = entry.depth
@ -209,6 +225,14 @@ function PatchVisualizer:render()
end end
drawNode(entry.node, depth, i) 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
end end