Skip to content
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.

Sign up for GitHub

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

feat: retire deprecated api #558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: retire deprecated api #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion docs/demo/visible.md → docs/demo/open.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ nav:
path: /demo
---

<code src="../../examples/visible.tsx"></code>
<code src="../../examples/open.tsx"></code>
6 changes: 3 additions & 3 deletions examples/visible.tsx → examples/open.tsx
View file
Open in desktop
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)}
onChange={value => setValue(value)}
>
<input value={getLabel()} readOnly />
Expand Down
23 changes: 6 additions & 17 deletions src/Cascader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,18 @@ interface BaseCascaderProps<
loadData?: (selectOptions: OptionType[]) => void;

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

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

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

/** @deprecated Use `onDropdownVisibleChange` instead */
onPopupVisibleChange?: (open: boolean) => void;
/** @deprecated Use `onPopupVisibleChange` instead */
onDropdownVisibleChange?: (open: boolean) => void;
onPopupVisibleChange?: (open: boolean) => void;

// Icon
expandIcon?: React.ReactNode;
Expand Down Expand Up @@ -209,15 +205,13 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
loadData,

// Open
popupVisible,
open,

popupClassName,
dropdownClassName,
dropdownMenuColumnStyle,
dropdownStyle: customDropdownStyle,

popupPlacement,
placement,

onDropdownVisibleChange,
Expand Down Expand Up @@ -371,13 +365,8 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
onInternalSelect(valueCells);
};

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

const mergedDropdownClassName = dropdownClassName || popupClassName;

const mergedPlacement = placement || popupPlacement;

const onInternalDropdownVisibleChange = (nextVisible: boolean) => {
onDropdownVisibleChange?.(nextVisible);
onPopupVisibleChange?.(nextVisible);
Expand Down Expand Up @@ -468,9 +457,9 @@ const Cascader = React.forwardRef<CascaderRef, InternalCascaderProps>((props, re
OptionList={OptionList}
emptyOptions={emptyOptions}
// Open
open={mergedOpen}
open={open}
dropdownClassName={mergedDropdownClassName}
placement={mergedPlacement}
placement={placement}
onDropdownVisibleChange={onInternalDropdownVisibleChange}
// Children
getRawInputElement={() => children as React.ReactElement}
Expand Down
15 changes: 5 additions & 10 deletions src/utils/warningPropsUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,15 @@ import warning from 'rc-util/lib/warning';
import type { DefaultOptionType, FieldNames, InternalCascaderProps } from '../Cascader';

function warningProps(props: InternalCascaderProps) {
const { onPopupVisibleChange, popupVisible, popupClassName, popupPlacement } = props;
const { onDropdownVisibleChange, dropdownClassName } = props;

warning(
!onPopupVisibleChange,
'`onPopupVisibleChange` is deprecated. Please use `onDropdownVisibleChange` instead.',
!onDropdownVisibleChange,
'`onDropdownVisibleChange` is deprecated. Please use `onPopupVisibleChange` instead.',
Comment on lines +8 to +9
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 名称似乎颠倒了：

  1. 声明 onDropdownVisibleChange 已废弃，建议使用 onPopupVisibleChange
  2. 声明 dropdownClassName 已废弃，建议使用 popupClassName

这与实际的 API 迁移方向（从 popup 到 dropdown）相反。

建议修改为：

-    !onDropdownVisibleChange,
-    '`onDropdownVisibleChange` is deprecated. Please use `onPopupVisibleChange` instead.',
+    !onPopupVisibleChange,
+    '`onPopupVisibleChange` is deprecated. Please use `onDropdownVisibleChange` instead.',

-    dropdownClassName === undefined,
-    '`dropdownClassName` is deprecated. Please use `popupClassName` instead.',
+    popupClassName === undefined,
+    '`popupClassName` is deprecated. Please use `dropdownClassName` instead.',

Also applies to: 12-13

);
warning(popupVisible === undefined, '`popupVisible` is deprecated. Please use `open` instead.');
warning(
popupClassName === undefined,
'`popupClassName` is deprecated. Please use `dropdownClassName` instead.',
);
warning(
popupPlacement === undefined,
'`popupPlacement` is deprecated. Please use `placement` instead.',
dropdownClassName === undefined,
'`dropdownClassName` is deprecated. Please use `popupClassName` instead.',
);
}

Expand Down
26 changes: 10 additions & 16 deletions tests/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,30 +514,24 @@ describe('Cascader.Basic', () => {
expect(wrapper.isOpen()).toBeTruthy();
});

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

expect(errorSpy).toHaveBeenCalledWith(
'Warning: `onPopupVisibleChange` is deprecated. Please use `onDropdownVisibleChange` instead.',
);
expect(errorSpy).toHaveBeenCalledWith(
'Warning: `popupVisible` is deprecated. Please use `open` instead.',
'Warning: `onDropdownVisibleChange` is deprecated. Please use `onPopupVisibleChange` instead.',
);
expect(errorSpy).toHaveBeenCalledWith(
'Warning: `popupClassName` is deprecated. Please use `dropdownClassName` instead.',
);
expect(errorSpy).toHaveBeenCalledWith(
'Warning: `popupPlacement` is deprecated. Please use `placement` instead.',
'Warning: `dropdownClassName` is deprecated. Please use `popupClassName` instead.',
Comment on lines +517 to +534
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. 测试用例使用新 API（onDropdownVisibleChange）但期望看到废弃警告
  2. 警告信息的断言与实际迁移方向相反

建议修改测试用例：

-    expect(errorSpy).toHaveBeenCalledWith(
-      'Warning: `onDropdownVisibleChange` is deprecated. Please use `onPopupVisibleChange` instead.',
-    );
+    expect(errorSpy).toHaveBeenCalledWith(
+      'Warning: `onPopupVisibleChange` is deprecated. Please use `onDropdownVisibleChange` instead.',
+    );

-    expect(errorSpy).toHaveBeenCalledWith(
-      'Warning: `dropdownClassName` is deprecated. Please use `popupClassName` instead.',
-    );
+    expect(errorSpy).toHaveBeenCalledWith(
+      'Warning: `popupClassName` is deprecated. Please use `dropdownClassName` instead.',
+    );

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

);

expect(wrapper.exists('.legacy-cls')).toBeTruthy();
Expand Down Expand Up @@ -610,7 +604,7 @@ describe('Cascader.Basic', () => {
},
];
const wrapper = mount(
<Cascader options={options} popupVisible>
<Cascader options={options} open>
<input readOnly />
</Cascader>,
);
Expand All @@ -622,7 +616,7 @@ describe('Cascader.Basic', () => {
const wrapper = mount(
<Cascader
options={addressOptions}
popupVisible
open
dropdownRender={menus => (
<div className="custom-dropdown">
{menus}
Expand Down
Loading