fix(dock): ensure drop safety and consistent reordering semantics

This commit is contained in:
2026-03-28 14:50:11 +09:00
parent 4aeaecfe81
commit 98405cb8ec
3 changed files with 110 additions and 78 deletions

View File

@@ -47,6 +47,10 @@ public partial class DockGroupNode : DockNode
/// </summary> /// </summary>
/// <param name="index">The zero-based index at which node should be inserted.</param> /// <param name="index">The zero-based index at which node should be inserted.</param>
/// <param name="node">The node to insert.</param> /// <param name="node">The node to insert.</param>
/// <remarks>
/// If the node is already a child of this group, it will be moved to the specified index.
/// The index represents the desired final position in the collection.
/// </remarks>
/// <exception cref="ArgumentNullException">Thrown if node is null.</exception> /// <exception cref="ArgumentNullException">Thrown if node is null.</exception>
/// <exception cref="ArgumentOutOfRangeException">Thrown if index is less than 0 or greater than Children.Count.</exception> /// <exception cref="ArgumentOutOfRangeException">Thrown if index is less than 0 or greater than Children.Count.</exception>
/// <exception cref="InvalidOperationException">Thrown if adding the node would create a cycle or if adding self.</exception> /// <exception cref="InvalidOperationException">Thrown if adding the node would create a cycle or if adding self.</exception>
@@ -69,14 +73,7 @@ public partial class DockGroupNode : DockNode
int oldIndex = _children.IndexOf(node); int oldIndex = _children.IndexOf(node);
if (oldIndex == index) return; if (oldIndex == index) return;
// If we're moving an item forward, the target index will shift after removal _children.Move(oldIndex, index);
int targetIndex = index;
if (targetIndex > oldIndex) targetIndex--;
if (oldIndex == targetIndex) return;
_children.RemoveAt(oldIndex);
_children.Insert(targetIndex, node);
return; return;
} }

View File

@@ -430,14 +430,17 @@ public sealed partial class DockLayout : Control
return; return;
} }
// Remove from source first (if it's the same node, we'll re-add it to the new structure) // If targetNode is the only child of Root, we wrap it.
if (Root.Children.Count == 1 && ReferenceEquals(Root.Children[0], targetNode))
{
// Remove from source first
if (!_sourceNode.Items.Remove(_draggedItem)) if (!_sourceNode.Items.Remove(_draggedItem))
{ {
ClearDragOperationState(); ClearDragOperationState();
return; return;
} }
var newRoot = new DockGroupNode var newGroup = new DockGroupNode
{ {
Orientation = isHorizontalSplit ? Orientation.Horizontal : Orientation.Vertical Orientation = isHorizontalSplit ? Orientation.Horizontal : Orientation.Vertical
}; };
@@ -445,21 +448,30 @@ public sealed partial class DockLayout : Control
var newNode = new DockPanelNode(); var newNode = new DockPanelNode();
newNode.Items.Add(_draggedItem); newNode.Items.Add(_draggedItem);
// If targetNode was the only child of Root, we replace it in Root. Root.RemoveChild(targetNode);
// If targetNode WAS the Root (impossible by type, but let's be safe with the model), we replace Root. Root.AddChild(newGroup);
// Actually, Root is always a DockGroupNode. So targetNode must be a child of Root if parent is null?
// No, if parent is null it's either detached or it IS the root.
// But targetNode is DockPanelNode, and Root is DockGroupNode.
// If targetNode is detached, we can't split it. if (isAfter)
// Let's assume it's a child of a group that we don't have a reference to? No, Parent should be set. {
// The only case where Parent is null for a DockPanelNode in a valid tree is if it's NOT in the tree. newGroup.AddChild(targetNode);
// Wait, if Root has only one child, that child's parent SHOULD be Root. newGroup.AddChild(newNode);
}
ClearDragOperationState(); else
return; // Abort for now, parent should not be null in a valid tree for a Panel. {
newGroup.AddChild(newNode);
newGroup.AddChild(targetNode);
} }
CleanupEmptyNodes(_sourceNode);
}
else
{
ClearDragOperationState();
return;
}
}
else
{
int targetIndex = parentGroup.Children.IndexOf(targetNode); int targetIndex = parentGroup.Children.IndexOf(targetNode);
if (targetIndex < 0) if (targetIndex < 0)
{ {
@@ -511,6 +523,7 @@ public sealed partial class DockLayout : Control
CleanupEmptyNodes(_sourceNode); CleanupEmptyNodes(_sourceNode);
} }
}
ClearDragOperationState(); ClearDragOperationState();
} }

View File

@@ -171,17 +171,23 @@ public class DockingModelTest
group.AddChild(child2); group.AddChild(child2);
group.AddChild(child3); group.AddChild(child3);
// Move child1 to end // Move child1 to end (index 2)
group.InsertChild(3, child1); group.InsertChild(2, child1);
Assert.AreEqual(child2, group.Children[0]); Assert.AreEqual(child2, group.Children[0]);
Assert.AreEqual(child3, group.Children[1]); Assert.AreEqual(child3, group.Children[1]);
Assert.AreEqual(child1, group.Children[2]); Assert.AreEqual(child1, group.Children[2]);
// Move child3 to start // Move child3 to start (index 0)
group.InsertChild(0, child3); group.InsertChild(0, child3);
Assert.AreEqual(child3, group.Children[0]); Assert.AreEqual(child3, group.Children[0]);
Assert.AreEqual(child2, group.Children[1]); Assert.AreEqual(child2, group.Children[1]);
Assert.AreEqual(child1, group.Children[2]); Assert.AreEqual(child1, group.Children[2]);
// Move child2 forward by one (index 1 -> 2)
group.InsertChild(2, child2);
Assert.AreEqual(child3, group.Children[0]);
Assert.AreEqual(child1, group.Children[1]);
Assert.AreEqual(child2, group.Children[2]);
} }
[TestMethod] [TestMethod]
@@ -198,4 +204,20 @@ public class DockingModelTest
Assert.AreEqual(child1, group.Children[0]); Assert.AreEqual(child1, group.Children[0]);
Assert.AreEqual(child2, group.Children[1]); Assert.AreEqual(child2, group.Children[1]);
} }
[TestMethod]
public void TestPanel_SetInvalidSelectedItem_ResetsSelection()
{
var panel = new DockPanelNode();
var item1 = new object();
var item2 = new object();
panel.Items.Add(item1);
panel.SelectedItem = item1;
panel.SelectedItem = item2; // Not in collection
Assert.IsNull(panel.SelectedItem);
Assert.AreEqual(-1, panel.SelectedIndex);
}
} }