feat(EditorForm): add IsShowDisplayTooltip parameter #6987
Merged
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
Reviewer's GuideThis PR introduces a new IsShowDisplayTooltip flag in EditorForm and propagates it through the display-rendering pipeline. It extends CreateDisplayByFieldType to accept a showTooltip parameter, refactors the rendering branches into reusable helper methods (RenderSwitch, RenderTextarea, RenderDisplay), and generalizes many AddAttribute calls to use open generic component types. Sequence diagram for propagating IsShowDisplayTooltip in EditorForm renderingsequenceDiagram
participant EditorForm
participant Utility
participant RenderTreeBuilder
EditorForm->>Utility: CreateDisplayByFieldType(item, Model, IsShowDisplayTooltip)
Utility->>RenderTreeBuilder: RenderDisplay(item, fieldType, fieldValue, displayName, showTooltip)
RenderTreeBuilder-->>Utility: Display component rendered with ShowTooltip = IsShowDisplayTooltip
Class diagram for EditorForm and Utility changesclassDiagram
class EditorForm_TModel {
+bool IsDisplay
+bool IsShowDisplayTooltip
+RenderFragment AutoGenerateTemplate(IEditorItem item)
}
class Utility {
+void CreateDisplayByFieldType(RenderTreeBuilder builder, IEditorItem item, object model, bool showTooltip = false)
-void RenderSwitch(RenderTreeBuilder builder, IEditorItem item, object? fieldValue, string? displayName)
-void RenderTextarea(RenderTreeBuilder builder, IEditorItem item, object? fieldValue, string? displayName)
-void RenderDisplay(RenderTreeBuilder builder, IEditorItem item, Type fieldType, object? fieldValue, string? displayName, bool showTooltip)
}
EditorForm_TModel --> Utility: uses CreateDisplayByFieldType
Utility o-- RenderSwitch
Utility o-- RenderTextarea
Utility o-- RenderDisplay
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.
Pull Request Overview
This PR adds a new IsShowDisplayTooltip parameter to the EditorForm component to control whether tooltips are displayed on Display components in display mode. The implementation refactors the CreateDisplayByFieldType method by extracting rendering logic into separate helper methods and updates XML documentation comments.
Key Changes:
- Added
IsShowDisplayTooltipparameter toEditorFormcomponent with default value offalse - Refactored
CreateDisplayByFieldTypeto use extracted helper methods (RenderSwitch,RenderTextarea,RenderDisplay) - Updated XML documentation with detailed parameter descriptions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/EditorForm/EditorForm.razor.cs | Adds IsShowDisplayTooltip parameter and passes it to CreateDisplayByFieldType |
| src/BootstrapBlazor/Utils/Utility.cs | Refactors display component creation logic and adds showTooltip parameter support |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Updates version numbers for beta releases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Utils/Utility.cs:461` </location>
<code_context>
+ builder.AddAttribute(10, nameof(Display<>.DisplayText), displayName);
+ builder.AddAttribute(20, nameof(Display<>.Value), fieldValue);
+ builder.AddAttribute(30, nameof(Display<>.Lookup), item.Lookup);
+ builder.AddAttribute(30, nameof(Display<>.LookupService), item.LookupService);
+ builder.AddAttribute(40, nameof(Display<>.LookupServiceKey), item.LookupServiceKey);
+ builder.AddAttribute(50, nameof(Display<>.LookupServiceData), item.LookupServiceData);
</code_context>
<issue_to_address>
**nitpick:** Duplicate attribute index (30) used for both Lookup and LookupService.
Assign unique indices to each attribute to prevent confusion and improve maintainability.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Utils/Utility.cs:419` </location>
<code_context>
+ }
+ }
+
+ private static void RenderTextarea(this RenderTreeBuilder builder, IEditorItem item, object? fieldValue, string? displayName)
+ {
+ builder.OpenComponent(0, typeof(Textarea));
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the three rendering methods into a single generic renderer with attribute lists to eliminate repetitive code and manual sequence numbers.
You’ve already split the big switch into three methods, but they’re still almost line-for-line duplicates of each other. You can collapse all of them into a single “generic” renderer + a tiny helper that drives `AddAttribute` from a simple list or dictionary. That way you:
- eliminate all of your hard-coded sequence numbers
- stop repeating the same `OpenComponent`/`CloseComponent`/`AddMultipleAttributes` logic
- can inline conditional attributes (e.g. the `class` on `ITableColumn`) in a single loop
For example, add a helper like this:
```csharp
private static void RenderComponent(
this RenderTreeBuilder builder,
Type componentType,
IEditorItem item,
IEnumerable<(string Name, object? Value)> attributes)
{
builder.OpenComponent(0, componentType);
var seq = 1;
foreach (var (name, value) in attributes)
{
if (value != null)
builder.AddAttribute(seq++, name, value);
}
builder.AddMultipleAttributes(seq, item.ComponentParameters);
builder.CloseComponent();
}
```
Then rewrite each of your three methods into just building an attribute list:
```csharp
private static void RenderSwitch(this RenderTreeBuilder builder, IEditorItem item, object? fieldValue, string? displayName)
{
var attrs = new[]
{
(nameof(Switch.Value) , (object?) fieldValue),
(nameof(Switch.IsDisabled) , true),
(nameof(Switch.DisplayText) , displayName),
(nameof(Switch.ShowLabelTooltip), (object?) item.ShowLabelTooltip),
( "class" , item is ITableColumn col ? col.CssClass : null )
};
builder.RenderComponent(typeof(Switch), item, attrs);
}
```
And similarly for `Textarea` and `Display<>`:
```csharp
private static void RenderTextarea(...){
var attrs = new List<(string, object?)>{
(nameof(Textarea.DisplayText), displayName),
(nameof(Textarea.Value), fieldValue),
/* etc */
};
if (item.Rows > 0) attrs.Add(("rows", item.Rows));
attrs.Add(("class", item is ITableColumn col ? col.CssClass : null));
builder.RenderComponent(typeof(Textarea), item, attrs);
}
private static void RenderDisplay(...){
var displayType = typeof(Display<>).MakeGenericType(fieldType);
var attrs = new List<(string, object?)>{
(nameof(Display<object>.DisplayText), displayName),
(nameof(Display<object>.Value), fieldValue),
/* all the lookup/formatter/tooltip attrs */
};
if (item is ITableColumn col) { /* add format/FormatterAsync/class */ }
builder.RenderComponent(displayType, item, attrs);
}
```
This preserves exactly the same behavior but removes hundreds of lines of boilerplate, collapses all your `AddAttribute(…)` calls into simple loops, and you never have to hand-maintain sequence numbers again.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6987 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 741 741
Lines 32352 32365 +13
Branches 4481 4481
=========================================
+ Hits 32352 32365 +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:
|
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 #6986
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable configurable tooltip display for Display components in EditorForm and refactor CreateDisplayByFieldType with helper methods for better maintainability
New Features:
Bug Fixes:
Enhancements: