-
-
Notifications
You must be signed in to change notification settings - Fork 5
feat(UniverSheet): add IsDarkMode parameter #857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideAdds an optional IsDarkMode parameter to UniverSheet, threads it through the Blazor-to-JS interop layer, and adjusts the Univer initialization logic so theme selection and dark mode are determined consistently, with automatic dark-mode detection when not explicitly set. Sequence diagram for UniverSheet initialization with IsDarkModesequenceDiagram
participant Blazor_UniverSheet as Blazor_UniverSheet
participant JS_UniverSheetInterop as UniverSheet_razor_js
participant Univer_Module as univer_js
participant Univer_Core as Univer_library
Blazor_UniverSheet->>JS_UniverSheetInterop: InvokeVoidAsync init(Id, Interop, { theme, lang, plugins, data, darkMode, ribbonType })
JS_UniverSheetInterop->>JS_UniverSheetInterop: Build univerSheet { el, invoke, plugins, theme, lang, ribbonType, darkMode }
JS_UniverSheetInterop->>Univer_Module: createUniverSheetAsync(univerSheet)
Univer_Module->>Univer_Module: Build options { theme: sheet.theme, darkMode: sheet.darkMode, locale, locales, plugins }
Univer_Module->>Univer_Module: if options.theme == greenTheme then options.theme = UniverDesign.greenTheme else options.theme = UniverDesign.defaultTheme
Univer_Module->>Univer_Module: if options.darkMode == null then options.darkMode = (getTheme() == dark)
Univer_Module->>Univer_Core: createUniver(options)
Univer_Core-->>Univer_Module: { univer, univerAPI }
Univer_Module-->>JS_UniverSheetInterop: univerSheet initialized
JS_UniverSheetInterop-->>Blazor_UniverSheet: init completed
Class diagram for UniverSheet component with IsDarkModeclassDiagram
class UniverSheet {
+string Id
+Dictionary~string,string~ Plugins
+string Theme
+bool IsDarkMode
+string Lang
+object Data
+Enum RibbonType
+Task OnAfterRenderAsync(bool firstRender)
+Task InvokeInitAsync()
+Task PushAsync(object data)
}
class JSInterop_UniverSheet_razor_js {
+Task init(string id, object invoke, object options)
}
class JS_Univer_Module_univer_js {
+Task createUniverSheetAsync(object sheet)
}
UniverSheet --> JSInterop_UniverSheet_razor_js : uses_init
JSInterop_UniverSheet_razor_js --> JS_Univer_Module_univer_js : calls_createUniverSheetAsync
Flow diagram for theme and darkMode resolution in univer.jsflowchart TD
A[Input sheet.theme and sheet.darkMode] --> B{Is sheet.theme equal to greenTheme?}
B -- Yes --> C[Set options.theme to UniverDesign.greenTheme]
B -- No --> D[Set options.theme to UniverDesign.defaultTheme]
C --> E{Is options.darkMode null?}
D --> E
E -- Yes --> F[Call getTheme]
F --> G{Does getTheme return dark?}
G -- Yes --> H[Set options.darkMode to true]
G -- No --> I[Set options.darkMode to false]
E -- No --> J[Keep options.darkMode as provided]
H --> K[Call createUniver with resolved options]
I --> K
J --> K
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/wwwroot/univer.js:90-95` </location>
<code_context>
options.plugins.push(plugin);
}
+ if (options.theme === 'greenTheme') {
+ options.theme = UniverDesign.greenTheme;
+ }
+ else {
+ options.theme = UniverDesign.defaultTheme;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Make the theme mapping more flexible instead of hard-coding only `greenTheme` vs default.
This change maps all non-`greenTheme` values (including any future valid themes) to `defaultTheme`. Previously, any `UniverDesign[options.theme]` key was used, with `defaultTheme` only as a fallback. This will block new themes from working. Instead, resolve the theme dynamically, e.g. `options.theme = UniverDesign[options.theme] ?? UniverDesign.defaultTheme;`, and add a special case only if you need a `'greenTheme'` alias.
```suggestion
options.theme = UniverDesign[options.theme] ?? UniverDesign.defaultTheme;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (options.theme === 'greenTheme') { | ||
| options.theme = UniverDesign.greenTheme; | ||
| } | ||
| else { | ||
| options.theme = UniverDesign.defaultTheme; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Make the theme mapping more flexible instead of hard-coding only greenTheme vs default.
This change maps all non-greenTheme values (including any future valid themes) to defaultTheme. Previously, any UniverDesign[options.theme] key was used, with defaultTheme only as a fallback. This will block new themes from working. Instead, resolve the theme dynamically, e.g. options.theme = UniverDesign[options.theme] ?? UniverDesign.defaultTheme;, and add a special case only if you need a 'greenTheme' alias.
| if (options.theme === 'greenTheme') { | |
| options.theme = UniverDesign.greenTheme; | |
| } | |
| else { | |
| options.theme = UniverDesign.defaultTheme; | |
| } | |
| options.theme = UniverDesign[options.theme] ?? UniverDesign.defaultTheme; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for configuring dark mode in the UniverSheet component. Previously, the dark mode setting was handled internally, but now users can explicitly control it through a new IsDarkMode parameter. When not set, the component automatically detects the theme from the Bootstrap Blazor theme system.
Key Changes:
- Added
IsDarkModenullable boolean parameter to the UniverSheet component - Implemented automatic dark mode detection when the parameter is null
- Updated theme handling logic to properly resolve theme strings to UniverDesign objects
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| UniverSheet.razor.cs | Added IsDarkMode parameter and passed it to JavaScript initialization |
| UniverSheet.razor.js | Added darkMode to destructured options and passed it to sheet creation |
| univer.js | Implemented theme resolution logic and automatic dark mode detection using getTheme() |
| BootstrapBlazor.UniverSheet.csproj | Incremented version from 10.0.3 to 10.0.4 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public string? Theme { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// 获得/设置 是否为暗黑模式 默认 null 未设置 自动 |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment uses '暗黑模式' which should be '深色模式' for consistency with standard Chinese terminology for dark mode.
| /// 获得/设置 是否为暗黑模式 默认 null 未设置 自动 | |
| /// 获得/设置 是否为深色模式 默认 null 未设置 自动 |
Link issues
fixes #856
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add optional dark mode support and adjust theme handling for the UniverSheet component.
New Features:
Enhancements: