fix(ui): 修复删除弹窗崩溃并清理 lint 阻塞#1
Conversation
删除确认弹窗中的 AlertDialogAction 通过 Button asChild 组合 Radix Slot，按钮内部额外渲染 loading 图标会让 Slot 收到多个子节点并导致 /ai-providers 删除操作崩溃，本次修复 asChild 场景的子节点结构。 变更： - 将 loading 图标抽出为可复用节点 - 在 asChild 且 loading 时把图标注入唯一的 React 子元素 - 保持普通 button 场景继续渲染 loading 图标与原 children 影响： - 修复 /ai-providers 点击删除时的 Radix Slot 崩溃 - 保留 Button 组件现有 loading 表现，降低对其他按钮的回归风险
📝 WalkthroughWalkthroughExtracts CVA variant definitions into new modules, updates component prop typings and module exports, centralizes sidebar context into a new hook, refactors Button loading/asChild rendering, and refactors AuthFilesPage selection/persistence and ConfigPage's handleSave to use callbacks. ChangesUI variant extraction, sidebar context, button render, and pages refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Button component to support a loading state when asChild is true by cloning the child element and injecting a loader icon. The review feedback points out a potential React key warning when cloning elements with multiple children and suggests using React.Children.toArray to safely flatten and key the children.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const slottedChildren = | ||
| asChild && loader && React.isValidElement(children) | ||
| ? React.cloneElement( | ||
| children, | ||
| undefined, | ||
| loader, | ||
| (children.props as { children?: React.ReactNode }).children | ||
| ) | ||
| : children |
There was a problem hiding this comment.
When asChild is true and the slotted element contains multiple children (e.g., <Button asChild><a><Icon /> <span>Text</span></a></Button>), children.props.children will be an array. Passing this array directly as an argument to React.cloneElement alongside loader will cause React to treat it as a nested array and trigger a console warning:
Warning: Each child in a list should have a unique "key" prop.
To prevent this warning, we can flatten and safely key the children using React.Children.toArray and spread them into React.cloneElement.
| const slottedChildren = | |
| asChild && loader && React.isValidElement(children) | |
| ? React.cloneElement( | |
| children, | |
| undefined, | |
| loader, | |
| (children.props as { children?: React.ReactNode }).children | |
| ) | |
| : children | |
| const slottedChildren = | |
| asChild && loader && React.isValidElement(children) | |
| ? React.cloneElement( | |
| children, | |
| undefined, | |
| loader, | |
| ...React.Children.toArray((children.props as { children?: React.ReactNode }).children) | |
| ) | |
| : children |
前端 lint 当前被 AuthFilesPage 同步 setState、Fast Refresh 非组件导出以及 ConfigPage 不稳定 hook 依赖阻塞，本次集中修复这些已知问题以恢复 lint 通过。 变更： - 将 AuthFilesPage 持久化状态改为 lazy initializer，并把批量操作栏打开动作移到用户事件 - 拆出 Button、Toggle variants 与 Sidebar context，收窄 UI 组件文件导出 - 将 ConfigPage 的 handleSave 包装为 useCallback，稳定 pageActions 依赖 影响： - bun run lint 可通过，减少 Fast Refresh 与 hooks 规则噪声 - 保留批量操作栏动画与现有按钮样式行为
LeekJay
changed the title
在 Claude 和 OpenAI Compatibility 的新建/编辑表单里，测试模型的“自动”选项会生成空字符串项，触发 Radix Select 的约束并中断表单。改用非空哨兵值后，界面仍保留自动选择语义，保存和连通性测试都继续按空值处理。 变更： - 为自动测试模型选项引入 `__auto__` 哨兵值 - 将 Select 当前值映射为哨兵，避免向 `Select.Item` 传入空字符串 - 在选择变化时把哨兵还原为空字符串，保持现有持久化语义 影响： - 新建和编辑 Claude / OpenAI Compatibility 提供商时不再直接报错 - 自动测试模型的交互和保存结果保持不变
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/features/providers/sheets/forms/BaseProviderForm.tsx`:
- Line 69: The constant AUTO_TEST_MODEL_VALUE ('__auto__') can collide with a
real model name; change the approach in BaseProviderForm by making the sentinel
collision-proof: either generate a unique sentinel per options set (e.g., append
a UUID or index) or escape/unescape real model values before mapping to the
sentinel, and update all places that map model values back in onChange (see
usages around AUTO_TEST_MODEL_VALUE and the mapping logic at the regions
corresponding to lines 313-315 and 616-618) so that selection and save correctly
round-trip without losing a real model named '__auto__'.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41319ba0-f633-4277-b3c8-d0aad1b5bfd7
📒 Files selected for processing (1)
src/features/providers/sheets/forms/BaseProviderForm.tsx
| apiKey: '', | ||
| proxyUrl: '', | ||
| }); | ||
| const AUTO_TEST_MODEL_VALUE = '__auto__'; |
There was a problem hiding this comment.
Avoid sentinel collision with real model values
Using a fixed sentinel ('__auto__') can corrupt intent when a real model name matches it: selecting that model is converted to '' on save. Please make the auto sentinel collision-proof per options set (or escape/unescape model values) before mapping back in onChange.
Suggested direction
-const AUTO_TEST_MODEL_VALUE = '__auto__';
+const AUTO_TEST_MODEL_VALUE_BASE = '__auto__';
-const testModelOptions = useMemo(() => {
+const { testModelOptions, autoTestModelValue } = useMemo(() => {
const seen = new Set<string>();
const names: string[] = [];
...
- const opts: Array<{ value: string; label: string }> = [
- { value: AUTO_TEST_MODEL_VALUE, label: autoLabel },
- ];
+ let autoTestModelValue = AUTO_TEST_MODEL_VALUE_BASE;
+ while (seen.has(autoTestModelValue)) autoTestModelValue += '_';
+ const opts: Array<{ value: string; label: string }> = [
+ { value: autoTestModelValue, label: autoLabel },
+ ];
...
- return opts;
+ return { testModelOptions: opts, autoTestModelValue };
}, [form.models, form.testModel, t]);
-const testModelValue = form.testModel?.trim() ? form.testModel : AUTO_TEST_MODEL_VALUE;
+const testModelValue = form.testModel?.trim() ? form.testModel : autoTestModelValue;
-onChange={(value) => updateField('testModel', value === AUTO_TEST_MODEL_VALUE ? '' : value)}
+onChange={(value) => updateField('testModel', value === autoTestModelValue ? '' : value)}Also applies to: 313-315, 616-618
🤖 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 `@src/features/providers/sheets/forms/BaseProviderForm.tsx` at line 69, The
constant AUTO_TEST_MODEL_VALUE ('__auto__') can collide with a real model name;
change the approach in BaseProviderForm by making the sentinel collision-proof:
either generate a unique sentinel per options set (e.g., append a UUID or index)
or escape/unescape real model values before mapping to the sentinel, and update
all places that map model values back in onChange (see usages around
AUTO_TEST_MODEL_VALUE and the mapping logic at the regions corresponding to
lines 313-315 and 616-618) so that selection and save correctly round-trip
without losing a real model named '__auto__'.
1 participant
Summary
Verification
Note: Windows local build still prints the existing git describe /dev/null path noise before Vite build, but the build exits successfully.
Summary by CodeRabbit