Browse Source

Land external file drops anywhere on the assembly tree

Two failure modes that both showed the user the "no" cursor with no
other feedback:

1. ResolveDropTarget returned null when e.Source had no
SharpTreeViewItem ancestor, so Explorer drops landing in the gap
beneath the last row (or onto an empty list) never reached
SharpTreeNode.InternalDrop.

2. The middle 50% of a row produces a DropPlace.Inside target on the
row's own node. Most concrete SharpTreeNodes inherit the base CanDrop
(returns false), so a literal "drop a .dll onto an assembly row"
refused even though AssemblyListTreeNode happily accepts that payload.

For (1), fall back to (Root, Children.Count, DropPlace.Inside) when no
row is hit. For (2), introduce PickAcceptingTarget which retries with
the empty-space (root) target when the initial CanDrop is false. Both
OnDragOver and OnDrop go through the same picker so the cursor and the
actual drop agree on the outcome. The retry skips when the initial
target already IS the root, so a real refusal still surfaces as "no".

The marker adorner stays hidden for empty-space drops because the
place is always Inside (Item is null for that case); the existing
DropPlace.Inside early return covers it.

DropTarget, DropPlace, and the two new helpers are internal-visible to
the ILSpy.Tests project for the unit tests that assert the fallback
chain.

Assisted-by: Claude:claude-opus-4-7[1m]:Claude Code
pull/3769/head
Siegfried Pammer 4 weeks ago committed by Christoph Wille
parent
commit
61bd014c2d
  1. 127
      ILSpy.Tests/Controls/SharpTreeViewEmptySpaceDropTests.cs
  2. 52
      ILSpy/Controls/TreeView/SharpTreeView.cs

127
ILSpy.Tests/Controls/SharpTreeViewEmptySpaceDropTests.cs

@ -0,0 +1,127 @@ @@ -0,0 +1,127 @@
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
// software and associated documentation files (the "Software"), to deal in the Software
// without restriction, including without limitation the rights to use, copy, modify, merge,
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
// to whom the Software is furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all copies or
// substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE.
using Avalonia.Controls;
using Avalonia.Headless.NUnit;
using Avalonia.Threading;
using AwesomeAssertions;
using ICSharpCode.ILSpyX.TreeView;
using ICSharpCode.ILSpyX.TreeView.PlatformAbstractions;
using ILSpy.Controls.TreeView;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests.Controls;
// External Explorer drops landed only when the cursor was over a row -- ResolveDropTarget bails
// when e.Source has no SharpTreeViewItem ancestor, so a drop into the empty space below the last
// row never reached SharpTreeNode.InternalDrop. In the assembly tree that meant "drop a DLL onto
// an empty list" silently did nothing. The fallback is the same as what the WPF SharpTreeView
// did: target the root node at its end with DropPlace.Inside, then let CanDrop decide.
[TestFixture]
public class SharpTreeViewEmptySpaceDropTests
{
sealed class RootNode : SharpTreeNode
{
public bool AcceptDrops { get; init; }
public override object Text => "root";
public override bool CanDrop(IPlatformDragEventArgs e, int index) => AcceptDrops;
public override void Drop(IPlatformDragEventArgs e, int index) { }
}
static (Window window, SharpTreeView tree) Host(SharpTreeNode? root)
{
var tree = new SharpTreeView { ShowRoot = false, Root = root };
var window = new Window { Content = tree, Width = 300, Height = 400 };
window.Show();
Dispatcher.UIThread.RunJobs();
return (window, tree);
}
[AvaloniaTest]
public void Empty_space_drop_resolves_to_root_inside_at_end()
{
var root = new RootNode { AcceptDrops = true };
root.Children.Add(new RootNode { AcceptDrops = false });
var (_, tree) = Host(root);
var target = tree.ResolveEmptySpaceDropTarget();
target.Should().NotBeNull();
ReferenceEquals(target!.Value.Node, root).Should().BeTrue();
target.Value.Index.Should().Be(root.Children.Count);
target.Value.Place.Should().Be(SharpTreeView.DropPlace.Inside);
target.Value.Item.Should().BeNull();
}
[AvaloniaTest]
public void Empty_space_drop_returns_null_when_no_root()
{
var (_, tree) = Host(root: null);
tree.ResolveEmptySpaceDropTarget().Should().BeNull();
}
// "Drop directly onto a row" lands DropPlace.Inside on the row's own node; most concrete
// SharpTreeNodes inherit the base CanDrop (returns false) so the row would refuse the payload.
// PickAcceptingTarget falls back to the root in that case so the file still gets opened.
[AvaloniaTest]
public void Drop_on_rejecting_row_falls_back_to_root()
{
var root = new RootNode { AcceptDrops = true };
var child = new RootNode { AcceptDrops = false };
root.Children.Add(child);
var (_, tree) = Host(root);
var initial = new SharpTreeView.DropTarget(child, child.Children.Count,
SharpTreeView.DropPlace.Inside, Item: null);
var picked = tree.PickAcceptingTarget(new AvaloniaPlatformDragEventArgs(new AvaloniaDataObject()), initial);
picked.Should().NotBeNull();
ReferenceEquals(picked!.Value.Node, root).Should().BeTrue();
picked.Value.Index.Should().Be(root.Children.Count);
}
[AvaloniaTest]
public void Drop_on_accepting_row_keeps_the_row_target()
{
var root = new RootNode { AcceptDrops = true };
var child = new RootNode { AcceptDrops = true };
root.Children.Add(child);
var (_, tree) = Host(root);
var initial = new SharpTreeView.DropTarget(child, child.Children.Count,
SharpTreeView.DropPlace.Inside, Item: null);
var picked = tree.PickAcceptingTarget(new AvaloniaPlatformDragEventArgs(new AvaloniaDataObject()), initial);
picked.Should().NotBeNull();
ReferenceEquals(picked!.Value.Node, child).Should().BeTrue();
}
[AvaloniaTest]
public void Drop_with_no_target_and_no_root_returns_null()
{
var (_, tree) = Host(root: null);
tree.PickAcceptingTarget(new AvaloniaPlatformDragEventArgs(new AvaloniaDataObject()), initial: null)
.Should().BeNull();
}
}

52
ILSpy/Controls/TreeView/SharpTreeView.cs

@ -441,7 +441,7 @@ namespace ILSpy.Controls.TreeView @@ -441,7 +441,7 @@ namespace ILSpy.Controls.TreeView
static readonly DataFormat<string> InternalDragFormat =
DataFormat.CreateStringApplicationFormat("sharptreeview-drag");
enum DropPlace { Before, Inside, After }
internal enum DropPlace { Before, Inside, After }
SharpTreeNode[]? draggedNodes;
IPlatformDataObject? dragData;
@ -515,15 +515,8 @@ namespace ILSpy.Controls.TreeView @@ -515,15 +515,8 @@ namespace ILSpy.Controls.TreeView
void OnDragOver(object? sender, DragEventArgs e)
{
if (ResolveDropTarget(e) is not { } target)
{
e.DragEffects = DragDropEffects.None;
HideInsertMarker();
e.Handled = true;
return;
}
var args = new AvaloniaPlatformDragEventArgs(BuildPlatformData(e));
if (target.Node.CanDrop(args, target.Index))
if (PickAcceptingTarget(args, ResolveDropTarget(e)) is { } target)
{
e.DragEffects = args.Effects.ToAvalonia();
ShowInsertMarker(target.Item, target.Place);
@ -539,10 +532,8 @@ namespace ILSpy.Controls.TreeView @@ -539,10 +532,8 @@ namespace ILSpy.Controls.TreeView
void OnDrop(object? sender, DragEventArgs e)
{
HideInsertMarker();
if (ResolveDropTarget(e) is not { } target)
return;
var args = new AvaloniaPlatformDragEventArgs(BuildPlatformData(e));
if (target.Node.CanDrop(args, target.Index))
if (PickAcceptingTarget(args, ResolveDropTarget(e)) is { } target)
{
target.Node.InternalDrop(args, target.Index);
e.DragEffects = args.Effects.ToAvalonia();
@ -550,13 +541,33 @@ namespace ILSpy.Controls.TreeView @@ -550,13 +541,33 @@ namespace ILSpy.Controls.TreeView
e.Handled = true;
}
readonly record struct DropTarget(SharpTreeNode Node, int Index, DropPlace Place, SharpTreeViewItem Item);
// Dropping onto the middle 50% of a row lands DropPlace.Inside on the row's own node. Most
// concrete SharpTreeNode subclasses inherit the base CanDrop (returns false), so a literal
// "drop onto an assembly row" would otherwise show the no-cursor even though the root happily
// accepts the payload. Fall back to the empty-space (root) target in that case -- same
// fallback as the empty-space-below-last-row path.
internal DropTarget? PickAcceptingTarget(IPlatformDragEventArgs args, DropTarget? initial)
{
if (initial is { } first && first.Node.CanDrop(args, first.Index))
return first;
if (ResolveEmptySpaceDropTarget() is { } fallback
&& (initial is null || !ReferenceEquals(fallback.Node, initial.Value.Node))
&& fallback.Node.CanDrop(args, fallback.Index))
return fallback;
return null;
}
internal readonly record struct DropTarget(SharpTreeNode Node, int Index, DropPlace Place, SharpTreeViewItem? Item);
DropTarget? ResolveDropTarget(DragEventArgs e)
{
// External Explorer drops over the gap below the last row arrive with e.Source pointing at
// the ListBox-inner ScrollViewer / ItemsPresenter / the tree itself rather than a row.
// Fall back to "target the root, Inside, at the end of its children" so dropping onto an
// empty list still calls the root's CanDrop/Drop -- the WPF SharpTreeView did the same.
if (e.Source is not Visual hit
|| hit.FindAncestorOfType<SharpTreeViewItem>(includeSelf: true) is not { Node: { } node } item)
return null;
return ResolveEmptySpaceDropTarget();
double h = item.Bounds.Height;
double y = e.GetPosition(item).Y;
DropPlace place = y < h * 0.25 ? DropPlace.Before : y > h * 0.75 ? DropPlace.After : DropPlace.Inside;
@ -572,6 +583,13 @@ namespace ILSpy.Controls.TreeView @@ -572,6 +583,13 @@ namespace ILSpy.Controls.TreeView
}
}
internal DropTarget? ResolveEmptySpaceDropTarget()
{
if (Root is not { } root)
return null;
return new DropTarget(root, root.Children.Count, DropPlace.Inside, Item: null);
}
IPlatformDataObject BuildPlatformData(DragEventArgs e)
{
// Internal drag: the node-built payload from Copy. External: pack the dropped file paths.
@ -591,9 +609,11 @@ namespace ILSpy.Controls.TreeView @@ -591,9 +609,11 @@ namespace ILSpy.Controls.TreeView
return data;
}
void ShowInsertMarker(SharpTreeViewItem item, DropPlace place)
void ShowInsertMarker(SharpTreeViewItem? item, DropPlace place)
{
if (place == DropPlace.Inside || AdornerLayer.GetAdornerLayer(this) is not { } layer)
// item is only null for the empty-space fallback target, whose place is always
// DropPlace.Inside -- the early return below covers that and the marker stays hidden.
if (place == DropPlace.Inside || item is null || AdornerLayer.GetAdornerLayer(this) is not { } layer)
{
HideInsertMarker();
return;

Loading…
Cancel
Save