Skip to content

feat(Table): improve performance#7913

Merged
ArgoZhang merged 38 commits intomainfrom
fix-table-mmo
Apr 25, 2026
Merged

feat(Table): improve performance#7913
ArgoZhang merged 38 commits intomainfrom
fix-table-mmo

Conversation

@ArgoZhang
Copy link
Copy Markdown
Member

@ArgoZhang ArgoZhang commented Apr 25, 2026

Link issues

fixes #7912

Summary By Copilot

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Merge the latest code from the main branch

Summary by Sourcery

Optimize dynamic DataTable handling and dynamic object editing to improve table performance and consistency.

Bug Fixes:

  • Fix deletion and editing behavior for DataTable-backed tables by keeping DataRow and dynamic object state in sync, avoiding operations on deleted/detached rows.

Enhancements:

  • Refactor DataTableDynamicContext to use a direct DataRow-backed dynamic object model instead of runtime-Emitted types, simplifying column construction and caching.
  • Introduce IDynamicObject editing lifecycle (Get/Set, Accept, Cancel) and integrate it into table editing flows, including DataTable-backed tables.
  • Adjust validation and binding utilities to use dynamic-object-specific value change callbacks, reducing reflection and cache invalidations.
  • Enhance dynamic column metadata handling for DataTable contexts, including display names, required attributes, and visibility/ignore flags.
  • Improve InternalTableColumn display name fallback behavior and sample initialization for dynamic tables.

Copilot AI review requested due to automatic review settings April 25, 2026 02:17
@bb-auto bb-auto Bot added the enhancement New feature or request label Apr 25, 2026
@bb-auto bb-auto Bot added this to the v10.5.0 milestone Apr 25, 2026
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Apr 25, 2026

Reviewer's Guide

Refactors the DataTable-based dynamic table pipeline to avoid runtime type emission, operate directly on DataRow-backed dynamic objects, and streamline value propagation and editing workflows for better performance and consistency.

Sequence diagram for dynamic DataTable cell edit flow

sequenceDiagram
    actor User
    participant Table as Table
    participant Validate as ValidateBase_T
    participant Utility as Utility
    participant DynObj as DataTableDynamicObject
    participant Context as DataTableDynamicContext
    participant Row as DataRow

    User->>Table: Edit cell in dynamic DataTable row
    Table->>Validate: Render editor with model as IDynamicObject
    Note over Validate,Utility: For IDynamicObject model, use OnValueChanged instead of ValueChanged
    Validate->>Utility: GenerateOnValueChanged(model, fieldName, fieldType)
    Utility-->>Validate: Func newValue => DynObj.SetValue(fieldName, newValue)

    User->>Validate: Change field value
    Validate->>Validate: Validate and update Value
    Validate->>DynObj: OnValueChanged(newValue)
    DynObj->>Row: SetValue(propertyName, newValue)

    Note over DynObj,Row: DataRow updated directly, no emitted runtime type

    User->>Table: Save edit in dialog
    Table->>DynObj: Accept()
    DynObj->>Row: Table.AcceptChanges()

    User->>Table: Cancel edit in dialog (without save)
    Table->>DynObj: Cancel()
    DynObj->>Row: Table.RejectChanges()
Loading

Sequence diagram for dynamic DataTable add and delete operations

sequenceDiagram
    actor User
    participant Table as Table
    participant Context as DataTableDynamicContext
    participant DynObj as DataTableDynamicObject
    participant DT as DataTable
    participant Row as DataRow

    User->>Table: Click Add
    Table->>Context: AddAsync(SelectedRows as IDynamicObject)
    alt OnAddAsync provided
        Context->>Context: OnAddAsync callback
    else default add
        Context->>DT: NewRow()
        DT-->>Context: Row
        Context->>DT: Rows.InsertAt(row, index)
        Context->>DT: AcceptChanges()
        Context->>DynObj: new DataTableDynamicObject(row)
        Context->>DynObj: set DynamicObjectPrimaryKey
        Context->>Context: _dataCache.Add(primaryKey, DynObj)
        Context->>Table: OnChanged(DynamicObjectContextArgs with DynObj)
        Table->>Table: QueryAsync(false) to refresh
    end

    User->>Table: Click Delete
    Table->>Context: DeleteAsync(SelectedRows as IDynamicObject)
    alt OnDeleteAsync provided
        Context->>Context: OnDeleteAsync(items)
        Context->>Context: _items = null
    else default delete
        loop for each item
            Context->>Context: _dataCache.TryRemove(primaryKey)
            Context->>DT: Rows.Remove(row)
            opt _items not null
                Context->>Context: remove from _items by primaryKey
            end
        end
        Context->>DT: AcceptChanges()
        Context->>Table: OnChanged(DynamicObjectContextArgs with deleted items)
    end
    Table->>Table: TriggerDeleteCallback()
    Table->>Table: QueryAsync(SelectedRowsChanged.HasDelegate)
    Table->>Table: SelectedRows.Clear()
Loading

Class diagram for refactored dynamic DataTable pipeline

classDiagram
    direction LR

    class IDynamicObject {
        <<interface>>
        Guid DynamicObjectPrimaryKey
        +object GetValue(string propertyName)
        +void SetValue(string propertyName, object value)
        +void Cancel()
        +void Accept()
    }

    class DynamicObject {
        +Guid DynamicObjectPrimaryKey
        +DynamicObject()
        +virtual object GetValue(string propertyName)
        +virtual void SetValue(string propertyName, object value)
        +virtual void Cancel()
        +virtual void Accept()
    }

    class DataTableDynamicObject {
        +DataTableDynamicObject(DataRow row)
        +DataRow Row
        +override object GetValue(string propertyName)
        +override void SetValue(string propertyName, object value)
        +override void Cancel()
        +override void Accept()
    }

    class DynamicColumnsObject {
        +DynamicColumnsObject(Dictionary~string, object~ columnsData)
        +DynamicColumnsObject()
        +Dictionary~string, object~ Columns
        +virtual object GetValue(string propertyName)
        +virtual void SetValue(string propertyName, object value)
        +virtual void Cancel()
        +virtual void Accept()
    }

    class DataTableDynamicContext {
        +DataTable DataTable
        +bool UseCache
        +Func~IEnumerable~IDynamicObject~~, Task~bool~~ OnDeleteAsync
        -ConcurrentDictionary~Guid, DataTableDynamicObject~ _dataCache
        -List~ITableColumn~ _columns
        -Action~DataTableDynamicContext, ITableColumn~ _addAttributesCallback
        -List~IDynamicObject~ _items
        +DataTableDynamicContext(DataTable table, Action~DataTableDynamicContext, ITableColumn~ addAttributesCallback, IEnumerable~string~ invisibleColumns, IEnumerable~string~ shownColumns, IEnumerable~string~ hiddenColumns)
        +override IEnumerable~IDynamicObject~ GetItems()
        +override IEnumerable~ITableColumn~ GetColumns()
        -List~IDynamicObject~ BuildItems()
        -void BuildTableColumns(IEnumerable~string~ invisibleColumns, IEnumerable~string~ shownColumns, IEnumerable~string~ hiddenColumns)
        -static void GetShownColumns(ITableColumn col, IEnumerable~string~ invisibleColumns, IEnumerable~string~ shownColumns, IEnumerable~string~ hiddenColumns)
        +internal void ApplyColumnAttribute~TAttribute~(string columnName, IEnumerable~KeyValuePair~string, object~~ parameters)
        +internal void ApplyDisplayName(string columnName, string displayName)
        +override Task AddAsync(IEnumerable~IDynamicObject~ selectedItems)
        +override Task~bool~ DeleteAsync(IEnumerable~IDynamicObject~ items)
        -Task OnCellValueChanged(IDynamicObject item, ITableColumn column, object val)
    }

    class InternalTableColumn {
        +string FieldName
        +Type FieldType
        +string Text
        +bool Visible
        +bool Ignore
        +InternalTableColumn(string fieldName, Type fieldType, string fieldText)
        +string GetDisplayName()
    }

    class CacheManager {
        <<static>>
        +static TResult GetPropertyValue~TModel, TResult~(TModel model, string fieldName)
        +static void SetPropertyValue~TModel, TValue~(TModel model, string fieldName, TValue value)
    }

    class Utility {
        <<static>>
        +static object GenerateValue(object model, string fieldName)
        +static object GenerateValueChanged(ComponentBase component, object model, string fieldName, Type fieldType)
        +static object GenerateOnValueChanged(object model, string fieldName, Type fieldType)
        +static EventCallback~TType~ CreateCallback~TType~(ComponentBase component, object model, string fieldName)
        -static Expression~Func~ComponentBase, object, string, object~~ CreateValueChangedLambda(Type fieldType)
        -static Expression~Func~object, string, object~~ CreateDynamicObjectOnValueChangedLambda(Type fieldType)
        -static Func~TValue, Task~ CreateDynamicObjectOnValueChangedCallback~TValue~(object model, string fieldName)
    }

    class DataRowExtensions {
        <<static>>
        +static bool IsDeletedOrDetached(this DataRow row)
    }

    class ValidateBase_T~T~ {
        +T Value
        +EventCallback~T~ ValueChanged
        +Expression ValueExpression
        +Func~T, Task~ OnValueChanged
        +bool ShowRequired
        +string RequiredErrorMessage
        +bool SkipValidate
    }

    class Table_TItem~TItem~ {
        +DynamicObjectContext DynamicContext
        +IEnumerable~TItem~ SelectedRows
        +object EditModel
        +Task AddAsync()
        +Task DeleteAsync()
        +Task ShowEditDialog(ItemChangedType changedType)
        -Task OnCloseEditDialogCallbackAsync(bool saved)
        +Task QueryAsync(bool searchInPage)
    }

    IDynamicObject <|.. DynamicObject
    DynamicObject <|-- DataTableDynamicObject
    DynamicObject <|-- DynamicColumnsObject

    DataTableDynamicContext --> DataTableDynamicObject : uses
    DataTableDynamicContext --> InternalTableColumn : builds
    DataTableDynamicContext --> DataRow : backs_items
    DataTableDynamicObject --> DataRow : wraps
    DataRowExtensions ..> DataRow : extends

    CacheManager ..> IDynamicObject : uses GetValue SetValue
    Utility ..> CacheManager : uses
    Utility ..> IDynamicObject : GenerateOnValueChanged

    Table_TItem --> DynamicObjectContext : uses
    Table_TItem --> IDynamicObject : SelectedRows, EditModel
    ValidateBase_T --> IDynamicObject : model when dynamic
    ValidateBase_T ..> Utility : GenerateOnValueChanged, GenerateValueChanged
Loading

File-Level Changes

Change Details Files
Refactor DataTableDynamicContext to operate on DataRow-backed DataTableDynamicObject instances, simplify column generation, and adjust caching and lifecycle behavior.
  • Make DataTable a get-only property and replace DynamicObjectType/Columns with an in-memory _columns list and _items cache.
  • Replace the original ConcurrentDictionary mapping to (dynamicObject, DataRow) with one mapping to DataTableDynamicObject and update GetItems/BuildItems to construct DataTableDynamicObject directly from DataRow while skipping deleted/detached rows.
  • Replace dynamic type emission and Utility.GetTableColumns with BuildTableColumns and GetShownColumns helpers that configure InternalTableColumn instances, respect visibility/ignore flags, and invoke the column-attribute callback.
  • Update AddAsync/DeleteAsync/OnCellValueChanged to work with DataTableDynamicObject and the new cache, reset _items instead of rebuilding inline, and ensure DataTable.AcceptChanges is called appropriately.
src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs
Rework DataTableDynamicObject and IDynamicObject implementations to manipulate DataRow directly, handle null/DBNull safely, and support cancel/accept semantics used by the table editing flow.
  • Change DataTableDynamicObject to be constructed with a DataRow, expose Row as a read-only property, and implement GetValue/SetValue based on the underlying DataRow with DBNull and deleted/detached row handling.
  • Add Cancel and Accept overrides in DataTableDynamicObject to call RejectChanges/AcceptChanges on the underlying DataTable.
  • Extend IDynamicObject with Cancel and Accept methods and provide default no-op implementations in DynamicObject and DynamicColumnsObject, updating CacheManager to use IDynamicObject for dynamic get/set routing instead of IDynamicColumnsObject.
src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs
src/BootstrapBlazor/Dynamic/IDynamicObject.cs
src/BootstrapBlazor/Dynamic/DynamicObject.cs
src/BootstrapBlazor/Dynamic/DynamicColumnsObject.cs
src/BootstrapBlazor/Services/CacheManager.cs
Optimize component generation and value-change handling for dynamic table fields, separating dynamic-object handling from standard models and introducing an OnValueChanged pipeline.
  • Adjust Utility.CreateComponentByFieldType to use GenerateOnValueChanged for IDynamicObject models while keeping existing Value/ValueChanged/ValueExpression/validation wiring for regular models, and move SkipValidate wiring into the non-dynamic branch.
  • Rename and slightly refactor GenerateValueChanged internals (CreateValueChangedLambda) for clarity and add GenerateOnValueChanged plus dynamic callback helpers that call IDynamicObject.SetValue asynchronously.
  • Ensure ValidateBase attributes like ShowRequired/RequiredErrorMessage/SkipValidate are only applied in the non-dynamic branch to avoid unnecessary validation overhead for dynamic objects.
src/BootstrapBlazor/Utils/Utility.cs
Wire dynamic context column attribute and display name APIs to the new DataTableDynamicContext internals for better column customization without reflection-heavy metadata flow.
  • Add ApplyColumnAttribute and ApplyDisplayName hooks to DataTableDynamicContext (currently skeleton/commented logic for future extension) and wire them into DynamicObjectContextExtensions.AddMultipleParameterAttribute/AddDisplayNameAttribute/AddDescriptionAttribute when the context is a DataTableDynamicContext.
  • Update InternalTableColumn.GetDisplayName to fall back to FieldName when Text is null to ensure a non-empty display name for generated columns.
src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs
src/BootstrapBlazor/Extensions/DynamicObjectContextExtensions.cs
src/BootstrapBlazor/Components/Table/InternalTableColumn.cs
Align Table editing workflow with the new dynamic object semantics, ensuring changes can be committed or rolled back on cancel without rebuilding the dynamic context.
  • Change Table toolbar AddAsync/DeleteAsync to call DynamicContext.AddAsync/DeleteAsync and then re-query data via QueryAsync instead of resetting the dynamic context, and adjust SelectedRows clearing timing.
  • In ShowEditDialog, call Accept on IDynamicObject models after a successful save and set triggerFromSave based on the save result; on dialog close, call Cancel on IDynamicObject models to roll back pending changes instead of relying only on the data service.
  • Minor cleanup such as clarifying comments and ensuring EF data service lookup uses a local variable, plus a small initialization ordering tweak in the TablesDynamic sample.
src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs
src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamic.razor.cs
Introduce a reusable helper for DataRow state checks to simplify deleted/detached row handling.
  • Add a DataRowExtensions helper providing an IsDeletedOrDetached convenience method and use it when building dynamic items and in DataTableDynamicObject value accessors.
src/BootstrapBlazor/Extensions/DataRowExtensions.cs
src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs
src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs

Assessment against linked issues

Issue Objective Addressed Explanation
#7912 Improve performance of the Table component, especially for DataTable-based dynamic tables (DataTableDynamicContext, DataTableDynamicObject, and related utility paths).
#7912 Preserve or improve correctness of core Table operations (rendering, value change handling, add/delete/edit workflows) while applying the performance improvements.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 7 issues, and left some high level feedback:

  • The new DataRowExtensions class is not valid C# (the extension(DataRow row) construct won’t compile); this should be implemented as a standard extension method, e.g. public static bool IsDeletedOrDetached(this DataRow row) inside the static class.
  • In DynamicObjectContextExtensions, AddDescriptionAttribute calls tableContext.ApplyDisplayName(columnName, description) instead of a description-specific method, which looks unintentional and will overwrite the display name rather than storing a description.
  • In Utility.CreateComponentByFieldType, when the model is an IDynamicObject you no longer set ShowRequired, RequiredErrorMessage, or SkipValidate on ValidateBase; confirm this behavioral change is intentional, as dynamic-object fields will now skip those validation-related attributes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new DataRowExtensions class is not valid C# (the `extension(DataRow row)` construct won’t compile); this should be implemented as a standard extension method, e.g. `public static bool IsDeletedOrDetached(this DataRow row)` inside the static class.
- In DynamicObjectContextExtensions, `AddDescriptionAttribute` calls `tableContext.ApplyDisplayName(columnName, description)` instead of a description-specific method, which looks unintentional and will overwrite the display name rather than storing a description.
- In Utility.CreateComponentByFieldType, when the model is an IDynamicObject you no longer set ShowRequired, RequiredErrorMessage, or SkipValidate on ValidateBase; confirm this behavioral change is intentional, as dynamic-object fields will now skip those validation-related attributes.

## Individual Comments

### Comment 1
<location path="src/BootstrapBlazor/Extensions/DataRowExtensions.cs" line_range="10-12" />
<code_context>
+
+namespace BootstrapBlazor.Components;
+
+internal static class DataRowExtensions
+{
+    extension(DataRow row)
+    {
+        public bool IsDeletedOrDetached() => row.RowState == DataRowState.Deleted || row.RowState == DataRowState.Detached;
</code_context>
<issue_to_address>
**issue (bug_risk):** DataRowExtensions is not valid C# syntax for an extension method and will not compile

`extension(DataRow row)` is invalid C# and prevents this class from compiling. For an extension method, declare a static class and a static method with a `this` parameter, e.g.:

```csharp
internal static class DataRowExtensions
{
    public static bool IsDeletedOrDetached(this DataRow row) =>
        row.RowState == DataRowState.Deleted || row.RowState == DataRowState.Detached;
}
```

As written, all calls to `row.IsDeletedOrDetached()` in `DataTableDynamicContext` and `DataTableDynamicObject` will not compile.
</issue_to_address>

### Comment 2
<location path="src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs" line_range="78-87" />
<code_context>
+    /// <para lang="zh">撤销数据更改</para>
+    /// <para lang="en">Cancels data changes</para>
+    /// </summary>
+    public override void Cancel()
+    {
+        row.Table.RejectChanges();
+    }
+
+    /// <summary>
+    /// <para lang="zh">接受数据更改</para>
+    /// <para lang="en">Accepts data changes</para>
+    /// </summary>
+    public override void Accept()
+    {
+        row.Table.AcceptChanges();
+    }
 }
</code_context>
<issue_to_address>
**issue (bug_risk):** Cancel/Accept operate on the entire DataTable instead of the current row

Both methods call `row.Table.RejectChanges()` / `row.Table.AcceptChanges()`, which applies to every row in the `DataTable`, not just the current row. For a per-row editing context, these should operate on `row` itself:

```csharp
public override void Cancel() => row.RejectChanges();
public override void Accept() => row.AcceptChanges();
```

Otherwise, cancelling/accepting one item will also commit or roll back changes on other rows in the table.
</issue_to_address>

### Comment 3
<location path="src/BootstrapBlazor/Utils/Utility.cs" line_range="740-742" />
<code_context>
-            builder.AddAttribute(41, nameof(ValidateBase<>.ShowRequired), GetRequired(item, changedType));
-            builder.AddAttribute(42, nameof(ValidateBase<>.RequiredErrorMessage), item.RequiredErrorMessage);
+
+            if (model is IDynamicObject d)
+            {
+                var fieldOnValueChanged = GenerateOnValueChanged(model, fieldName, fieldType);
+                builder.AddAttribute(21, nameof(ValidateBase<>.OnValueChanged), fieldOnValueChanged);
+            }
</code_context>
<issue_to_address>
**question (bug_risk):** Dynamic-object branch no longer wires ValueChanged/ValueExpression/validation attributes

In the `IDynamicObject` path we only set `OnValueChanged`, so `ValueChanged`, `ValueExpression`, `ShowRequired` / `RequiredErrorMessage`, and `SkipValidate` are no longer wired, unlike the non-dynamic path. This may change validation and notification behavior for dynamic rows (e.g., no validation or missing UI updates).

If the intent is just to change the update mechanism, please ensure the dynamic branch still configures `ValueExpression`, `ShowRequired`, `RequiredErrorMessage`, and `SkipValidate`, and verify whether `ValueChanged` must also be set alongside `OnValueChanged`.
</issue_to_address>

### Comment 4
<location path="src/BootstrapBlazor/Extensions/DynamicObjectContextExtensions.cs" line_range="164-168" />
<code_context>
     ///   <para lang="en">Description</para>
     /// </param>
-    public static void AddDescriptionAttribute(this DynamicObjectContext context, string columnName, string description) => context.AddAttribute<DescriptionAttribute>(columnName, [typeof(string)], [description]);
+    public static void AddDescriptionAttribute(this DynamicObjectContext context, string columnName, string description)
+    {
+        if (context is DataTableDynamicContext tableContext)
+        {
+            tableContext.ApplyDisplayName(columnName, description);
+        }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** AddDescriptionAttribute uses ApplyDisplayName, which looks like a copy-paste mistake

`AddDescriptionAttribute` currently routes to `tableContext.ApplyDisplayName(columnName, description)`, which updates the display name, not a description field. To align with the method name and XML docs, this should use a description-specific API (e.g., `ApplyDescription`) rather than repurposing `ApplyDisplayName`, otherwise calling `AddDescriptionAttribute` will overwrite the column’s display text instead of its description.
</issue_to_address>

### Comment 5
<location path="src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs" line_range="200-204" />
<code_context>
+        //}
+    }
+
+    internal void ApplyDisplayName(string columnName, string displayName)
+    {
+        //if (InternalColumns.TryGetValue(columnName, out var column))
+        //{
+        //    column.Text = displayName;
+        //}
     }
</code_context>
<issue_to_address>
**issue (bug_risk):** ApplyDisplayName and ApplyColumnAttribute are effectively no-ops, but are now used by the extensions

Because these methods are effectively no-ops, any calls from `DynamicObjectContextExtensions` (e.g., `AddDisplayNameAttribute`, `AddRequiredAttribute`) won’t update the in-memory `InternalTableColumn` state for `DataTableDynamicContext`. That’s a behavior change from when attributes lived on the dynamic type and is likely a regression. If the goal is to move attribute handling into `InternalTableColumn`, these method bodies should be implemented rather than commented out so the column configuration is actually updated.
</issue_to_address>

### Comment 6
<location path="src/BootstrapBlazor/Services/CacheManager.cs" line_range="547-548" />
<code_context>
         return propertyInfo != null;
     }

-    public static TResult GetPropertyValue<TModel, TResult>(TModel model, string fieldName) => (model is IDynamicColumnsObject d)
+    public static TResult GetPropertyValue<TModel, TResult>(TModel model, string fieldName) => (model is IDynamicObject d)
         ? (TResult)d.GetValue(fieldName)!
         : GetValue<TModel, TResult>(model, fieldName);

</code_context>
<issue_to_address>
**question (bug_risk):** Changing GetPropertyValue to use IDynamicObject may alter behavior for non-column-backed properties

`GetPropertyValue` used to special-case `IDynamicColumnsObject`, but now applies to all `IDynamicObject`. Combined with `DynamicObject.GetValue` now returning `null` by default, any `IDynamicObject` that still relies on regular CLR properties (without overriding `GetValue` accordingly) may now resolve those properties differently. Consider either keeping the special case to `IDynamicColumnsObject` or verifying that all `IDynamicObject` implementations override `GetValue` to maintain the previous behavior.
</issue_to_address>

### Comment 7
<location path="src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs" line_range="157" />
<code_context>
+        }
+    }
+
+    internal void ApplyColumnAttribute<TAttribute>(string columnName, IEnumerable<KeyValuePair<string, object?>> parameters) where TAttribute : Attribute
+    {
+        //if (!InternalColumns.TryGetValue(columnName, out var column))
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing or simplifying the partially implemented column-configuration helpers so that column setup logic is explicit, local, and free of dead code.

The biggest complexity increase here comes from the partially‑implemented / dead abstractions around column configuration. You can cut that down without losing any functionality.

### 1. Remove or fully implement no‑op attribute APIs

`ApplyColumnAttribute<TAttribute>` and `ApplyDisplayName` are currently public-ish entry points with only large commented‑out bodies. They read as “important” but actually do nothing, which adds cognitive overhead.

If you don’t need them yet, delete them and re‑introduce later from git history:

```csharp
// REMOVE these until you actually wire them through:

// internal void ApplyColumnAttribute<TAttribute>(string columnName, IEnumerable<KeyValuePair<string, object?>> parameters) where TAttribute : Attribute
// { ... }

// internal void ApplyDisplayName(string columnName, string displayName)
// { ... }
```

If you *do* need them as part of the contract, make their behavior explicit and minimal instead of commented out. For example, implement only the parts you actually support today and drive it off the existing `_columns`:

```csharp
internal void ApplyDisplayName(string columnName, string displayName)
{
    var column = _columns.FirstOrDefault(c =>
        c.GetFieldName().Equals(columnName, StringComparison.OrdinalIgnoreCase));
    if (column is null) return;

    column.Text = displayName;
}

internal void ApplyColumnAttribute<TAttribute>(
    string columnName,
    IEnumerable<KeyValuePair<string, object?>> parameters
) where TAttribute : Attribute
{
    var column = _columns.FirstOrDefault(c =>
        c.GetFieldName().Equals(columnName, StringComparison.OrdinalIgnoreCase));
    if (column is null) return;

    if (typeof(TAttribute) == typeof(RequiredAttribute))
    {
        column.Required = true;
        var message = parameters.FirstOrDefault(p => p.Key == nameof(RequiredAttribute.ErrorMessage)).Value as string;
        if (!string.IsNullOrEmpty(message))
        {
            column.RequiredErrorMessage = message;
        }
        return;
    }

    // Add more cases only as needed, or explicitly no-op:
    // else if (typeof(TAttribute) == typeof(...)) { ... }
}
```

This removes the “mystery commented code” and makes it obvious what is and isn’t supported.

### 2. Collapse `GetShownColumns` indirection into `BuildTableColumns`

`GetShownColumns` currently mutates `col.Ignore`/`Visible` and is called only from `BuildTableColumns`. That two‑step (mutate, then check `Ignore`) is more indirect than necessary.

You can inline the logic and drop the helper without changing behavior:

```csharp
private void BuildTableColumns(
    IEnumerable<string>? invisibleColumns = null,
    IEnumerable<string>? shownColumns = null,
    IEnumerable<string>? hiddenColumns = null)
{
    _columns.Clear();

    foreach (DataColumn col in DataTable.Columns)
    {
        var column = new InternalTableColumn(col.ColumnName, col.DataType);
        var columnName = column.GetFieldName();

        if (invisibleColumns != null &&
            invisibleColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
        {
            // skip entirely
            continue;
        }

        if (hiddenColumns != null &&
            hiddenColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
        {
            column.Visible = false;
        }

        if (shownColumns != null &&
            shownColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
        {
            column.Visible = true;
        }

        OnColumnCreating(column);
        _columns.Add(column);
    }
}
```

Then you can safely remove `GetShownColumns`:

```csharp
// REMOVE:
// private static void GetShownColumns(ITableColumn col, ...)
// { ... }
```

This keeps all existing behaviors (invisible → not added; hidden/shown → adjust `Visible`) while making the column‑building flow local and easier to follow.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/BootstrapBlazor/Extensions/DataRowExtensions.cs
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs Outdated
Comment thread src/BootstrapBlazor/Utils/Utility.cs Outdated
Comment thread src/BootstrapBlazor/Extensions/DynamicObjectContextExtensions.cs Outdated
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs Outdated
Comment thread src/BootstrapBlazor/Services/CacheManager.cs
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve Table performance (fixes #7912) primarily by optimizing DataTable-backed dynamic table editing/rendering and reducing per-cell binding overhead for dynamic models.

Changes:

  • Refactors DataTable dynamic support to wrap DataRow directly (DataTableDynamicObject) and rebuild columns/items more efficiently.
  • Updates editor component binding to use ValidateBase.OnValueChanged for IDynamicObject models instead of generating ValueChanged/ValueExpression.
  • Extends IDynamicObject with Cancel() / Accept() and wires these into the Table edit dialog close/save flow.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/BootstrapBlazor/Utils/Utility.cs Adjusts binding logic for IDynamicObject editors; adds GenerateOnValueChanged helper
src/BootstrapBlazor/Services/CacheManager.cs Changes dynamic property access handling (Get vs Set)
src/BootstrapBlazor/Extensions/DynamicObjectContextExtensions.cs Routes attribute helper calls into DataTableDynamicContext hooks
src/BootstrapBlazor/Extensions/DataRowExtensions.cs Adds DataRow helper to detect Deleted/Detached state
src/BootstrapBlazor/Dynamic/IDynamicObject.cs Adds Cancel() / Accept() to the public interface
src/BootstrapBlazor/Dynamic/DynamicObject.cs Updates base IDynamicObject implementation behavior
src/BootstrapBlazor/Dynamic/DynamicColumnsObject.cs Implements new IDynamicObject members
src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs Introduces row-wrapping dynamic object + Accept/Cancel behavior
src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs Major refactor of DataTable dynamic context (columns/items/cache)
src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs Uses query refresh after add/delete; integrates Accept/Cancel into dialog flow
src/BootstrapBlazor/Components/Table/InternalTableColumn.cs Makes display name fall back to field name
src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamic.razor.cs Minor formatting change

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/BootstrapBlazor/Dynamic/IDynamicObject.cs Outdated
Comment thread src/BootstrapBlazor/Utils/Utility.cs Outdated
Comment thread src/BootstrapBlazor/Utils/Utility.cs Outdated
Comment thread src/BootstrapBlazor/Services/CacheManager.cs
Comment thread src/BootstrapBlazor/Extensions/DynamicObjectContextExtensions.cs Outdated
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs Outdated
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs Outdated
Comment thread src/BootstrapBlazor/Dynamic/DataTableDynamicObject.cs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 92.72727% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.98%. Comparing base (8a65a04) to head (2805305).

Files with missing lines Patch % Lines
...BootstrapBlazor/Dynamic/DataTableDynamicContext.cs 92.68% 2 Missing and 1 partial ⚠️
...rc/BootstrapBlazor/Extensions/DataRowExtensions.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              main    #7913      +/-   ##
===========================================
- Coverage   100.00%   99.98%   -0.02%     
===========================================
  Files          764      765       +1     
  Lines        34337    34325      -12     
  Branches      4711     4710       -1     
===========================================
- Hits         34337    34321      -16     
- Misses           0        2       +2     
- Partials         0        2       +2     
Flag Coverage Δ
BB 99.98% <92.72%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ArgoZhang ArgoZhang merged commit 605f639 into main Apr 25, 2026
3 of 5 checks passed
@ArgoZhang ArgoZhang deleted the fix-table-mmo branch April 25, 2026 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(Table): improve performance

2 participants