feat(TreeView): add OnBeforeTreeItemClick parameter#7375
Conversation
Reviewer's GuideAdds a new OnBeforeTreeItemClick callback to TreeView that can asynchronously veto item click handling, wires it into the click pipeline, updates samples/docs metadata to use generic TreeViewItem types, and adds a unit test and tip entry to cover and document the new behavior. Class diagram for TreeView with OnBeforeTreeItemClickclassDiagram
class TreeView_TItem {
+Func~TreeViewItem_TItem, Task~?~~ OnTreeItemClick
+Func~TreeViewItem_TItem, Task_bool~~ OnBeforeTreeItemClick
+Func~TreeViewItem_TItem, Task~?~~ OnTreeItemChecked
+Func~TreeViewItem_TItem, Task~?~~ OnExpandNodeAsync
-TreeViewItem_TItem _activeItem
-bool ClickToggleNode
-bool ShowCheckbox
-bool ClickToggleCheck
-bool IsDisabled
-bool CanExpandWhenDisabled
+Task OnClick(TreeViewItem_TItem item)
+Task OnToggleNodeAsync(TreeViewItem_TItem item)
+Task OnCheckStateChanged(TreeViewItem_TItem item, CheckedState state)
+CheckedState ToggleCheckState(CheckedState current)
+void StateHasChanged()
}
class TreeViewItem_TItem {
+CheckedState CheckedState
+bool CanTriggerClickNode(bool isDisabled, bool canExpandWhenDisabled)
}
TreeView_TItem --> TreeViewItem_TItem : contains
class AttributeItem {
+string Name
+string Description
+string Type
+string ValueList
+string DefaultValue
}
class TreeViews_Sample {
+static AttributeItem[] GetAttributes()
}
TreeViews_Sample --> AttributeItem : uses
%% AttributeItem entries updated to use generic TreeViewItem_TItem types
class TreeView_string {
+Func~TreeViewItem_TItem, Task~?~~ OnTreeItemClick
+Func~TreeViewItem_TItem, Task_bool~~ OnBeforeTreeItemClick
+Func~TreeViewItem_TItem, Task~?~~ OnTreeItemChecked
+Func~TreeViewItem_TItem, Task~?~~ OnExpandNodeAsync
}
TreeView_string --|> TreeView_TItem
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You added a new
TreeViewsTipsOnBeforeTreeItemClickusage inTreeViews.razor, but there are no corresponding entries shown inen-US.jsonorzh-CN.json; make sure the new localization keys are added so the tip renders correctly in both languages.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You added a new `TreeViewsTipsOnBeforeTreeItemClick` usage in `TreeViews.razor`, but there are no corresponding entries shown in `en-US.json` or `zh-CN.json`; make sure the new localization keys are added so the tip renders correctly in both languages.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Components/TreeViewTest.cs:829-838` </location>
<code_context>
+ public async Task OnBeforeTreeItemClick_Ok()
+ {
+ var clicked = false;
+ var cut = Context.Render<TreeView<TreeFoo>>(pb =>
+ {
+ pb.Add(a => a.ClickToggleNode, true);
+ pb.Add(a => a.Items, TreeFoo.GetTreeItems());
+ pb.Add(a => a.OnTreeItemClick, node =>
+ {
+ clicked = true;
+ return Task.CompletedTask;
+ });
+ pb.Add(a => a.OnBeforeTreeItemClick, node =>
+ {
+ return Task.FromResult(false);
+ });
+ });
+ var node = cut.FindAll(".tree-node").Skip(1).First();
+ await cut.InvokeAsync(() => node.Click());
+ Assert.False(clicked);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the test to verify that `OnBeforeTreeItemClick` also blocks other click side effects (not just `OnTreeItemClick`).
The test currently only asserts that `OnTreeItemClick` is not invoked when `OnBeforeTreeItemClick` returns `false`. Since the implementation also blocks `_activeItem` updates, node toggling (`OnToggleNodeAsync`), and checkbox toggling (`OnCheckStateChanged`) when `confirm` is false, please extend this test (or add separate tests) to also verify that:
- The clicked node is not expanded/collapsed when `ClickToggleNode` is `true`.
- Checkbox state is unchanged when `ShowCheckbox` and `ClickToggleCheck` are enabled.
- The selected/active node does not change.
Suggested implementation:
```csharp
Assert.True(clicked);
}
[Fact]
public async Task OnBeforeTreeItemClick_Ok()
{
// arrange
var clicked = false;
var cut = Context.Render<TreeView<TreeFoo>>(pb =>
{
pb.Add(a => a.ClickToggleNode, true);
pb.Add(a => a.ShowCheckbox, true);
pb.Add(a => a.ClickToggleCheck, true);
pb.Add(a => a.Items, TreeFoo.GetTreeItems());
pb.Add(a => a.OnTreeItemClick, node =>
{
clicked = true;
return Task.CompletedTask;
});
pb.Add(a => a.OnBeforeTreeItemClick, node =>
{
// Block the click side-effects.
return Task.FromResult(false);
});
});
// capture initial state before click
var nodes = cut.FindAll(".tree-node");
var node = nodes.Skip(1).First();
var initialClasses = node.ClassList.ToArray();
var checkbox = node.QuerySelector("input[type=checkbox]") as AngleSharp.Dom.IElement;
var initialChecked = checkbox?.HasAttribute("checked") ?? false;
AngleSharp.Dom.IElement? activeNodeBefore = null;
string? activeNodeIdBefore = null;
try
{
activeNodeBefore = cut.Find(".tree-node.active");
activeNodeIdBefore = activeNodeBefore.GetAttribute("data-id");
}
catch
{
// No active node before click – keep both "before" and "after"
// values null so the assertion still verifies they are equal.
}
// act
await cut.InvokeAsync(() => node.Click());
// assert
// 1. OnTreeItemClick is not invoked
Assert.False(clicked);
// 2. Node expand/collapse state is unchanged
nodes = cut.FindAll(".tree-node");
node = nodes.Skip(1).First();
Assert.Equal(initialClasses, node.ClassList);
// 3. Checkbox state is unchanged
checkbox = node.QuerySelector("input[type=checkbox]") as AngleSharp.Dom.IElement;
var afterChecked = checkbox?.HasAttribute("checked") ?? false;
Assert.Equal(initialChecked, afterChecked);
// 4. Active/selected node is unchanged
AngleSharp.Dom.IElement? activeNodeAfter = null;
string? activeNodeIdAfter = null;
try
{
activeNodeAfter = cut.Find(".tree-node.active");
activeNodeIdAfter = activeNodeAfter.GetAttribute("data-id");
}
catch
{
// Still no active node after click
}
Assert.Equal(activeNodeIdBefore, activeNodeIdAfter);
}
```
1. Ensure `using AngleSharp.Dom;` is present at the top of `TreeViewTest.cs` so that `AngleSharp.Dom.IElement` and `ClassList` compile.
2. If your DOM structure or CSS classes differ (for example, if the active/selected node uses a different class than `.tree-node.active`, or the checkbox uses a different selector than `input[type=checkbox]`), adjust:
- The selector used to locate the checkbox.
- The selector used to locate the active node.
- The way expansion/collapse state is asserted (you may prefer to assert on specific classes like `"is-expanded"` / `"is-collapsed"` instead of comparing the full `ClassList` arrays).
3. If the TreeView component exposes a more direct way to inspect the active or expanded node (e.g., via attributes like `data-expanded` / `data-active-id`), consider replacing the `ClassList` and `data-id` checks with those more explicit indicators to make the test more robust.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7375 +/- ##
========================================
Coverage ? 100.00%
========================================
Files ? 748
Lines ? 32766
Branches ? 4545
========================================
Hits ? 32766
Misses ? 0
Partials ? 0
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:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new OnBeforeTreeItemClick parameter to the TreeView component that allows consumers to intercept and potentially prevent node click actions. When this callback returns false, the click action and all subsequent operations (expand/collapse, OnTreeItemClick, checkbox toggle) are cancelled.
Key Changes:
- Added
OnBeforeTreeItemClickcallback parameter that returns a boolean to control whether the click proceeds - Modified the
OnClickmethod to check the callback result before executing node click logic - Added comprehensive test coverage and documentation in both English and Chinese
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/TreeView/TreeView.razor.cs | Added OnBeforeTreeItemClick parameter and integrated callback check into OnClick method logic; includes minor formatting fix for spacing |
| test/UnitTest/Components/TreeViewTest.cs | Added unit test to verify OnBeforeTreeItemClick prevents node click when returning false |
| src/BootstrapBlazor.Server/Components/Samples/TreeViews.razor.cs | Updated attribute documentation to include OnBeforeTreeItemClick and corrected generic type parameters for consistency |
| src/BootstrapBlazor.Server/Locales/zh-CN.json | Added Chinese documentation explaining the new OnBeforeTreeItemClick callback functionality |
| src/BootstrapBlazor.Server/Locales/en-US.json | Added English documentation explaining the new OnBeforeTreeItemClick callback functionality |
| src/BootstrapBlazor.Server/Components/Samples/TreeViews.razor | Added display of the new OnBeforeTreeItemClick documentation in the tips section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| new() | ||
| { | ||
| Name = nameof(TreeView<string>.OnBeforeTreeItemClick), | ||
| Description = "点击节点前回调方法", |
There was a problem hiding this comment.
The Description field contains Chinese text while other attributes in this file use English descriptions. This is inconsistent with the rest of the file where descriptions are in English. The Description should be translated to English to maintain consistency.
| Description = "点击节点前回调方法", | |
| Description = "Callback delegate before tree control node is clicked", |
| { | ||
| _activeItem = item; | ||
| if (ClickToggleNode && item.CanTriggerClickNode(IsDisabled, CanExpandWhenDisabled)) | ||
| var confirm = true; |
There was a problem hiding this comment.
The variable name 'confirm' is misleading in this context. Since the callback is named 'OnBeforeTreeItemClick' and returns a boolean to indicate whether the click should proceed, a more descriptive name would be 'shouldProceed' or 'canProceed' to better convey the intent that this controls whether the click action continues.
| "TreeViewsTips6": "Set whether the node is <b>expanded</b> state through <code>TreeViewItem<TItem>.IsExpand</code>", | ||
| "TreeViewsTips7": "Set whether the node is <b>selected</b> state through <code>TreeViewItem<TItem>.IsActive</code>", | ||
| "TreeViewsTips8": "Through <code>TreeViewItem<TItem>.Checked</code>, set whether the node is in <b>checked/single selection</b> state", | ||
| "TreeViewsTipsOnBeforeTreeItemClick": "You can prevent a node click by setting the <code>OnBeforeTreeItemClick</code> callback method, and cancel the click action when it returns <code>false</code>.", |
There was a problem hiding this comment.
The English translation is somewhat redundant with "prevent" and "cancel" conveying the same meaning. The sentence can be simplified to be more concise and clear.
| "TreeViewsTipsOnBeforeTreeItemClick": "You can prevent a node click by setting the <code>OnBeforeTreeItemClick</code> callback method, and cancel the click action when it returns <code>false</code>.", | |
| "TreeViewsTipsOnBeforeTreeItemClick": "Use the <code>OnBeforeTreeItemClick</code> callback to cancel a node click by returning <code>false</code>.", |
Link issues
fixes #7374
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a pre-click callback to TreeView items and update samples and tests to support the new behavior.
New Features:
Enhancements:
Tests: