fix: render select input as text combobox#1222
Conversation
|
@QDyanbing is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough将 SelectInput 的内部输入 type 固定为 "text"，默认 autocomplete 改为 "new-password"，并更新相关测试以断言这些属性（包括不存在 list 属性）。 变更内容ARIA 合规性修复
预期代码审查工作量🎯 2 (Simple) | ⏱️ ~10 分钟 可能相关的 PR
建议审查者
庆祝之诗 🐰
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 timed out. The project may have too many dependencies for the sandbox. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1222 +/- ##
==========================================
- Coverage 99.44% 99.44% -0.01%
==========================================
Files 31 31
Lines 1266 1265 -1
Branches 442 461 +19
==========================================
- Hits 1259 1258 -1
Misses 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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 Select component's input element by changing its type from search to text and its default autoComplete value from off to new-password. These changes are reflected across the component's implementation, accessibility tests, and various snapshots to ensure consistent behavior and prevent browser-level autocomplete interference. I have no feedback to provide as there were no review comments.
![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/SelectInput/Input.tsx`:
- Line 174: The autoComplete fallback uses logical OR which treats an explicit
empty string as falsy; update the assignment in Input.tsx (the autoComplete
property where it currently reads autoComplete: autoComplete || 'new-password')
to use the nullish coalescing operator so that only null or undefined trigger
the default (i.e., replace the `||` fallback with `??` for the autoComplete
property in the SelectInput/Input component).
🪄 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: 694f759d-0632-42e2-a47c-d07d9e0fdaf2
⛔ Files ignored due to path filters (4)
tests/__snapshots__/Combobox.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Select.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Tags.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/ssr.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/SelectInput/Input.tsxtests/Accessibility.test.tsxtests/Select.test.tsx
| } as React.CSSProperties, | ||
| autoFocus, | ||
| autoComplete: autoComplete || 'off', | ||
| autoComplete: autoComplete || 'new-password', |
There was a problem hiding this comment.
避免用 || 覆盖显式空字符串的 autoComplete 值
Line 174 建议改为 ??。当前写法会把调用方传入的 "" 覆盖为 'new-password'，行为不够精确。
🔧 建议修改
- autoComplete: autoComplete || 'new-password',
+ autoComplete: autoComplete ?? 'new-password',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| autoComplete: autoComplete || 'new-password', | |
| autoComplete: autoComplete ?? 'new-password', |
🤖 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/SelectInput/Input.tsx` at line 174, The autoComplete fallback uses
logical OR which treats an explicit empty string as falsy; update the assignment
in Input.tsx (the autoComplete property where it currently reads autoComplete:
autoComplete || 'new-password') to use the nullish coalescing operator so that
only null or undefined trigger the default (i.e., replace the `||` fallback with
`??` for the autoComplete property in the SelectInput/Input component).
2 participants
What changed
type="text"instead oftype="search"so it can validly carryrole="combobox".new-passwordto preserve the Chromium autocomplete workaround that originally motivatedtype="search".Why
input type="search"without a nativelistattribute has an implicit searchbox role in ARIA in HTML, so overriding it withrole="combobox"creates invalid markup. A text input allows the combobox role whileautocomplete="new-password"keeps browser autocomplete suppressed.Fixes ant-design/ant-design#57904.
Validation
npx rc-test --updateSnapshot --runInBandnpm run tscSummary by CodeRabbit
变更说明