feat(TreeView): support Darg and Drop function #6121
+607
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your PR, @syminomega. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideThis PR adds full drag-and-drop reordering support to the TreeView component by introducing draggable parameters and callbacks, rendering drop zones with preview indicators, and updating the component logic to handle item reordering on drop. Sequence Diagram for TreeView Item Drag-and-DropsequenceDiagram
actor User
participant SourceRow as TreeViewRow (Source)
participant TargetRow as TreeViewRow (Target)
participant TreeViewComp as TreeView
User->>SourceRow: Initiates drag (ondragstart)
SourceRow->>SourceRow: DragStart(eventArgs)
SourceRow->>TreeViewComp: OnItemDragStart(SourceItem)
TreeViewComp->>TreeViewComp: OnItemDragStart(item): sets _draggingItem, _previewDrop, StateHasChanged()
User->>TargetRow: Drags over drop zone (ondragenter)
TargetRow->>TargetRow: DragEnterChildInside() / DragEnterChildBelow()
TargetRow->>TargetRow: Sets preview flags (e.g., _previewChildLast), Renders drop preview
User->>TargetRow: Drops item (ondrop)
TargetRow->>TargetRow: DropChildInside() / DropChildBelow()
TargetRow->>TreeViewComp: OnItemDrop(TreeDropEventArgs)
TreeViewComp->>TreeViewComp: OnItemDrop(args): sets args.Source
TreeViewComp->>TreeViewComp: Invokes user OnDrop(args) callback
alt User OnDrop returns true
TreeViewComp->>TreeViewComp: Reorders items (updates TreeViewItem parents/collections)
TreeViewComp->>TreeViewComp: Resets _draggingItem, _previewDrop, StateHasChanged()
end
User->>SourceRow: Drag operation ends (ondragend)
SourceRow->>SourceRow: DragEnd(eventArgs)
SourceRow->>TreeViewComp: OnItemDragEnd()
TreeViewComp->>TreeViewComp: OnItemDragEnd(): resets _previewDrop, _draggingItem, StateHasChanged()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6121 +/- ##
===========================================
- Coverage 100.00% 99.81% -0.19%
===========================================
Files 704 704
Lines 31093 31256 +163
Branches 4395 4414 +19
===========================================
+ Hits 31093 31198 +105
- Misses 0 45 +45
- Partials 0 13 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @syminomega - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// Drop action can be cancelled by returning false. | ||
| /// </summary> | ||
| [Parameter] | ||
| public Func<TreeDropEventArgs<TItem>, Task<bool>> OnDrop { get; set; } = _ => Task.FromResult(true); |
There was a problem hiding this comment.
suggestion (bug_risk): Default value for OnDrop may mask missing handler.
Consider making OnDrop a required parameter or log a warning when the default is used to help identify missing handlers.
Suggested implementation:
[Parameter]
public Func<TreeDropEventArgs<TItem>, Task<bool>> OnDrop { get; set; }
private bool _previewDrop; private async Task<bool> InvokeOnDropAsync(TreeDropEventArgs<TItem> args)
{
if (OnDrop != null)
{
return await OnDrop.Invoke(args);
}
else
{
Console.WriteLine("Warning: OnDrop handler is not set. Drop action will be ignored.");
return false;
}
}
private void OnItemDragEnd()You will need to replace any direct calls to OnDrop.Invoke(...) elsewhere in this file with await InvokeOnDropAsync(...) to ensure the warning and fallback behavior are applied consistently.
|
|
||
| _draggingItem = null; | ||
| _previewDrop = false; | ||
| _rows = GetTreeItems().ToFlat(); |
There was a problem hiding this comment.
suggestion (performance): Potential performance issue with full tree flattening after each drop.
Consider updating only the modified tree sections instead of flattening the entire tree after each drop to enhance performance, especially for large trees.
Suggested implementation:
_draggingItem = null;
_previewDrop = false;
UpdateRowsAfterDrop();
StateHasChanged(); /// <summary>
/// Updates only the affected rows in the _rows collection after a drop operation,
/// instead of flattening the entire tree for performance.
/// </summary>
private void UpdateRowsAfterDrop()
{
// TODO: Implement logic to update only the modified sections of _rows
// based on the drag-and-drop operation.
// This should update, insert, or remove only the affected items in _rows.
// For now, this is a placeholder to be implemented according to your tree structure.
}
#region DraggableYou will need to implement the logic inside UpdateRowsAfterDrop() to update only the relevant parts of _rows based on your tree structure and the specifics of your drag-and-drop operation. This may involve:
- Identifying which nodes were moved or changed.
- Updating, inserting, or removing only those nodes in
_rows. - Ensuring the UI reflects only the necessary changes for optimal performance.
If you have helper methods or data structures that track the affected nodes, use them here.
Comment on lines
+338
to
+339
| await ToggleNodeAsync(); | ||
| StateHasChanged(); |
There was a problem hiding this comment.
suggestion (performance): Calling StateHasChanged inside DragStart may cause unnecessary re-renders.
Consider removing or batching this StateHasChanged call if the UI is already updated by the drag operation to avoid redundant renders.
Suggested change
| await ToggleNodeAsync(); | |
| StateHasChanged(); | |
| await ToggleNodeAsync(); |
| OnItemDragEnd?.Invoke(); | ||
| } | ||
|
|
||
| private bool _previewChildLast; |
There was a problem hiding this comment.
issue (complexity): Consider replacing multiple boolean preview flags with a single enum and centralizing drop logic to simplify state management.
// 1. Introduce a single enum to represent all preview states:
private enum PreviewState
{
None,
InsideLast,
InsideFirst,
Below
}
private PreviewState _previewState;
// 2. Helper methods to set/clear the preview state:
private void SetPreview(PreviewState state) => _previewState = state;
private void ClearPreview() => _previewState = PreviewState.None;
// 3. Simplify DragEnter/Leave:
private void DragEnterChildInside(DragEventArgs e) => SetPreview(PreviewState.InsideLast);
private void DragLeaveChildInside(DragEventArgs e) => ClearPreview();
private void DragEnterChildBelow(DragEventArgs e)
{
var canBecomeFirst = (Item.HasChildren || Item.Items.Any()) && Item.IsExpand;
SetPreview(canBecomeFirst ? PreviewState.InsideFirst : PreviewState.Below);
}
private void DragLeaveChildBelow(DragEventArgs e) => ClearPreview();
// 4. Simplify Drop handlers into one method:
private async Task HandleDrop(DragEventArgs e)
{
if (OnItemDrop == null) return;
var args = new TreeDropEventArgs<TItem>
{
Target = Item,
ExpandAfterDrop = _expandAfterDrop,
DropType = _previewState switch
{
PreviewState.InsideLast => TreeDropType.AsLastChild,
PreviewState.InsideFirst => TreeDropType.AsFirstChild,
PreviewState.Below => TreeDropType.AsSiblingBelow,
_ => throw new InvalidOperationException()
}
};
ClearPreview();
await OnItemDrop.Invoke(args);
}
// 5. Wire up DropChildInside/DropChildBelow to the unified handler:
private Task DropChildInside(DragEventArgs e) => HandleDrop(e);
private Task DropChildBelow(DragEventArgs e) => HandleDrop(e);This replaces the three booleans with a single _previewState enum, centralizes preview‐state toggling in SetPreview/ClearPreview, and merges the drop logic into one HandleDrop method. All existing functionality is preserved.
| StateHasChanged(); | ||
| } | ||
|
|
||
| private async Task OnItemDrop(TreeDropEventArgs<TItem> e) |
There was a problem hiding this comment.
issue (complexity): Consider extracting the container/index computation and item insertion into helper methods to reduce nesting and duplication in OnItemDrop.
private async Task OnItemDrop(TreeDropEventArgs<TItem> e)
{
if (_draggingItem is null)
throw new InvalidOperationException("拖拽的项为空");
// … pre–checks, remove from old parent, set expand …
_draggingItem.Parent?.Items.Remove(_draggingItem);
_draggingItem.IsExpand = e.ExpandAfterDrop;
// compute target container, parent & insert index in one place
(TreeViewItem<TItem>? newParent, IList<TreeViewItem<TItem>> container, int idx) = e.DropType switch
{
TreeDropType.AsFirstChild => ( e.Target, e.Target.Items, 0 ),
TreeDropType.AsLastChild => ( e.Target, e.Target.Items, e.Target.Items.Count ),
TreeDropType.AsSiblingBelow => GetSiblingBelowInfo(e.Target),
_ => throw new ArgumentOutOfRangeException()
};
InsertDraggedItem(_draggingItem, newParent, container, idx);
_draggingItem = null;
_previewDrop = false;
_rows = GetTreeItems().ToFlat();
StateHasChanged();
}
private static (TreeViewItem<TItem>? parent, IList<TreeViewItem<TItem>> items, int index)
GetSiblingBelowInfo(TreeViewItem<TItem> target)
{
if (target.Parent is { } p)
{
var idx = p.Items.IndexOf(target) + 1;
return (p, p.Items, idx);
}
else
{
var idx = Items.IndexOf(target) + 1;
return (null, Items, idx);
}
}
private void InsertDraggedItem(
TreeViewItem<TItem> dragged,
TreeViewItem<TItem>? parent,
IList<TreeViewItem<TItem>> list,
int index)
{
if (index < 0 || index > list.Count)
list.Add(dragged);
else
list.Insert(index, dragged);
dragged.Parent = parent;
}Extracting the “compute container/index” logic and a single InsertDraggedItem helper:
- Removes the deep nesting and code‐duplication in each
case. - Consolidates index bounds checks.
- Centralizes parent assignment.
ArgoZhang
changed the title
ArgoZhang
approved these changes
Jun 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issues
#6216
New Feature
Add Draggable TreeView
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Implement drag-and-drop support for TreeView nodes with customizable drop behaviors and visual previews.
New Features:
Enhancements:
Documentation: