feat(design-system): DS-6 完成表单与确认组件复用 - #4135
Conversation
Signed-off-by: kirozeng <kirozeng@gmail.com>
Signed-off-by: kirozeng <kirozeng@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/components/settings/CustomProviderDialog.tsx | 将供应商表单迁移到共享字段和按钮组件，加入字段首错定位、稳定动态行身份及同步保存守卫。 |
| apps/desktop/src/renderer/components/settings/McpServerDialog.tsx | 使用 Radix Dialog 和共享表单控件重构 MCP 编辑界面，同时保留配置、token 与 clearToken 参数语义。 |
| apps/desktop/src/renderer/components/ui/form-field.tsx | 新增负责 label、hint、error 和 ARIA 关系的轻量 FormField 组合组件。 |
| apps/desktop/src/renderer/components/ui/button.tsx | 新增保留可访问名称和尺寸、阻止重复激活的 loading 呈现。 |
| apps/desktop/src/renderer/components/ui/input.tsx | 更新 soft focus ring、错误 ARIA 状态及密码显隐按钮的禁用和提示行为。 |
| apps/desktop/src/renderer/components/ui/confirm-dialog.tsx | 增加显式 standard 呈现分支，并保留旧调用者、动作顺序和焦点策略。 |
| apps/desktop/src/renderer/components/settings/tests/McpServerDialog.test.tsx | 覆盖字段校验、保存锁定、失败重试、token 语义、动态行身份和焦点恢复。 |
| apps/desktop/src/renderer/components/settings/tests/CustomProviderDialogAccessibility.test.tsx | 补充供应商字段错误关联、首错聚焦以及保存期间防重复和失败恢复测试。 |
| docs/design-rules/DESIGN.md | 回写 DS-6 表单、loading、普通确认、焦点及表面选择规范。 |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[供应商或 MCP 设置入口] --> B[共享 FormField + SettingsTextInput]
B --> C{客户端校验}
C -->|失败| D[字段内显示错误并聚焦首错]
C -->|通过| E[Button loading / 同步重复提交守卫]
E --> F{保存请求}
F -->|成功| G[父级关闭弹窗并刷新列表]
F -->|失败| H[恢复编辑并允许重试]
A --> I[删除操作]
I --> J[共享 ConfirmDialog standard 呈现]
Reviews (1): Last reviewed commit: "chore(design-system): DS-6 对齐最新主线规则并补交付证..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cd62f0c59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MagicLizi
added
touches:core
|
命中 UI 路径(apps/desktop/src/renderer/components/settings/CustomProviderDialog.tsx / apps/desktop/src/renderer/components/settings/McpServerDialog.tsx / apps/desktop/src/renderer/components/settings/McpServersSection.tsx 等)但 description 未附界面效果证据——建议补充改动后效果:截图/录屏,或改动后界面的 HTML 页面(```html 代码块、.html 附件或在线预览链接),便于确认界面符合 DESIGN.md 设计规范 |
|
@kirozeng 👋 这个 PR 还有 1 条 review conversation 没 resolve(apps/desktop/src/renderer/components/ui/input.tsx),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
review P2:CustomProviderDialog / McpServerDialog 的密码显隐按钮与 行删除按钮的 Tip 经 Portal 渲染,内容层默认 z-[60] 低于手写模态的 z-[10000],悬停/聚焦时提示被宿主弹窗盖住。 - Input 为 secret 眼睛按钮新增 secretTipContentClassName 透传,两张 表单的密钥/Token 输入传 z-[10001] - 行删除 Tip 传 contentClassName="z-[10001]",沿用仓内既有惯例 (SessionShareExportDialog / AccountSwitcherDialog 等) - DESIGN.md Dialog & Modal 段回写规则,DS-6 证据文件补记录 - 回归测试:两张表单悬停触发后断言可见 tooltip 层含 z-[10001] Signed-off-by: Kirozeng <kirozeng@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecafd62a62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
本轮按 review 修了表单提示被弹窗盖住的问题(P2),并补上界面效果说明。 改了什么:
验证:
界面效果:下面是静态 HTML 复刻(字段几何与配色 token 取自 cindy-light / cindy-dark,非实机截图)。保存为 <!DOCTYPE html>
<html lang="zh-CN">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>DS-6 MCP 表单 · 模态内 Tooltip 层级演示</title>
<style>
:root, [data-theme="light"] {
--page-bg: #F5F5F0; --surface-elevated: #ffffff; --card-ivory: #FDFDF8;
--confirm-bg: #FDFDF8; --confirm-title: #1A1A1A; --confirm-desc: #888883;
--confirm-btn-primary-bg: #3C3F43; --confirm-btn-primary-text: #FCFCFC;
--border-default: #d7d7d4; --text-primary: #262626; --text-secondary: #888883;
--text-placeholder: #6B6B67; --eye-icon: #6B6B67; --eye-hover: #262626;
--tooltip-bg: #3C3F43; --tooltip-text: #FFFFFF;
--overlay-modal: rgba(0,0,0,0.5); --focus-ring: #417CDD;
--btn-secondary-hover: #EEEEE9; --shadow: 0 24px 48px rgba(0,0,0,0.18);
}
[data-theme="dark"] {
--page-bg: #181818; --surface-elevated: #2c2c2a; --card-ivory: #1F1F1F;
--confirm-bg: #1F1F1F; --confirm-title: #D4D4D4; --confirm-desc: #6F6F6F;
--confirm-btn-primary-bg: #EEEEEE; --confirm-btn-primary-text: #151515;
--border-default: #3c3c3a; --text-primary: #d4d4d4; --text-secondary: #6F6F6F;
--text-placeholder: #C1C1C1; --eye-icon: #C1C1C1; --eye-hover: #d4d4d4;
--tooltip-bg: #181818; --tooltip-text: #FFFFFF;
--overlay-modal: rgba(0,0,0,0.7); --focus-ring: #417CDD;
--btn-secondary-hover: #1D1D1D; --shadow: 0 24px 48px rgba(0,0,0,0.5);
}
* { box-sizing: border-box; margin: 0; }
body { font-family: -apple-system, "PingFang SC", "Segoe UI", sans-serif; background: var(--page-bg); min-height: 100vh; padding: 24px; }
.controls { display: flex; gap: 12px; align-items: center; margin-bottom: 20px; flex-wrap: wrap; }
.controls button { border: 1px solid var(--border-default); background: var(--surface-elevated); color: var(--text-primary); border-radius: 9999px; padding: 6px 14px; cursor: pointer; font-size: 13px; }
.controls button.active { background: var(--confirm-btn-primary-bg); color: var(--confirm-btn-primary-text); }
.controls label { font-size: 13px; color: var(--text-primary); display: flex; gap: 6px; align-items: center; }
.note { font-size: 12px; color: var(--text-secondary); line-height: 1.6; max-width: 640px; margin-bottom: 8px; }
/* ── 模态层:真实结构 fixed inset-0 z-[10000](遮罩 + 面板同层) ── */
.scrim { position: fixed; inset: 0; z-index: 10000; background: var(--overlay-modal); }
.panel { position: fixed; inset: 0; z-index: 10000; display: flex; align-items: flex-start; justify-content: center; padding-top: 7vh; pointer-events: none; }
.dialog { pointer-events: auto; width: min(600px, calc(100vw - 32px)); max-height: 88vh; overflow-y: auto; background: var(--confirm-bg); border: 1px solid var(--border-default); border-radius: 12px; box-shadow: var(--shadow); padding: 16px; display: flex; flex-direction: column; gap: 12px; }
.dialog h2 { font-size: 16px; color: var(--confirm-title); font-weight: 600; }
.dialog .desc { font-size: 13px; color: var(--confirm-desc); line-height: 1.5; }
.field { display: flex; flex-direction: column; gap: 4px; }
.field label { font-size: 13px; font-weight: 500; color: var(--confirm-title); }
.field .help { font-size: 12px; color: var(--confirm-desc); line-height: 1.5; }
.input-wrap { position: relative; }
.input { width: 100%; height: 40px; border-radius: 9999px; border: 1px solid var(--border-default); background: var(--card-ivory); color: var(--text-primary); padding: 0 44px 0 12px; font-size: 14px; outline: none; }
.input:focus { border-color: var(--text-secondary); box-shadow: 0 0 0 2px rgba(65,124,221,0.5); }
.input::placeholder { color: var(--text-placeholder); }
.eye { position: absolute; right: 12px; top: 50%; transform: translateY(-50%); border: none; background: transparent; color: var(--eye-icon); cursor: pointer; border-radius: 50%; display: flex; align-items: center; justify-content: center; width: 24px; height: 24px; }
.eye:hover { color: var(--eye-hover); }
.eye:focus-visible { outline: none; box-shadow: 0 0 0 2px var(--focus-ring); }
.row { display: flex; gap: 8px; align-items: center; }
.row .input { padding-right: 12px; }
.icon-btn { flex-shrink: 0; width: 36px; height: 36px; border-radius: 9999px; border: 1px solid var(--border-default); background: var(--surface-elevated); color: var(--text-primary); cursor: pointer; display: flex; align-items: center; justify-content: center; }
.icon-btn:hover { background: var(--btn-secondary-hover); }
.icon-btn:focus-visible { outline: none; box-shadow: 0 0 0 2px var(--focus-ring); }
.legend { font-size: 13px; font-weight: 500; color: var(--confirm-title); margin-top: 2px; }
.add-link { background: none; border: none; color: var(--text-primary); font-size: 13px; cursor: pointer; padding: 0; align-self: flex-start; }
.add-link:hover { text-decoration: underline; }
footer { display: flex; justify-content: flex-end; gap: 8px; padding-top: 4px; }
.btn { height: 36px; border-radius: 9999px; padding: 0 18px; font-size: 13px; font-weight: 500; cursor: pointer; }
.btn.secondary { border: 1px solid var(--border-default); background: var(--surface-elevated); color: var(--text-primary); }
.btn.secondary:hover { background: var(--btn-secondary-hover); }
.btn.primary { border: none; background: var(--confirm-btn-primary-bg); color: var(--confirm-btn-primary-text); }
/* ── Tooltip:12px 圆角 / 13px 文本 / 深底白字,Portal 渲染在 body 下 ── */
.tooltip { position: fixed; z-index: 10001; background: var(--tooltip-bg); color: var(--tooltip-text); border-radius: 12px; padding: 6px 10px; font-size: 13px; line-height: 1.4; box-shadow: 0 4px 16px rgba(0,0,0,0.25); opacity: 0; pointer-events: none; transition: opacity 0.12s ease-out; white-space: nowrap; }
body.before .tooltip { z-index: 60; }
.has-tip { position: relative; }
.tag { display: inline-block; font-size: 11px; color: var(--text-secondary); border: 1px solid var(--border-default); border-radius: 9999px; padding: 1px 8px; margin-left: 8px; vertical-align: 1px; }
</style>
</head>
<body data-theme="light">
<div class="controls">
<b style="font-size:13px;color:var(--text-primary)">DS-6 · MCP 服务器表单(静态复刻)</b>
<button id="light" class="active">Light</button>
<button id="dark">Dark</button>
<label><input type="checkbox" id="before"> 模拟修复前(Tooltip z-60,被模态盖住)</label>
</div>
<p class="note">静态 HTML 复刻 McpServerDialog 的字段几何与 token 配色(cindy-light / cindy-dark),用于核对本轮 review 修复:模态为手写 <code>fixed inset-0 z-[10000]</code> 层,Tip 经 Portal 渲染,默认内容层 z-[60] 低于模态;修复后传 <code>contentClassName="z-[10001]"</code> / <code>secretTipContentClassName="z-[10001]"</code>。悬停或键盘聚焦(Tab)密码眼睛按钮与删除行按钮可查看提示层级。非实机截图。</p>
<div class="scrim"></div>
<div class="panel">
<div class="dialog" role="dialog" aria-label="添加 MCP 服务器">
<h2>添加 MCP 服务器</h2>
<p class="desc">仅支持远程 http / sse 型 MCP。保存后新建或重启的 Agent 会话即可调用,正在运行的 Agent 会话不受影响。</p>
<div class="field">
<label>显示名称</label>
<input class="input" placeholder="例如 我的工具箱" style="padding-right:12px">
</div>
<div class="field">
<label>端点 URL</label>
<input class="input" placeholder="https://example.com/mcp" style="padding-right:12px">
</div>
<div class="field">
<label>Bearer Token(可选)<span class="tag">已保存</span></label>
<div class="input-wrap">
<input class="input" type="password" value="sk-demo-1234" placeholder="留空将删除已存 token">
<button class="eye has-tip" data-tip="eye" aria-label="显示密钥" aria-pressed="false">
<svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M9.88 9.88a3 3 0 1 0 4.24 4.24"/><path d="M10.73 5.08A10.43 10.43 0 0 1 12 5c7 0 10 7 10 7a13.16 13.16 0 0 1-1.67 2.68"/><path d="M6.61 6.61A13.526 13.526 0 0 0 2 12s3 7 10 7a9.74 9.74 0 0 0 5.39-1.61"/><line x1="2" x2="22" y1="2" y2="22"/></svg>
</button>
</div>
<span class="help">用于鉴权:Claude 会作为 Authorization: Bearer 头发送;Codex 仅支持 Bearer 型鉴权。</span>
</div>
<fieldset style="border:none;display:flex;flex-direction:column;gap:8px">
<legend class="legend">请求头(可选)</legend>
<div class="row">
<input class="input" placeholder="Header 名" value="X-Test" style="flex:1">
<input class="input" placeholder="值" value="value" style="flex:1">
<button class="icon-btn has-tip" data-tip="remove" aria-label="删除此行 1">
<svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M3 6h18"/><path d="M19 6v14c0 1-1 2-2 2H7c-1 0-2-1-2-2V6"/><path d="M8 6V4c0-1 1-2 2-2h4c1 0 2 1 2 2v2"/></svg>
</button>
</div>
<button class="add-link">+ 添加请求头</button>
</fieldset>
<footer>
<button class="btn secondary">取消</button>
<button class="btn primary">保存</button>
</footer>
</div>
</div>
<!-- Portal 渲染到 body 的 tooltip 内容层 -->
<div class="tooltip" id="tip-eye" style="top: 0; left: 0;" role="tooltip">显示密钥</div>
<div class="tooltip" id="tip-remove" style="top: 0; left: 0;" role="tooltip">删除此行</div>
<script>
const light = document.getElementById('light'), dark = document.getElementById('dark');
light.onclick = () => { document.body.dataset.theme = 'light'; light.classList.add('active'); dark.classList.remove('active'); place(); };
dark.onclick = () => { document.body.dataset.theme = 'dark'; dark.classList.add('active'); light.classList.remove('active'); place(); };
const before = document.getElementById('before');
before.onchange = () => { document.body.classList.toggle('before', before.checked); };
// tooltip 跟随 trigger 定位(真实实现为 Radix Popper;此处仅静态演示层级)
const eyeTip = document.getElementById('tip-eye'), rmTip = document.getElementById('tip-remove');
function place() {
const eye = document.querySelector('.eye');
const rm = document.querySelector('.icon-btn');
const r1 = eye.getBoundingClientRect(), r2 = rm.getBoundingClientRect();
eyeTip.style.top = (r1.top - 34) + 'px'; eyeTip.style.left = Math.round(r1.left + r1.width/2 - eyeTip.offsetWidth/2) + 'px';
rmTip.style.top = (r2.top - 34) + 'px'; rmTip.style.left = Math.round(r2.left + r2.width/2 - rmTip.offsetWidth/2) + 'px';
}
document.querySelectorAll('.has-tip').forEach((el) => {
const tip = el.dataset.tip === 'eye' ? eyeTip : rmTip;
el.addEventListener('mouseenter', () => { place(); tip.style.opacity = 1; });
el.addEventListener('mouseleave', () => (tip.style.opacity = 0));
el.addEventListener('focus', () => { place(); tip.style.opacity = 1; });
el.addEventListener('blur', () => (tip.style.opacity = 0));
});
window.addEventListener('resize', place);
window.addEventListener('load', place);
place();
// 截图/演示用 URL 参数:?show=eye|remove 自动点亮提示,?before=1 复现修复前层级
const q = new URLSearchParams(location.search);
if (q.get('before') === '1') { before.checked = true; document.body.classList.add('before'); }
if (q.get('theme') === 'dark') { document.body.dataset.theme = 'dark'; dark.classList.add('active'); light.classList.remove('active'); place(); }
const show = q.get('show');
if (show === 'eye' || show === 'remove') {
const tip = show === 'eye' ? eyeTip : rmTip;
place(); setTimeout(() => { place(); tip.style.opacity = 1; }, 60);
}
</script>
</body>
</html>dispatch:07c79a07f8b0b71e |
review P1:向导内置供应商与预设 API Key 两个 SettingsTextInput secret 调用点同处 z-[10000] 遮罩内,Tip 默认 z-[60] 仍会被盖住;补传 secretTipContentClassName="z-[10001]"(与两张表单同批修复)。仓内 secret 调用点全景核查:两表单与向导均已抬层,ProvidersSection 的 ImageApiKeyRow 为设置页行内无模态遮罩,保持默认。 Signed-off-by: Kirozeng <kirozeng@gmail.com>
Signed-off-by: Kirozeng <kirozeng@gmail.com> # Conflicts: # docs/design-rules/design-inventory.md
|
本轮处理了 P1 反馈、CI 缺席排查与主线冲突,情况如下。 P1(向导内密钥 Tooltip 层级):已修复( CI 缺席(Windows unit tests 等 required context):核实这不是测试失败—— 主线冲突:已在本地 dispatch:1fc9248fe26310a4 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73290c4412
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
review P2 ×2: - McpServerDialog 的 transport 互斥选择从两个独立 Button + aria-pressed 改用 SettingsSegmentedControl(DESIGN.md §4 Settings segmented controls),获得单一 Tab 停靠点、方向键、Home/End 与 RTL 键盘行为, 不再自造第二套选中态视觉;测试同步改为 radiogroup/radio 断言。 - cindy-design-system.md「新贡献者从这里开始」第 3 条不再说 FormField 与 Button loading 尚未提供,改指向上方 DS-6 表单贡献入口。 Signed-off-by: Kirozeng <kirozeng@gmail.com>
|
本轮完成两条 P2 修复、主线冲突合并与 CI 恢复,情况如下。 P2 ×2(均已修复并 resolve 对应 thread):
主线冲突:已 CI 恢复:前两个 head 上 required context 缺席的根因即冲突——PR 处于 CONFLICTING 时 GitHub 不为其创建 pull_request workflow run。冲突解除后 dispatch:3390ff7e6f5f43b6 |
|
本轮无代码改动:CI 红灯经核实是 runner 负载下的 flake,已对 failed jobs 重跑确认。 上一轮已论证 当前 PR 全部 required checks 绿(DCO / Git integration / Linux ×2 / Windows ×2 / verify / pr-design-basis),merge 状态 CLEAN。review conversation 已全部 resolve。 dispatch:f77c18a8f05a02e5 |
Signed-off-by: Kirozeng <kirozeng@gmail.com> # Conflicts: # apps/desktop/src/renderer/components/settings/CustomProviderDialog.tsx # docs/design-rules/design-decision-log.md
|
本轮处理新的主线冲突:main 在 CI 转绿后又有约 10 个提交合入,PR 再次 CONFLICTING。已在本地 冲突三处及解法:
验证: dispatch:f77c18a8f05a02e5 |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Review Report: PR #4135
PR: feat(design-system): DS-6 complete form and confirm component reuse
Author: kirozeng
Head: d9daf75
Base: main (0d1a5c7)
Tier: standard
Reviewer: magiclizi (auto-review)
Date: 2026-09-09
Summary
DS-6 migrates the custom provider and MCP server settings forms to shared components (FormField, Button loading, Input enhancements), adds field-level validation with first-error focus, and opts two specific delete confirmations into the standard button presentation. Existing callers and business semantics are preserved through backward-compatible opt-in APIs.
Findings
0 P0, 0 P1 discovered.
Verification
Security and credentials
- Security content scan: pass (0 hard hits, 0 soft hits)
- No credentials, tokens, or authorization files in diff
- Existing credential handling (provider API keys, MCP tokens, safeStorage) unchanged
- clearToken/hasToken semantics preserved in McpServerDialog
Shared symbols and callers
- Button: added optional loading prop, backward compatible (defaults false)
- Input: added optional secretTipContentClassName, backward compatible
- ConfirmDialog: added optional presentation, unset callers keep existing presentation
- FormField: new component, no existing callers affected
- ConfirmDialogProvider: passes through presentation, existing calls unchanged
- Two explicit presentation standard opt-ins: ProvidersSection.handleDelete, McpServersSection.handleDelete
Error and recovery paths
- savingRef.current synchronous guard prevents duplicate save submissions
- Save failure resets both savingRef.current and setSaving(false) on all failure paths
- Image generation reload confirmation path properly resets saving state
- Field validation errors use reportFieldError with focus and scroll-into-view
- Service failures remain as toasts (not field errors)
- Cancel/Esc/overlay blocked only during actual save via ref guard
Accessibility
- FormField associates label, hint, error via proper ARIA attributes
- aria-required (not native required) prevents browser validation
- aria-invalid set on error state
- aria-busy on loading buttons
- aria-pressed on eye toggle and auth mode/wire protocol buttons
- Eye button disabled when input is disabled
- MCP transport uses SettingsSegmentedControl (radiogroup/radio) instead of independent buttons
Design rule compliance
- Focus ring: soft/50% (--focus-ring-soft) per approved DS-6 D1
- Ivory surface: explicit white-panel usage documented per DS-6 D2
- Standard confirmation: uses Button component, preserves ordering/focus per DS-6 D3
- Field errors with first-error focus per DS-6 D4
- Save lock with recovery per DS-6 D5
- Modal tooltip z-index: z-[10001] on all modal-internal tips per DS-6 review fix
- All colors use semantic tokens; no hardcoded color values introduced
- Light/Dark dual-mode: all styling through themed tokens
Test coverage
- New: McpServerDialog.test.tsx (field validation, save lock, token semantics, header identity, tooltip z-index)
- New: formControls.test.tsx (button loading, input secret disabled, confirm ordering/typed/busy)
- New: formField.test.tsx (ARIA associations, dynamic field identity)
- Extended: CustomProviderDialogAccessibility.test.tsx (field error focus, save ownership, tooltip z-index)
- Updated: input.test.tsx (soft focus ring, ivory label)
PR description consistency
- Description accurately reflects all changes
- Explicitly scoped: two forms, two delete confirmations, shared components
- Non-goals clearly stated (full settings, permissions, DS-7)
- Risk section identifies cross-platform gaps and unverified items
- Rollback plan documented (revert PR, no data migration needed)
Merge status
- CI: pass (client-ci success, pr-design-basis success)
- Maintainer signoff: all three triggers (product, rules, arch) released by zqchris
- All review threads: resolved
- Bot settle: timed out (copilot-pull-request-reviewer, chatgpt-codex-connector) -- their opinions may arrive post-merge
- mergeStateStatus: UNKNOWN at time of live context refresh
Conclusion
Clean review. 0 P0, 0 P1. Recommend merge.
Signed-off-by: Kirozeng <kirozeng@gmail.com> # Conflicts: # docs/design-rules/design-inventory.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b8e588d2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
冲突已解决,CI 全绿。 main 在上一轮 CI 转绿后又合入两批提交,PR 两度 CONFLICTING。本轮已连续两次
推送前已跑 |
review P2:面板级 onChangeCapture 原先在任意后代输入变化时清空唯一的 fieldError——Base URL 无效时改名称或 API Key 会立即消失 URL 的错误 提示与 aria-invalid,尽管 URL 未修正。现仅在 event.target 即报错字段 本身时清除;其它字段的编辑保留提示直至再次保存重新校验。回归测试 覆盖「改其它字段错误保留 / 改报错字段错误清除」两分支。 Signed-off-by: Kirozeng <kirozeng@gmail.com>
|
本轮修复字段错误清除逻辑的 P2,并核实 CI 信号。 P2(字段错误过度清除):已修复( CI 信号核实:派发时的 "Windows unit tests 缺席" 是陈旧快照—— review conversation 5/5 已 resolve,PR 仅待 review approval。 dispatch:2704bd8962b256eb |
Signed-off-by: Kirozeng <kirozeng@gmail.com> # Conflicts: # docs/design-rules/design-inventory.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4ff17f9f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
本轮解决主线冲突:main 又合入一批提交(远程桌面 #4065 等),PR 再次 CONFLICTING。已 CI 在 注:本次解冲突 push 触发了分支保护的 stale review dismiss,此前的 approval 已失效,需要重新批准后才能合并。 dispatch:d4785b024fa6c549 |
review P1:上一轮把错误清除收敛到「报错元素自身的 change」后,两个
非输入控件路径会让错误滞留到再次保存:
- 列表级错误(`${agent}:add-model`,模型列表为空,提示挂在「添加模型」
按钮旁):点该按钮新增行并填写内容时,change 目标是新行输入而非按钮,
提示不清。现在该 agent 模型行的输入变化同样视为对列表错误的修正。
- applyPreset 程序化替换名称/鉴权/全部 runtime:不触发任何输入 change,
既有错误指向整体失效。函数内显式清除。
回归测试 ×2:删光模型行保存→重添行填写即清;应用预设即清并核对
名称已被程序化替换。
Signed-off-by: Kirozeng <kirozeng@gmail.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
本轮修复上一轮错误清除语义的反例(P1):非输入控件修正字段时错误滞留。
普通输入的语义保持上一轮的收敛(只在报错字段自身被编辑时清除)。回归测试 ×2(33 项全过), dispatch:f16e1502cfe07773 |
|
已核实本轮反馈,无需代码改动:该评论是 Codex 的用量通知(code review 额度已用尽,提示仓库管理员在设置中增加 credits),属于机器人运营消息,不含任何代码审查意见。PR 当前状态不变:CI 全绿、无冲突、已获批准,等待合并。 dispatch:3371894b6adb46db |
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
Findings
- 无 P0/P1。
Rule coverage
- AGENTS.md / docs/dev-rules/development-workflow.md：范围与 PR 描述一致；未改凭证存储、MCP/provider 配置语义、协议或 CI。提交带 Signed-off-by。
- docs/design-rules/DESIGN.md §4 Buttons / Inputs & Forms / Dialog & Modal、§5、cindy-design-system.md、design-governance.md：FormField 只做 label/hint/error 关联且 required 仅 aria；Input 焦点环改为 --focus-ring-soft，错误态优先；ivory 仍为显式白面板；Button loading 保留可访问名称、aria-busy、禁用再激活；ConfirmDialog presentation=standard 为 opt-in，未选调用者走旧分支；两处删除确认显式 opt-in。模态内 Tip 抬到 z-[10001]（表单、向导密钥）。规范回写与 D1—D5 裁决一致。
- docs/dev-rules/engineering-conventions.md：未新增 IPC/日志协议；服务失败仍 Toast；i18n 走既有 key。
- docs/dev-rules/electron-security-and-process-boundaries.md：仍走既有 maker IPC / customMcpServers / customProviders；token 不改 clearToken 语义；Renderer 无 Node/凭证直写。
- docs/dev-rules/configuration-and-overrides.md：未改配置分层、默认值或 override 语义。
- docs/dev-rules/credentials-and-local-storage.md：密钥仍经既有 read/update；hydration 未完成时空 token 不清已存值（测试覆盖）。
- PR 描述吻合：两张设置表单、两处普通删除确认、FormField/Button loading/Input 焦点与文档测试；未夹带 Permission/Full Access/插件授权或依赖/CI。讨论 issue 无。既有 thread 均已 resolve，对应修复在 head 上可核对。
Verification
- 凭证/隐私门：security.pass=true，hardHits=0。
- 格式门：formatPass=true。uiEvidenceMissing=true（模板未嵌截图），作者后续评论已附静态 HTML 复刻；不阻断。
- CI：gate.ci.status=pass（client-ci / pr-design-basis success）；mergeable CLEAN。
- 本 worktree 无 node_modules，未在本 agent 重跑 pnpm test:unit:related / typecheck。CI 与作者声明的 related/typecheck 作为外部证据，不把未运行写成通过。
- botSettle：timedOut=true（copilot-pull-request-reviewer / chatgpt-codex-connector 未对当前 head 表态），按上限放行。
Overall
pass — 没有 P0/P1。
3 participants
设计系统改造系列 DS-6；路线见 docs/design-rules/design-governance.md §12。
这次改了什么
摘要
把添加/编辑自定义模型供应商、添加/编辑 MCP 服务器整理为可复用的真实设置表单。必填和格式错误显示在对应字段并聚焦首错；实际保存时阻止重复提交和关闭，失败后可继续修改重试。补最小 FormField 与 Button loading，让后续贡献者复用现有输入和按钮。
变更类型
feat新功能refactor/perf重构或性能优化docs/test/chore文档、测试或工程维护范围
UI 变化
docs/design-rules/DESIGN.md§4 Buttons / Inputs & Forms / Dialog & Modal（标准控件、就近错误、实际提交状态、elevated 默认与明确 ivory 用途）；§5 Border Radius Scale（先登记可见层，再给普通控件分档，保留 docs(design): 按可见层与登记分配圆角并明确审查边界 #4072）；§10（语义 token、Light/Dark、旧局部覆盖）；§14.2/14.3/14.6（焦点、IME、图标按钮提示）；design-governance.md§4/6/10/13（主题兼容、两级证据、D1—D5 裁决、圆角审查边界）。6d4af6733a中的表单源码。完整证据类别与未验项见 本批证据。怎么验证的
自动验证
VITE_CINDY_AUTH_REGION=global pnpm test:unit:related：退出 0。合并兼容阶段包含上游测试调度/lockfile 增量，按现行调度扩大全量；runner 506 通过、1 既有跳过，Desktop/Mobile 与全部适用 workspace unit 通过。未收集测试的包仍按既有 manifest 跳过。pnpm --filter desktop run --if-present typecheck、pnpm --filter mobile typecheck：退出 0。test:unit:related，runner 506 通过/1 既有跳过、Desktop 相关单测通过（42.7s）；Desktop typecheck 再次退出 0。最终仅调整证据文本和截图交付，14 个源码/测试与已验收实现逐文件 blob 一致。pnpm check:endpoints、check:i18n、check:brand-terminology、check:i18n-glossary、--filter desktop db:validate、ci:scheduler-guard、--filter mobile test:scope、check:design-inventory：均退出 0。35bac027eb，独立 frozen-lockfile 安装，Desktop typecheck 通过；高级模型/上下文/兼容选择 3 文件32项，最新统一模型/远程模型选择 4 文件231项全部通过。git diff --check、pnpm check:dco：通过；实现和合并提交均带 Signed-off-by。追加 fetch 至5c0e4e0da0，相对上述兼容模拟只新增 Mobile 图片缓存/分享四文件，不与本批 Desktop 改动交叉。node scripts/hardcoded-color-audit.mjs --base-ref f3203a7049：退出 1，9 项报告为4处hsl(var(--destructive))语义 token 消费和5处文档 PR 编号。逐条核实不含新增颜色值；未改扫描器/豁免，也不声称该扫描通过，交独立审查核验并留 DS-7 误报样本。.env的 CN 区域曾造成3个测试文件9项失败，命令级明确 Global 后原55项及全量门禁通过；未修改.env或测试断言。临时合并工作区补齐 lockfile 对应 Electron 二进制后，模型选择回归重新通过。手工验证
macOS arm64、Global 的独立 ds6 Desktop 开发沙箱，实际 Settings 路由。自动化验证使用虚构服务返回；用户手动审核前已移除所有 renderer 替身并重载。已覆盖的正常/错误/禁用/提交中/失败重试/成功、密码显隐、键盘焦点、嵌套弹层、长文与字号 24px、Light/Dark、旧主题及局部/全局覆盖见证据索引。真实组件样例与完整设置页分别登记；模型高级面板保留 Codex 编辑/未改不写入/恢复默认，模型选择受控样例不冒充完整生产入口。
未执行的验证
风险
风险分类
影响与回滚
提交前检查
目标：完成 DS-6 两张真实设置表单与指定普通确认的组件复用，让非设计师能按仓内规范复用，用户获得一致且状态完整的操作。
范围：自定义供应商、MCP 表单及两处删除确认，FormField、Button loading、Input soft/50% 焦点与文档/必要测试。D1—D5 已获批准，实际保存防重复与防关闭、字段首错定位；保留 local/global 主题覆盖。
非目标：全设置页迁移、Permission/Full Access/插件授权、配置/鉴权/请求结算/存储/协议/用户数据语义、依赖或 CI、Token JSON、DS-7 实施。相邻模型面板仅回归。
验收：主计划 SC-01—14 按范围记录；本地 related 与受影响包 typecheck 通过；Light/Dark 与旧主题代表场景、键盘/提交失败恢复有真实运行证据；用户已在测试版手动通过。G2 独立贡献者、公开附件、未验证平台明确待项，不以 PR 合入或 Agent 自测冒充目标全部完成。