-
-
Notifications
You must be signed in to change notification settings - Fork 334
feat:add maxTagPlaceholder api #938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat:add maxTagPlaceholder api #938
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @rubbishmaker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
本次拉取请求为多选日期选择器组件引入了新的maxTagPlaceholder属性。此增强功能允许开发者在选中项数量超过maxTagCount限制时,自定义“剩余”或“省略”标签的显示方式。它提供了灵活性,可以渲染自定义React节点,或使用函数根据省略的值动态生成占位符内容,从而改善选中项较多组件的用户体验。
Highlights
- 新属性引入: 为多选组件引入maxTagPlaceholder属性,允许自定义超出maxTagCount限制时的占位符显示。
- 灵活性增强: maxTagPlaceholder属性支持传入React节点或一个接收被省略值数组的函数,提供了高度的定制能力。
- 接口更新: 更新了相关组件接口(BasePickerProps、SingleSelectorProps、MultipleDatesProps)以包含新属性。
- 默认行为与兼容性: 为maxTagPlaceholder提供了默认实现,确保向后兼容性,并在未指定时提供标准显示。
- 测试覆盖: 新增了全面的单元测试,验证maxTagPlaceholder在各种场景下的行为,包括边界情况。
- 文档与示例: 更新了示例文档,展示了maxTagPlaceholder的实际使用方法。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个 PR 很好地实现了多选超出最大数量后自定义 placeholder 的功能。代码结构清晰,并且添加了全面的单元测试,覆盖了多种场景。我发现了一些可以改进的小地方,主要涉及类型定义、React key 的使用和一些冗余代码,这些改动将有助于提升代码质量和可维护性。请查看我的具体评论。
docs/examples/multiple.tsx
Outdated
const maxTagPlaceholder = (value: any[]) => { | ||
return ( | ||
<ul> | ||
{value?.map((item) => { | ||
return <li>{item?.format('YYYY-MM-DD')}</li>; | ||
})} | ||
</ul> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxTagPlaceholder
函数存在两个问题:
value
参数的类型是any[]
,这太宽泛了。考虑到这个文件的上下文,它应该被更具体地类型化为dayjs.Dayjs[]
。- 在
map
函数中生成的<li>
元素缺少key
属性。在 React 中,列表中的每个元素都必须有一个唯一的key
,以避免重新渲染时的性能问题和潜在的 bug。
const maxTagPlaceholder = (value: any[]) => { | |
return ( | |
<ul> | |
{value?.map((item) => { | |
return <li>{item?.format('YYYY-MM-DD')}</li>; | |
})} | |
</ul> | |
); | |
}; | |
const maxTagPlaceholder = (value: dayjs.Dayjs[]) => { | |
return ( | |
<ul> | |
{value?.map((item) => { | |
const formattedDate = item?.format('YYYY-MM-DD'); | |
return <li key={formattedDate}>{formattedDate}</li>; | |
})} | |
</ul> | |
); | |
}; |
if (!value.length) { | ||
return null; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onInputChange, | ||
multiple, | ||
maxTagCount, | ||
maxTagPlaceholder = (omittedValues: string[]) => `+ ${omittedValues.length} ...`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxTagPlaceholder
默认函数的 omittedValues
参数类型是 string[]
,但这与属性的定义和用法不匹配。该函数实际接收的是被省略的 DateType
对象数组。为了类型安全和代码清晰,应将其类型更正为 DateType[]
。
maxTagPlaceholder = (omittedValues: string[]) => `+ ${omittedValues.length} ...`, | |
maxTagPlaceholder = (omittedValues: DateType[]) => `+ ${omittedValues.length} ...`, |
tests/multiple.spec.tsx
Outdated
); | ||
|
||
// Test maxTagCount = 0 | ||
const { container, rerender } = render( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/PickerInput/SinglePicker.tsx (1)
49-50
: 为新公开属性补充注释与约束说明建议为
maxTagPlaceholder
增加 JSDoc,明确:
- 仅在
multiple
模式下生效;- 当溢出时将接收被省略的
DateType[]
;- 与
maxTagCount="responsive" | number
的配合行为。/** Only work when `multiple` is in used */ maxTagCount?: number | 'responsive'; - maxTagPlaceholder?: React.ReactNode | ((omittedValues: DateType[]) => React.ReactNode); + /** + * When selected items exceed `maxTagCount`, render custom placeholder. + * Only works in `multiple` mode. Receives omitted values. + */ + maxTagPlaceholder?: React.ReactNode | ((omittedValues: DateType[]) => React.ReactNode);docs/examples/multiple.tsx (2)
41-56
: 示例中复用相同 ref 可能引起困惑同一个
singleRef
同时赋给多个SinglePicker
,后者会覆盖前者引用;建议仅对需要暴露操作的实例赋ref
,或为每个实例独立创建ref
。- const singleRef = React.useRef<PickerRef>(null); + const singleRef = React.useRef<PickerRef>(null); + const weekRef = React.useRef<PickerRef>(null); ... - <SinglePicker {...sharedLocale} multiple ref={singleRef} onOpenChange={console.error} /> - <SinglePicker {...sharedLocale} multiple ref={singleRef} needConfirm /> + <SinglePicker {...sharedLocale} multiple onOpenChange={console.error} /> + <SinglePicker {...sharedLocale} multiple ref={singleRef} needConfirm /> ... - <SinglePicker + <SinglePicker {...sharedLocale} multiple - picker="week" + picker="week" + ref={weekRef} defaultValue={[dayjs('2021-01-01'), dayjs('2021-01-08')]} />
16-18
: 示例中的console.clear()
可去除文档示例中清空控制台无必要,影响阅读历史日志。
-console.clear();
tests/multiple.spec.tsx (2)
158-201
: 补充“默认占位”用例,覆盖未自定义maxTagPlaceholder
的场景当前仅验证自定义占位符;建议再加一例未传入
maxTagPlaceholder
时默认"+ N ..."
的渲染验证。describe('maxTagPlaceholder', () => { + it('uses default placeholder when not provided', () => { + const { container } = render( + <DayPicker + multiple + maxTagCount={1} + defaultValue={[getDay('2000-01-01'), getDay('2000-01-02'), getDay('2000-01-03')]} + />, + ); + // 1 个可见条目 + 1 个占位 + expect(container.querySelectorAll('.rc-picker-selection-item')).toHaveLength(2); + // 默认文案形如 "+ 2 ..." + expect(container.textContent).toMatch(/\+\s*2\s*\.\.\./); + });
224-241
: 测试名与变量冗余小修
it('should handle maxTagCount edge cases1' ...)
名称中的 “1” 多余;且rerender
未使用。可去除或补充二次渲染断言。- it('should handle maxTagCount edge cases1', () => { + it('should handle maxTagCount edge cases', () => { @@ - const { container, rerender } = render( + const { container } = render(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
docs/examples/multiple.tsx
(1 hunks)src/PickerInput/Selector/SingleSelector/MultipleDates.tsx
(3 hunks)src/PickerInput/Selector/SingleSelector/index.tsx
(3 hunks)src/PickerInput/SinglePicker.tsx
(1 hunks)tests/multiple.spec.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (1)
src/PickerInput/SinglePicker.tsx (1)
PickerProps
(103-105)
src/PickerInput/Selector/SingleSelector/index.tsx (1)
src/PickerInput/SinglePicker.tsx (1)
PickerProps
(103-105)
docs/examples/multiple.tsx (1)
src/interface.tsx (1)
PickerRef
(452-456)
tests/multiple.spec.tsx (1)
tests/util/commonUtil.tsx (2)
DayPicker
(168-173)getDay
(186-195)
🪛 Biome (2.1.2)
docs/examples/multiple.tsx
[error] 30-30: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (4)
src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (2)
7-8
: 类型扩展方向正确,接口变更 LGTM将
MultipleDatesProps
扩展为同时接收maxTagCount
与maxTagPlaceholder
合理,能与上层PickerProps
对齐。
28-30
: 解构新增 prop 合理,保持向下传递新增解构
maxTagPlaceholder
并与maxTagCount
一并传入渲染逻辑,方向正确。src/PickerInput/Selector/SingleSelector/index.tsx (2)
15-16
: Prop 扩展 LGTM向
SingleSelectorProps
增加maxTagPlaceholder
并保持与上层PickerProps
对齐,方向正确。
168-178
: 向下传递maxTagPlaceholder
的做法正确默认值在此处统一生效,保证
MultipleDates
始终拿到可用占位符,避免空节点。
docs/examples/multiple.tsx
Outdated
const maxTagPlaceholder = (value: any[]) => { | ||
return ( | ||
<ul> | ||
{value?.map((item) => { | ||
return <li>{item?.format('YYYY-MM-DD')}</li>; | ||
})} | ||
</ul> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
为可迭代元素添加 key(静态检查已提示)
为 <li>
添加稳定 key
,消除 React/BIOME 警告。
-const maxTagPlaceholder = (value: any[]) => {
+const maxTagPlaceholder = (value: any[]) => {
return (
<ul>
- {value?.map((item) => {
- return <li>{item?.format('YYYY-MM-DD')}</li>;
+ {value?.map((item, idx) => {
+ return <li key={item?.valueOf?.() ?? idx}>{item?.format('YYYY-MM-DD')}</li>;
})}
</ul>
);
};
📝 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.
const maxTagPlaceholder = (value: any[]) => { | |
return ( | |
<ul> | |
{value?.map((item) => { | |
return <li>{item?.format('YYYY-MM-DD')}</li>; | |
})} | |
</ul> | |
); | |
}; | |
const maxTagPlaceholder = (value: any[]) => { | |
return ( | |
<ul> | |
{value?.map((item, idx) => { | |
return <li key={item?.valueOf?.() ?? idx}>{item?.format('YYYY-MM-DD')}</li>; | |
})} | |
</ul> | |
); | |
}; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 30-30: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In docs/examples/multiple.tsx around lines 26 to 34, the <li> elements inside
the map are missing a stable key causing React/BIOME warnings; update the map to
pass a unique key to each <li> (for example use item.format('YYYY-MM-DD') or
another unique identifier on the item, and fall back to the index only if no
stable id exists) so each list item has a stable key prop.
if (!value.length) { | ||
return null; | ||
} | ||
const content = | ||
typeof maxTagPlaceholder === 'function' | ||
? maxTagPlaceholder(omittedValues) | ||
: maxTagPlaceholder; | ||
return renderSelector(content); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
应使用 omittedValues 判空,避免非预期渲染 Rest 占位
这里应判断 omittedValues.length
而不是 value.length
,以避免在没有溢出时误渲染占位内容(或与 rc-overflow
的调用时机耦合过深)。
- function renderRest(omittedValues: DateType[]) {
- if (!value.length) {
- return null;
- }
+ function renderRest(omittedValues: DateType[]) {
+ if (!omittedValues?.length) {
+ return null;
+ }
const content =
typeof maxTagPlaceholder === 'function'
? maxTagPlaceholder(omittedValues)
- : maxTagPlaceholder;
+ : (maxTagPlaceholder ?? `+ ${omittedValues.length} ...`);
return renderSelector(content);
}
📝 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.
if (!value.length) { | |
return null; | |
} | |
const content = | |
typeof maxTagPlaceholder === 'function' | |
? maxTagPlaceholder(omittedValues) | |
: maxTagPlaceholder; | |
return renderSelector(content); | |
function renderRest(omittedValues: DateType[]) { | |
if (!omittedValues?.length) { | |
return null; | |
} | |
const content = | |
typeof maxTagPlaceholder === 'function' | |
? maxTagPlaceholder(omittedValues) | |
: (maxTagPlaceholder ?? `+ ${omittedValues.length} ...`); | |
return renderSelector(content); | |
} |
🤖 Prompt for AI Agents
In src/PickerInput/Selector/SingleSelector/MultipleDates.tsx around lines 72 to
79, the early-return currently checks value.length which can wrongly suppress or
render the maxTagPlaceholder when there are no omitted items; change the
condition to check omittedValues.length instead so the component returns null
when there are no omitted items to display, and only computes/render the
maxTagPlaceholder when omittedValues.length > 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (1)
72-76
: renderRest 增强:空/false 占位应返回 null,且做防御性判空当用户显式传入
null/false
或(极端情况下)无溢出项时,当前实现会渲染一个空的占位气泡。建议提前返回以避免空节点;同时补充对omittedValues
的防御性判空,降低对rc-overflow
调用时机的耦合。应用以下补丁:
- function renderRest(omittedValues: DateType[]) { - const content = - typeof maxTagPlaceholder === 'function' - ? maxTagPlaceholder(omittedValues) - : maxTagPlaceholder; - return renderSelector(content); - } + function renderRest(omittedValues: DateType[]) { + if (!omittedValues?.length) { + return null; + } + const node = + typeof maxTagPlaceholder === 'function' + ? maxTagPlaceholder(omittedValues) + : maxTagPlaceholder; + if (node == null || node === false) { + return null; + } + return renderSelector(node); + }src/PickerInput/Selector/SingleSelector/index.tsx (1)
78-78
: 默认占位函数类型正确(已从 string[] 修正为 DateType[])默认实现直观、与 MultipleDates 的使用一致。
tests/multiple.spec.tsx (1)
224-241
: 测试命名请更具体并修正编号样式将 “edge cases1” 更名为能直指场景的名称,便于定位与维护。
- it('should handle maxTagCount edge cases1', () => { + it('should handle maxTagCount=0 edge case', () => {
🧹 Nitpick comments (2)
tests/multiple.spec.tsx (2)
160-162
: 收敛 any:测试里为 omittedValues 使用 Dayjs 类型用
Dayjs[]
提升类型提示与重构友好度(不影响断言语义)。+import type { Dayjs } from 'dayjs'; @@ - const maxTagPlaceholder = (omittedValues: any[]) => ( + const maxTagPlaceholder = (omittedValues: Dayjs[]) => ( <span className="custom-max-tag-placeholder">+{omittedValues.length} more</span> ); @@ - const maxTagPlaceholder = (omittedValues: any[]) => ( + const maxTagPlaceholder = (omittedValues: Dayjs[]) => ( <span className="custom-max-tag-placeholder">+{omittedValues.length} items</span> ); @@ - const CustomPlaceholder = ({ omittedValues }: { omittedValues: any[] }) => ( + const CustomPlaceholder = ({ omittedValues }: { omittedValues: Dayjs[] }) => ( <div className="custom-placeholder-wrapper"> @@ - const maxTagPlaceholder = (omittedValues: any[]) => ( + const maxTagPlaceholder = (omittedValues: Dayjs[]) => ( <span className="edge-case-placeholder">+{omittedValues.length}</span> ); @@ - const maxTagPlaceholder = jest.fn((omittedValues) => ( + const maxTagPlaceholder = jest.fn((omittedValues: Dayjs[]) => ( <span className="test-placeholder">+{omittedValues.length}</span> ));Also applies to: 179-181, 203-208, 225-227, 244-246
158-266
: 补充一个“未提供自定义占位时”的默认占位渲染用例验证默认 “+ N ...” 的回退逻辑,防回归。
describe('maxTagPlaceholder', () => { @@ it('should pass correct omittedValues to maxTagPlaceholder', () => { const maxTagPlaceholder = jest.fn((omittedValues) => ( <span className="test-placeholder">+{omittedValues.length}</span> )); @@ expect(maxTagPlaceholder).toHaveBeenCalledWith([values[2], values[3]]); }); + + it('should render default "+ N ..." when maxTagPlaceholder is not provided', () => { + const { container } = render( + <DayPicker + multiple + maxTagCount={2} + defaultValue={[ + getDay('2000-01-01'), + getDay('2000-01-02'), + getDay('2000-01-03'), + getDay('2000-01-04'), + ]} + />, + ); + const items = container.querySelectorAll('.rc-picker-selection-item'); + expect(items).toHaveLength(3); + // 最后一个为 Rest 占位 + expect(items[items.length - 1].textContent).toBe('+ 2 ...'); + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/examples/multiple.tsx
(1 hunks)src/PickerInput/Selector/SingleSelector/MultipleDates.tsx
(3 hunks)src/PickerInput/Selector/SingleSelector/index.tsx
(3 hunks)tests/multiple.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/examples/multiple.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (1)
src/PickerInput/SinglePicker.tsx (1)
PickerProps
(103-105)
tests/multiple.spec.tsx (1)
tests/util/commonUtil.tsx (2)
DayPicker
(168-173)getDay
(186-195)
src/PickerInput/Selector/SingleSelector/index.tsx (1)
src/PickerInput/SinglePicker.tsx (1)
PickerProps
(103-105)
🔇 Additional comments (5)
src/PickerInput/Selector/SingleSelector/MultipleDates.tsx (2)
7-7
: 扩展 props 到 MultipleDatesProps 合理且一致新增对
maxTagPlaceholder
的透传定义与本次 PR 目标一致。无需改动。
28-29
: 从 props 解构 maxTagPlaceholder 正确命名与上游一致,便于直传到 Overflow 的
renderRest
。src/PickerInput/Selector/SingleSelector/index.tsx (2)
174-175
: Props 透传无误
maxTagPlaceholder
正确下传至MultipleDates
,行为与预期一致。
15-15
: 确认 maxTagPlaceholder 类型一致
PickerProps 中已将 maxTagPlaceholder 声明为 React.ReactNode | ((omittedValues: DateType[]) => React.ReactNode),与 SingleSelector 和 MultipleDates 中的 Pick 用法及默认实现保持一致,无需调整。tests/multiple.spec.tsx (1)
158-166
: 新增 maxTagPlaceholder 用例覆盖面良好基础场景(未超限不出现占位)验证到位。
ant-design/ant-design#54822 对应此需求
Summary by CodeRabbit
新功能
体验
文档
测试