Skip to content
Merged
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
6 changes: 3 additions & 3 deletions examples/visible.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const addressOptions = [

const Demo = () => {
const [value, setValue] = useState<string[]>([]);
const [popupVisible, setPopupVisible] = useState(false);
const [open, setOpen] = useState(false);

const getLabel = () => {
return arrayTreeFilter(addressOptions, (o, level) => o.value === value[level])
Expand All @@ -69,10 +69,10 @@ const Demo = () => {

return (
<Cascader
popupVisible={popupVisible}
open={open}
value={value}
options={addressOptions}
onPopupVisibleChange={open => setPopupVisible(open)}
onPopupVisibleChange={open => setOpen(open)}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

⚠️ Potential issue

🧩 Analysis chain

事件处理器更新不完整

根据 PR 的目标是废弃旧 API,onPopupVisibleChange 也应该更新为新的 API 名称。

-  onPopupVisibleChange={open => setOpen(open)}
+  onDropdownVisibleChange={open => setOpen(open)}

🏁 Script executed:

#!/bin/bash
# 检查 src/Cascader.tsx 中 onPopupVisibleChange 和 onDropdownVisibleChange 的使用情况
grep -n "onPopupVisibleChange\|onDropdownVisibleChange" src/Cascader.tsx

Length of output: 267


请更新所有事件处理器引用

根据最新的检查,在 examples/visible.tsx(第 75 行)以及 src/Cascader.tsx 中(例如第 109、212、363 和 455 行)的代码依然使用了旧 API onPopupVisibleChange。而本次 PR 的目标是废弃旧 API,因此需要将所有相关引用统一更新为新版 API onDropdownVisibleChange

示例修改差异:

-  onPopupVisibleChange={open => setOpen(open)}
+  onDropdownVisibleChange={open => setOpen(open)}

请同步检查并更新 src/Cascader.tsx 中所有涉及该事件处理器的位置,以确保整个代码库的一致性。

📝 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
onPopupVisibleChange={open => setOpen(open)}
onDropdownVisibleChange={open => setOpen(open)}

onChange={value => setValue(value)}
>
<input value={getLabel()} readOnly />
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"@types/classnames": "^2.2.6",
"@types/enzyme": "^3.1.15",
"@types/jest": "^29.4.0",
"@types/react": "^17.0.38",
"@types/react": "^18.0.38",
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

React类型定义版本更新

此更改将 @types/react 的版本从 ^17.0.38 更新到了 ^18.0.38,这是一个主要版本更新。确保项目中的其他依赖和代码与 React 18 的类型定义兼容。


🏁 Script executed:

#!/bin/bash
# 检查 React 依赖版本与类型定义版本是否匹配
grep -E '"react": ".*"' package.json
grep -E '"react-dom": ".*"' package.json
grep -E '"@types/react": ".*"' package.json
grep -E '"@types/react-dom": ".*"' package.json

Length of output: 350


React类型定义版本升级存在依赖版本不匹配问题

当前仅升级了 @types/react(及相应的 @types/react-dom)至 React 18 的类型定义,但项目中的 reactreact-dom 依然是 React 16 的版本。这种不一致可能引发类型或运行时的问题。建议同步更新 reactreact-dom 至 React 18 版本,或确保项目代码与新版类型定义保持兼容。

  • 请检查并更新 package.json 中的 reactreact-dom 至兼容 React 18 的版本

"@types/react-dom": "^18.0.11",
"@types/warning": "^3.0.0",
"@umijs/fabric": "^4.0.0",
Expand Down
59 changes: 24 additions & 35 deletions src/Cascader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
toRawValues,
} from './utils/commonUtil';
import { formatStrategyValues, toPathOptions } from './utils/treeUtil';
import warningProps, { warningNullOptions } from './utils/warningPropsUtil';
import { warningNullOptions } from './utils/warningPropsUtil';

export interface BaseOptionType {
disabled?: boolean;
Expand Down Expand Up @@ -67,9 +67,9 @@ interface BaseCascaderProps<
OptionType extends DefaultOptionType = DefaultOptionType,
ValueField extends keyof OptionType = keyof OptionType,
> extends Omit<
BaseSelectPropsWithoutPrivate,
'tokenSeparators' | 'labelInValue' | 'mode' | 'showSearch'
> {
BaseSelectPropsWithoutPrivate,
'tokenSeparators' | 'labelInValue' | 'mode' | 'showSearch'
> {
// MISC
id?: string;
prefixCls?: string;
Expand All @@ -95,18 +95,14 @@ interface BaseCascaderProps<
// Options
options?: OptionType[];
/** @private Internal usage. Do not use in your production. */
dropdownPrefixCls?: string;
popupPrefixCls?: string;
loadData?: (selectOptions: OptionType[]) => void;

// Open
/** @deprecated Use `open` instead */
popupVisible?: boolean;

popupClassName?: string;
/** @deprecated Use `popupMenuColumnStyle` instead */
dropdownMenuColumnStyle?: React.CSSProperties;
popupMenuColumnStyle?: React.CSSProperties;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

❓ Verification inconclusive

缺少类名属性更新

popupClassName 属性应该标记为过时,并添加 dropdownClassName 作为新的 API。根据之前的评审意见,类名也需要更新。

 popupClassName?: string;
+/** @deprecated Use `dropdownClassName` instead */

🏁 Script executed:

#!/bin/bash
# 检查测试中是否使用了 dropdownClassName
grep -n "dropdownClassName" tests/index.spec.tsx

Length of output: 48


请更新类名属性和相关测试用例

  • 在文件 src/Cascader.tsx 中,将 popupClassName 属性标记为过时,并新增 dropdownClassName 属性作为替代方案。
  • 根据已有评审意见,需要在代码中添加如下变更:
     popupClassName?: string;
    +/** @deprecated 使用 `dropdownClassName` 代替 */
    +dropdownClassName?: string;
  • 请注意,目前在测试文件 tests/index.spec.tsx 中没有发现 dropdownClassName 的使用。请务必手动检查或更新相关测试,确保新 API 能覆盖所有预期场景。

Committable suggestion skipped: line range outside the PR's diff.


/** @deprecated Use `placement` instead */
popupPlacement?: Placement;
placement?: Placement;
builtinPlacements?: BuildInPlacements;

Expand All @@ -131,8 +127,8 @@ export type ValueType<
ValueField extends keyof OptionType = keyof OptionType,
> = keyof OptionType extends ValueField
? unknown extends OptionType['value']
? OptionType[ValueField]
: OptionType['value']
? OptionType[ValueField]
: OptionType['value']
: OptionType[ValueField];

export type GetValueType<
Expand Down Expand Up @@ -201,18 +197,16 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re

// Options
options,
dropdownPrefixCls,
popupPrefixCls,
loadData,

// Open
popupVisible,
open,

popupClassName,
dropdownMenuColumnStyle,
popupMenuColumnStyle,
popupStyle: customPopupStyle,

popupPlacement,
placement,

onPopupVisibleChange,
Expand Down Expand Up @@ -272,7 +266,7 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
mergedSearchValue,
mergedOptions,
mergedFieldNames,
dropdownPrefixCls || prefixCls,
popupPrefixCls || prefixCls,
searchConfig,
changeOnSelect || multiple,
);
Expand Down Expand Up @@ -365,19 +359,14 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
onInternalSelect(valueCells);
};

// ============================ Open ============================
const mergedOpen = open !== undefined ? open : popupVisible;


const mergedPlacement = placement || popupPlacement;

const onInternalPopupVisibleChange = (nextVisible: boolean) => {
onPopupVisibleChange?.(nextVisible);
};

const mergedPopupMenuColumnStyle = popupMenuColumnStyle || dropdownMenuColumnStyle;

// ========================== Warning ===========================
if (process.env.NODE_ENV !== 'production') {
warningProps(props);
warningNullOptions(mergedOptions, mergedFieldNames);
}

Expand All @@ -392,12 +381,12 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
onSelect: onInternalSelect,
checkable,
searchOptions,
dropdownPrefixCls,
popupPrefixCls,
loadData,
expandTrigger,
expandIcon,
loadingIcon,
dropdownMenuColumnStyle,
popupMenuColumnStyle: mergedPopupMenuColumnStyle,
optionRender,
}),
[
Expand All @@ -409,12 +398,12 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
onInternalSelect,
checkable,
searchOptions,
dropdownPrefixCls,
popupPrefixCls,
loadData,
expandTrigger,
expandIcon,
loadingIcon,
dropdownMenuColumnStyle,
mergedPopupMenuColumnStyle,
optionRender,
],
);
Expand All @@ -427,12 +416,12 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
const popupStyle: React.CSSProperties =
// Search to match width
(mergedSearchValue && searchConfig.matchInputWidth) ||
// Empty keep the width
emptyOptions
// Empty keep the width
emptyOptions
? {}
: {
minWidth: 'auto',
};
minWidth: 'auto',
};

return (
<CascaderContext.Provider value={cascaderContext}>
Expand Down Expand Up @@ -460,9 +449,9 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
OptionList={OptionList}
emptyOptions={emptyOptions}
// Open
open={mergedOpen}
open={open}
popupClassName={popupClassName}
placement={mergedPlacement}
placement={placement}
onPopupVisibleChange={onInternalPopupVisibleChange}
// Children
getRawInputElement={() => children as React.ReactElement}
Expand Down
4 changes: 2 additions & 2 deletions src/OptionList/Column.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
expandTrigger,
expandIcon,
loadingIcon,
dropdownMenuColumnStyle,
popupMenuColumnStyle,
optionRender,
} = React.useContext(CascaderContext);

Expand Down Expand Up @@ -155,7 +155,7 @@ export default function Column<OptionType extends DefaultOptionType = DefaultOpt
[`${menuItemPrefixCls}-disabled`]: isOptionDisabled(disabled),
[`${menuItemPrefixCls}-loading`]: isLoading,
})}
style={dropdownMenuColumnStyle}
style={popupMenuColumnStyle}
role="menuitemcheckbox"
title={title}
aria-checked={checked}
Expand Down
4 changes: 2 additions & 2 deletions src/OptionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ const RawOptionList = React.forwardRef<RefOptionListProps, RawOptionListProps>((
changeOnSelect,
onSelect,
searchOptions,
dropdownPrefixCls,
popupPrefixCls,
loadData,
expandTrigger,
} = React.useContext(CascaderContext);

const mergedPrefixCls = dropdownPrefixCls || prefixCls;
const mergedPrefixCls = popupPrefixCls || prefixCls;

// ========================= loadData =========================
const [loadingKeys, setLoadingKeys] = React.useState<React.Key[]>([]);
Expand Down
4 changes: 2 additions & 2 deletions src/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,12 @@ export default function Panel<
onSelect: onInternalSelect,
checkable,
searchOptions: [],
dropdownPrefixCls: undefined,
popupPrefixCls: undefined,
loadData,
expandTrigger,
expandIcon,
loadingIcon,
dropdownMenuColumnStyle: undefined,
popupMenuColumnStyle: undefined,
}),
[
mergedOptions,
Expand Down
4 changes: 2 additions & 2 deletions src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ export interface CascaderContextProps {
onSelect: (valuePath: SingleValueType) => void;
checkable?: boolean | React.ReactNode;
searchOptions: DefaultOptionType[];
dropdownPrefixCls?: string;
popupPrefixCls?: string;
loadData?: (selectOptions: DefaultOptionType[]) => void;
expandTrigger?: 'hover' | 'click';
expandIcon?: React.ReactNode;
loadingIcon?: React.ReactNode;
dropdownMenuColumnStyle?: React.CSSProperties;
popupMenuColumnStyle?: React.CSSProperties;
optionRender?: CascaderProps['optionRender'];
}

Expand Down
14 changes: 1 addition & 13 deletions src/utils/warningPropsUtil.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
import warning from '@rc-component/util/lib/warning';
import type { DefaultOptionType, FieldNames, InternalCascaderProps } from '../Cascader';

function warningProps(props: InternalCascaderProps) {
const { popupVisible, popupPlacement } = props;

warning(popupVisible === undefined, '`popupVisible` is deprecated. Please use `open` instead.');
warning(
popupPlacement === undefined,
'`popupPlacement` is deprecated. Please use `placement` instead.',
);
}
import type { DefaultOptionType, FieldNames } from '../Cascader';

// value in Cascader options should not be null
export function warningNullOptions(options: DefaultOptionType[], fieldNames: FieldNames) {
Expand All @@ -35,5 +25,3 @@ export function warningNullOptions(options: DefaultOptionType[], fieldNames: Fie
recursiveOptions(options);
}
}

export default warningProps;
18 changes: 5 additions & 13 deletions tests/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -514,28 +514,20 @@ describe('Cascader.Basic', () => {
expect(wrapper.isOpen()).toBeTruthy();
});

it('warning popupVisible & onPopupVisibleChange & popupClassName', () => {
it('warning onPopupVisibleChange & popupClassName', () => {
resetWarned();
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const onPopupVisibleChange = jest.fn();
const wrapper = mount(
<Cascader
popupVisible
open
onPopupVisibleChange={onPopupVisibleChange}
popupClassName="legacy-cls"
popupPlacement="topRight"
placement="topRight"
/>,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

测试用例与 API 更新方向不一致

测试用例的描述和使用的属性与实际 API 迁移方向不符:

  1. 测试描述仍然使用 onPopupVisibleChange 而非 onDropdownVisibleChange
  2. 使用 popupClassName 而非 dropdownClassName

根据 PR 的目标是废弃旧 API,测试用例应该使用新的 API 名称。

-  it('warning onPopupVisibleChange & popupClassName', () => {
+  it('warning onDropdownVisibleChange & dropdownClassName', () => {
     resetWarned();
     const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
     const onPopupVisibleChange = jest.fn();
     const wrapper = mount(
       <Cascader
         open
-        onPopupVisibleChange={onPopupVisibleChange}
-        popupClassName="legacy-cls"
+        onDropdownVisibleChange={onPopupVisibleChange}
+        dropdownClassName="legacy-cls"
         placement="topRight"
       />,
     );
📝 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
it('warning onPopupVisibleChange & popupClassName', () => {
resetWarned();
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const onPopupVisibleChange = jest.fn();
const wrapper = mount(
<Cascader
popupVisible
open
onPopupVisibleChange={onPopupVisibleChange}
popupClassName="legacy-cls"
popupPlacement="topRight"
placement="topRight"
/>,
it('warning onDropdownVisibleChange & dropdownClassName', () => {
resetWarned();
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const onPopupVisibleChange = jest.fn();
const wrapper = mount(
<Cascader
open
onDropdownVisibleChange={onPopupVisibleChange}
dropdownClassName="legacy-cls"
placement="topRight"
/>,
);

);

expect(errorSpy).toHaveBeenCalledWith(
'Warning: `popupVisible` is deprecated. Please use `open` instead.',
);
expect(errorSpy).toHaveBeenCalledWith(
'Warning: `popupPlacement` is deprecated. Please use `placement` instead.',
);

expect(wrapper.exists('.legacy-cls')).toBeTruthy();
expect(wrapper.find('Trigger').prop('popupPlacement')).toEqual('topRight');

errorSpy.mockRestore();
});
Expand Down Expand Up @@ -604,7 +596,7 @@ describe('Cascader.Basic', () => {
},
];
const wrapper = mount(
<Cascader options={options} popupVisible>
<Cascader options={options} open>
<input readOnly />
</Cascader>,
);
Expand All @@ -616,7 +608,7 @@ describe('Cascader.Basic', () => {
const wrapper = mount(
<Cascader
options={addressOptions}
popupVisible
open
popupRender={menus => (
<div className="custom-dropdown">
{menus}
Expand Down
2 changes: 1 addition & 1 deletion tests/private.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('Cascader.Private', () => {
]}
open
prefixCls="bamboo"
dropdownPrefixCls="little"
popupPrefixCls="little"
/>,
);

Expand Down
Loading