feat: support tagRender in multiple picker#963
feat: support tagRender in multiple picker#963zombieJ merged 2 commits intoreact-component:masterfrom
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. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
总览该PR为 变更
代码审查工作量估算🎯 3 (Moderate) | ⏱️ ~25 minutes 可能相关的 PR
建议审核人
诗句
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a tagRender prop to the SinglePicker component, allowing for custom rendering of tags when the multiple selection mode is active. The changes include updating the MultipleDates and SingleSelector components to support the new prop, exporting the CustomTagProps interface, and adding relevant tests and documentation examples. Feedback suggests adding a safety check within the onClose callback in MultipleDates.tsx to prevent date removal when the component is disabled, ensuring robust behavior for custom tag implementations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
docs/examples/multiple.tsx (2)
26-38: 多个SinglePicker共用同一个singleRef。
singleRef被同时挂载到 4 个SinglePicker实例上,后挂载的会覆盖前者,ref.current的语义不明确。虽然这是 demo 代码,但作为 tagRender 示例建议仅在需要的实例上保留 ref,避免误导使用者。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/multiple.tsx` around lines 26 - 38, The demo mistakenly mounts the same React ref variable singleRef onto multiple SinglePicker instances so later mounts overwrite earlier ones; update the JSX to only attach a ref to the SinglePicker(s) that actually need it (e.g., keep singleRef on the instance demonstrating tagRender or the one using onOpenChange) and remove the ref prop from the other SinglePicker components, or create distinct refs (singleRef1, singleRef2) if you need to control multiple pickers individually; adjust any code that reads singleRef.current accordingly.
43-68: demo 中tagRender未使用closable字段。既然在
CustomTagProps中新增了closable,建议 demo 示范如何根据closable决定是否显示 remove 按钮(而不是仅用业务逻辑locked),让使用者更直观地了解该字段用途。此外,自定义 tag 的<button>建议加上onMouseDown={e => e.preventDefault()},以避免点击 remove 时输入框失焦/重新打开面板,这也是更贴近实际使用的范例。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/multiple.tsx` around lines 43 - 68, 在 tagRender 回调中使用 CustomTagProps 的 closable 字段来决定是否渲染删除按钮(而不是仅依赖业务变量 locked):检查传入的 closable 和当前逻辑中的 locked 并在两者都允许时才渲染 remove 按钮,调用现有的 onClose 保持不变;同时给自定义的 button 添加 onMouseDown={e => e.preventDefault()} 以防止点击 remove 时输入框失焦/面板重开,修改点集中在 tagRender、closable、locked、onClose 以及自定义 button 上。src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (2)
64-69:closable的语义建议确认。目前
closable = !disabled,在disabled为 true 时传false。不过disabled与closable在用户侧语义并不完全一致(例如未来可能支持 tag 级别的closable控制)。当前实现作为首版没问题,但建议在 JSDoc 中明确closable当前仅受组件disabled影响,避免使用者基于更细粒度语义作假设。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/PickerInput/Selector/SingleSelector/MultipleDates.tsx` around lines 64 - 69, Add a JSDoc comment clarifying the current semantics of the closable flag: explain that the local variable closable (in MultipleDates.tsx) is derived solely from the component-level prop disabled (closable = !disabled) and that tag-level or per-item closable control is not supported currently; update the comment near the closable declaration (and mention related symbols like the disabled prop and the onClose handler/CustomTagProps<DateType>) so users won't assume finer-grained closable behavior in this first release.
62-82: 建议:tagRender返回节点未包裹容器 &renderRest未走自定义渲染。两点建议,供取舍:
- 当提供
tagRender时,返回值未包裹selection-item容器。这是符合“完全自定义”的常见设计,但需要在文档中明确说明 —— 使用者将失去默认的 title、mouseDown preventDefault(点击 tag 时输入焦点丢失)、以及rc-picker-selection-item相关样式。建议在BasePickerProps.tagRender的 JSDoc 中补充说明,或参考 antdSelect在外层追加onMouseDown={e => e.preventDefault()}的 wrapper。renderRest(+ N ...折叠项)仍然使用默认renderSelector,当maxTagCount触发折叠时,视觉风格会与自定义 tag 不一致,使用者可能会感到意外。可考虑在文档中说明此行为,或提供单独的自定义入口(例如maxTagPlaceholder)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/PickerInput/Selector/SingleSelector/MultipleDates.tsx` around lines 62 - 82, The tagRender return value is currently injected raw (so users lose default container behaviors like rc-picker-selection-item styling, title, and onMouseDown preventDefault) and renderRest still uses renderSelector which makes folded "+ N" items visually inconsistent with custom tags; update docs and code by either documenting this behavior in BasePickerProps.tagRender JSDoc (explicitly state that tagRender must include its own wrapper and event handlers) and/or wrap tagRender output with the default selection container and onMouseDown prevention inside MultipleDates.renderItem, and add support for a separate custom placeholder hook (e.g., maxTagPlaceholder) or route renderRest through tagRender when maxTagCount folding occurs so the collapsed item can be customized to match tagRender styling; reference tagRender, renderRest, renderSelector, maxTagCount and BasePickerProps.tagRender when applying the change.src/PickerInput/SinglePicker.tsx (1)
39-45:CustomTagProps接口定义清晰。字段命名与 antd
Select的CustomTagProps对齐,便于使用者迁移认知。可选建议:为每个字段补充 JSDoc(尤其是closable与disabled的区别),提升 API 可发现性。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/PickerInput/SinglePicker.tsx` around lines 39 - 45, Add JSDoc comments to the CustomTagProps interface fields to improve API discoverability: document label, value, disabled, onClose, and closable in the interface declaration (CustomTagProps<DateType>) and explicitly clarify the difference between disabled and closable (e.g., disabled means the tag is non-interactive, closable controls whether the close icon is shown/can trigger onClose). Keep the comments concise and consistent with antd Select's CustomTagProps semantics so users can map behavior easily.tests/multiple.spec.tsx (1)
143-168: 测试覆盖核心路径,建议补充边界用例。当前测试验证了自定义渲染与
onClose触发onChange的主流程。可以考虑补充:
disabled场景下tagRender收到的disabled/closable值是否符合预期。- 自定义
tagRender与maxTagCount同时使用时的折叠(renderRest)行为(目前MultipleDates.renderRest仍走默认renderSelector,未使用tagRender,用户可能期望一致的外观)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/multiple.spec.tsx` around lines 143 - 168, Add tests covering two edge cases: (1) verify tagRender receives correct disabled/closable flags and that the close button is not rendered/clickable when the DayPicker or a specific date is disabled (exercise DayPicker with multiple + disabled date(s) and assert the tagRender props and that onClose/onChange are not invoked); (2) verify behavior when maxTagCount is set so that MultipleDates.renderRest collapses tags using the custom tagRender (not the default renderSelector) by rendering DayPicker with multiple + tagRender + maxTagCount and asserting the rendered "rest" node matches the custom tag appearance and that clicking the rest expands or triggers expected callbacks; reference DayPicker, tagRender, MultipleDates.renderRest, renderRest, renderSelector, disabled and closable in your assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/PickerInput/SinglePicker.tsx`:
- Around line 58-59: Fix the comment grammar for the tagRender prop in
SinglePicker.tsx: change "Only work when `multiple` is in used" to a consistent
and correct phrasing (e.g., "Only works when `multiple` is in use" or "Only used
when `multiple` is enabled") to match the style of the maxTagCount comment;
update the comment immediately above the tagRender?: (props:
CustomTagProps<DateType>) => React.ReactNode; declaration so both wording and
verb agreement ("works"/"used"/"enabled") are consistent with existing
annotations.
---
Nitpick comments:
In `@docs/examples/multiple.tsx`:
- Around line 26-38: The demo mistakenly mounts the same React ref variable
singleRef onto multiple SinglePicker instances so later mounts overwrite earlier
ones; update the JSX to only attach a ref to the SinglePicker(s) that actually
need it (e.g., keep singleRef on the instance demonstrating tagRender or the one
using onOpenChange) and remove the ref prop from the other SinglePicker
components, or create distinct refs (singleRef1, singleRef2) if you need to
control multiple pickers individually; adjust any code that reads
singleRef.current accordingly.
- Around line 43-68: 在 tagRender 回调中使用 CustomTagProps 的 closable
字段来决定是否渲染删除按钮(而不是仅依赖业务变量 locked):检查传入的 closable 和当前逻辑中的 locked 并在两者都允许时才渲染
remove 按钮,调用现有的 onClose 保持不变;同时给自定义的 button 添加 onMouseDown={e =>
e.preventDefault()} 以防止点击 remove 时输入框失焦/面板重开,修改点集中在
tagRender、closable、locked、onClose 以及自定义 button 上。
In `@src/PickerInput/Selector/SingleSelector/MultipleDates.tsx`:
- Around line 64-69: Add a JSDoc comment clarifying the current semantics of the
closable flag: explain that the local variable closable (in MultipleDates.tsx)
is derived solely from the component-level prop disabled (closable = !disabled)
and that tag-level or per-item closable control is not supported currently;
update the comment near the closable declaration (and mention related symbols
like the disabled prop and the onClose handler/CustomTagProps<DateType>) so
users won't assume finer-grained closable behavior in this first release.
- Around line 62-82: The tagRender return value is currently injected raw (so
users lose default container behaviors like rc-picker-selection-item styling,
title, and onMouseDown preventDefault) and renderRest still uses renderSelector
which makes folded "+ N" items visually inconsistent with custom tags; update
docs and code by either documenting this behavior in BasePickerProps.tagRender
JSDoc (explicitly state that tagRender must include its own wrapper and event
handlers) and/or wrap tagRender output with the default selection container and
onMouseDown prevention inside MultipleDates.renderItem, and add support for a
separate custom placeholder hook (e.g., maxTagPlaceholder) or route renderRest
through tagRender when maxTagCount folding occurs so the collapsed item can be
customized to match tagRender styling; reference tagRender, renderRest,
renderSelector, maxTagCount and BasePickerProps.tagRender when applying the
change.
In `@src/PickerInput/SinglePicker.tsx`:
- Around line 39-45: Add JSDoc comments to the CustomTagProps interface fields
to improve API discoverability: document label, value, disabled, onClose, and
closable in the interface declaration (CustomTagProps<DateType>) and explicitly
clarify the difference between disabled and closable (e.g., disabled means the
tag is non-interactive, closable controls whether the close icon is shown/can
trigger onClose). Keep the comments concise and consistent with antd Select's
CustomTagProps semantics so users can map behavior easily.
In `@tests/multiple.spec.tsx`:
- Around line 143-168: Add tests covering two edge cases: (1) verify tagRender
receives correct disabled/closable flags and that the close button is not
rendered/clickable when the DayPicker or a specific date is disabled (exercise
DayPicker with multiple + disabled date(s) and assert the tagRender props and
that onClose/onChange are not invoked); (2) verify behavior when maxTagCount is
set so that MultipleDates.renderRest collapses tags using the custom tagRender
(not the default renderSelector) by rendering DayPicker with multiple +
tagRender + maxTagCount and asserting the rendered "rest" node matches the
custom tag appearance and that clicking the rest expands or triggers expected
callbacks; reference DayPicker, tagRender, MultipleDates.renderRest, renderRest,
renderSelector, disabled and closable in your assertions.
🪄 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: 303be436-38a5-40f1-84ad-75b4c377880f
📒 Files selected for processing (6)
docs/examples/multiple.tsxsrc/PickerInput/Selector/SingleSelector/MultipleDates.tsxsrc/PickerInput/Selector/SingleSelector/index.tsxsrc/PickerInput/SinglePicker.tsxsrc/index.tsxtests/multiple.spec.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #963 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 65 65
Lines 2691 2695 +4
Branches 722 749 +27
=======================================
+ Hits 2659 2663 +4
Misses 29 29
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
tagRendersupport for single pickermultiplemodeTesting
Summary by CodeRabbit
发布说明
新功能
tagRender回调自定义标签外观和交互。文档
测试