-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): support theme variant #4747
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
Walkthrough本次更改引入了“主题变体(theme variant)”的概念,包括类型声明、主题变体的设置与获取函数、相关测试用例及全新 CSS 变量文件。还将“elevo”主题样式通过 CSS 文件集成到主题体系中,并在运行时逻辑和类型声明中增加对变体的支持。 Changes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/runtime/src/index.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs packages/runtime/src/internal/Router.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs packages/runtime/src/themeAndMode.spec.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/theme/src/elevo.css (1)
8-8: 分隔符颜色透明度值需要验证分隔符颜色使用了
rgba(222, 222, 223, 0.6)的透明度值。建议验证这个透明度在不同背景下的视觉效果是否合适。建议考虑将分隔符颜色也定义为基于主色调的变体:
- --elevo-divider-color: rgba(222, 222, 223, 0.6); + --elevo-divider-color: rgba(0, 10, 26, 0.1);这样可以与
--elevo-placeholder-color保持色调一致性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (8)
etc/runtime.api.md(2 hunks)packages/runtime/src/index.ts(1 hunks)packages/runtime/src/internal/Router.ts(2 hunks)packages/runtime/src/themeAndMode.spec.ts(2 hunks)packages/runtime/src/themeAndMode.ts(2 hunks)packages/theme/src/elevo.css(1 hunks)packages/theme/src/index.css(1 hunks)packages/types/src/manifest.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/runtime/src/internal/Router.ts (2)
packages/runtime/src/themeAndMode.ts (1)
setThemeVariant(107-120)packages/runtime/src/internal/Runtime.ts (1)
getRuntime(191-191)
packages/runtime/src/themeAndMode.ts (2)
packages/types/src/manifest.ts (1)
SiteVariant(1624-1624)packages/runtime/src/index.ts (1)
getThemeVariant(28-28)
packages/runtime/src/themeAndMode.spec.ts (2)
packages/runtime/src/themeAndMode.ts (2)
setThemeVariant(107-120)getThemeVariant(122-124)packages/runtime/src/index.ts (1)
getThemeVariant(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (16)
packages/types/src/manifest.ts (1)
1621-1624: 类型定义看起来很好!新增的
SiteVariant类型定义符合现有代码模式,与SiteTheme和SiteMode的定义结构一致。类型限制为两个字符串字面量值是合理的设计选择。packages/theme/src/index.css (1)
8-8: CSS 导入添加正确!新增的
elevo.css导入语句位置合适,与现有的主题相关导入保持一致的结构。etc/runtime.api.md (2)
41-41: 类型导入正确!正确导入了
SiteVariant类型,为新的主题变体功能提供类型支持。
236-237: 函数声明符合规范!
getThemeVariant()函数声明正确,返回类型与实现一致。注意该函数当前标记为未文档化,后续可能需要补充文档。packages/runtime/src/index.ts (1)
28-28: 导出添加正确!新增的
getThemeVariant导出与其他主题相关函数归组合理,遵循现有的导出模式。packages/runtime/src/internal/Router.ts (2)
33-33: 导入添加正确!
setThemeVariant函数导入与其他主题相关函数归组合理,遵循现有的导入模式。
408-408: 主题变体设置调用位置合适!
setThemeVariant调用位置恰当,紧跟在setTheme和setMode之后,作为主题配置流程的一部分。根据相关代码片段,该函数内部已处理无效值的情况,会默认设置为 "default",因此即使globalThemeVariant未定义或无效也能安全处理。packages/runtime/src/themeAndMode.spec.ts (2)
10-11: 函数导入命名一致性良好导入的函数使用下划线前缀以避免与测试中的局部变量冲突,命名约定与现有的 theme 和 mode 测试保持一致。
151-162: 测试设置遵循现有模式测试设置正确地重置模块并初始化 DOM 状态,与现有的 theme 和 mode 测试保持一致的模式。
packages/runtime/src/themeAndMode.ts (3)
1-1: 类型导入正确正确导入了
SiteVariant类型,与现有的SiteMode和SiteTheme类型保持一致。
105-105: 变量初始化遵循现有模式变量初始化为 "default" 值,与现有的
theme和mode变量的初始化模式保持一致。
122-124: getter 函数实现正确
getThemeVariant函数实现简洁正确,返回类型明确,与现有的getTheme()和getMode()函数保持一致。packages/theme/src/elevo.css (4)
1-17: CSS 变量命名规范良好CSS 变量使用
--elevo-前缀保持命名空间隔离,变量名描述性强且结构清晰。颜色变量同时提供了 RGB 通道和十六进制值,便于不同场景使用。
10-11: 组件背景样式设计合理组件背景使用半透明白色配合模糊效果,这是现代 UI 设计中常见的毛玻璃效果实现方式。
backdrop-filter属性提供了良好的视觉层次。
14-15: 输入框阴影使用 RGB 通道变量输入框阴影巧妙地使用了 RGB 通道变量
var(--elevo-color-brand-rgb-channel)配合透明度,这样可以保持与品牌色的一致性,同时允许透明度调整。
2-5: 确认品牌色对比度符合 WCAG AA 要求
- #2540ff 在白色背景(#ffffff)下的对比度为 5.64:1(≥4.5:1,符合 AA 普通文本要求)。
- #2540ff 在黑色背景(#000000)下的对比度为 8.76:1(符合所有文本和 UI 组件要求)。
如果在其他背景色上使用,请使用 WebAIM Contrast Checker、AccessibleWeb 等工具验证是否满足 4.5:1(文字/UI 元素)或 3:1(图形/非文字元素)的最低对比度标准。
无需对
packages/theme/src/elevo.css中定义的品牌色进行修改。
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-theme-variant
|
| Run status |
|
| Run duration | 00m 27s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
17
|
| View all changes introduced in this branch ↗︎ | |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v3 #4747 +/- ##
=======================================
Coverage 95.23% 95.23%
=======================================
Files 209 209
Lines 9120 9131 +11
Branches 1756 1758 +2
=======================================
+ Hits 8685 8696 +11
Misses 322 322
Partials 113 113
🚀 New features to boost your workflow:
|
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
Adds support for a “theme variant” concept, introducing a new SiteVariant type, CSS variables for an “elevo” variant, and runtime APIs to get/set the current variant.
- Defined
SiteVariantin type manifests. - Imported and defined CSS vars for the “elevo” variant.
- Implemented
setThemeVariant/getThemeVariant, added tests, integrated in router, and exposed API for reading the variant.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/manifest.ts | Introduced SiteVariant type |
| packages/theme/src/index.css | Imported elevo.css for new variant |
| packages/theme/src/elevo.css | Defined CSS variables for “elevo” theme variant |
| packages/runtime/src/themeAndMode.ts | Implemented setThemeVariant/getThemeVariant |
| packages/runtime/src/themeAndMode.spec.ts | Added unit tests for variant behavior |
| packages/runtime/src/internal/Router.ts | Integrated variant init into router setup |
| packages/runtime/src/index.ts | Exposed getThemeVariant in public API exports |
| etc/runtime.api.md | Documented getThemeVariant in API reference |
Comments suppressed due to low confidence (5)
packages/runtime/src/index.ts:28
- Consider also exporting setThemeVariant in the public API so consumers can programmatically switch theme variants.
getThemeVariant,
etc/runtime.api.md:237
- Add documentation for setThemeVariant to the API reference to reflect its availability and behavior.
export function getThemeVariant(): SiteVariant;
packages/runtime/src/themeAndMode.spec.ts:151
- Add a test case to cover the branch where setThemeVariant receives an invalid value and falls back to "default".
describe("variant", () => {
packages/runtime/src/themeAndMode.ts:107
- Add JSDoc comments to describe the behavior of setThemeVariant, including its fallback logic and dispatched "variant.change" event.
export function setThemeVariant(value: unknown) {
packages/runtime/src/themeAndMode.ts:115
- [nitpick] Consider namespacing the custom event (e.g., "theme.variant.change") to avoid potential conflicts with other global events.
window.dispatchEvent(
| test("should set theme variant", () => { | ||
| expect(getThemeVariant()).toEqual("default"); | ||
| expect(document.documentElement.dataset.variant).toBe("default"); | ||
| setThemeVariant("elevo"); | ||
| expect(getThemeVariant()).toEqual("elevo"); | ||
| expect(document.documentElement.dataset.variant).toBe("elevo"); | ||
| }); | ||
|
|
||
| test("should apply the current theme variant", () => { | ||
| expect(getThemeVariant()).toEqual("default"); | ||
| expect(document.documentElement.dataset.variant).toBe("default"); | ||
| setThemeVariant("elevo"); | ||
| expect(document.documentElement.dataset.variant).toBe("elevo"); | ||
| }); | ||
|
|
||
| test("should apply the specified theme variant", () => { | ||
| expect(getThemeVariant()).toEqual("default"); | ||
| expect(document.documentElement.dataset.variant).toBe("default"); | ||
| setThemeVariant("elevo"); | ||
| expect(getThemeVariant()).toEqual("elevo"); | ||
| expect(document.documentElement.dataset.variant).toBe("elevo"); | ||
| }); |
Copilot
AI
Jul 9, 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.
[nitpick] This test overlaps significantly with other variant tests; consider consolidating or removing redundant cases to streamline maintenance.
| test("should set theme variant", () => { | |
| expect(getThemeVariant()).toEqual("default"); | |
| expect(document.documentElement.dataset.variant).toBe("default"); | |
| setThemeVariant("elevo"); | |
| expect(getThemeVariant()).toEqual("elevo"); | |
| expect(document.documentElement.dataset.variant).toBe("elevo"); | |
| }); | |
| test("should apply the current theme variant", () => { | |
| expect(getThemeVariant()).toEqual("default"); | |
| expect(document.documentElement.dataset.variant).toBe("default"); | |
| setThemeVariant("elevo"); | |
| expect(document.documentElement.dataset.variant).toBe("elevo"); | |
| }); | |
| test("should apply the specified theme variant", () => { | |
| expect(getThemeVariant()).toEqual("default"); | |
| expect(document.documentElement.dataset.variant).toBe("default"); | |
| setThemeVariant("elevo"); | |
| expect(getThemeVariant()).toEqual("elevo"); | |
| expect(document.documentElement.dataset.variant).toBe("elevo"); | |
| }); | |
| test("should handle theme variant correctly", () => { | |
| // Verify initial state | |
| expect(getThemeVariant()).toEqual("default"); | |
| expect(document.documentElement.dataset.variant).toBe("default"); | |
| // Set a new theme variant and verify changes | |
| setThemeVariant("elevo"); | |
| expect(getThemeVariant()).toEqual("elevo"); | |
| expect(document.documentElement.dataset.variant).toBe("elevo"); | |
| // Verify that the current theme variant is applied correctly | |
| expect(document.documentElement.dataset.variant).toBe("elevo"); | |
| }); |
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
测试