Conversation
Reviewer's GuideThis PR cleans up and clarifies XML documentation comments for several BootstrapBlazor components and services, primarily improving English descriptions, adding or refining parameter documentation, and removing redundant or empty comments without changing runtime behavior. 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.
Hey - I've found 2 issues, and left some high level feedback:
- Some of the updated English XML comments could be polished for clarity and grammar (e.g.,
Default is #000 Supports hexadecimal and color namesandMaskService Mask Serviceread a bit awkwardly); consider tightening the wording and punctuation for consistency across components. - Where you removed empty
<param>tags (e.g., inSetPlacementandDismiss), there are still methods likeGetClassString(MenuItem item)without parameter documentation but with non-obvious behavior; consider either documenting the parameter or removing stray, now-misleading comment remnants to keep the API docs coherent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Some of the updated English XML comments could be polished for clarity and grammar (e.g., `Default is #000 Supports hexadecimal and color names` and `MaskService Mask Service` read a bit awkwardly); consider tightening the wording and punctuation for consistency across components.
- Where you removed empty `<param>` tags (e.g., in `SetPlacement` and `Dismiss`), there are still methods like `GetClassString(MenuItem item)` without parameter documentation but with non-obvious behavior; consider either documenting the parameter or removing stray, now-misleading comment remnants to keep the API docs coherent.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Components/Marquee/Marquee.razor.cs" line_range="23" />
<code_context>
- /// <para lang="en">Gets or sets component值 文本color Default is #000 <para>支持16进制和color名称</para>
- ///</para>
+ /// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 支持16进制和颜色名称</para>
+ /// <para lang="en">Gets or sets the component's text color. Default is #000 Supports hexadecimal and color names</para>
/// </summary>
[Parameter]
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar issue in the English description of the text color parameter.
Add punctuation to clearly separate the default value from the capability description, e.g. `Default is #000. Supports hexadecimal and color names.`
```suggestion
/// <para lang="en">Gets or sets the component's text color. Default is #000. Supports hexadecimal and color names.</para>
```
</issue_to_address>
### Comment 2
<location path="src/BootstrapBlazor/Components/Marquee/Marquee.razor.cs" line_range="30" />
<code_context>
- /// <para lang="en">Gets or sets component值 背景color Default is #fff <para>支持16进制和color名称</para>
- ///</para>
+ /// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff 支持16进制和颜色名称</para>
+ /// <para lang="en">Gets or sets the component's background color. Default is #fff Supports hexadecimal and color names</para>
/// </summary>
[Parameter]
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar issue in the English description of the background color parameter.
You could mirror the text color description and update this to: `Default is #fff. Supports hexadecimal and color names.`
Suggested implementation:
```csharp
/// <summary>
/// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff 支持16进制和颜色名称</para>
/// <para lang="en">Gets or sets the component's background color. Default is #fff. Supports hexadecimal and color names.</para>
/// </summary>
[Parameter]
public string? Text { get; set; }
/// <summary>
/// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 支持16进制和颜色名称</para>
/// <para lang="en">Gets or sets the component's text color. Default is #000. Supports hexadecimal and color names.</para>
/// </summary>
```
If the `[Parameter]` attribute for `Text` is already present elsewhere around this property in the actual file, remove the duplicate `[Parameter]` I added above and keep only one attribute on the property.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// <para lang="en">Gets or sets component值 文本color Default is #000 <para>支持16进制和color名称</para> | ||
| ///</para> | ||
| /// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 支持16进制和颜色名称</para> | ||
| /// <para lang="en">Gets or sets the component's text color. Default is #000 Supports hexadecimal and color names</para> |
There was a problem hiding this comment.
nitpick (typo): Minor grammar issue in the English description of the text color parameter.
Add punctuation to clearly separate the default value from the capability description, e.g. Default is #000. Supports hexadecimal and color names.
| /// <para lang="en">Gets or sets the component's text color. Default is #000 Supports hexadecimal and color names</para> | |
| /// <para lang="en">Gets or sets the component's text color. Default is #000. Supports hexadecimal and color names.</para> |
| /// <para lang="en">Gets or sets component值 背景color Default is #fff <para>支持16进制和color名称</para> | ||
| ///</para> | ||
| /// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff 支持16进制和颜色名称</para> | ||
| /// <para lang="en">Gets or sets the component's background color. Default is #fff Supports hexadecimal and color names</para> |
There was a problem hiding this comment.
nitpick (typo): Minor grammar issue in the English description of the background color parameter.
You could mirror the text color description and update this to: Default is #fff. Supports hexadecimal and color names.
Suggested implementation:
/// <summary>
/// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff 支持16进制和颜色名称</para>
/// <para lang="en">Gets or sets the component's background color. Default is #fff. Supports hexadecimal and color names.</para>
/// </summary>
[Parameter]
public string? Text { get; set; }
/// <summary>
/// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 支持16进制和颜色名称</para>
/// <para lang="en">Gets or sets the component's text color. Default is #000. Supports hexadecimal and color names.</para>
/// </summary>If the [Parameter] attribute for Text is already present elsewhere around this property in the actual file, remove the duplicate [Parameter] I added above and keep only one attribute on the property.
There was a problem hiding this comment.
Pull request overview
Updates XML documentation across several BootstrapBlazor components/services to improve bilingual (zh/en) comment quality and consistency.
Changes:
- Refined/expanded component/service
<summary>and parameter XML docs (Menu, MaskService, MenuItem, SubMenu, Marquee). - Removed some redundant/empty XML doc tags (e.g.,
<param>/<value>placeholders) and internal-only helper docs. - Adjusted
Message’s injectedMessageServicevisibility (nowprivate).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Message/Message.razor.cs | Removes XML docs on injected service and changes injected MessageService property visibility to private. |
| src/BootstrapBlazor/Components/Menu/SubMenu.razor.cs | Fills in previously empty class summary; removes doc block for a private helper. |
| src/BootstrapBlazor/Components/Menu/MenuItem.cs | Reformats constructor parameter XML docs into multiline bilingual blocks. |
| src/BootstrapBlazor/Components/Menu/Menu.razor.cs | Minor XML doc formatting tweaks; removes an empty <value> tag. |
| src/BootstrapBlazor/Components/Mask/MaskService.cs | Improves English service summary; reformats method parameter XML docs. |
| src/BootstrapBlazor/Components/Marquee/Marquee.razor.cs | Rewrites multiple English/zh XML docs for clarity and consistency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [Inject] | ||
| [NotNull] | ||
| public MessageService? MessageService { get; set; } | ||
| private MessageService? MessageService { get; set; } |
There was a problem hiding this comment.
Changing the injected MessageService property from public to private is a public API breaking change for consumers compiling against this library (the member is removed from the public surface). If the intent is only to hide the injected service, consider an alternative that preserves compatibility (e.g., keep it public but mark it non-browsable / internal via a wrapper) or explicitly call out the breaking change in release notes/PR description.
| /// <summary> | ||
| /// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 <para>支持16进制和颜色名称</para> | ||
| ///</para> | ||
| /// <para lang="en">Gets or sets component值 文本color Default is #000 <para>支持16进制和color名称</para> | ||
| ///</para> | ||
| /// <para lang="zh">获得/设置 组件值 文本颜色 默认 #000 支持16进制和颜色名称</para> | ||
| /// <para lang="en">Gets or sets the component's text color. Default is #000 Supports hexadecimal and color names</para> | ||
| /// </summary> |
There was a problem hiding this comment.
In the English doc text for Color, there’s a missing sentence separator: "Default is #000 Supports..." reads as two sentences but lacks punctuation. Consider adding a period or rewriting so generated docs read cleanly.
| /// <summary> | ||
| /// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff <para>支持16进制和颜色名称</para> | ||
| ///</para> | ||
| /// <para lang="en">Gets or sets component值 背景color Default is #fff <para>支持16进制和color名称</para> | ||
| ///</para> | ||
| /// <para lang="zh">获得/设置 组件值 背景颜色 默认 #fff 支持16进制和颜色名称</para> | ||
| /// <para lang="en">Gets or sets the component's background color. Default is #fff Supports hexadecimal and color names</para> | ||
| /// </summary> |
There was a problem hiding this comment.
In the English doc text for BackgroundColor, there’s a missing sentence separator: "Default is #fff Supports..." would read better with a period (or similar) between the default value sentence and the supported formats sentence.
| /// <summary> | ||
| /// <para lang="zh">获得/设置 组件值 动画时间 默认 14s <para>值越小滚动越快</para> | ||
| ///</para> | ||
| /// <para lang="en">Gets or sets component值 动画时间 Default is 14s <para>值越小滚动越快</para> | ||
| /// <para lang="en">Gets or sets the component's animation duration. Default is 14s <para>The smaller the value, the faster the scroll</para> | ||
| ///</para> |
There was a problem hiding this comment.
The Duration XML docs still use nested <para> tags inside <para lang="..."> (and the closing tags are formatted as ///</para>), while other properties in this file were updated to a flatter style. Consider splitting this into separate <para lang="..."> blocks to keep the XML docs consistent and easier for doc tooling to parse.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7745 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 750 750
Lines 33269 33269
Branches 4612 4612
=========================================
Hits 33269 33269
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:
|
Link issues
fixes #7744
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Improve XML documentation and component comments for several UI components to clarify behavior and usage in both Chinese and English.
Enhancements: