UX update

This commit is contained in:
Joe 2026-02-01 07:58:06 -06:00
parent 3e6a50ce4b
commit 0e5e809894
5 changed files with 397 additions and 48 deletions

View file

@ -0,0 +1,178 @@
---
name: Destructive Syncback Mode
overview: Add a `--clean` flag to syncback that ignores existing file structure and middleware, treating all instances as new to ensure consistent 1:1 output regardless of existing filesystem state.
todos: []
isProject: false
---
# Destructive Syncback Mode
## Root Cause Analysis
Your inconsistency stems from three preservation mechanisms in syncback:
### 1. File Name Preservation (`[src/syncback/file_names.rs](src/syncback/file_names.rs)` lines 19-34)
When an old instance exists, the **existing filename is always preserved**, even if the instance was renamed:
```rust
if let Some(old_inst) = old_inst {
if let Some(source) = old_inst.metadata().relevant_paths.first() {
source.file_name() // Returns OLD filename, ignores new instance name
}
}
```
### 2. Middleware Preservation (`[src/syncback/mod.rs](src/syncback/mod.rs)` lines 353-357)
When an old instance exists, its middleware format is preserved:
```rust
} else if let Some(old_middleware) = old_middleware {
// Use old middleware, even if a clean sync would choose differently
middleware = old_middleware;
}
```
This means a `Part.rbxm` stays as `.rbxm` instead of potentially becoming a `Part/` directory with children.
### 3. Name-Based Child Matching (`[src/snapshot_middleware/dir.rs](src/snapshot_middleware/dir.rs)` lines 196-216)
Children are matched by name between old and new DOMs. If a child exists in both, it inherits the old structure:
```rust
if let Some(old_child) = old_child_map.remove(new_child.name.as_str()) {
// Existing child - pass old reference, preserving its structure
children.push(snapshot.with_joined_path(*new_child_ref, Some(old_child.id()))?);
} else {
// New child - gets fresh structure
children.push(snapshot.with_joined_path(*new_child_ref, None)?);
}
```
## Solution: Add `--clean` Flag
Add a `--clean` flag that makes syncback treat all instances as "new" by setting `old` references to `None`, ensuring consistent output regardless of existing filesystem state.
### Changes Required
#### 1. Add CLI Flag (`[src/cli/syncback.rs](src/cli/syncback.rs)`)
Add `--clean` flag to the command definition:
```rust
/// If provided, syncback will ignore existing file structure and create
/// a fresh project layout. This ensures consistent output regardless of
/// existing filesystem state.
#[clap(long)]
pub clean: bool,
```
#### 2. Pass Flag to `syncback_loop` (`[src/cli/syncback.rs](src/cli/syncback.rs)`)
Modify the `syncback_loop` call to pass the clean flag:
```rust
let snapshot = syncback_loop(
session_old.vfs(),
&mut dom_old,
dom_new,
session_old.root_project(),
self.clean, // New parameter
)?;
```
#### 3. Modify `syncback_loop` (`[src/syncback/mod.rs](src/syncback/mod.rs)`)
Update function signature and initial snapshot:
```rust
pub fn syncback_loop(
vfs: &Vfs,
old_tree: &mut RojoTree,
mut new_tree: WeakDom,
project: &Project,
clean_mode: bool, // New parameter
) -> anyhow::Result<FsSnapshot>
```
When `clean_mode` is true, set the initial `old` reference to `None`:
```rust
let mut snapshots = vec![SyncbackSnapshot {
data: syncback_data,
old: if clean_mode { None } else { Some(old_tree.get_root_id()) },
new: new_tree.root_ref(),
path: project.file_location.clone(),
middleware: Some(Middleware::Project),
}];
```
#### 4. Track Removed Files in Clean Mode (`[src/syncback/mod.rs](src/syncback/mod.rs)`)
In clean mode, we need to remove all existing files in the target paths. Add logic to collect existing paths for removal:
```rust
if clean_mode {
// Collect all existing paths under the project tree for removal
for ref_id in descendants(old_tree.inner(), old_tree.get_root_id()) {
if let Some(inst) = old_tree.get_instance(ref_id) {
for path in inst.metadata().relevant_paths.iter() {
if path.is_dir() {
fs_snapshot.remove_dir(path);
} else {
fs_snapshot.remove_file(path);
}
}
}
}
}
```
#### 5. Update Child Snapshot Creation (`[src/snapshot_middleware/dir.rs](src/snapshot_middleware/dir.rs)`)
The `SyncbackSnapshot::with_joined_path` method already handles `old = None` correctly - it will create a fresh child snapshot without old references.
### Data Flow Diagram
```mermaid
flowchart TD
subgraph currentBehavior [Current Behavior]
A1[Syncback on Existing Files] --> B1{Old Instance Exists?}
B1 -->|Yes| C1[Preserve Filename]
B1 -->|Yes| D1[Preserve Middleware]
B1 -->|Yes| E1[Match Children by Name]
C1 --> F1[Inconsistent Structure]
D1 --> F1
E1 --> F1
end
subgraph cleanMode [Clean Mode]
A2[Syncback with --clean] --> B2[Set old = None]
B2 --> C2[Generate Fresh Filename]
B2 --> D2[Choose Best Middleware]
B2 --> E2[All Children Are New]
C2 --> F2[Consistent 1:1 Structure]
D2 --> F2
E2 --> F2
end
```
### Usage
```bash
# Current behavior (preserves existing structure)
rojo syncback -i game.rbxl
# Destructive mode (ignores existing structure, creates fresh layout)
rojo syncback -i game.rbxl --clean
# Combine with other flags
rojo syncback -i game.rbxl --clean --list --dry-run
```
### Alternative Consideration
If you also want to support project-level configuration (so you don't need to pass `--clean` every time), we could add a `syncbackRules.cleanMode: true` option in the project file. Let me know if you'd prefer that approach.

View file

@ -0,0 +1,132 @@
---
name: Merge UI Button Changes
overview: Rename Push/Pull buttons to Rojo/Studio, remove "all" helper buttons, add double-click cascade functionality with hover-reveal buttons for parent nodes.
todos:
- id: rename-buttons
content: Rename Pull->Studio and Push->Rojo in DomLabel.lua SelectionRadio
status: completed
- id: remove-all-buttons
content: Remove PullAll, SkipAll, PushAll buttons from Confirming.lua
status: completed
- id: add-subtree-callback
content: Create onSubtreeSelectionChange callback in Confirming.lua that sets selection for node and all descendants
status: completed
- id: add-double-click
content: Add double-click handler to SelectionOption buttons with caret indicator
status: completed
- id: hover-reveal-buttons
content: Show selection buttons on hover for parent nodes without patchType
status: completed
isProject: false
---
# Merge UI Button Improvements
## Key Files to Modify
- `[plugin/src/App/Components/PatchVisualizer/DomLabel.lua](plugin/src/App/Components/PatchVisualizer/DomLabel.lua)` - Selection buttons and hover behavior
- `[plugin/src/App/StatusPages/Confirming.lua](plugin/src/App/StatusPages/Confirming.lua)` - Remove "all" buttons, add subtree selection callback
- `[plugin/src/App/Components/PatchVisualizer/init.lua](plugin/src/App/Components/PatchVisualizer/init.lua)` - Pass patchTree to DomLabel
## Changes
### 1. Rename Buttons (DomLabel.lua)
In `SelectionRadio` function, change button text:
- "Pull" -> "Studio" (line 113)
- "Push" -> "Rojo" (line 137)
### 2. Remove "All" Button Helpers (Confirming.lua)
Remove these elements from the `Buttons` frame (lines 156-184):
- `PullAll` button
- `SkipAll` button
- `PushAll` button
Keep the `setAllSelections` function as it will be repurposed for subtree selection.
### 3. Add Double-Click Cascade with Caret (DomLabel.lua)
Modify `SelectionOption` component:
- Add a "▼" caret character after the button text
- Track double-click using `clickCount` in the `Activated` event
- On double-click, call a new `onSubtreeSelectionChange` callback
Add to `SelectionRadio`:
- Pass `onSubtreeSelectionChange` prop to each option
### 4. Add Subtree Selection Callback (Confirming.lua)
Create new callback `onSubtreeSelectionChange(nodeId, selection)`:
```lua
self.onSubtreeSelectionChange = function(nodeId, selection)
if not self.props.patchTree then return end
local node = self.props.patchTree:getNode(nodeId)
if not node then return end
self:setState(function(state)
local newSelections = table.clone(state.selections)
-- Set this node if it has a patchType
if node.patchType then
newSelections[nodeId] = selection
end
-- Set all descendants
self.props.patchTree:forEach(function(childNode)
if childNode.patchType then
newSelections[childNode.id] = selection
end
end, node)
return { selections = newSelections }
end)
end
```
### 5. Show Buttons on Hover for Parents Without Changes (DomLabel.lua)
For nodes where `patchType == nil` but `hasChildren == true`:
Add hover state tracking in `DomLabel`:
```lua
self:setState({ isHovered = false })
```
Add `MouseEnter`/`MouseLeave` events to the main Frame.
Modify `SelectionRadio` visibility logic:
- `visible = (props.patchType ~= nil) or (props.hasChildren and self.state.isHovered)`
The buttons for parent-only nodes will use `onSubtreeSelectionChange` directly (no single-click behavior needed since there's no selection to make for the node itself).
### 6. Pass Required Props Through Component Tree
In `PatchVisualizer/init.lua`, pass to DomLabel:
- `patchTree = patchTree`
- `onSubtreeSelectionChange = self.props.onSubtreeSelectionChange`
In `Confirming.lua`, pass to PatchVisualizer:
- `onSubtreeSelectionChange = self.onSubtreeSelectionChange`
## Component Flow
```mermaid
flowchart TD
A[Confirming.lua] -->|selections, onSelectionChange, onSubtreeSelectionChange| B[PatchVisualizer]
B -->|node props, selection callbacks| C[DomLabel]
C -->|single click| D[onSelectionChange]
C -->|double click| E[onSubtreeSelectionChange]
D --> F[Update single node selection]
E --> G[Update node + all descendants]
```

View file

@ -61,24 +61,40 @@ local function SelectionOption(props)
else Color3.fromHex("7F8C8D")) else Color3.fromHex("7F8C8D"))
else theme.BorderedContainer.BackgroundColor else theme.BorderedContainer.BackgroundColor
local textColor = if isSelected then Color3.new(1, 1, 1) else theme.TextColor local textColor = if isSelected then Color3.new(1, 1, 1) else theme.TextColor
-- Add caret if this button can cascade to children
local displayText = if props.showCaret then props.text .. " ▼" else props.text
local buttonWidth = if props.showCaret then 48 else 36
return e("TextButton", { return e("TextButton", {
Size = UDim2.new(0, 36, 0, 18), Size = UDim2.new(0, buttonWidth, 0, 18),
BackgroundColor3 = bgColor, BackgroundColor3 = bgColor,
BackgroundTransparency = props.transparency:map(function(t) BackgroundTransparency = props.transparency:map(function(t)
return if isSelected then 0.1 + (0.9 * t) else 0.7 + (0.3 * t) return if isSelected then 0.1 + (0.9 * t) else 0.7 + (0.3 * t)
end), end),
BorderSizePixel = 0, BorderSizePixel = 0,
Text = props.text, Text = displayText,
FontFace = if isSelected then theme.Font.Bold else theme.Font.Main, FontFace = if isSelected then theme.Font.Bold else theme.Font.Main,
TextSize = theme.TextSize.Small, TextSize = theme.TextSize.Small,
TextColor3 = textColor, TextColor3 = textColor,
TextTransparency = props.transparency, TextTransparency = props.transparency,
LayoutOrder = props.layoutOrder, LayoutOrder = props.layoutOrder,
ZIndex = 10, ZIndex = 10,
[Roact.Event.Activated] = function() [Roact.Event.Activated] = function(_rbx, _input, clickCount)
if props.onClick then -- For parent-only nodes, any click triggers subtree selection
props.onClick() if props.isParentOnly then
if props.onDoubleClick then
props.onDoubleClick()
end
elseif clickCount == 1 then
-- Double click: apply to subtree
if props.onDoubleClick then
props.onDoubleClick()
end
else
-- Single click: apply to this node only
if props.onClick then
props.onClick()
end
end end
end, end,
}, { }, {
@ -95,8 +111,12 @@ local function SelectionRadio(props)
return nil return nil
end end
local showCaret = props.hasChildren
local frameWidth = if showCaret then 154 else 118
local isParentOnly = props.isParentOnly
return e("Frame", { return e("Frame", {
Size = UDim2.new(0, 118, 0, 18), Size = UDim2.new(0, frameWidth, 0, 18),
BackgroundTransparency = 1, BackgroundTransparency = 1,
Position = UDim2.new(1, -5, 0, 3), Position = UDim2.new(1, -5, 0, 3),
AnchorPoint = Vector2.new(1, 0), AnchorPoint = Vector2.new(1, 0),
@ -109,17 +129,24 @@ local function SelectionRadio(props)
SortOrder = Enum.SortOrder.LayoutOrder, SortOrder = Enum.SortOrder.LayoutOrder,
Padding = UDim.new(0, 4), Padding = UDim.new(0, 4),
}), }),
Pull = e(SelectionOption, { Studio = e(SelectionOption, {
text = "Pull", text = "Studio",
optionType = "pull", optionType = "pull",
isSelected = props.selection == "pull", isSelected = props.selection == "pull",
transparency = props.transparency, transparency = props.transparency,
layoutOrder = 1, layoutOrder = 1,
showCaret = showCaret,
isParentOnly = isParentOnly,
onClick = function() onClick = function()
if props.onSelectionChange then if props.onSelectionChange then
props.onSelectionChange(props.nodeId, "pull") props.onSelectionChange(props.nodeId, "pull")
end end
end, end,
onDoubleClick = function()
if props.onSubtreeSelectionChange then
props.onSubtreeSelectionChange(props.nodeId, "pull")
end
end,
}), }),
Skip = e(SelectionOption, { Skip = e(SelectionOption, {
text = "Skip", text = "Skip",
@ -127,23 +154,37 @@ local function SelectionRadio(props)
isSelected = props.selection == "ignore", isSelected = props.selection == "ignore",
transparency = props.transparency, transparency = props.transparency,
layoutOrder = 2, layoutOrder = 2,
showCaret = showCaret,
isParentOnly = isParentOnly,
onClick = function() onClick = function()
if props.onSelectionChange then if props.onSelectionChange then
props.onSelectionChange(props.nodeId, "ignore") props.onSelectionChange(props.nodeId, "ignore")
end end
end, end,
onDoubleClick = function()
if props.onSubtreeSelectionChange then
props.onSubtreeSelectionChange(props.nodeId, "ignore")
end
end,
}), }),
Push = e(SelectionOption, { Rojo = e(SelectionOption, {
text = "Push", text = "Rojo",
optionType = "push", optionType = "push",
isSelected = props.selection == "push", isSelected = props.selection == "push",
transparency = props.transparency, transparency = props.transparency,
layoutOrder = 3, layoutOrder = 3,
showCaret = showCaret,
isParentOnly = isParentOnly,
onClick = function() onClick = function()
if props.onSelectionChange then if props.onSelectionChange then
props.onSelectionChange(props.nodeId, "push") props.onSelectionChange(props.nodeId, "push")
end end
end, end,
onDoubleClick = function()
if props.onSubtreeSelectionChange then
props.onSubtreeSelectionChange(props.nodeId, "push")
end
end,
}), }),
}) })
end end
@ -183,6 +224,7 @@ function DomLabel:init()
self:setState({ self:setState({
renderExpansion = self.expanded, renderExpansion = self.expanded,
isHovered = false,
}) })
self.motorStepConnection = self.motor:onStep(function(value) self.motorStepConnection = self.motor:onStep(function(value)
if not self.isMounted then if not self.isMounted then
@ -301,6 +343,12 @@ function DomLabel:render()
BackgroundTransparency = 1, BackgroundTransparency = 1,
Text = "", Text = "",
Size = UDim2.new(1, 0, 1, 0), Size = UDim2.new(1, 0, 1, 0),
[Roact.Event.MouseEnter] = function()
self:setState({ isHovered = true })
end,
[Roact.Event.MouseLeave] = function()
self:setState({ isHovered = false })
end,
[Roact.Event.Activated] = function(_rbx: Instance, _input: InputObject, clickCount: number) [Roact.Event.Activated] = function(_rbx: Instance, _input: InputObject, clickCount: number)
if clickCount == 1 then if clickCount == 1 then
-- Double click opens the instance in explorer -- Double click opens the instance in explorer
@ -413,11 +461,17 @@ function DomLabel:render()
else nil, else nil,
}), }),
SelectionRadio = e(SelectionRadio, { SelectionRadio = e(SelectionRadio, {
visible = props.patchType ~= nil and props.onSelectionChange ~= nil, -- Visible for nodes with patchType, or for parent-only nodes on hover
visible = (props.patchType ~= nil and props.onSelectionChange ~= nil)
or (props.patchType == nil and props.hasChildren and self.state.isHovered and props.onSubtreeSelectionChange ~= nil),
nodeId = props.nodeId, nodeId = props.nodeId,
selection = props.selection, selection = props.selection,
transparency = props.transparency, transparency = props.transparency,
hasChildren = props.hasChildren,
-- For parent-only nodes (no patchType), only subtree selection makes sense
isParentOnly = props.patchType == nil,
onSelectionChange = props.onSelectionChange, onSelectionChange = props.onSelectionChange,
onSubtreeSelectionChange = props.onSubtreeSelectionChange,
}), }),
LineGuides = e("Folder", nil, lineGuides), LineGuides = e("Folder", nil, lineGuides),
}) })

