Conversation
Contributor
Reviewer's GuideRefactors component parameter documentation to use auto-generated AttributeTable metadata, adds version annotations to several parameters, and makes minor doc and styling tweaks, removing now-redundant hand-maintained attribute/method lists for Table and Charts samples. Class diagram for AttributeTable usage with auto-generated metadataclassDiagram
class AttributeTable {
+string? Title
+IEnumerable~AttributeItem~? Items
+Type? Type
+bool ShowFooter
}
class AttributeItem {
+string Name
+string Description
+string Type
+string ValueList
+string DefaultValue
+string Version
}
class Table_TItem_ {
+bool IsBordered
+bool IsStriped
}
class Chart {
}
class BootstrapComponentBase {
+Dictionary~string, object~ AdditionalAttributes
}
AttributeTable ..> AttributeItem : displays
AttributeTable ..> Table_TItem_ : wraps
AttributeTable --|> BootstrapComponentBase
Table_TItem_ ..> AttributeItem : metadata source
Chart ..> AttributeItem : metadata source
AttributeItem --> "10.2.2" Version : default
Flow diagram for AttributeTable auto-generation of parameter docsflowchart TD
A_Start["Render AttributeTable"] --> B_CheckType["Type is null?"]
B_CheckType -->|Yes| C_UseItems["Use Items parameter as attribute metadata"]
B_CheckType -->|No| D_AutoGenerate["Reflect over Type to build AttributeItem collection"]
D_AutoGenerate --> E_SetMark["Set table-attr-mark text to 'Automatically generated'"]
C_UseItems --> F_RenderTable["Render Table AttributeItem with IsBordered=true IsStriped=true"]
E_SetMark --> F_RenderTable
F_RenderTable --> G_ShowFooter["Show footer when ShowFooter is true"]
G_ShowFooter --> H_End["Attribute documentation table displayed"]
File-Level Changes
Assessment against 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 1 issue, and left some high level feedback:
- AttributeTable now supports a Type parameter but the markup still always renders the Title span; when using Type without a Title (e.g., in Tables.razor and Charts/Index.razor), this will produce an empty label, so consider hiding the label element or providing a sensible default when Title is null or empty.
- AttributeItem.Version is now hardcoded to "10.2.2"; to avoid future drift, consider deriving this from a central version source (e.g., assembly or package version) instead of a literal string.
- The Table sample no longer renders MethodTable or exposes GetMethods, while other samples (like Charts) still show method metadata; if the intent is to keep method documentation consistent across components, consider adding an equivalent auto-generated method table or clarifying the difference in behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- AttributeTable now supports a Type parameter but the markup still always renders the Title span; when using Type without a Title (e.g., in Tables.razor and Charts/Index.razor), this will produce an empty label, so consider hiding the label element or providing a sensible default when Title is null or empty.
- AttributeItem.Version is now hardcoded to "10.2.2"; to avoid future drift, consider deriving this from a central version source (e.g., assembly or package version) instead of a literal string.
- The Table sample no longer renders MethodTable or exposes GetMethods, while other samples (like Charts) still show method metadata; if the intent is to keep method documentation consistent across components, consider adding an equivalent auto-generated method table or clarifying the difference in behavior.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Components/AttributeTable.razor:2-8` </location>
<code_context>
<div class="table-attr">
- <h4>@Title</h4>
+ <p class="table-attr-desc">
+ <span class="code-label">@Title</span>
+ @if (Type != null)
+ {
+ <span class="table-attr-mark">Automatically generated</span>
+ }
</code_context>
<issue_to_address>
**suggestion:** Handle cases where `Title` is null but `Type` is set to avoid an empty label next to the marker.
In this layout, when `Title` is null but `Type` is not, the `code-label` span will be empty while the `Automatically generated` marker still shows, which is visually awkward. Consider either falling back to something like `Type.Name` (or a localized default) when `Title` is null, or omitting the title span entirely in that case so the marker doesn’t appear detached.
```suggestion
<p class="table-attr-desc">
@if (!string.IsNullOrEmpty(Title))
{
<span class="code-label">@Title</span>
}
else if (Type != null)
{
<span class="code-label">@Type.Name</span>
}
@if (Type != null)
{
<span class="table-attr-mark">Automatically generated</span>
}
</p>
```
</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 #7537 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 748 748
Lines 33012 33012
Branches 4588 4588
=========================================
Hits 33012 33012
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
This pull request refactors the documentation generation system to be more automated by removing hardcoded attribute lists and leveraging reflection-based attribute generation. The PR removes over 1,300 lines of redundant code while improving maintainability.
Changes:
- Removed hardcoded attribute definitions from Tables and Charts sample components (1,149 lines)
- Updated AttributeTable component to support automatic attribute generation via reflection
- Cleaned up localization files by removing 170 unused Table attribute strings from both zh-CN and en-US
- Improved XML documentation formatting for parameters in several components
- Changed default version value in AttributeItem from empty string to "10.2.2"
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Tables.razor.cs | Removed 1,013 lines of hardcoded GetAttributes() and GetMethods() methods |
| Tables.razor | Changed to use Type parameter for automatic attribute generation |
| Charts/Index.razor.cs | Removed 136 lines of hardcoded GetAttributes() method |
| Charts/Index.razor | Changed to use Type parameter for automatic attribute generation |
| AttributeTable.razor | Added UI improvements for displaying automatically generated attributes |
| AttributeTable.razor.css | Added new styling for the updated AttributeTable component |
| zh-CN.json, en-US.json | Removed 170 unused Table attribute localization strings from each file |
| LambdaExtensions.cs | Fixed English translation and removed redundant remarks/returns tags |
| JsonLocalizationServiceCollectionExtensions.cs | Improved parameter documentation formatting |
| Layout.razor.cs, ContextMenuTrigger.cs, BootstrapComponentBase.cs | Added version tags for parameters |
| AttributeItem.cs | Changed default version from empty string to "10.2.2" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 #7536
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Replace manually maintained Table and Chart parameter documentation in samples with automatically generated attribute tables, and align related metadata and styling with version 10.2.2.
Enhancements:
Documentation: