feat(IEditorItem): add EditorItem generic type#7642
Merged
Conversation
Contributor
Reviewer's GuideIntroduce a generic EditorItem implementation and supporting utilities for strongly-typed edit templates and parameter binding, including a RenderEditTemplate helper on BootstrapDynamicComponent and a PropertyInfo extension for typed [Parameter] detection, with unit tests covering the new behavior. Sequence diagram for BootstrapDynamicComponent.RenderEditTemplate_TModel_sequenceDiagram
actor Caller
participant BootstrapDynamicComponent
participant RenderFragment_TModel_ as RenderFragment_TModel_
participant RenderTreeBuilder as Builder
participant ComponentType as componentType
participant PropertyInfoExtensions
Caller->>BootstrapDynamicComponent: RenderEditTemplate_TModel_(modelParameterName)
activate BootstrapDynamicComponent
BootstrapDynamicComponent-->>Caller: RenderFragment_TModel_
deactivate BootstrapDynamicComponent
Caller->>RenderFragment_TModel_: invoke with model
activate RenderFragment_TModel_
RenderFragment_TModel_->>Builder: build fragment
activate Builder
Builder->>Builder: index = 0
Builder->>componentType: OpenComponent(index, componentType)
alt parameters not null
RenderFragment_TModel_->>RenderFragment_TModel_: modelParameterName ??= Model
RenderFragment_TModel_->>componentType: GetPropertyByName(modelParameterName)
componentType-->>RenderFragment_TModel_: modelProperty
RenderFragment_TModel_->>PropertyInfoExtensions: HasParameterAttribute(modelProperty, typeof(TModel))
PropertyInfoExtensions-->>RenderFragment_TModel_: bool hasParameter
alt hasParameter
RenderFragment_TModel_->>Builder: AddAttribute(index, modelParameterName, model)
end
loop for each p in parameters
RenderFragment_TModel_->>Builder: AddAttribute(index, p.Key, p.Value)
end
end
Builder-->>Builder: CloseComponent()
deactivate Builder
deactivate RenderFragment_TModel_
Class diagram for the new generic EditorItem_TModel implementationclassDiagram
direction TB
class IEditorItem {
<<interface>>
bool SkipValidate
bool? Ignore
bool? Readonly
bool? Required
string? RequiredErrorMessage
bool? ShowLabelTooltip
string? PlaceHolder
Type PropertyType
bool Editable
string? Step
int Rows
int Cols
string? Text
RenderFragment_object_ EditTemplate
Type? ComponentType
IEnumerable_KeyValuePair_string_object__? ComponentParameters
IEnumerable_SelectedItem_? Items
int Order
IEnumerable_SelectedItem_? Lookup
bool ShowSearchWhenSelect
bool IsFixedSearchWhenSelect
bool IsPopover
StringComparison LookupStringComparison
string? LookupServiceKey
object? LookupServiceData
ILookupService? LookupService
Action_TableCellArgs_? OnCellRender
List_IValidator_? ValidateRules
string? GroupName
int GroupOrder
string GetDisplayName()
string GetFieldName()
}
class ILookup {
<<interface>>
IEnumerable_SelectedItem_? Lookup
StringComparison LookupStringComparison
string? LookupServiceKey
object? LookupServiceData
ILookupService? LookupService
}
class ITableColumn {
<<interface>>
Action_TableCellArgs_? OnCellRender
}
class EditorItem_TModel_ {
- string FieldName
+ bool SkipValidate
+ bool? Ignore
+ bool? Readonly
+ bool? Required
+ string? RequiredErrorMessage
+ bool? ShowLabelTooltip
+ string? PlaceHolder
+ Type PropertyType
+ string? Step
+ int Rows
+ int Cols
+ string? Text
+ RenderFragment_TModel_? EditTemplate
+ Type? ComponentType
+ IEnumerable_KeyValuePair_string_object__? ComponentParameters
+ IEnumerable_SelectedItem_? Items
+ int Order
+ IEnumerable_SelectedItem_? Lookup
+ bool ShowSearchWhenSelect
+ bool IsPopover
+ StringComparison LookupStringComparison
+ string? LookupServiceKey
+ object? LookupServiceData
+ ILookupService? LookupService
+ Action_TableCellArgs_? OnCellRender
+ List_IValidator_? ValidateRules
+ string? GroupName
+ int GroupOrder
+ EditorItem_TModel_(string fieldName, Type fieldType, string? fieldText)
+ string GetDisplayName()
+ string GetFieldName()
+ RenderFragment_object_ IEditorItem_EditTemplate
}
class RenderFragment_TModel_ {
<<delegate>>
}
class RenderFragment_object_ {
<<delegate>>
}
class ILookupService {
<<interface>>
}
class SelectedItem
class TableCellArgs
class IValidator {
<<interface>>
}
EditorItem_TModel_ ..|> IEditorItem
EditorItem_TModel_ ..|> ILookup
EditorItem_TModel_ ..|> ITableColumn
EditorItem_TModel_ --> RenderFragment_TModel_ : uses
EditorItem_TModel_ --> RenderFragment_object_ : explicit implementation
EditorItem_TModel_ --> SelectedItem
EditorItem_TModel_ --> ILookupService
EditorItem_TModel_ --> TableCellArgs
EditorItem_TModel_ --> IValidator
Flow diagram for PropertyInfoExtensions.HasParameterAttributeflowchart TD
A[Start HasParameterAttribute
modelProperty, type] --> B{modelProperty is null}
B -->|yes| C[Return false]
B -->|no| D{Has ParameterAttribute
on modelProperty}
D -->|no| C
D -->|yes| E[Get propertyType
= underlying type of
modelProperty.PropertyType
or PropertyType]
E --> F[Get targetType
= underlying type of
input type
or type]
F --> G{targetType
IsAssignableFrom
propertyType}
G -->|yes| H[Return true]
G -->|no| C
C --> I[End]
H --> I
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
EditorItem<TModel>.IEditorItem.EditTemplatethe cast(TModel)itemassumes the runtime model type always matchesTModel; consider adding a type check or safer cast path to avoid potentialInvalidCastExceptionwhen used with loosely-typed callers. PropertyInfoExtensions.HasParameterAttributecurrently requiresmodelProperty.PropertyType == type; if the intent is to support derived or assignable types, consider usingtype.IsAssignableFrom(modelProperty.PropertyType)instead.BootstrapDynamicComponent.RenderEditTemplate<TModel>performs reflection (GetPropertyByNameandHasParameterAttribute) on each render; if this is used frequently, consider caching the resolved property info percomponentType/modelParameterNameto reduce overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `EditorItem<TModel>.IEditorItem.EditTemplate` the cast `(TModel)item` assumes the runtime model type always matches `TModel`; consider adding a type check or safer cast path to avoid potential `InvalidCastException` when used with loosely-typed callers.
- `PropertyInfoExtensions.HasParameterAttribute` currently requires `modelProperty.PropertyType == type`; if the intent is to support derived or assignable types, consider using `type.IsAssignableFrom(modelProperty.PropertyType)` instead.
- `BootstrapDynamicComponent.RenderEditTemplate<TModel>` performs reflection (`GetPropertyByName` and `HasParameterAttribute`) on each render; if this is used frequently, consider caching the resolved property info per `componentType`/`modelParameterName` to reduce overhead.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Extensions/PropertyInfoExtensions.cs:55-57` </location>
<code_context>
+ /// </summary>
+ /// <param name="modelProperty"></param>
+ /// <param name="type"></param>
+ public static bool HasParameterAttribute(this PropertyInfo? modelProperty, Type type) => modelProperty != null
+ && modelProperty.IsDefined(typeof(ParameterAttribute), true)
+ && modelProperty.PropertyType == type;
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `PropertyType == type` is likely too restrictive for many real-world Parameter scenarios.
This currently returns true only for exact type matches, which will miss valid cases like nullable vs non-nullable, base/derived types, and interface vs implementation (e.g., `IEnumerable<T>` vs a concrete collection). If the goal is type compatibility, consider `type.IsAssignableFrom(modelProperty.PropertyType)` with nullable unwrapping so this behaves as expected at call sites like `RenderEditTemplate`.
Suggested implementation:
```csharp
/// <summary>
/// <para lang="zh">判断属性是否有指定类型的 Parameter 特性</para>
/// <para lang="en">Determines whether the property has a Parameter attribute of the specified type</para>
/// </summary>
/// <param name="modelProperty"></param>
/// <param name="type"></param>
public static bool HasParameterAttribute(this PropertyInfo? modelProperty, Type type)
{
if (modelProperty is null)
{
return false;
}
// 必须带有 Parameter 特性
if (!modelProperty.IsDefined(typeof(ParameterAttribute), inherit: true))
{
return false;
}
// 处理可空类型，并使用可赋值性检查类型兼容性
var propertyType = Nullable.GetUnderlyingType(modelProperty.PropertyType) ?? modelProperty.PropertyType;
var targetType = Nullable.GetUnderlyingType(type) ?? type;
return targetType.IsAssignableFrom(propertyType);
}
/// <summary>
```
If `ParameterAttribute` is not already in scope in this file, you will need to add the appropriate using at the top of the file, typically:
- `using Microsoft.AspNetCore.Components;`
This implementation assumes that the intended semantics are "the property type is compatible with the requested type" after unwrapping nullable types. If your call sites need the opposite relationship, adjust the last line accordingly (e.g., `propertyType.IsAssignableFrom(targetType)`).
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Misc/EditorItem.cs:98-100` </location>
<code_context>
+ /// </summary>
+ public RenderFragment<TModel>? EditTemplate { get; set; }
+
+ RenderFragment<object>? IEditorItem.EditTemplate
+ {
+ get
+ {
+ return EditTemplate == null ? null : new RenderFragment<object>(item => builder =>
+ {
+ builder.AddContent(0, EditTemplate((TModel)item));
+ });
+ }
+ set
+ {
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The explicit interface EditTemplate setter is a silent no-op, which may hide misuses.
The `IEditorItem.EditTemplate` setter currently ignores the value, which can confuse consumers who expect it to work. Consider either mapping the setter to the strongly-typed `EditTemplate` (with an appropriate cast/check) or throwing `NotSupportedException` so incorrect usage fails fast instead of silently doing nothing.
```suggestion
set
{
if (value is null)
{
EditTemplate = null;
}
else
{
EditTemplate = model => value(model!);
}
}
```
</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 #7642 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 750 +1
Lines 33109 33179 +70
Branches 4593 4602 +9
=========================================
+ Hits 33109 33179 +70
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:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a generic EditorItem<TModel> model-aware edit-template pathway to support strongly-typed editor rendering scenarios (per #7641), alongside utility updates to dynamically pass a model into dynamically-rendered components.
Changes:
- Added
BootstrapDynamicComponent.RenderEditTemplate<TModel>()to render a dynamic component while attempting to pass a model parameter. - Introduced new
EditorItem<TModel>(entity/config class) implementingIEditorItem, including a strongly-typedRenderFragment<TModel>edit template. - Extended
PropertyInfoExtensionswithHasParameterAttribute(...)and improved some XML documentation text.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Utils/BootstrapDynamicComponent.cs | Adds a new RenderEditTemplate<TModel> API intended to pass a model parameter when rendering a dynamic component. |
| src/BootstrapBlazor/Misc/EditorItem.cs | Adds new EditorItem<TModel> implementation of IEditorItem with typed EditTemplate. |
| src/BootstrapBlazor/Extensions/PropertyInfoExtensions.cs | Updates XML docs and adds HasParameterAttribute helper used by RenderEditTemplate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The explicit
IEditorItem.EditTemplatesetter inEditorItem<TModel>is a no-op, which is surprising for consumers working via the interface; consider either wiring it to the strongly-typedEditTemplateor throwing/notifying to make the behavior explicit. BootstrapDynamicComponent.RenderEditTemplate<TModel>currently assumes a default parameter name of"Model"; you might want to optionally fall back to discovering a[Parameter]property by assignable type whenmodelParameterNameis null or not found to make it more robust to different component conventions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The explicit `IEditorItem.EditTemplate` setter in `EditorItem<TModel>` is a no-op, which is surprising for consumers working via the interface; consider either wiring it to the strongly-typed `EditTemplate` or throwing/notifying to make the behavior explicit.
- `BootstrapDynamicComponent.RenderEditTemplate<TModel>` currently assumes a default parameter name of `"Model"`; you might want to optionally fall back to discovering a `[Parameter]` property by assignable type when `modelParameterName` is null or not found to make it more robust to different component conventions.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Extensions/PropertyInfoExtensions.cs:55-64` </location>
<code_context>
+ /// </summary>
+ /// <param name="modelProperty"></param>
+ /// <param name="type"></param>
+ public static bool HasParameterAttribute(this PropertyInfo? modelProperty, Type type)
+ {
+ if (modelProperty is null)
+ {
+ return false;
+ }
+
+ // 必须带有 Parameter 特性
+ if (!modelProperty.IsDefined(typeof(ParameterAttribute), inherit: true))
+ {
+ return false;
+ }
+
+ // 处理可空类型，并使用可赋值性检查类型兼容性
+ var propertyType = Nullable.GetUnderlyingType(modelProperty.PropertyType) ?? modelProperty.PropertyType;
+ var targetType = Nullable.GetUnderlyingType(type) ?? type;
+
+ return targetType.IsAssignableFrom(propertyType);
+ }
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The type compatibility check in HasParameterAttribute is likely reversed, which can cause false negatives.
Because this is later called as `HasParameterAttribute(typeof(TModel))` and a `TModel` instance is assigned to a `propertyType` property, the assignability check needs to reflect that the property must accept `TModel`. That means the condition should be `propertyType.IsAssignableFrom(targetType)`. As written, scenarios like `propertyType = object` and `TModel = string` return false even though the assignment is valid, leading to false negatives.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Misc/EditorItem.cs:81-90` </location>
<code_context>
+ [NotNull]
</code_context>
<issue_to_address>
**issue (bug_risk):** Text can be null at runtime but GetDisplayName assumes it is non-null, which may lead to null-related issues.
`Text` is `string?` (from `fieldText` and future assignments), but `GetDisplayName()` returns `string` and just returns `Text`. `[NotNull]` only informs static analysis, so callers may still receive null at runtime. Either enforce `Text` as non-null (e.g., default to `FieldName` when null) or update `GetDisplayName()` to handle null explicitly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was referenced Feb 25, 2026
This was referenced Mar 10, 2026
This was referenced Mar 23, 2026
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
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.
Link issues
fixes #7641
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce a strongly-typed generic EditorItem model and enhance dynamic component rendering support for model-bound edit templates.
New Features:
Enhancements:
Tests: