feat(UniverSheet): add RibbonType parameter#855
Conversation
Reviewer's GuideAdds a configurable ribbon style to the UniverSheet component by introducing a UniverSheetRibbonType enum parameter and propagating its value through the Blazor-to-JavaScript initialization pipeline. Sequence diagram for passing RibbonType from Blazor to JavaScriptsequenceDiagram
actor Developer
participant Blazor as UniverSheet
participant JSInterop
participant Script as UniverSheet_razor_js
participant UniverInstance as UniverSheetInstance
Developer ->> Blazor: Set RibbonType parameter
Developer ->> Blazor: Render UniverSheet
Blazor ->> JSInterop: InvokeVoidAsync init(id, interop, options with RibbonType.ToDescriptionString)
JSInterop ->> Script: init(id, invoke, options)
Script ->> Script: Read theme, lang, plugins, data, ribbonType from options
Script ->> UniverInstance: createUniverSheetAsync(univerSheet with ribbonType)
Class diagram for UniverSheet ribbon type parameterclassDiagram
class UniverSheet {
string Lang
UniverSheetRibbonType RibbonType
InvokeInitAsync() Task
}
class UniverSheetRibbonType {
<<enumeration>>
Default
Classic
Simple
}
UniverSheet --> UniverSheetRibbonType
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.UniverSheet/Components/UniverSheet.razor.cs:33-37` </location>
<code_context>
[Parameter]
public string? Lang { get; set; }
+ /// <summary>
+ /// 获得/设置 设置工具栏样式 默认 default 未设置
+ /// </summary>
+ [Parameter]
+ public UniverSheetRibbonType RibbonType { get; set; }
+
/// <summary>
</code_context>
<issue_to_address>
**suggestion:** Explicitly initialize `RibbonType` to `UniverSheetRibbonType.Default` to make the intended default resilient to future enum changes.
This currently depends on `RibbonType` defaulting to the enum’s implicit `0` value being `Default`. If the enum order changes or a new value is added before `Default`, the runtime default will change unintentionally. Initializing it explicitly (e.g. `public UniverSheetRibbonType RibbonType { get; set; } = UniverSheetRibbonType.Default;`) makes the intended default robust to such changes.
```suggestion
/// <summary>
/// 获得/设置 设置工具栏样式 默认 default 未设置
/// </summary>
[Parameter]
public UniverSheetRibbonType RibbonType { get; set; } = UniverSheetRibbonType.Default;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// <summary> | ||
| /// 获得/设置 设置工具栏样式 默认 default 未设置 | ||
| /// </summary> | ||
| [Parameter] | ||
| public UniverSheetRibbonType RibbonType { get; set; } |
There was a problem hiding this comment.
suggestion: Explicitly initialize RibbonType to UniverSheetRibbonType.Default to make the intended default resilient to future enum changes.
This currently depends on RibbonType defaulting to the enum’s implicit 0 value being Default. If the enum order changes or a new value is added before Default, the runtime default will change unintentionally. Initializing it explicitly (e.g. public UniverSheetRibbonType RibbonType { get; set; } = UniverSheetRibbonType.Default;) makes the intended default robust to such changes.
| /// <summary> | |
| /// 获得/设置 设置工具栏样式 默认 default 未设置 | |
| /// </summary> | |
| [Parameter] | |
| public UniverSheetRibbonType RibbonType { get; set; } | |
| /// <summary> | |
| /// 获得/设置 设置工具栏样式 默认 default 未设置 | |
| /// </summary> | |
| [Parameter] | |
| public UniverSheetRibbonType RibbonType { get; set; } = UniverSheetRibbonType.Default; |
There was a problem hiding this comment.
Pull request overview
This PR adds a RibbonType parameter to the UniverSheet component, allowing users to configure the toolbar style. The implementation introduces a new enum with three style options (Default, Classic, Simple) and passes this configuration from C# to JavaScript.
Key Changes:
- Introduces
UniverSheetRibbonTypeenum with three toolbar style options - Adds RibbonType parameter to UniverSheet component with automatic conversion to JavaScript-compatible string format
- Version bump from 10.0.2 to 10.0.3
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| UniverSheetRibbonType.cs | New enum defining three ribbon/toolbar style options with Description attributes for JavaScript interop |
| UniverSheet.razor.cs | Adds RibbonType parameter and includes it in JavaScript initialization call |
| UniverSheet.razor.js | Extracts ribbonType from options and passes it to the univer initialization |
| BootstrapBlazor.UniverSheet.csproj | Version bump to 10.0.3 for the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,31 @@ | |||
| // Copyright (c) BootstrapBlazor & Argo Zhang (argo@live.ca). All rights reserved. | |||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | |||
| // Website: https://www.blazor.zone | |||
There was a problem hiding this comment.
The website URL in this file is "https://www.blazor.zone", which is inconsistent with other files in the same directory (e.g., UniverSheetData.cs, UniverSheet.razor.cs) that use "https://www.blazor.zone or https://argozhang.github.io/". The header format should be consistent across all files in the component.
| // Website: https://www.blazor.zone | |
| // Website: https://www.blazor.zone or https://argozhang.github.io/ |
| /// <summary> | ||
| /// 获得/设置 设置工具栏样式 默认 default 未设置 | ||
| /// </summary> | ||
| [Parameter] | ||
| public UniverSheetRibbonType RibbonType { get; set; } |
There was a problem hiding this comment.
The RibbonType parameter will always send a value to JavaScript (defaulting to "default" when not explicitly set by the user), which means the JavaScript fallback ?? 'simple' in univer.js line 62 will never be used. Additionally, the documentation comment "默认 default 未设置" (defaults to default, not set) is ambiguous. Consider making the RibbonType property nullable (UniverSheetRibbonType?) to allow it to truly be unset and align with the JavaScript fallback behavior, or update the documentation to clearly state that it defaults to the Default enum value.
| @@ -0,0 +1,31 @@ | |||
| // Copyright (c) BootstrapBlazor & Argo Zhang (argo@live.ca). All rights reserved. | |||
There was a problem hiding this comment.
The copyright header in this file uses "BootstrapBlazor & Argo Zhang (argo@live.ca)", which is inconsistent with other files in the same directory (e.g., UniverSheetData.cs, UniverSheet.razor.cs) that use "Argo Zhang (argo@163.com)". The copyright header should be consistent across all files in the component.
| // Copyright (c) BootstrapBlazor & Argo Zhang (argo@live.ca). All rights reserved. | |
| // Copyright (c) Argo Zhang (argo@163.com). All rights reserved. |
Link issues
fixes #854
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add support for configuring the UniverSheet toolbar style via a new ribbon type parameter and propagate it through to the JavaScript initializer.
New Features: