-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: add palette config types for plugin color selection #4650
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
base: master
Are you sure you want to change the base?
Conversation
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.
嗨,我在这里给出了一些高层次的反馈:
PaletteEditor.vue中的颜色解析和转换辅助方法(hex/rgb/hsv 的解析和转换)体量比较大,建议抽取到一个共享的工具模块中,这样可以复用、更容易测试,也能让该组件更专注于 UI 逻辑。PaletteEditor.vue中的剪贴板处理假设navigator.clipboard一定可用,并使用了一个硬编码的英文回退消息("Copy failed");更安全的做法是先判断 API 是否可用,并将所有面向用户的文案都通过 i18n 层处理。- 当主文本字段通过直接输入(
onDirectInput)发生变化时,内部的拾色器/预览状态要等到菜单打开才会同步;建议在输入变更时立刻解析并同步该值,这样预览和校验就能在弹出层之外的用户编辑场景下保持一致。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The color parsing and conversion helpers (hex/rgb/hsv parsing and transforms) in `PaletteEditor.vue` are quite substantial; consider extracting them into a shared utility module so they can be reused, more easily tested, and keep the component focused on UI logic.
- Clipboard handling in `PaletteEditor.vue` assumes `navigator.clipboard` is available and uses a hard-coded English fallback message (`'Copy failed'`); it would be safer to guard for API availability and route all user-visible text through the i18n layer.
- When the main text field value changes via direct input (`onDirectInput`), the internal picker/preview state is not synchronized until the menu opens; consider parsing and syncing that value immediately so the preview and validation stay consistent with user edits outside the popup.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据这些反馈改进后续的审查。
Original comment in English
Hey - I've left some high level feedback:
- The color parsing and conversion helpers (hex/rgb/hsv parsing and transforms) in
PaletteEditor.vueare quite substantial; consider extracting them into a shared utility module so they can be reused, more easily tested, and keep the component focused on UI logic. - Clipboard handling in
PaletteEditor.vueassumesnavigator.clipboardis available and uses a hard-coded English fallback message ('Copy failed'); it would be safer to guard for API availability and route all user-visible text through the i18n layer. - When the main text field value changes via direct input (
onDirectInput), the internal picker/preview state is not synchronized until the menu opens; consider parsing and syncing that value immediately so the preview and validation stay consistent with user edits outside the popup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The color parsing and conversion helpers (hex/rgb/hsv parsing and transforms) in `PaletteEditor.vue` are quite substantial; consider extracting them into a shared utility module so they can be reused, more easily tested, and keep the component focused on UI logic.
- Clipboard handling in `PaletteEditor.vue` assumes `navigator.clipboard` is available and uses a hard-coded English fallback message (`'Copy failed'`); it would be safer to guard for API availability and route all user-visible text through the i18n layer.
- When the main text field value changes via direct input (`onDirectInput`), the internal picker/preview state is not synchronized until the menu opens; consider parsing and syncing that value immediately so the preview and validation stay consistent with user edits outside the popup.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
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 - 我在这里给出了一些高层次的反馈:
- 与调色板相关的三个类型(
palette、palette_rgb、palette_hsv)在多个位置被硬编码(后端校验、默认映射、前端渲染器);建议将它们集中到一个共享的常量或枚举中,这样在添加或修改调色板格式时可以避免出现不一致。 - 在
PaletteEditor.vue中,任何无效的直接输入仍然会更新modelValue,并且只展示一个校验提示气泡;如果后端对每种类型都要求严格格式化的值,你可能需要在前端对输入进行规范化或直接拒绝,而不是原样传递无效输入。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The three palette-related types ('palette', 'palette_rgb', 'palette_hsv') are hard-coded in several places (backend validation, default maps, front-end renderer); consider centralizing them in a shared constant/enum to avoid divergence when adding or changing palette formats.
- In `PaletteEditor.vue`, any invalid direct input still updates `modelValue` and only shows a validation tooltip; if backend expects strictly formatted values per type, you may want to normalize or reject invalid input instead of propagating it unchanged.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The three palette-related types ('palette', 'palette_rgb', 'palette_hsv') are hard-coded in several places (backend validation, default maps, front-end renderer); consider centralizing them in a shared constant/enum to avoid divergence when adding or changing palette formats.
- In
PaletteEditor.vue, any invalid direct input still updatesmodelValueand only shows a validation tooltip; if backend expects strictly formatted values per type, you may want to normalize or reject invalid input instead of propagating it unchanged.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The three palette-related types ('palette', 'palette_rgb', 'palette_hsv') are hard-coded in several places (backend validation, default maps, front-end renderer); consider centralizing them in a shared constant/enum to avoid divergence when adding or changing palette formats.
- In `PaletteEditor.vue`, any invalid direct input still updates `modelValue` and only shows a validation tooltip; if backend expects strictly formatted values per type, you may want to normalize or reject invalid input instead of propagating it unchanged.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
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 - 我在这里给了一些总体反馈:
- 新的
palette、palette_rgb和palette_hsv类型标识符在多个地方被重复为字符串字面量(Python 校验、默认映射、Vue 组件);建议在后端和前端分别集中到共享的常量/枚举中,以减少拼写错误的风险并保持行为一致。 - 在
TemplateListEditor和default.py中,palette_rgb和palette_hsv的默认值是空字符串,而 UI 期望的是结构化格式(例如rgb(...)、hsv(...));建议将默认值与期望格式对齐,或者在编辑器中显式处理空值情况,以避免出现不一致的初始状态。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- 新的 `palette`、`palette_rgb` 和 `palette_hsv` 类型标识符在多个地方被重复为字符串字面量(Python 校验、默认映射、Vue 组件);建议在后端和前端分别集中到共享的常量/枚举中,以减少拼写错误的风险并保持行为一致。
- 在 `TemplateListEditor` 和 `default.py` 中,`palette_rgb` 和 `palette_hsv` 的默认值是空字符串,而 UI 期望的是结构化格式(例如 `rgb(...)`、`hsv(...)`);建议将默认值与期望格式对齐,或者在编辑器中显式处理空值情况,以避免出现不一致的初始状态。帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈来改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- The new
palette,palette_rgb, andpalette_hsvtype identifiers are repeated as string literals in multiple places (Python validation, default maps, Vue components); consider centralizing them in a shared constant/enum on both backend and frontend to reduce the risk of typos and keep behavior in sync. - In
TemplateListEditoranddefault.pythe default values forpalette_rgbandpalette_hsvare empty strings while the UI expects structured formats (e.g.,rgb(...),hsv(...)); consider aligning the defaults with the expected format or handling the empty case explicitly in the editor to avoid inconsistent initial states.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `palette`, `palette_rgb`, and `palette_hsv` type identifiers are repeated as string literals in multiple places (Python validation, default maps, Vue components); consider centralizing them in a shared constant/enum on both backend and frontend to reduce the risk of typos and keep behavior in sync.
- In `TemplateListEditor` and `default.py` the default values for `palette_rgb` and `palette_hsv` are empty strings while the UI expects structured formats (e.g., `rgb(...)`, `hsv(...)`); consider aligning the defaults with the expected format or handling the empty case explicitly in the editor to avoid inconsistent initial states.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
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 - 我发现了 1 个问题,并给出了一些总体性反馈:
- 你现在有两个独立的 DEFAULT_VALUE_MAP 定义(一个是新加在 config_types.py 中的,另一个是 PR 描述中提到的现有 core/config/default.py 中的);建议把它们统一整合,这样后端默认配置值就只有一个权威来源。
- 在前端,ConfigType 和 ColorFormat 被硬编码在 dashboard/src/utils/color.ts,而它们在后端的对应定义位于 astrbot/core/config/config_types.py;为了避免两边逐渐偏离,可能值得引入一个简单的共享 schema 或生成步骤,而不是在前后端分别维护这些字符串枚举。
- PaletteEditor.vue 在一个基于 TypeScript 的 dashboard 中使用的是纯 JS,并且 props 使用的是字符串类型(例如 format: String);如果把这些收紧为使用 ColorFormatType/PaletteConfigType 类型并启用 lang="ts",会让这个组件更安全,也更符合整个代码库的风格。
给 AI Agent 的提示
请根据这次代码评审中的评论进行修改:
## 整体评论
- 你现在有两个独立的 DEFAULT_VALUE_MAP 定义(一个是新加在 config_types.py 中的,另一个是 PR 描述中提到的现有 core/config/default.py 中的);建议把它们统一整合,这样后端默认配置值就只有一个权威来源。
- 在前端,ConfigType 和 ColorFormat 被硬编码在 dashboard/src/utils/color.ts,而它们在后端的对应定义位于 astrbot/core/config/config_types.py;为了避免两边逐渐偏离,可能值得引入一个简单的共享 schema 或生成步骤,而不是在前后端分别维护这些字符串枚举。
- PaletteEditor.vue 在一个基于 TypeScript 的 dashboard 中使用的是纯 JS,并且 props 使用的是字符串类型(例如 format: String);如果把这些收紧为使用 ColorFormatType/PaletteConfigType 类型并启用 lang="ts",会让这个组件更安全,也更符合整个代码库的风格。
## 单条评论
### 评论 1
<location> `dashboard/src/utils/color.ts:99-100` </location>
<code_context>
+export function hsvToRgb(h: number, s: number, v: number): RgbColor {
</code_context>
<issue_to_address>
**suggestion:** 建议在 `hsvToRgb` 中对 `h`、`s` 和 `v` 做范围限制,以避免超出范围的输入导致意外结果。
目前,`hsvToRgb` 依赖调用方传入已归一化的值,这一点和会在调用前进行截断的 `parseAnyColor` 不同。如果直接用超出范围的输入调用 `hsvToRgb`,可能会产生意外输出。在函数开头添加范围限制(例如对每个参数使用 `Math.max/Math.min`)可以让它对将来的调用方更安全、更不易出错。
```suggestion
export function hsvToRgb(h: number, s: number, v: number): RgbColor {
h = Math.max(0, Math.min(360, h))
s = Math.max(0, Math.min(100, s))
v = Math.max(0, Math.min(100, v))
h /= 360
s /= 100
v /= 100
```
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈来改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- You now have two separate DEFAULT_VALUE_MAP definitions (new one in config_types.py and the existing one in core/config/default.py referenced in the PR description); consider consolidating these so there is a single source of truth for default config values on the backend.
- On the frontend, ConfigType and ColorFormat are hard-coded in dashboard/src/utils/color.ts while their backend counterparts live in astrbot/core/config/config_types.py; to avoid drift, it may be worth introducing a simple shared schema or generation step rather than maintaining these string enums in parallel.
- PaletteEditor.vue is written in plain JS in a TypeScript-based dashboard and uses string-typed props (e.g., format: String); tightening these to use the ColorFormatType/PaletteConfigType types and enabling lang="ts" would make the component safer and better aligned with the rest of the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You now have two separate DEFAULT_VALUE_MAP definitions (new one in config_types.py and the existing one in core/config/default.py referenced in the PR description); consider consolidating these so there is a single source of truth for default config values on the backend.
- On the frontend, ConfigType and ColorFormat are hard-coded in dashboard/src/utils/color.ts while their backend counterparts live in astrbot/core/config/config_types.py; to avoid drift, it may be worth introducing a simple shared schema or generation step rather than maintaining these string enums in parallel.
- PaletteEditor.vue is written in plain JS in a TypeScript-based dashboard and uses string-typed props (e.g., format: String); tightening these to use the ColorFormatType/PaletteConfigType types and enabling lang="ts" would make the component safer and better aligned with the rest of the codebase.
## Individual Comments
### Comment 1
<location> `dashboard/src/utils/color.ts:99-100` </location>
<code_context>
+export function hsvToRgb(h: number, s: number, v: number): RgbColor {
</code_context>
<issue_to_address>
**suggestion:** Consider clamping `h`, `s`, and `v` in `hsvToRgb` to avoid unexpected results for out-of-range inputs.
Right now, `hsvToRgb` relies on callers to pass normalized values, unlike `parseAnyColor`, which clamps before calling it. If `hsvToRgb` is used directly with out-of-range inputs, it can yield unexpected outputs. Adding clamping at the top of the function (e.g. with `Math.max/Math.min` for each parameter) makes it safer and less error-prone for future callers.
```suggestion
export function hsvToRgb(h: number, s: number, v: number): RgbColor {
h = Math.max(0, Math.min(360, h))
s = Math.max(0, Math.min(100, s))
v = Math.max(0, Math.min(100, v))
h /= 360
s /= 100
v /= 100
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
脑袋疼,改不动了,我只是想加个配置项,优化超出我能力范围了。如果确实还有什么需要优化的地方,来个佬捞捞我求求了 |
添加了 palette, palette_rgb, palette_hsv 新配置项,以期解决插件配置中需要保存&管理 HEX、RGB、HSV 等不同格式的颜色时麻烦、不直观的问题
Modifications / 改动点
后端 (Python)
astrbot/core/config/default.py | 修改 | DEFAULT_VALUE_MAP 添加 palette, palette_rgb, palette_hsv(新配置项)
astrbot/dashboard/routes/config.py | 修改 | 验证逻辑添加新类型支持
前端 (Vue/TypeScript)
dashboard/src/components/shared/PaletteEditor.vue | 新增 | 调色板核心组件
dashboard/src/components/shared/ConfigItemRenderer.vue | 修改 | 导入并渲染 palette 类型
dashboard/src/components/shared/TemplateListEditor.vue | 修改 | defaultValueMap 添加新类型
国际化 (i18n)
dashboard/src/i18n/locales/en-US/core/common.json | 修改 | 英文翻译
dashboard/src/i18n/locales/zh-CN/core/common.json | 修改 | 中文翻译
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.由 Sourcery 提供的摘要
在插件配置中新增可配置的调色板类型颜色字段,并在控制台中支持对其进行编辑。
新功能:
palette、palette_rgb和palette_hsv配置类型,并提供默认值。PaletteEditorUI 组件,支持在 HEX、RGB 和 HSV 格式之间选择和转换颜色,并集成剪贴板功能。增强:
Original summary in English
Summary by Sourcery
引入基于调色板的颜色配置类型,以及统一的颜色选择器 UI 用于插件配置,实现对 HEX/RGB/HSV 值的端到端支持。
新功能:
palette、palette_rgb和palette_hsv配置类型,并为它们提供共享的默认值,同时将这些类型暴露给仪表盘(dashboard)配置系统。PaletteEditor组件和颜色工具模块,使用户能够在插件配置表单中以 HEX、RGB 或 HSV 格式选择、查看和编辑颜色。改进:
Original summary in English
Summary by Sourcery
Introduce palette-based color configuration types and a unified color picker UI for plugin configuration, supporting HEX/RGB/HSV values end-to-end.
New Features:
Enhancements:
新功能:
palette、palette_rgb和palette_hsv配置类型,并支持默认值。改进:
Original summary in English
Summary by Sourcery
引入基于调色板的颜色配置类型,以及统一的颜色选择器 UI 用于插件配置,实现对 HEX/RGB/HSV 值的端到端支持。
新功能:
palette、palette_rgb和palette_hsv配置类型,并为它们提供共享的默认值,同时将这些类型暴露给仪表盘(dashboard)配置系统。PaletteEditor组件和颜色工具模块,使用户能够在插件配置表单中以 HEX、RGB 或 HSV 格式选择、查看和编辑颜色。改进:
Original summary in English
Summary by Sourcery
Introduce palette-based color configuration types and a unified color picker UI for plugin configuration, supporting HEX/RGB/HSV values end-to-end.
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
引入基于调色板的颜色配置类型,以及统一的颜色选择器 UI 用于插件配置,实现对 HEX/RGB/HSV 值的端到端支持。
新功能:
palette、palette_rgb和palette_hsv配置类型,并为它们提供共享的默认值,同时将这些类型暴露给仪表盘(dashboard)配置系统。PaletteEditor组件和颜色工具模块,使用户能够在插件配置表单中以 HEX、RGB 或 HSV 格式选择、查看和编辑颜色。改进:
Original summary in English
Summary by Sourcery
Introduce palette-based color configuration types and a unified color picker UI for plugin configuration, supporting HEX/RGB/HSV values end-to-end.
New Features:
Enhancements:
新功能:
palette、palette_rgb和palette_hsv配置类型,并支持默认值。改进:
Original summary in English
Summary by Sourcery
引入基于调色板的颜色配置类型,以及统一的颜色选择器 UI 用于插件配置,实现对 HEX/RGB/HSV 值的端到端支持。
新功能:
palette、palette_rgb和palette_hsv配置类型,并为它们提供共享的默认值,同时将这些类型暴露给仪表盘(dashboard)配置系统。PaletteEditor组件和颜色工具模块,使用户能够在插件配置表单中以 HEX、RGB 或 HSV 格式选择、查看和编辑颜色。改进:
Original summary in English
Summary by Sourcery
Introduce palette-based color configuration types and a unified color picker UI for plugin configuration, supporting HEX/RGB/HSV values end-to-end.
New Features:
Enhancements: