Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions docs/examples/multiple.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,37 @@ const sharedLocale = {
style: { width: 300 },
};

const maxTagPlaceholder = (value: any[]) => {
return (
<ul>
{value?.map((item) => {
return <li>{item?.format('YYYY-MM-DD')}</li>;
})}
</ul>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

maxTagPlaceholder 函数存在两个问题:

  1. value 参数的类型是 any[],这太宽泛了。考虑到这个文件的上下文,它应该被更具体地类型化为 dayjs.Dayjs[]
  2. map 函数中生成的 <li> 元素缺少 key 属性。在 React 中,列表中的每个元素都必须有一个唯一的 key,以避免重新渲染时的性能问题和潜在的 bug。
Suggested change
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>
);
};

Copy link

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.

Suggested change
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.


export default () => {
const singleRef = React.useRef<PickerRef>(null);

return (
<div>
<SinglePicker {...sharedLocale} multiple ref={singleRef} onOpenChange={console.error} />
<SinglePicker {...sharedLocale} multiple ref={singleRef} needConfirm />
<SinglePicker {...sharedLocale} multiple picker="week" defaultValue={[
dayjs('2021-01-01'),
dayjs('2021-01-08'),
]} />
<SinglePicker
{...sharedLocale}
multiple
picker="week"
defaultValue={[dayjs('2021-01-01'), dayjs('2021-01-08')]}
/>
<SinglePicker
maxTagCount={10}
{...sharedLocale}
multiple
ref={singleRef}
maxTagPlaceholder={maxTagPlaceholder}
defaultValue={[dayjs('2021-01-01'), dayjs('2021-01-08')]}
/>
</div>
);
};
12 changes: 9 additions & 3 deletions src/PickerInput/Selector/SingleSelector/MultipleDates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as React from 'react';
import type { PickerProps } from '../../SinglePicker';

export interface MultipleDatesProps<DateType extends object = any>
extends Pick<PickerProps, 'maxTagCount'> {
extends Pick<PickerProps, 'maxTagCount' | 'maxTagPlaceholder'> {
prefixCls: string;
value: DateType[];
onRemove: (value: DateType) => void;
Expand All @@ -25,6 +25,7 @@ export default function MultipleDates<DateType extends object = any>(
formatDate,
disabled,
maxTagCount,
maxTagPlaceholder,
placeholder,
} = props;

Expand Down Expand Up @@ -68,8 +69,13 @@ export default function MultipleDates<DateType extends object = any>(

// ========================= Rest =========================
function renderRest(omittedValues: DateType[]) {
const content = `+ ${omittedValues.length} ...`;

if (!value.length) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

if (!value.length) { return null; } 这段检查是多余的。renderRest 函数仅在有省略项时由 rc-overflow 组件调用,这意味着 value(即 rc-overflowdata 属性)不为空,并且其长度大于 maxTagCount。因此,这个检查可以安全地移除。

const content =
typeof maxTagPlaceholder === 'function'
? maxTagPlaceholder(omittedValues)
: maxTagPlaceholder;
return renderSelector(content);
Copy link

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.

Suggested change
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.

}

Expand Down
4 changes: 3 additions & 1 deletion src/PickerInput/Selector/SingleSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import MultipleDates from './MultipleDates';

export interface SingleSelectorProps<DateType extends object = any>
extends SelectorProps<DateType>,
Pick<PickerProps, 'multiple' | 'maxTagCount'> {
Pick<PickerProps, 'multiple' | 'maxTagCount' | 'maxTagPlaceholder'> {
id?: string;

value?: DateType[];
Expand Down Expand Up @@ -75,6 +75,7 @@ function SingleSelector<DateType extends object = any>(
onInputChange,
multiple,
maxTagCount,
maxTagPlaceholder = (omittedValues: string[]) => `+ ${omittedValues.length} ...`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

maxTagPlaceholder 默认函数的 omittedValues 参数类型是 string[],但这与属性的定义和用法不匹配。该函数实际接收的是被省略的 DateType 对象数组。为了类型安全和代码清晰,应将其类型更正为 DateType[]

Suggested change
maxTagPlaceholder = (omittedValues: string[]) => `+ ${omittedValues.length} ...`,
maxTagPlaceholder = (omittedValues: DateType[]) => `+ ${omittedValues.length} ...`,


// Valid
format,
Expand Down Expand Up @@ -170,6 +171,7 @@ function SingleSelector<DateType extends object = any>(
onRemove={onMultipleRemove}
formatDate={getText}
maxTagCount={maxTagCount}
maxTagPlaceholder={maxTagPlaceholder}
disabled={disabled}
removeIcon={removeIcon}
placeholder={placeholder}
Expand Down
1 change: 1 addition & 0 deletions src/PickerInput/SinglePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export interface BasePickerProps<DateType extends object = any>
removeIcon?: React.ReactNode;
/** Only work when `multiple` is in used */
maxTagCount?: number | 'responsive';
maxTagPlaceholder?: React.ReactNode | ((omittedValues: DateType[]) => React.ReactNode);

// Value
value?: DateType | DateType[] | null;
Expand Down
109 changes: 109 additions & 0 deletions tests/multiple.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,115 @@ describe('Picker.Multiple', () => {
).toBeFalsy();
});
});
describe('maxTagPlaceholder', () => {
it('should not show maxTagPlaceholder when items count is within maxTagCount', () => {
const maxTagPlaceholder = (omittedValues: any[]) => (
<span className="custom-max-tag-placeholder">+{omittedValues.length} more</span>
);

const { container } = render(
<DayPicker
multiple
maxTagCount={3}
maxTagPlaceholder={maxTagPlaceholder}
defaultValue={[getDay('2000-01-01'), getDay('2000-01-02')]}
/>,
);

// Should show all items, no placeholder
expect(container.querySelectorAll('.rc-picker-selection-item')).toHaveLength(2);
expect(container.querySelector('.custom-max-tag-placeholder')).toBeFalsy();
});

it('should show maxTagPlaceholder when items count exceeds maxTagCount', () => {
const maxTagPlaceholder = (omittedValues: any[]) => (
<span className="custom-max-tag-placeholder">+{omittedValues.length} items</span>
);

const { container } = render(
<DayPicker
multiple
maxTagCount={2}
maxTagPlaceholder={maxTagPlaceholder}
defaultValue={[
getDay('2000-01-01'),
getDay('2000-01-02'),
getDay('2000-01-03'),
getDay('2000-01-04'),
]}
/>,
);

// Should show maxTagCount items + placeholder
expect(container.querySelectorAll('.rc-picker-selection-item')).toHaveLength(3);
expect(container.querySelector('.custom-max-tag-placeholder').textContent).toBe('+2 items');
});

it('should work with custom maxTagPlaceholder component', () => {
const CustomPlaceholder = ({ omittedValues }: { omittedValues: any[] }) => (
<div className="custom-placeholder-wrapper">
<span className="omitted-count">{omittedValues.length}</span>
<span className="omitted-text">hidden dates</span>
</div>
);

const { container } = render(
<DayPicker
multiple
maxTagCount={1}
maxTagPlaceholder={(omittedValues) => <CustomPlaceholder omittedValues={omittedValues} />}
defaultValue={[getDay('2000-01-01'), getDay('2000-01-02'), getDay('2000-01-03')]}
/>,
);

expect(container.querySelector('.custom-placeholder-wrapper')).toBeTruthy();
expect(container.querySelector('.omitted-count').textContent).toBe('2');
expect(container.querySelector('.omitted-text').textContent).toBe('hidden dates');
});

it('should handle maxTagCount edge cases1', () => {
const maxTagPlaceholder = (omittedValues: any[]) => (
<span className="edge-case-placeholder">+{omittedValues.length}</span>
);

// Test maxTagCount = 0
const { container, rerender } = render(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

render 函数解构出的 rerender 未在测试用例 should handle maxTagCount edge cases1 中被使用。为了提高代码清晰度,应移除这个未使用的变量。另外,测试用例的名称 “edge cases1” 不够具体,可以考虑修改为更具描述性的名称,例如 “should handle maxTagCount=0 edge case”。

Suggested change
const { container, rerender } = render(
const { container } = render(

<DayPicker
multiple
maxTagCount={0}
maxTagPlaceholder={maxTagPlaceholder}
defaultValue={[getDay('2000-01-01')]}
/>,
);
expect(container.querySelectorAll('.rc-picker-selection-item')).toHaveLength(1);
expect(container.querySelector('.edge-case-placeholder')).toBeTruthy();
expect(container.querySelector('.edge-case-placeholder').textContent).toBe('+1');
});

it('should pass correct omittedValues to maxTagPlaceholder', () => {
const maxTagPlaceholder = jest.fn((omittedValues) => (
<span className="test-placeholder">+{omittedValues.length}</span>
));

const values = [
getDay('2000-01-01'),
getDay('2000-01-02'),
getDay('2000-01-03'),
getDay('2000-01-04'),
];

render(
<DayPicker
multiple
maxTagCount={2}
maxTagPlaceholder={maxTagPlaceholder}
defaultValue={values}
/>,
);

expect(maxTagPlaceholder).toHaveBeenCalledWith([values[2], values[3]]);
});
});

it('click year panel should not select', () => {
const onChange = jest.fn();
Expand Down