Skip to content

feat(Select): use DynamicElement ensure release event callback #7894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ArgoZhang merged 1 commit into from
Apr 19, 2026
+29 −5
Merged

feat(Select): use DynamicElement ensure release event callback #7894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/BootstrapBlazor/Components/Select/Select.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@namespace BootstrapBlazor.Components
@namespace BootstrapBlazor.Components
@using Microsoft.AspNetCore.Components.Web.Virtualization
@typeparam TValue
@inherits SimpleSelectBase<TValue>
Expand All @@ -22,7 +22,8 @@
}
else
{
<input type="text" id="@InputId" disabled="@Disabled" placeholder="@PlaceHolder" class="@InputClassString" value="@SelectedRow?.Text" @onchange="OnChange" readonly="@ReadonlyString" />
<input type="text" id="@InputId" disabled="@Disabled" placeholder="@PlaceHolder" class="@InputClassString"
value="@SelectedRow?.Text" @onchange="@_onChangeEventCallback" readonly="@ReadonlyString" />
}
<span class="@AppendClassString"><i class="@DropdownIcon"></i></span>
</div>
Expand Down Expand Up @@ -92,7 +93,7 @@

@code {
RenderFragment<SelectedItem> RenderRow => item =>
@<div class="@ActiveItem(item)" @onclick="() => OnClickItem(item)">
@<DynamicElement class="@ActiveItem(item)" OnClick="() => OnClickItem(item)">
@if (ItemTemplate != null)
{
@ItemTemplate(item)
Expand All @@ -105,7 +106,7 @@
{
@item.Text
}
</div>;
</DynamicElement>;

RenderFragment<PlaceholderContext> RenderPlaceHolderRow => context =>
@<div class="dropdown-item">
Expand Down
25 changes: 24 additions & 1 deletion src/BootstrapBlazor/Components/Select/Select.razor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ public bool IsUseActiveWhenValueIsNull

private string _defaultVirtualizedItemText = "";

private EventCallback<ChangeEventArgs> _onChangeEventCallback = EventCallback<ChangeEventArgs>.Empty;
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider removing the stateful EventCallback field and explicit StateHasChanged call by handling editability and rendering declaratively or within the handler to keep the component logic simpler and more self-contained.

You can simplify this change by removing the stateful EventCallback field and its disposal, and by tightening the click handler.

1. Remove _onChangeEventCallback field and lifecycle wiring

The field and OnParametersSet / DisposeAsync logic are only used to conditionally wire OnChange. You can achieve the same behavior declaratively in the .razor file without extra mutable state or lifecycle coupling.

Instead of:

// .razor.cs
private EventCallback<ChangeEventArgs> _onChangeEventCallback = EventCallback<ChangeEventArgs>.Empty;

protected override void OnParametersSet()
{
    base.OnParametersSet();

    PlaceHolder ??= Localizer[nameof(PlaceHolder)];
    NoSearchDataText ??= Localizer[nameof(NoSearchDataText)];
    DropdownIcon ??= IconTheme.GetIconByKey(ComponentIcons.SelectDropdownIcon);
    ClearIcon ??= IconTheme.GetIconByKey(ComponentIcons.SelectClearIcon);

    _onChangeEventCallback = IsEditable
        ? EventCallback.Factory.Create<ChangeEventArgs>(this, OnChange)
        : EventCallback<ChangeEventArgs>.Empty;
}

protected override ValueTask DisposeAsync(bool disposing)
{
    if (disposing)
    {
        _onChangeEventCallback = EventCallback<ChangeEventArgs>.Empty;
    }

    return base.DisposeAsync(disposing);
}

Bind directly in the .razor:

<input
    @onchange="(IsEditable
        ? EventCallback.Factory.Create<ChangeEventArgs>(this, OnChange)
        : EventCallback<ChangeEventArgs>.Empty)" />

Then you can delete:

private EventCallback<ChangeEventArgs> _onChangeEventCallback = EventCallback<ChangeEventArgs>.Empty;

and the DisposeAsync override entirely. The component instance is short‑lived; clearing a value-type callback in DisposeAsync brings no real benefit.

If you prefer to keep the binding simple, another option is to short‑circuit inside the handler:

private Task OnChange(ChangeEventArgs e)
{
    if (!IsEditable)
    {
        return Task.CompletedTask;
    }

    // existing logic...
}

and then use a straightforward binding:

<input @onchange="OnChange" />

Either pattern removes the extra field and lifecycle complexity while preserving behavior.

2. Reconsider explicit StateHasChanged in OnClickItem

OnClickItem currently forces a render:

private async Task OnClickItem(SelectedItem item)
{
    if (!item.IsDisabled)
    {
        _defaultVirtualizedItemText = item.Text;
        await SelectedItemChanged(item);
    }

    StateHasChanged();
}

If SelectedItemChanged flows into CurrentValue / parameter updates that already trigger a render (which is common in Blazor forms), the explicit StateHasChanged() is redundant and can be removed:

private async Task OnClickItem(SelectedItem item)
{
    if (!item.IsDisabled)
    {
        _defaultVirtualizedItemText = item.Text;
        await SelectedItemChanged(item);
    }
}

If you’ve hit a specific scenario where a manual refresh is required (e.g., async external state not bound to the component), consider adding a brief comment so future readers understand why it’s needed:

private async Task OnClickItem(SelectedItem item)
{
    if (!item.IsDisabled)
    {
        _defaultVirtualizedItemText = item.Text;
        await SelectedItemChanged(item);
    }

    // Explicit refresh required because <reason>, this is not triggered by normal binding.
    StateHasChanged();
}


private SelectedItem? SelectedItem { get; set; }

private SelectedItem? SelectedRow
Expand Down Expand Up @@ -234,6 +236,10 @@ protected override void OnParametersSet()
NoSearchDataText ??= Localizer[nameof(NoSearchDataText)];
DropdownIcon ??= IconTheme.GetIconByKey(ComponentIcons.SelectDropdownIcon);
ClearIcon ??= IconTheme.GetIconByKey(ComponentIcons.SelectClearIcon);

_onChangeEventCallback = IsEditable
? EventCallback.Factory.Create<ChangeEventArgs>(this, OnChange)
: EventCallback<ChangeEventArgs>.Empty;
}

/// <summary>
Expand Down Expand Up @@ -389,6 +395,8 @@ private async Task OnClickItem(SelectedItem item)
_defaultVirtualizedItemText = item.Text;
await SelectedItemChanged(item);
}

StateHasChanged();
}
Comment on lines 396 to 400
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OnClickItem now calls StateHasChanged(), but ConfirmSelectedItem also calls StateHasChanged() after awaiting OnClickItem. This means selecting via keyboard Enter (JS-invoked) will trigger two renders. Consider removing the extra StateHasChanged() from ConfirmSelectedItem so the selection path renders only once.

Copilot uses AI. Check for mistakes.

private async Task SelectedItemChanged(SelectedItem item)
Expand Down Expand Up @@ -494,11 +502,26 @@ private async Task OnChange(ChangeEventArgs args)
// 修复：使用完整的未过滤列表来查找当前选中项
// 避免在用户搜索时被外部 StateHasChanged 影响导致值被错误修改
var allItems = GetRowsByItems();

var item = GetItemWithEnumValue()
?? allItems.Find(i => i.Value == CurrentValueAsString)
?? allItems.Find(i => i.Active)
?? allItems.Find(i => !i.IsDisabled);
return item;
}

/// <summary>
/// <inheritdoc/>
/// </summary>
/// <param name="disposing"></param>
/// <returns></returns>
protected override ValueTask DisposeAsync(bool disposing)
{
if (disposing)
{
_onChangeEventCallback = EventCallback<ChangeEventArgs>.Empty;
}

return base.DisposeAsync(disposing);
}
}
Loading