Skip to content

feat(web-ui): optimize provider card layout and i18n#187

Open
ymkiux wants to merge 4 commits into
mainfrom
feat/web-ui-provider-grid-layout
Open

feat(web-ui): optimize provider card layout and i18n#187
ymkiux wants to merge 4 commits into
mainfrom
feat/web-ui-provider-grid-layout

Conversation

@ymkiux
Copy link
Copy Markdown
Collaborator

@ymkiux ymkiux commented Jun 3, 2026

Summary

  • 提供商卡片改为2列网格布局,提升空间利用率
  • 统一配置状态徽章样式,缩小字号至9px
  • 补全 common.configured/notConfigured 多语言翻译
  • 移除提示词工具 tab 中硬编码消息,改用 i18n

Technical changes

  • titles-cards.css: .card-list 改用 grid 2列布局,移动端自动降级单列
  • titles-cards.css: .pill 字号从11px缩至9px,padding同步调整
  • i18n: 在 zh/en/ja/vi 四语言中添加 common.configured/notConfigured
  • i18n: 简化 ruleAck plugin desc,移除占位符
  • i18n: 新增5个模板相关 toast key(template.loadFail/empty/applied/appliedButRefreshFail/applyFail)
  • panel-config-claude.html: 统一使用 common.* namespace
  • app.methods.codex-config.mjs: 替换5处硬编码消息为 i18n 调用

Test plan

  • 配置页卡片横向2列显示
  • 移动端(<768px)降级单列
  • 徽章字号缩小,视觉紧凑
  • 多语言切换显示正常
  • 提示词工具列表不显示占位符
  • 模板操作消息支持多语言

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Expanded translations (EN/JA/VI/ZH): added common configured/not-configured labels, many new toast/notification messages (operations, templates, provider/Claude flows) and localized modal/validation text.
    • Replaced hardcoded UI messages with localized strings across multiple config and action flows.
  • Style

    • Configuration card list now uses a responsive two-column grid; status pill typography/padding tightened.
  • Chores

    • Tool configuration permissions are now persisted to local storage.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds common configured/notConfigured and many toast keys across locales; replaces hardcoded Chinese toasts with i18n lookups across app methods; updates Claude card UI and card layout; persists tool-config permissions to localStorage; tightens render memo guards; and injects i18n into unit test contexts.

Changes

i18n, Codex config toasts, UI layout, localStorage persistence, and tests

Layer / File(s) Summary
Add/modify locale keys and toast messages
web-ui/modules/i18n/locales/en.mjs, web-ui/modules/i18n/locales/ja.mjs, web-ui/modules/i18n/locales/vi.mjs, web-ui/modules/i18n/locales/zh.mjs
Adds common.configured and common.notConfigured, updates plugins.builtin.ruleAck.desc, and inserts many toast.*, validation.*, and modal.claudeDelete.* keys across locales.
Switch Codex config messages to i18n toasts
web-ui/modules/app.methods.codex-config.mjs, tests/unit/agents-modal-guards.test.mjs
Replaces hardcoded toast strings with this.t('toast.*') keys for export success, doctor/health-check success/fail, template load/empty/apply/applied/appliedButRefreshFail/applyFail, and updates related tests to include i18n context.
Localize Claude & provider flows
web-ui/modules/app.methods.claude-config.mjs, web-ui/modules/app.methods.providers.mjs, web-ui/modules/app.methods.openclaw-persist.mjs
Localizes Claude validation, add/update/delete/apply, local bridge, and provider CRUD/reset/model flows to use translated toast keys and modal strings.
Localize agents and session actions; update tests
web-ui/modules/app.methods.agents.mjs, web-ui/modules/app.methods.session-actions.mjs, tests/unit/*
Replaces hardcoded messages with this.t('toast.*') for agent editors, apply-agents, copy/share/clone/delete session actions; tests are updated to dynamically import/create i18n methods and set lang: 'zh' in mocked contexts.
Update Claude panel translation key usage
web-ui/partials/index/panel-config-claude.html, web-ui/res/web-ui-render.precompiled.js
Switches the Claude configuration card pill from claude.configured/claude.notConfigured to common.configured/common.notConfigured (template and precompiled render change).
Update card list layout and pill styling
web-ui/styles/titles-cards.css
Changes .card-list from flex column to responsive two-column CSS grid (collapses under 768px) and reduces .pill padding and fixes font-size: 9px.
Persist toolConfigPermissions to localStorage
web-ui/app.js, web-ui/modules/app.methods.startup-claude.mjs, web-ui/modules/app.methods.tool-config-permissions.mjs
toolConfigPermissions is lazily initialized from localStorage and persisted back after startup/computation and updates; storage errors are caught and ignored.
Require _cached.el in render memo reuse
web-ui/res/web-ui-render.precompiled.js
Three memoization fast-paths (session list, session preview messages, session timeline) now require _cached.el before reusing cached VNodes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • awsl233777

"🐰 I hopped through keys and toasts anew,
Replaced hard strings with translations true.
Cards now grid, small pills align,
Tests speak zh when mocked contexts shine.
A bunny cheers — messages neatly glue."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main changes: optimization of provider card layout (CSS grid) and comprehensive i18n additions/improvements across the codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/web-ui-provider-grid-layout

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
web-ui/partials/index/panel-config-claude.html (1)

157-157: ⚡ Quick win

Unify the remaining local-card pill with common.* keys.

Line 157 switches provider cards to common.configured / common.notConfigured, but the local card still uses claude.configured / claude.notConfigured on Line 138. This leaves the panel on mixed namespaces.

♻️ Suggested follow-up patch
-                            <span :class="['pill', claudeLocalBridgeConfigured() ? 'configured' : 'empty']">{{ claudeLocalBridgeConfigured() ? t('claude.configured') : t('claude.notConfigured') }}</span>
+                            <span :class="['pill', claudeLocalBridgeConfigured() ? 'configured' : 'empty']">{{ claudeLocalBridgeConfigured() ? t('common.configured') : t('common.notConfigured') }}</span>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/partials/index/panel-config-claude.html` at line 157, The local-card
pill is using claude.configured / claude.notConfigured while other provider
cards use common.configured / common.notConfigured; update the template
expression for the local card (the span that binds :class="['pill',
config.hasKey ? 'configured' : 'empty']" and its displayed translation) to use
t('common.configured') and t('common.notConfigured') instead of
t('claude.configured') / t('claude.notConfigured') so all provider cards use the
common namespace and remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@web-ui/partials/index/panel-config-claude.html`:
- Line 157: The local-card pill is using claude.configured /
claude.notConfigured while other provider cards use common.configured /
common.notConfigured; update the template expression for the local card (the
span that binds :class="['pill', config.hasKey ? 'configured' : 'empty']" and
its displayed translation) to use t('common.configured') and
t('common.notConfigured') instead of t('claude.configured') /
t('claude.notConfigured') so all provider cards use the common namespace and
remain consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c45a2724-b2ec-4826-b6c2-bb729a314a1d

📥 Commits

Reviewing files that changed from the base of the PR and between 6cd3134 and c27caa5.

📒 Files selected for processing (6)
  • web-ui/modules/i18n/locales/en.mjs
  • web-ui/modules/i18n/locales/ja.mjs
  • web-ui/modules/i18n/locales/vi.mjs
  • web-ui/modules/i18n/locales/zh.mjs
  • web-ui/partials/index/panel-config-claude.html
  • web-ui/styles/titles-cards.css
📜 Review details
🔇 Additional comments (5)
web-ui/modules/i18n/locales/en.mjs (1)

64-65: LGTM!

web-ui/modules/i18n/locales/ja.mjs (1)

65-66: LGTM!

web-ui/modules/i18n/locales/vi.mjs (1)

74-75: LGTM!

web-ui/modules/i18n/locales/zh.mjs (1)

64-65: LGTM!

web-ui/styles/titles-cards.css (1)

78-79: LGTM!

Also applies to: 84-88, 377-379

ymkiux added 2 commits June 4, 2026 01:10
## Summary
- 提供商卡片改为2列网格布局,提升空间利用率
- 统一配置状态徽章样式,缩小字号至9px
- 补全 common.configured/notConfigured 多语言翻译

## Technical changes
- titles-cards.css: .card-list 改用 grid 2列布局,移动端自动降级单列
- titles-cards.css: .pill 字号从11px缩至9px,padding同步调整
- i18n: 在 zh/en/ja/vi 四语言中添加 common.configured/notConfigured
- panel-config-claude.html: 统一使用 common.* namespace

## Test plan
- [x] 配置页卡片横向2列显示
- [x] 移动端(<768px)降级单列
- [x] 徽章字号缩小,视觉紧凑
- [x] 多语言切换显示正常
- 移除 ruleAck plugin desc 中的占位符,简化列表显示
- 新增5个模板相关 toast i18n key(zh/en/ja/vi)
- 替换 codex-config.mjs 中5处硬编码消息
@ymkiux ymkiux force-pushed the feat/web-ui-provider-grid-layout branch from c58de2d to 61849cb Compare June 3, 2026 17:11
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web-ui/modules/app.methods.codex-config.mjs`:
- Line 69: The factory createCodexConfigMethods assumes this.t exists and calls
this.t(...) in many methods (e.g., at the sites referenced), causing TypeError
in tests; fix by injecting a translator into the factory signature (e.g.,
createCodexConfigMethods({ ..., t }) with a sensible default noop/identity
translator) and replace uses of this.t(...) inside the returned methods with the
injected t(...), or alternatively update unit tests to compose
createI18nMethods() or provide a t function on the test context so the factory
no longer depends on a runtime-mixed-in i18n implementation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4c58813e-674c-4961-8f5b-3b32cba7c18d

📥 Commits

Reviewing files that changed from the base of the PR and between c27caa5 and c58de2d.

📒 Files selected for processing (5)
  • web-ui/modules/app.methods.codex-config.mjs
  • web-ui/modules/i18n/locales/en.mjs
  • web-ui/modules/i18n/locales/ja.mjs
  • web-ui/modules/i18n/locales/vi.mjs
  • web-ui/modules/i18n/locales/zh.mjs
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: ci / 0_ci (18).txt
web-ui/modules/app.methods.codex-config.mjs

[error] 833-833: TypeError: this.t is not a function at performApply.


[error] 833-833: TypeError: this.t is not a function at performApply.


[error] 833-833: TypeError: this.t is not a function at performApply.

🪛 GitHub Actions: ci / 1_ci (20).txt
web-ui/modules/app.methods.codex-config.mjs

[error] 833-833: TypeError: this.t is not a function at performApply (file .../web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate and failing test 'applyConfigTemplate force closes the modal after a successful apply' (tests/unit/agents-modal-guards.test.mjs:75:5).


[error] 833-833: TypeError: this.t is not a function at performApply (file .../web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate and failing test 'applyConfigTemplate keeps the successful apply result when only the refresh fails' (tests/unit/agents-modal-guards.test.mjs:124:5).


[error] 833-833: TypeError: this.t is not a function at performApply (file .../web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate and failing test 'applyConfigTemplate applies immediately when diff confirm is disabled' (tests/unit/agents-modal-guards.test.mjs:176:5).

🪛 GitHub Actions: ci / ci (18)
web-ui/modules/app.methods.codex-config.mjs

[error] 833-833: TypeError: this.t is not a function in performApply(file:///home/runner/work/codexmate/codexmate/web-ui/modules/app.methods.codex-config.mjs:833:43).

🪛 GitHub Actions: ci / ci (20)
web-ui/modules/app.methods.codex-config.mjs

[error] 833-833: Test failed: "applyConfigTemplate force closes the modal after a successful apply". TypeError: this.t is not a function at performApply.


[error] 833-833: Test failed: "applyConfigTemplate keeps the successful apply result when only the refresh fails". TypeError: this.t is not a function at performApply.


[error] 833-833: Test failed: "applyConfigTemplate applies immediately when diff confirm is disabled". TypeError: this.t is not a function at performApply.

🪛 GitHub Actions: ci / ci (22)
web-ui/modules/app.methods.codex-config.mjs

[error] 833-833: TypeError: this.t is not a function at performApply (web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate in tests/unit/agents-modal-guards.test.mjs:75:5.


[error] 833-833: TypeError: this.t is not a function at performApply (web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate in tests/unit/agents-modal-guards.test.mjs:124:5.


[error] 833-833: TypeError: this.t is not a function at performApply (web-ui/modules/app.methods.codex-config.mjs:833:43). Triggered by applyConfigTemplate in tests/unit/agents-modal-guards.test.mjs:176:5.

🔇 Additional comments (4)
web-ui/modules/i18n/locales/en.mjs (1)

64-65: LGTM!

Also applies to: 363-363, 390-401

web-ui/modules/i18n/locales/ja.mjs (1)

65-66: LGTM!

Also applies to: 365-365, 396-407

web-ui/modules/i18n/locales/vi.mjs (1)

9-9: LGTM!

Also applies to: 74-75, 238-259

web-ui/modules/i18n/locales/zh.mjs (1)

64-65: LGTM!

Also applies to: 363-363, 391-405

Comment thread web-ui/modules/app.methods.codex-config.mjs
• 修通 agents-modal-guards 等 7 個測試檔案的 this.t 呼叫
• 在測試 context 顯式組合 createI18nMethods() + lang: 'zh'
• 重新生成預編譯渲染(web-ui-render.precompiled.js)
• 單測全過(585 個)
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
web-ui/modules/app.methods.providers.mjs (1)

296-309: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Localize the auto-switch delete success branch too.

When delete-provider returns res.switched, this method still emits a hard-coded Chinese toast (已删除提供商,自动切换到 ...), while the non-switched branch now uses i18n. That makes the delete flow language-dependent on backend state. Please move this branch to a translated key with interpolation for provider/model.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/modules/app.methods.providers.mjs` around lines 296 - 309, Replace the
hard-coded Chinese toast in the res.switched branch with a localized message
using the same i18n pattern as the other branch: call this.t with a new
translation key (e.g., 'toast.delete.switched') and pass an interpolation object
containing provider: res.provider and model: res.model (or model: res.model ||
''), then pass that result to this.showMessage; update the call that currently
constructs the string and leave the rest of the logic (setting
this.currentProvider/this.currentModel and updating this.providersList)
unchanged so the UI text is language-agnostic when delete-provider returns
res.switched.
web-ui/modules/app.methods.claude-config.mjs (1)

181-219: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Finish localizing the remaining save/apply toasts.

This method now translates the failure path, but the success/info branches still hard-code Chinese (已保存(未填写 API Key) and Claude 配置已生效). That leaves the same save/apply flow mixed-locale for en/ja/vi users. Please route those branches through i18n as well—toast.apply.success already exists for the applied case, and the “saved without API key” branch likely needs its own key.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/modules/app.methods.claude-config.mjs` around lines 181 - 219, In
saveAndApplyConfig replace the two hard-coded Chinese toasts with i18n keys: use
this.t('toast.apply.success') instead of 'Claude 配置已生效' when apply succeeds
(where _lastAppliedClaudeKey is updated) and add/use a new i18n key for the
saved-without-API-Key message (e.g. this.t('toast.save.no_api_key') or project
naming convention) instead of '已保存(未填写 API Key)'; update any callers in this
method (this.showMessage sites) and add the new translation entries to the
locale files so all locales get the message.
🧹 Nitpick comments (1)
web-ui/app.js (1)

369-375: ⚡ Quick win

Consider validating the cached structure.

The initialization reads from localStorage and parses JSON, but doesn't validate that the result has the expected { codex: boolean, claude: boolean } shape. If the cached data is corrupted or from an incompatible version, the object might have unexpected properties or missing keys until loadAll() overwrites it.

Additionally, the localStorage key 'toolConfigPermissions' is duplicated across three files (here, startup-claude.mjs, and tool-config-permissions.mjs), violating DRY.

♻️ Proposed improvements

Extract the key to a constant in app.constants.mjs:

+export const TOOL_CONFIG_PERMISSIONS_STORAGE_KEY = 'toolConfigPermissions';

Then add validation in the initialization:

 toolConfigPermissions: (function() {
     try {
         const cached = localStorage.getItem('toolConfigPermissions');
-        if (cached) return JSON.parse(cached);
+        if (cached) {
+            const parsed = JSON.parse(cached);
+            if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+                return {
+                    codex: parsed.codex === true,
+                    claude: parsed.claude === true
+                };
+            }
+        }
     } catch (_) {}
     return { codex: false, claude: false };
 })(),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/app.js` around lines 369 - 375, The toolConfigPermissions initializer
should validate parsed localStorage data and centralize the storage key: add a
constant (e.g. TOOL_CONFIG_PERMISSIONS_KEY) in app.constants.mjs and replace the
literal 'toolConfigPermissions' usages in web-ui/app.js, startup-claude.mjs, and
tool-config-permissions.mjs; modify the IIFE that sets toolConfigPermissions to
parse JSON safely then check that the result is an object with boolean codex and
claude properties (otherwise fall back to { codex: false, claude: false }) so
corrupted or incompatible cached values are rejected until loadAll() updates
them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@web-ui/modules/app.methods.claude-config.mjs`:
- Around line 181-219: In saveAndApplyConfig replace the two hard-coded Chinese
toasts with i18n keys: use this.t('toast.apply.success') instead of 'Claude
配置已生效' when apply succeeds (where _lastAppliedClaudeKey is updated) and add/use
a new i18n key for the saved-without-API-Key message (e.g.
this.t('toast.save.no_api_key') or project naming convention) instead of
'已保存(未填写 API Key)'; update any callers in this method (this.showMessage sites)
and add the new translation entries to the locale files so all locales get the
message.

In `@web-ui/modules/app.methods.providers.mjs`:
- Around line 296-309: Replace the hard-coded Chinese toast in the res.switched
branch with a localized message using the same i18n pattern as the other branch:
call this.t with a new translation key (e.g., 'toast.delete.switched') and pass
an interpolation object containing provider: res.provider and model: res.model
(or model: res.model || ''), then pass that result to this.showMessage; update
the call that currently constructs the string and leave the rest of the logic
(setting this.currentProvider/this.currentModel and updating this.providersList)
unchanged so the UI text is language-agnostic when delete-provider returns
res.switched.

---

Nitpick comments:
In `@web-ui/app.js`:
- Around line 369-375: The toolConfigPermissions initializer should validate
parsed localStorage data and centralize the storage key: add a constant (e.g.
TOOL_CONFIG_PERMISSIONS_KEY) in app.constants.mjs and replace the literal
'toolConfigPermissions' usages in web-ui/app.js, startup-claude.mjs, and
tool-config-permissions.mjs; modify the IIFE that sets toolConfigPermissions to
parse JSON safely then check that the result is an object with boolean codex and
claude properties (otherwise fall back to { codex: false, claude: false }) so
corrupted or incompatible cached values are rejected until loadAll() updates
them.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72a15606-c68f-4c24-b8a9-8d7cf33b7b16

📥 Commits

Reviewing files that changed from the base of the PR and between 61849cb and 66aab65.

📒 Files selected for processing (16)
  • tests/unit/agents-modal-guards.test.mjs
  • tests/unit/claude-settings-sync.test.mjs
  • tests/unit/openclaw-persist-regression.test.mjs
  • tests/unit/provider-switch-regression.test.mjs
  • tests/unit/providers-validation.test.mjs
  • tests/unit/session-trash-state.test.mjs
  • tests/unit/web-ui-behavior-parity.test.mjs
  • web-ui/app.js
  • web-ui/modules/app.methods.agents.mjs
  • web-ui/modules/app.methods.claude-config.mjs
  • web-ui/modules/app.methods.openclaw-persist.mjs
  • web-ui/modules/app.methods.providers.mjs
  • web-ui/modules/app.methods.session-actions.mjs
  • web-ui/modules/app.methods.startup-claude.mjs
  • web-ui/modules/app.methods.tool-config-permissions.mjs
  • web-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (10)
web-ui/modules/app.methods.startup-claude.mjs (1)

123-131: LGTM!

web-ui/modules/app.methods.tool-config-permissions.mjs (1)

66-69: LGTM!

tests/unit/agents-modal-guards.test.mjs (1)

14-16: LGTM!

Also applies to: 26-28

web-ui/modules/app.methods.openclaw-persist.mjs (1)

277-277: LGTM!

tests/unit/openclaw-persist-regression.test.mjs (1)

11-13: LGTM!

Also applies to: 27-29

web-ui/modules/app.methods.agents.mjs (2)

63-63: LGTM!

Also applies to: 95-95, 130-130, 174-174


591-591: LGTM!

Also applies to: 645-645

web-ui/modules/app.methods.session-actions.mjs (2)

129-129: LGTM!

Also applies to: 155-155, 315-315, 320-320, 323-323, 347-347, 362-362, 373-373, 379-379, 383-383, 449-449, 455-455, 459-459, 488-488, 494-494, 498-498


527-527: LGTM!

Also applies to: 611-611

web-ui/res/web-ui-render.precompiled.js (1)

1929-1929: LGTM!

Also applies to: 2414-2414, 2660-2660, 2694-2694

…figPermissions

- Add missing locale keys for provider operations (add/delete/update/reset)
- Add missing locale keys for Claude config (validation, deletion dialog, load balancing)
- Replace all hardcoded Chinese strings with this.t() calls in:
  - web-ui/modules/app.methods.providers.mjs
  - web-ui/modules/app.methods.claude-config.mjs
- Validate toolConfigPermissions structure when reading from localStorage
  - Normalize to { codex: boolean, claude: boolean } in app.js
  - Prevents UI state issues from corrupted/legacy cache
- Add i18nMethods to test contexts in claude-settings-sync.test.mjs
  - Fixes "vm.t is not a function" test failures

All tests pass: npm run test:unit (585 passed), npm run lint (228 files)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.providers.mjs (1)

484-485: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Provider model add/remove flows still contain hardcoded Chinese toasts.

Line 484, Line 509, and Line 532 still emit fixed Chinese strings, so these paths are not localized.

Suggested fix
- return this.showMessage('请输入模型', 'error');
+ return this.showMessage(this.t('toast.provider.modelRequired'), 'error');
@@
- this.showMessage('新增模型失败', 'error');
+ this.showMessage(this.t('toast.provider.addModelFail'), 'error');
@@
- this.showMessage('删除模型失败', 'error');
+ this.showMessage(this.t('toast.provider.removeModelFail'), 'error');

Add corresponding locale keys for all supported languages.

Also applies to: 509-510, 532-533

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/modules/app.methods.providers.mjs` around lines 484 - 485, The
provider add/remove flows call this.showMessage with hardcoded Chinese strings
(e.g., '请输入模型' in app.methods.providers.mjs around the showMessage calls), so
replace those literal messages with localized keys (e.g.,
this.showMessage(this.$t('providers.enterModel'), 'error') or the project’s
i18n.t helper) and add corresponding locale entries for all supported languages;
update the three occurrences referenced (lines around the showMessage calls at
~484, ~509-510, ~532-533) to use the new locale keys so all toasts are
localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web-ui/modules/app.methods.providers.mjs`:
- Line 173: The validation messages in getProviderValidationForContext are
returning hardcoded Chinese strings which bypass i18n when showMessage uses
validation.errors.name/url/key/model; update getProviderValidationForContext to
return either i18n keys (e.g. 'provider.error.nameRequired') or return values
already passed through this.t so callers can display localized text, then adjust
usages in app.methods.providers.mjs (the showMessage call that references
validation.errors.name|url|key|model and the similar occurrence later) to rely
on those keys or localized strings; ensure consistency by mapping any existing
Chinese literals in getProviderValidationForContext to translation keys and/or
calling this.t before returning.
- Line 473: The success message in resetConfig uses a hardcoded Chinese backup
suffix; change the code that builds the backup string so it uses a translation
key (e.g., call this.t('toast.provider.resetBackupSuffix', { backup }) or
similar) instead of the literal "(已备份: ...)", and pass that localized suffix
into this.t('toast.provider.resetSuccess', { backup }); also add the
corresponding toast.provider.resetBackupSuffix entries to each locale file
(en/ja/vi/etc.) so the suffix text is localized.

---

Outside diff comments:
In `@web-ui/modules/app.methods.providers.mjs`:
- Around line 484-485: The provider add/remove flows call this.showMessage with
hardcoded Chinese strings (e.g., '请输入模型' in app.methods.providers.mjs around the
showMessage calls), so replace those literal messages with localized keys (e.g.,
this.showMessage(this.$t('providers.enterModel'), 'error') or the project’s
i18n.t helper) and add corresponding locale entries for all supported languages;
update the three occurrences referenced (lines around the showMessage calls at
~484, ~509-510, ~532-533) to use the new locale keys so all toasts are
localized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: df5a7b1d-30f4-4502-a1b0-ff6b1b376b8d

📥 Commits

Reviewing files that changed from the base of the PR and between 66aab65 and 3dc5307.

📒 Files selected for processing (8)
  • tests/unit/claude-settings-sync.test.mjs
  • web-ui/app.js
  • web-ui/modules/app.methods.claude-config.mjs
  • web-ui/modules/app.methods.providers.mjs
  • web-ui/modules/i18n/locales/en.mjs
  • web-ui/modules/i18n/locales/ja.mjs
  • web-ui/modules/i18n/locales/vi.mjs
  • web-ui/modules/i18n/locales/zh.mjs
✅ Files skipped from review due to trivial changes (3)
  • web-ui/modules/i18n/locales/zh.mjs
  • web-ui/modules/i18n/locales/vi.mjs
  • web-ui/modules/app.methods.claude-config.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
  • web-ui/app.js
  • web-ui/modules/i18n/locales/ja.mjs
📜 Review details
🔇 Additional comments (2)
web-ui/modules/i18n/locales/en.mjs (1)

402-433: LGTM!

Also applies to: 64-65

tests/unit/claude-settings-sync.test.mjs (1)

9-11: LGTM!

Also applies to: 271-275, 315-319, 572-576, 634-638, 684-688, 722-726, 771-775

const validation = getProviderValidationForContext(this, 'add');
if (!validation.ok) {
return this.showMessage(validation.errors.name || validation.errors.url || validation.errors.key || validation.errors.model || '名称、URL、API Key 和模型名称必填', 'error');
return this.showMessage(validation.errors.name || validation.errors.url || validation.errors.key || validation.errors.model || this.t('toast.provider.fieldsRequired'), 'error');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validation errors still bypass i18n and return hardcoded Chinese text.

Line 173 and Line 409 prioritize validation.errors.*, but those strings are hardcoded in getProviderValidationForContext, so non-zh users still get Chinese validation messages.

Suggested fix
 function getProviderValidationForContext(vm, mode = 'add') {
+    const tr = (key, fallback) => (vm && typeof vm.t === 'function' ? vm.t(key) : fallback);
@@
     if (mode === 'add') {
         if (!name) {
-            errors.name = '名称不能为空';
+            errors.name = tr('validation.provider.nameRequired', 'Name is required');
         } else if (!isValidProviderNameInputValue(name)) {
-            errors.name = '名称仅支持字母/数字/._-';
+            errors.name = tr('validation.provider.namePattern', 'Name only supports letters/numbers/._-');
         } else if (isReservedProviderCreationNameInput(name)) {
-            errors.name = 'codexmate-proxy 为保留名称,不可手动添加';
+            errors.name = tr('validation.provider.nameReserved', 'Reserved name cannot be added manually');
         } else if (findProviderByName(vm.providersList, name)) {
-            errors.name = '名称已存在';
+            errors.name = tr('validation.provider.nameExists', 'Name already exists');
         }
     } else if (!editingName) {
-        errors.name = '提供商名称不能为空';
+        errors.name = tr('validation.provider.nameRequired', 'Provider name is required');
     }
@@
     if (!url) {
-        errors.url = 'URL 必填';
+        errors.url = tr('validation.provider.urlRequired', 'URL is required');
     } else if (!isValidProviderUrlInputValue(url)) {
-        errors.url = 'URL 仅支持 http/https';
+        errors.url = tr('validation.provider.urlHttpOnly', 'URL only supports http/https');
     }
@@
     if (mode === 'add' && !key) {
-        errors.key = 'API Key 必填';
+        errors.key = tr('validation.provider.apiKeyRequired', 'API Key is required');
     }
@@
     if (mode === 'add' && !model) {
-        errors.model = '模型名称必填';
+        errors.model = tr('validation.provider.modelRequired', 'Model is required');
     }

Also applies to: 409-409

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/modules/app.methods.providers.mjs` at line 173, The validation
messages in getProviderValidationForContext are returning hardcoded Chinese
strings which bypass i18n when showMessage uses
validation.errors.name/url/key/model; update getProviderValidationForContext to
return either i18n keys (e.g. 'provider.error.nameRequired') or return values
already passed through this.t so callers can display localized text, then adjust
usages in app.methods.providers.mjs (the showMessage call that references
validation.errors.name|url|key|model and the similar occurrence later) to rely
on those keys or localized strings; ensure consistency by mapping any existing
Chinese literals in getProviderValidationForContext to translation keys and/or
calling this.t before returning.

}
const backup = res.backupFile ? `(已备份: ${res.backupFile})` : '';
this.showMessage(`配置已重装${backup}`, 'success');
this.showMessage(this.t('toast.provider.resetSuccess', { backup }), 'success');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

resetConfig success message still injects a hardcoded Chinese backup suffix.

Line 473 uses toast.provider.resetSuccess but backup is built as (已备份: ...) earlier, producing mixed-language output for en/ja/vi locales.

Suggested fix
-const backup = res.backupFile ? `(已备份: ${res.backupFile})` : '';
+const backup = res.backupFile
+    ? this.t('toast.provider.resetBackupSuffix', { file: res.backupFile })
+    : '';
 this.showMessage(this.t('toast.provider.resetSuccess', { backup }), 'success');

Also add toast.provider.resetBackupSuffix in each locale.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web-ui/modules/app.methods.providers.mjs` at line 473, The success message in
resetConfig uses a hardcoded Chinese backup suffix; change the code that builds
the backup string so it uses a translation key (e.g., call
this.t('toast.provider.resetBackupSuffix', { backup }) or similar) instead of
the literal "(已备份: ...)", and pass that localized suffix into
this.t('toast.provider.resetSuccess', { backup }); also add the corresponding
toast.provider.resetBackupSuffix entries to each locale file (en/ja/vi/etc.) so
the suffix text is localized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant