fix(ui): 修复创建用户对话框中供应商分组下拉无法点击展开 (#1212)#1237
Conversation
📝 WalkthroughWalkthroughTagInput组件新增异步建议加载处理逻辑：suggestions从空变为非空时若输入框仍聚焦则自动展开下拉。容器点击事件增强为始终focus输入框并在有建议时强制打开下拉。测试文件完整更新mock路径、新增异步驱动工具函数、添加Portal渲染位置查询器，以多个回归用例覆盖异步时序与交互场景。 Changes异步建议加载与下拉交互修复
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 addresses issue #1212 in the TagInput component by ensuring that the suggestions dropdown automatically expands when suggestions are loaded asynchronously while the input is already focused. It also ensures that clicking on an already-focused input re-opens the dropdown. Additionally, the mock path for provider actions was corrected in the tests, and several regression tests were added to verify these behaviors under different scenarios, including inside a Dialog. There are no review comments to address, so no feedback is provided.
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}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const hadSuggestionsRef = React.useRef(suggestions.length > 0); | ||
| React.useEffect(() => { | ||
| const hasSuggestions = suggestions.length > 0; | ||
| const wasEmpty = !hadSuggestionsRef.current; | ||
| hadSuggestionsRef.current = hasSuggestions; | ||
| if (hasSuggestions && wasEmpty && inputRef.current === document.activeElement) { | ||
| setShowSuggestions(true); | ||
| } | ||
| }, [suggestions.length]); |
There was a problem hiding this comment.
suggestions array causes the length to cycle through 0 and back to non-zero (e.g. a background refresh clears then repopulates the list), the effect will auto-reopen the dropdown while the input is still focused — overriding the user's explicit dismiss. Adding a guard on showSuggestions prevents this surprise for any of the ~11 callers that pass dynamic suggestions.
| const hadSuggestionsRef = React.useRef(suggestions.length > 0); | |
| React.useEffect(() => { | |
| const hasSuggestions = suggestions.length > 0; | |
| const wasEmpty = !hadSuggestionsRef.current; | |
| hadSuggestionsRef.current = hasSuggestions; | |
| if (hasSuggestions && wasEmpty && inputRef.current === document.activeElement) { | |
| setShowSuggestions(true); | |
| } | |
| }, [suggestions.length]); | |
| const hadSuggestionsRef = React.useRef(suggestions.length > 0); | |
| React.useEffect(() => { | |
| const hasSuggestions = suggestions.length > 0; | |
| const wasEmpty = !hadSuggestionsRef.current; | |
| hadSuggestionsRef.current = hasSuggestions; | |
| if (hasSuggestions && wasEmpty && !showSuggestions && inputRef.current === document.activeElement) { | |
| setShowSuggestions(true); | |
| } | |
| }, [suggestions.length, showSuggestions]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/ui/tag-input.tsx
Line: 181-189
Comment:
When a caller intentionally closes the dropdown via Escape, then later another update to the same `suggestions` array causes the length to cycle through 0 and back to non-zero (e.g. a background refresh clears then repopulates the list), the effect will auto-reopen the dropdown while the input is still focused — overriding the user's explicit dismiss. Adding a guard on `showSuggestions` prevents this surprise for any of the ~11 callers that pass dynamic suggestions.
```suggestion
const hadSuggestionsRef = React.useRef(suggestions.length > 0);
React.useEffect(() => {
const hasSuggestions = suggestions.length > 0;
const wasEmpty = !hadSuggestionsRef.current;
hadSuggestionsRef.current = hasSuggestions;
if (hasSuggestions && wasEmpty && !showSuggestions && inputRef.current === document.activeElement) {
setShowSuggestions(true);
}
}, [suggestions.length, showSuggestions]);
```
How can I resolve this? If you propose a fix, please make it concise.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![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 two targeted fixes in TagInput correctly address the race condition between async suggestion loading and user focus, and the click-to-reopen interaction is sound. The regression tests are well-scoped and each directly guards its corresponding fix hunk.
PR Size: M
- Lines changed: 236
- Files changed: 2
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
Notes
Mock path fix (test file): The corrected mock path from @/actions/providers to @/lib/api-client/v1/actions/providers is an important fix. The old mock was silently no-op, meaning the test was exercising the real REST client rather than the mock. This was a latent test defect that the PR correctly resolves.
hadSuggestionsRef effect (tag-input.tsx): The useEffect that auto-opens on the first 0->N suggestions transition is correctly bounded. The useRef persists across renders, so the effect only fires once per component lifetime. Concerns about the effect re-firing after an explicit Escape dismissal are not applicable — once hadSuggestionsRef.current is set to true, subsequent length transitions do not trigger the effect.
onClick handler (tag-input.tsx): Adding setShowSuggestions(true) to the container click handler correctly enables re-opening the dropdown on re-click of an already-focused input, without affecting the handleFocus behavior for first-focus interactions.
Automated review by Claude AI
… focused When TagInput receives suggestions asynchronously (e.g. ProviderGroupSelect fetching provider groups over HTTP), a user who focuses the field before the data arrives never saw the dropdown: focus fired while suggestions were empty, and an already-focused input never re-fires focus. This made it impossible to select existing provider groups in the create-user dialog. Add a fire-once, disabled-guarded effect that opens the dropdown the first time suggestions become non-empty while the input is focused. Firing only on the initial load (and never on later refreshes) avoids re-opening a dropdown the user has dismissed via Escape / click-outside when a consumer repopulates its suggestions. Also fix the test mock to target the module the component actually imports (@/lib/api-client/v1/actions/providers) and add regression tests, including the dialog/portal path. Fixes #1212 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
33bb34d to
be231ff
Compare
![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: be231ffd7f
ℹ️ 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".
| React.useEffect(() => { | ||
| if (didAutoOpenRef.current || suggestions.length === 0) return; | ||
| didAutoOpenRef.current = true; | ||
| if (!disabled && inputRef.current === document.activeElement) { | ||
| setShowSuggestions(true); |
There was a problem hiding this comment.
When async suggestions arrive while the input is focused, this one-shot effect opens the dropdown, but after the user dismisses it with Escape the input remains focused and a subsequent click still only refocuses the already-focused input, so handleFocus does not run and the list cannot be reopened without typing or blurring/refocusing. This leaves the same “click does nothing” behavior for the focused field after a normal dismiss; the click handler needs to explicitly show suggestions when suggestions are available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/ui/__tests__/provider-group-tag-input.test.tsx (1)
259-263: 💤 Low value可选:复用
suggestionButtonsFor的查找逻辑以减少重复。此处内联的按钮过滤与
suggestionButtonsFor(行 82-86)完全一致,仅查询根从document改为dialogContent。可将该 helper 改为接受可选根节点,避免两处textContent.includes逻辑分叉。此外includes为子串匹配,若后续出现互为前缀的分组名(如team与team-alpha)断言可能误判,顺带可考虑精确匹配。♻️ 建议:helper 接受可选根节点
-function suggestionButtonsFor(group: string) { - return Array.from(document.querySelectorAll("button")).filter((btn) => - (btn.textContent || "").includes(group) - ); -} +function suggestionButtonsFor(group: string, root: ParentNode = document) { + return Array.from(root.querySelectorAll("button")).filter((btn) => + (btn.textContent || "").includes(group) + ); +}- const dialogContent = container.querySelector('[data-slot="dialog-content"]') as HTMLElement; - const buttonsInDialog = Array.from(dialogContent.querySelectorAll("button")).filter((btn) => - (btn.textContent || "").includes("team-alpha") - ); - expect(buttonsInDialog.length).toBeGreaterThan(0); + const dialogContent = container.querySelector('[data-slot="dialog-content"]') as HTMLElement; + expect(suggestionButtonsFor("team-alpha", dialogContent).length).toBeGreaterThan(0);🤖 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/components/ui/__tests__/provider-group-tag-input.test.tsx` around lines 259 - 263, Refactor the duplicated button-search logic by updating the existing helper suggestionButtonsFor to accept an optional root HTMLElement (defaulting to document) so the test can call suggestionButtonsFor("team-alpha", dialogContent) instead of reimplementing the query; also change the matching from substring includes(...) to an exact text match (e.g., compare trimmed textContent === target) to avoid false positives when group names are prefixes of one another. Ensure you update all call sites (the helper usage at lines ~82-86 and this test's inline query) to pass the optional root where needed.
🤖 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 `@src/components/ui/__tests__/provider-group-tag-input.test.tsx`:
- Around line 259-263: Refactor the duplicated button-search logic by updating
the existing helper suggestionButtonsFor to accept an optional root HTMLElement
(defaulting to document) so the test can call suggestionButtonsFor("team-alpha",
dialogContent) instead of reimplementing the query; also change the matching
from substring includes(...) to an exact text match (e.g., compare trimmed
textContent === target) to avoid false positives when group names are prefixes
of one another. Ensure you update all call sites (the helper usage at lines
~82-86 and this test's inline query) to pass the optional root where needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b9ad746-ecc1-41c6-9b40-6bbc51549398
📒 Files selected for processing (2)
src/components/ui/__tests__/provider-group-tag-input.test.tsxsrc/components/ui/tag-input.tsx
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
问题 (Fixes #1212)
在「创建用户」对话框中，「供应商分组」多选框只显示默认的
default，点击无法展开下拉菜单去选择已有的供应商分组。根因
ProviderGroupSelect（创建用户对话框里经KeyEditSection使用）通过一次 异步 HTTP GET 加载建议列表，再传给TagInput。而TagInput只在以下时机展开建议下拉：focus时（handleFocus）handleInputChange）如果用户在数据返回之前就点击/聚焦了该字段（真实部署有网络延迟时很常见），
handleFocus看到suggestions.length === 0，什么都不做；而此后输入框已处于聚焦态，再次点击不会重新触发focus，于是下拉永远打不开 —— 用户的体感就是「点了没反应」（只有键入才会出现下拉）。修复（
src/components/ui/tag-input.tsx）onClick在聚焦输入框的同时，若suggestions.length > 0则主动setShowSuggestions(true)，使得对已聚焦输入框的再次点击也能重新打开下拉。document.activeElement时自动展开下拉（用hadSuggestionsRef仅在 0 → N 跳变时触发），覆盖「先聚焦、数据后到」的竞态。改动为
TagInput通用行为，所有传入suggestions的调用方（约 11 处）均受益，且符合标准 combobox 交互（点击即展开），不影响既有用例。测试（
provider-group-tag-input.test.tsx）@/lib/api-client/v1/actions/providers导入，旧测试却 mock 了@/actions/providers（mock 未生效，掩盖了真实数据流）。onClick改动）。data-slot="dialog-content"）中点击时，下拉应通过 Portal 渲染进 dialog-content —— 覆盖 创建用户无法选择已有供应商分组 #1212 的真实场景（含 portal 分支）。本地验证（全绿）
bun run build✓bun run lint✓（biome，0 错误）bun run typecheck✓（tsgo，0 错误）bun run test✓（6175 passed / 13 skipped）🤖 Generated with Claude Code
Greptile Summary
This PR fixes the #1212 bug where the "Provider Group" multi-select in the Create User dialog could not be expanded. The root cause was a race condition: when the user focused the input before the async suggestions loaded,
handleFocussaw an empty list and did nothing; once focused, no subsequent focus event fired to retry.tag-input.tsx): A one-shotuseEffectguarded bydidAutoOpenRefauto-opens the suggestions dropdown when the list transitions from empty to non-empty while the input is the active element, covering the focus-before-data race.provider-group-tag-input.test.tsx): Corrects the mock import path from@/actions/providersto the actual@/lib/api-client/v1/actions/providers, adds root-tracking cleanup for reliable test isolation, and introduces three regression tests guarding the async-load path, the deferred-focus race, and portal rendering inside a Dialog.Confidence Score: 5/5
Safe to merge; the core async-race fix is correct and well-tested, with one minor UX gap left from a described-but-absent onClick enhancement.
The useEffect logic is sound: it fires exactly once per mount (guarded by didAutoOpenRef), correctly checks activeElement, and handleFocus independently covers the non-race path. The only gap is that the PR description promises an onClick enhancement for re-clicking an already-focused input after manual dismissal, but that code is absent from the diff — leaving a secondary UX edge case unaddressed rather than any data-corrupting or security issue.
src/components/ui/tag-input.tsx — verify whether the onClick enhancement (setShowSuggestions when suggestions.length > 0) was intentionally deferred or accidentally omitted.
Important Files Changed
Sequence Diagram
sequenceDiagram participant U as User participant Input as TagInput (focused) participant Effect as useEffect (didAutoOpenRef) participant API as getProviderGroupsWithCount U->>Input: "click/focus (suggestions=[])" Input->>Input: "handleFocus → suggestions.length===0 → no-op" API-->>Input: "suggestions arrive (length > 0)" Input->>Effect: suggestions.length changed Effect->>Effect: "didAutoOpenRef.current===false → set true" Effect->>Effect: "inputRef === document.activeElement?" alt input still focused Effect->>Input: setShowSuggestions(true) ✓ else input not focused Effect->>Effect: skip (handleFocus covers next focus) end Note over U,Input: After auto-open, user dismisses (Escape) U->>Input: re-click (input already focused) Input->>Input: onClick → focus() no focus event fires Note over Input: didAutoOpenRef=true → effect skips, handleFocus not called → dropdown stays closedPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix(ui): auto-open TagInput suggestions ..." | Re-trigger Greptile