Refactor subtree highlight management in DomLabel and PatchVisualizer components

- Simplified the handling of subtree highlights by replacing the previous state management with a binding approach.
- Updated the SelectionOption component to derive hover effects directly from the new binding.
- Removed unnecessary props related to subtree highlighting from SelectionRadio, streamlining the component structure.
This commit is contained in:
Joe 2026-02-07 19:15:53 -06:00
parent 76d4e6aff0
commit dd57bad374
2 changed files with 38 additions and 44 deletions

View file

@ -80,32 +80,23 @@ function SelectionOption:render()
-- Use wider button for "Studio" text (6 chars vs 4 for others) -- Use wider button for "Studio" text (6 chars vs 4 for others)
local buttonWidth = if props.text == "Studio" then 44 else 36 local buttonWidth = if props.text == "Studio" then 44 else 36
-- Check if this button is subtree-highlighted (parent's matching button is hovered)
local subtreeInfo = props.subtreeHighlightInfo
local ancestorIds = props.ancestorIds
local subtreeHov = subtreeInfo ~= nil
and ancestorIds ~= nil
and ancestorIds[subtreeInfo.nodeId] == true
and subtreeInfo.optionType == props.optionType
local effectiveHovered = isHovered or subtreeHov
-- Transparency logic with hover effect -- Transparency logic with hover effect
local bgTransparency, txtTransparency local bgTransparency, txtTransparency
if isParentOnly then if isParentOnly then
-- Parent-only: partially transparent, less so when hovering the button -- Parent-only: partially transparent, less so when hovering the button
bgTransparency = props.transparency:map(function(t) bgTransparency = props.transparency:map(function(t)
return if effectiveHovered then 0.2 + (0.8 * t) else 0.5 + (0.5 * t) return if isHovered then 0.2 + (0.8 * t) else 0.5 + (0.5 * t)
end) end)
txtTransparency = props.transparency:map(function(t) txtTransparency = props.transparency:map(function(t)
return if effectiveHovered then 0 + t else 0.3 + (0.7 * t) return if isHovered then 0 + t else 0.3 + (0.7 * t)
end) end)
else else
-- Normal buttons: show hover effect by reducing transparency -- Normal buttons: show hover effect by reducing transparency
bgTransparency = props.transparency:map(function(t) bgTransparency = props.transparency:map(function(t)
if isSelected then if isSelected then
return if effectiveHovered then 0 + (1 * t) else 0.1 + (0.9 * t) return if isHovered then 0 + (1 * t) else 0.1 + (0.9 * t)
else else
return if effectiveHovered then 0.5 + (0.5 * t) else 0.7 + (0.3 * t) return if isHovered then 0.5 + (0.5 * t) else 0.7 + (0.3 * t)
end end
end) end)
txtTransparency = props.transparency txtTransparency = props.transparency
@ -127,7 +118,7 @@ function SelectionOption:render()
self:setState({ isHovered = true }) self:setState({ isHovered = true })
-- Trigger subtree highlight when this button would apply to children -- Trigger subtree highlight when this button would apply to children
if (isParentOnly or (isSelected and hasChildren)) and props.onSubtreeHoverStart then if (isParentOnly or (isSelected and hasChildren)) and props.onSubtreeHoverStart then
props.onSubtreeHoverStart(props.optionType) props.onSubtreeHoverStart()
end end
end, end,
[Roact.Event.MouseLeave] = function() [Roact.Event.MouseLeave] = function()
@ -181,7 +172,7 @@ local function SelectionRadio(props)
SortOrder = Enum.SortOrder.LayoutOrder, SortOrder = Enum.SortOrder.LayoutOrder,
Padding = UDim.new(0, 4), Padding = UDim.new(0, 4),
}), }),
Studio = e(SelectionOption, { Studio = e(SelectionOption, {
text = "Studio", text = "Studio",
optionType = "pull", optionType = "pull",
isSelected = props.selection == "pull", isSelected = props.selection == "pull",
@ -189,8 +180,6 @@ local function SelectionRadio(props)
layoutOrder = 1, layoutOrder = 1,
hasChildren = hasChildren, hasChildren = hasChildren,
isParentOnly = isParentOnly, isParentOnly = isParentOnly,
subtreeHighlightInfo = props.subtreeHighlightInfo,
ancestorIds = props.ancestorIds,
onSubtreeHoverStart = props.onSubtreeHoverStart, onSubtreeHoverStart = props.onSubtreeHoverStart,
onSubtreeHoverEnd = props.onSubtreeHoverEnd, onSubtreeHoverEnd = props.onSubtreeHoverEnd,
onClick = function() onClick = function()
@ -212,8 +201,6 @@ local function SelectionRadio(props)
layoutOrder = 2, layoutOrder = 2,
hasChildren = hasChildren, hasChildren = hasChildren,
isParentOnly = isParentOnly, isParentOnly = isParentOnly,
subtreeHighlightInfo = props.subtreeHighlightInfo,
ancestorIds = props.ancestorIds,
onSubtreeHoverStart = props.onSubtreeHoverStart, onSubtreeHoverStart = props.onSubtreeHoverStart,
onSubtreeHoverEnd = props.onSubtreeHoverEnd, onSubtreeHoverEnd = props.onSubtreeHoverEnd,
onClick = function() onClick = function()
@ -235,8 +222,6 @@ local function SelectionRadio(props)
layoutOrder = 3, layoutOrder = 3,
hasChildren = hasChildren, hasChildren = hasChildren,
isParentOnly = isParentOnly, isParentOnly = isParentOnly,
subtreeHighlightInfo = props.subtreeHighlightInfo,
ancestorIds = props.ancestorIds,
onSubtreeHoverStart = props.onSubtreeHoverStart, onSubtreeHoverStart = props.onSubtreeHoverStart,
onSubtreeHoverEnd = props.onSubtreeHoverEnd, onSubtreeHoverEnd = props.onSubtreeHoverEnd,
onClick = function() onClick = function()
@ -352,6 +337,17 @@ function DomLabel:render()
local props = self.props local props = self.props
local depth = props.depth or 1 local depth = props.depth or 1
-- Derive subtree highlight state from binding (no re-renders needed)
local ancestorIds = props.ancestorIds
local isSubtreeHighlighted = if props.subtreeHighlightNodeId
then props.subtreeHighlightNodeId:map(function(highlightId)
if highlightId == nil then
return false
end
return ancestorIds[highlightId] == true
end)
else nil
return Theme.with(function(theme) return Theme.with(function(theme)
local color = if props.isWarning local color = if props.isWarning
then theme.Diff.Warning then theme.Diff.Warning
@ -548,6 +544,17 @@ function DomLabel:render()
}) })
else nil, else nil,
}), }),
SubtreeHighlight = if isSubtreeHighlighted
then e("Frame", {
Size = UDim2.fromScale(1, 1),
BackgroundColor3 = theme.Diff.SubtreeHighlight,
BackgroundTransparency = isSubtreeHighlighted:map(function(highlighted)
return if highlighted then 0.92 else 1
end),
BorderSizePixel = 0,
ZIndex = 0,
})
else nil,
SelectionRadio = e(SelectionRadio, { SelectionRadio = e(SelectionRadio, {
-- Visible for nodes with patchType, or for parent-only nodes on hover -- Visible for nodes with patchType, or for parent-only nodes on hover
visible = (props.patchType ~= nil and props.onSelectionChange ~= nil) visible = (props.patchType ~= nil and props.onSelectionChange ~= nil)
@ -565,17 +572,14 @@ function DomLabel:render()
isParentOnly = props.patchType == nil, isParentOnly = props.patchType == nil,
onSelectionChange = props.onSelectionChange, onSelectionChange = props.onSelectionChange,
onSubtreeSelectionChange = props.onSubtreeSelectionChange, onSubtreeSelectionChange = props.onSubtreeSelectionChange,
-- Subtree hover highlight: pass info and ancestors to child buttons onSubtreeHoverStart = function()
subtreeHighlightInfo = props.subtreeHighlightInfo, if props.setSubtreeHighlightNodeId then
ancestorIds = props.ancestorIds, props.setSubtreeHighlightNodeId(props.nodeId)
onSubtreeHoverStart = function(optionType)
if props.onSubtreeHighlightChange then
props.onSubtreeHighlightChange({ nodeId = props.nodeId, optionType = optionType })
end end
end, end,
onSubtreeHoverEnd = function() onSubtreeHoverEnd = function()
if props.onSubtreeHighlightChange then if props.setSubtreeHighlightNodeId then
props.onSubtreeHighlightChange(nil) props.setSubtreeHighlightNodeId(nil)
end end
end, end,
}), }),

View file

@ -61,25 +61,15 @@ function PatchVisualizer:init()
-- causing expanded rows to visually glitch when unrelated props change. -- causing expanded rows to visually glitch when unrelated props change.
self.nodeHeightBindings = {} self.nodeHeightBindings = {}
self:setState({ -- Binding for subtree hover preview: stores the nodeId whose children should highlight
subtreeHighlightInfo = nil, self.subtreeHighlightNodeId, self.setSubtreeHighlightNodeId = Roact.createBinding(nil)
})
-- Callback for subtree hover preview: stores { nodeId, optionType } or nil
self.onSubtreeHighlightChange = function(info)
self:setState({ subtreeHighlightInfo = info })
end
end end
function PatchVisualizer:willUnmount() function PatchVisualizer:willUnmount()
self.updateEvent:Destroy() self.updateEvent:Destroy()
end end
function PatchVisualizer:shouldUpdate(nextProps, nextState) function PatchVisualizer:shouldUpdate(nextProps)
if self.state.subtreeHighlightInfo ~= nextState.subtreeHighlightInfo then
return true
end
if self.props.patchTree ~= nextProps.patchTree then if self.props.patchTree ~= nextProps.patchTree then
return true return true
end end
@ -230,8 +220,8 @@ function PatchVisualizer:render()
onSubtreeSelectionChange = self.props.onSubtreeSelectionChange, onSubtreeSelectionChange = self.props.onSubtreeSelectionChange,
-- Subtree hover highlight props -- Subtree hover highlight props
ancestorIds = ancestorIds, ancestorIds = ancestorIds,
subtreeHighlightInfo = self.state.subtreeHighlightInfo, subtreeHighlightNodeId = self.subtreeHighlightNodeId,
onSubtreeHighlightChange = self.onSubtreeHighlightChange, setSubtreeHighlightNodeId = self.setSubtreeHighlightNodeId,
}) })
if isFinalChild then if isFinalChild then