View file

@ -104,6 +104,7 @@ function PatchVisualizer:render()
nodeId = node.id, nodeId = node.id,
selection = self.props.selections and self.props.selections[node.id], selection = self.props.selections and self.props.selections[node.id],
onSelectionChange = self.props.onSelectionChange, onSelectionChange = self.props.onSelectionChange,
onSubtreeSelectionChange = self.props.onSubtreeSelectionChange,
}) })
if isFinalChild then if isFinalChild then

View file

@ -48,18 +48,31 @@ function ConfirmingPage:init()
end) end)
end end
-- Set all selections to a specific value -- Set selection for a node and all its descendants (subtree)
self.setAllSelections = function(value) self.onSubtreeSelectionChange = function(nodeId, selection)
if not self.props.patchTree then if not self.props.patchTree then
return return
end end
local newSelections = {}
self.props.patchTree:forEach(function(node) local node = self.props.patchTree:getNode(nodeId)
if not node then
return
end
self:setState(function(state)
local newSelections = table.clone(state.selections)
-- Set this node if it has a patchType
if node.patchType then if node.patchType then
newSelections[node.id] = value newSelections[nodeId] = selection
end end
-- Set all descendants
self.props.patchTree:forEach(function(childNode)
if childNode.patchType then
newSelections[childNode.id] = selection
end
end, node)
return { selections = newSelections }
end) end)
self:setState({ selections = newSelections })
end end
end end
@ -123,6 +136,7 @@ function ConfirmingPage:render()
patchTree = self.props.patchTree, patchTree = self.props.patchTree,
selections = self.state.selections, selections = self.state.selections,
onSelectionChange = self.onSelectionChange, onSelectionChange = self.onSelectionChange,
onSubtreeSelectionChange = self.onSubtreeSelectionChange,
showStringDiff = function(currentString: string, incomingString: string) showStringDiff = function(currentString: string, incomingString: string)
self:setState({ self:setState({
@ -153,41 +167,11 @@ function ConfirmingPage:render()
onClick = self.props.onAbort, onClick = self.props.onAbort,
}), }),
PullAll = e(TextButton, {
text = "Pull All",
style = "Danger",
transparency = self.props.transparency,
layoutOrder = 2,
onClick = function()
self.setAllSelections("pull")
end,
}),
SkipAll = e(TextButton, {
text = "Skip All",
style = "Neutral",
transparency = self.props.transparency,
layoutOrder = 3,
onClick = function()
self.setAllSelections("ignore")
end,
}),
PushAll = e(TextButton, {
text = "Push All",
style = "Success",
transparency = self.props.transparency,
layoutOrder = 4,
onClick = function()
self.setAllSelections("push")
end,
}),
Accept = e(TextButton, { Accept = e(TextButton, {
text = "Accept", text = "Accept",
style = "Primary", style = "Primary",
transparency = self.props.transparency, transparency = self.props.transparency,
layoutOrder = 5, layoutOrder = 2,
onClick = function() onClick = function()
if not self.props.onConfirm then if not self.props.onConfirm then
return return