fix(providers): 编辑/创建供应商弹窗失焦/点击外部不再关闭(Esc 仍可关闭)#1272
Conversation
Provider create and edit dialogs contain long forms that were previously dismissed by accidental outside clicks, window focus loss, or the Escape key, leading to potential data loss. Introduce explicitCloseOnlyDialogProps to neutralize implicit close triggers and apply it across all provider and vendor key dialogs. The dialogs now only close via the explicit close button, cancel, or successful submission. Includes unit tests for the new dialog utility.
📝 WalkthroughWalkthrough该 PR 新增 dialog 外部交互防护机制，创建 ProviderFormDialogContent 包装器组件预设布局与防护，并在四个 provider 相关组件中将 DialogContent 替换为该新组件，统一处理对话框外部交互关闭行为。 ChangesDialog 防护与 Provider 组件集成
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 introduces a utility helper explicitCloseOnlyDialogProps to prevent dialogs from closing accidentally when a user clicks outside or presses the Escape key. This utility is applied to several provider-related dialogs (add, edit, clone, and vendor keys) to protect long forms from being discarded. Unit tests have also been added to verify the behavior of these handlers. There are no review comments, so I have no additional feedback to provide.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR. The change is narrow, well-scoped, and mechanically consistent: a single utility (explicitCloseOnlyDialogProps) that calls preventDefault() on two Radix Dialog implicit-close events, spread onto all 6 provider form dialogs. Unit tests cover both handlers and the public surface. No type safety, security, error handling, or standards concerns.
PR Size: S
- Lines changed: 79
- Files changed: 6
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Claude AI
The previous explicitCloseOnlyDialogProps helper blocked the Escape key along with outside clicks and window blur, which was too restrictive for users trying to dismiss long forms. Rename the helper to preventCloseOnOutsideInteraction and drop the onEscapeKeyDown interceptor. This keeps the protection against accidental click-aways and focus loss while letting Escape close the dialog as expected.
ding113
changed the title
![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: 1e0bcfaefa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| export const preventCloseOnOutsideInteraction = { | ||
| // Covers both pointer-down-outside and focus-outside (window/tab blur). | ||
| // Escape is intentionally left untouched so it still dismisses the dialog. |
There was a problem hiding this comment.
For the provider create/edit dialogs using this helper, leaving Escape untouched means Radix still performs its default onOpenChange(false) when the user presses Escape. That still discards the long form without an explicit close action, so the new “explicit close only” behavior is not enforced for keyboard input; add an onEscapeKeyDown handler that calls preventDefault() if Escape should no longer close these dialogs.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
The file dialog-explicit-close.test.ts is renamed to dialog-prevent-close-on-outside-interaction.test.ts to align with the preventCloseOnOutsideInteraction helper it actually covers.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Consolidate the preventCloseOnOutsideInteraction guard and standard layout classes into a shared ProviderFormDialogContent component. This ensures all provider create, edit, and clone dialogs consistently prevent accidental dismissal via outside clicks or window blur without requiring manual prop spreading. Remove the standalone unit test for the interaction guard since the behavior is now encapsulated within the component.
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/app/`[locale]/settings/providers/_components/add-provider-dialog.tsx:
- Line 10: 当前提交新增的导入使用了相对路径（例如 import { ProviderFormDialogContent } from
"./provider-form-dialog-content";）违背仓库约定，应改为使用项目别名 '`@/`'. 在文件中定位包含
ProviderFormDialogContent 的 import 语句以及另外三处新增相对导入，并将它们替换为以 '`@/`...'
开头的别名导入（保持原导出符号名不变），确保所有四处导入统一使用 alias 风格。
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0837119b-e496-4372-8fd3-47cb22784d63
📒 Files selected for processing (5)
src/app/[locale]/settings/providers/_components/add-provider-dialog.tsxsrc/app/[locale]/settings/providers/_components/provider-form-dialog-content.tsxsrc/app/[locale]/settings/providers/_components/provider-manager.tsxsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/[locale]/settings/providers/_components/vendor-keys-compact-list.tsx
| import { Dialog, DialogContent, DialogTitle, DialogTrigger } from "@/components/ui/dialog"; | ||
| import { Dialog, DialogTitle, DialogTrigger } from "@/components/ui/dialog"; | ||
| import { ProviderForm } from "./forms/provider-form"; | ||
| import { ProviderFormDialogContent } from "./provider-form-dialog-content"; |
There was a problem hiding this comment.
跨文件同一根因：新增导入统一使用了相对路径。
这 4 处新增导入都偏离了仓库 @/ 别名规范，建议一次性统一替换，避免后续继续扩散相对路径风格。
As per coding guidelines: **/*.{ts,tsx,js,jsx}: Use path alias @/ to map to ./src/ for imports.
🤖 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/app/`[locale]/settings/providers/_components/add-provider-dialog.tsx at
line 10, 当前提交新增的导入使用了相对路径（例如 import { ProviderFormDialogContent } from
"./provider-form-dialog-content";）违背仓库约定，应改为使用项目别名 '`@/`'. 在文件中定位包含
ProviderFormDialogContent 的 import 语句以及另外三处新增相对导入，并将它们替换为以 '`@/`...'
开头的别名导入（保持原导出符号名不变），确保所有四处导入统一使用 alias 风格。
Source: Coding guidelines
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
问题
编辑/创建供应商的弹窗在窗口失焦或点击弹窗外部时会被关闭,导致填写中的长表单内容意外丢失。期望:失焦/点击外部不再关闭;关闭按钮 / 取消 / 提交成功 / Esc 仍可正常关闭。
修复
行为原语
preventCloseOnOutsideInteraction(src/lib/utils/dialog.ts):仅中和 Radix 的onInteractOutside(同时覆盖点击外部 + 窗口/标签页失焦),不拦截 Esc。封装组件
ProviderFormDialogContent(code-review 驱动的深层修复):把上面的关闭行为 与 6 处供应商表单弹窗重复的 shell className(max-h-[var(--cch-viewport-height-90)] flex flex-col overflow-hidden p-0 gap-0)一起内聚成一个组件,各站点只传各自的max-w-*。这样新增供应商弹窗不可能再漏掉该行为——不存在可被复制的裸<DialogContent>(消除"加站点忘加行为"的回归类)。应用到全部 6 个承载
<ProviderForm>的创建/编辑/克隆弹窗:add-provider-dialog、provider-manager(编辑)、vendor-keys-compact-list ×2、provider-rich-list-item(编辑 + 克隆)。范围说明
provider-list-item.legacy.tsx为无引用死代码,未改动。provider-batch-dialog.tsx,mode="batch")是独立功能,且其外部交互守卫是"提交中才阻止关闭"的不同语义,本 PR 未改。代码评审(/code-review max effort)
对本改动做了多代理 + 机器人交叉评审:无正确性缺陷——针对实际安装的 Radix 源码(
react-dismissable-layer@1.1.11)验证了内层 portal 子组件(Select/Popover/Combobox/嵌套 Dialog)注册在context.branches中、被排除在父弹窗的 outside 检测之外,故拦截onInteractOutside对它们是 no-op,无嵌套 UI 回归;Esc 行为保留;类型可赋值;6 站点全覆盖。评审提出的 altitude 建议(封装组件替代逐处 spread)已采纳实现。测试与验证
tests/unit/dialog-prevent-close-on-outside-interaction.test.ts(3 例):onInteractOutside调用preventDefault、不注册onEscapeKeyDown(Esc 仍可关)、仅暴露这一个守卫。build/typecheck/lint/test(全量 6439 passed)。🤖 Generated with Claude Code
Greptile Summary
This PR prevents provider create/edit dialogs from closing when clicking outside or on window/tab blur — protecting long in-progress form data from accidental loss. It introduces a shared
preventCloseOnOutsideInteractionutility and aProviderFormDialogContentwrapper component, then applies both to all 6 provider form dialogs.src/lib/utils/dialog.ts— new utility that callsevent.preventDefault()ononInteractOutside, covering both pointer-outside and focus-outside events while leaving Escape untouched.provider-form-dialog-content.tsx— thin wrapper aroundDialogContentthat bakes in the layout shell classes and the no-outside-close behavior, ensuring future dialogs cannot forget the protection.Confidence Score: 5/5
Safe to merge — the change is narrowly scoped to provider form dialogs and cannot affect unrelated dialog behavior.
All 6 call sites are migrated consistently, the new utility is simple and well-tested, and existing close paths (Escape, close button, cancel, successful submit) are untouched. The only minor concern is that the spread order in
ProviderFormDialogContentwould silently override a caller-suppliedonInteractOutside, but no current caller passes one.No files require special attention.
Important Files Changed
preventCloseOnOutsideInteraction; correctly usesonInteractOutside+preventDefaultto cover both click-outside and focus-outside without blocking Escape.onInteractOutsideprop is silently overridden rather than composed.ProviderFormDialogContent; identical behavior preserved, no regressions.ProviderFormDialogContent; correct controlled open/close pattern retained.preventDefaultcall, absence ofonEscapeKeyDown, and key-set shape — good coverage of the utility contract.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User interaction with provider dialog] --> B{Interaction type?} B -->|Outside click / window blur| C[onInteractOutside fires] B -->|Escape key| D[onEscapeKeyDown fires] B -->|Close button / Cancel| E[Controlled onOpenChange called] B -->|Form submit success| F[onSuccess calls setOpen false] C --> G[preventDefault called] G --> H[Dialog stays OPEN] D --> I[Default Radix behavior] I --> J[Dialog closes] E --> J F --> JPrompt To Fix All With AI
Reviews (4): Last reviewed commit: "refactor(providers): extract ProviderFor..." | Re-trigger Greptile