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.

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/actions #238

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
64 changes: 64 additions & 0 deletions components/actions/ActionMenu.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { EllipsisOutlined } from '@ant-design/icons';
import { Dropdown, MenuProps } from 'antd';
import React from 'react';
import { useXProviderContext } from '../x-provider';
import { ActionItemType, ActionsProps, SubItemType } from './interface';

export const findItem = (keyPath: string[], items: ActionItemType[]): ActionItemType | null => {
const keyToFind = keyPath[0]; // Get the first key from the keyPath

for (const item of items) {
if (item.key === keyToFind) {
// If the item is found and this is the last key in the path
if (keyPath.length === 1) return item;

// If it is a SubItemType, recurse to find in its children
if ('children' in item) {
return findItem(keyPath.slice(1), item.children!);
}
}
}

return null;
};
Comment on lines +7 to +23
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

需要增加空值检查以提高代码健壮性

在递归查找过程中,对 item.children 的访问可能会导致运行时错误。

建议按照以下方式修改代码:

 if ('children' in item) {
-  return findItem(keyPath.slice(1), item.children!);
+  if (item.children) {
+    return findItem(keyPath.slice(1), item.children);
+  }
 }
📝 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
export const findItem = (keyPath: string[], items: ActionItemType[]): ActionItemType | null => {
const keyToFind = keyPath[0]; // Get the first key from the keyPath
for (const item of items) {
if (item.key === keyToFind) {
// If the item is found and this is the last key in the path
if (keyPath.length === 1) return item;
// If it is a SubItemType, recurse to find in its children
if ('children' in item) {
return findItem(keyPath.slice(1), item.children!);
}
}
}
return null;
};
export const findItem = (keyPath: string[], items: ActionItemType[]): ActionItemType | null => {
const keyToFind = keyPath[0]; // Get the first key from the keyPath
for (const item of items) {
if (item.key === keyToFind) {
// If the item is found and this is the last key in the path
if (keyPath.length === 1) return item;
// If it is a SubItemType, recurse to find in its children
if ('children' in item) {
if (item.children) {
return findItem(keyPath.slice(1), item.children);
}
}
}
}
return null;
};


const ActionMenu = (props: { item: SubItemType } & Pick<ActionsProps, 'prefixCls' | 'onClick'>) => {
const { onClick: onMenuClick } = props;
const item = props.item;
const { children = [], triggerSubMenuAction = 'hover' } = item;
const { getPrefixCls } = useXProviderContext();
const prefixCls = getPrefixCls('actions', props.prefixCls);
const icon = item?.icon ?? <EllipsisOutlined />;

const menuProps: MenuProps = {
items: children,

onClick: ({ key, keyPath, domEvent }) => {
onMenuClick?.({
key,
keyPath: [item.key, ...keyPath],
domEvent,
item: findItem(keyPath, children)!,
});
},
};
Comment on lines +33 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

处理 findItem 可能返回 null 的情况

在第41行使用非空断言操作符 ! 可能导致运行时错误,因为 findItem 可能返回 null

建议修改为:

-item: findItem(keyPath, children)!,
+const foundItem = findItem(keyPath, children);
+if (!foundItem) {
+  console.warn(`未找到键路径为 ${keyPath.join('/')} 的菜单项`);
+  return;
+}
+item: foundItem,

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


return (
<Dropdown
menu={menuProps}
overlayClassName={`${prefixCls}-sub-item`}
arrow
trigger={[triggerSubMenuAction]}
>
<div className={`${prefixCls}-list-item`}>
<div className={`${prefixCls}-list-item-icon`}>{icon}</div>
</div>
</Dropdown>
);
};

if (process.env.NODE_ENV === 'production') {
ActionMenu.displayName = 'ActionMenu';
}

export default ActionMenu;
185 changes: 185 additions & 0 deletions components/actions/__tests__/__snapshots__/demo.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`renders components/actions/demo/basic.tsx correctly 1`] = `
<div
class="ant-actions"
>
<div
class="ant-actions-list borderless"
>
<div
class="ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="redo"
class="anticon anticon-redo"
role="img"
>
<svg
aria-hidden="true"
data-icon="redo"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M758.2 839.1C851.8 765.9 912 651.9 912 523.9 912 303 733.5 124.3 512.6 124 291.4 123.7 112 302.8 112 523.9c0 125.2 57.5 236.9 147.6 310.2 3.5 2.8 8.6 2.2 11.4-1.3l39.4-50.5c2.7-3.4 2.1-8.3-1.2-11.1-8.1-6.6-15.9-13.7-23.4-21.2a318.64 318.64 0 01-68.6-101.7C200.4 609 192 567.1 192 523.9s8.4-85.1 25.1-124.5c16.1-38.1 39.2-72.3 68.6-101.7 29.4-29.4 63.6-52.5 101.7-68.6C426.9 212.4 468.8 204 512 204s85.1 8.4 124.5 25.1c38.1 16.1 72.3 39.2 101.7 68.6 29.4 29.4 52.5 63.6 68.6 101.7 16.7 39.4 25.1 81.3 25.1 124.5s-8.4 85.1-25.1 124.5a318.64 318.64 0 01-68.6 101.7c-9.3 9.3-19.1 18-29.3 26L668.2 724a8 8 0 00-14.1 3l-39.6 162.2c-1.2 5 2.6 9.9 7.7 9.9l167 .8c6.7 0 10.5-7.7 6.3-12.9l-37.3-47.9z"
/>
</svg>
</span>
</div>
</div>
<div
class="ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="copy"
class="anticon anticon-copy"
role="img"
>
<svg
aria-hidden="true"
data-icon="copy"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M832 64H296c-4.4 0-8 3.6-8 8v56c0 4.4 3.6 8 8 8h496v688c0 4.4 3.6 8 8 8h56c4.4 0 8-3.6 8-8V96c0-17.7-14.3-32-32-32zM704 192H192c-17.7 0-32 14.3-32 32v530.7c0 8.5 3.4 16.6 9.4 22.6l173.3 173.3c2.2 2.2 4.7 4 7.4 5.5v1.9h4.2c3.5 1.3 7.2 2 11 2H704c17.7 0 32-14.3 32-32V224c0-17.7-14.3-32-32-32zM350 856.2L263.9 770H350v86.2zM664 888H414V746c0-22.1-17.9-40-40-40H232V264h432v624z"
/>
</svg>
</span>
</div>
</div>
</div>
</div>
`;

exports[`renders components/actions/demo/sub.tsx correctly 1`] = `
<div
class="ant-actions"
>
<div
class="ant-actions-list borderless"
>
<div
class="ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="redo"
class="anticon anticon-redo"
role="img"
>
<svg
aria-hidden="true"
data-icon="redo"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M758.2 839.1C851.8 765.9 912 651.9 912 523.9 912 303 733.5 124.3 512.6 124 291.4 123.7 112 302.8 112 523.9c0 125.2 57.5 236.9 147.6 310.2 3.5 2.8 8.6 2.2 11.4-1.3l39.4-50.5c2.7-3.4 2.1-8.3-1.2-11.1-8.1-6.6-15.9-13.7-23.4-21.2a318.64 318.64 0 01-68.6-101.7C200.4 609 192 567.1 192 523.9s8.4-85.1 25.1-124.5c16.1-38.1 39.2-72.3 68.6-101.7 29.4-29.4 63.6-52.5 101.7-68.6C426.9 212.4 468.8 204 512 204s85.1 8.4 124.5 25.1c38.1 16.1 72.3 39.2 101.7 68.6 29.4 29.4 52.5 63.6 68.6 101.7 16.7 39.4 25.1 81.3 25.1 124.5s-8.4 85.1-25.1 124.5a318.64 318.64 0 01-68.6 101.7c-9.3 9.3-19.1 18-29.3 26L668.2 724a8 8 0 00-14.1 3l-39.6 162.2c-1.2 5 2.6 9.9 7.7 9.9l167 .8c6.7 0 10.5-7.7 6.3-12.9l-37.3-47.9z"
/>
</svg>
</span>
</div>
</div>
<div
class="ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="copy"
class="anticon anticon-copy"
role="img"
>
<svg
aria-hidden="true"
data-icon="copy"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M832 64H296c-4.4 0-8 3.6-8 8v56c0 4.4 3.6 8 8 8h496v688c0 4.4 3.6 8 8 8h56c4.4 0 8-3.6 8-8V96c0-17.7-14.3-32-32-32zM704 192H192c-17.7 0-32 14.3-32 32v530.7c0 8.5 3.4 16.6 9.4 22.6l173.3 173.3c2.2 2.2 4.7 4 7.4 5.5v1.9h4.2c3.5 1.3 7.2 2 11 2H704c17.7 0 32-14.3 32-32V224c0-17.7-14.3-32-32-32zM350 856.2L263.9 770H350v86.2zM664 888H414V746c0-22.1-17.9-40-40-40H232V264h432v624z"
/>
</svg>
</span>
</div>
</div>
<div
class="ant-dropdown-trigger ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="ellipsis"
class="anticon anticon-ellipsis"
role="img"
>
<svg
aria-hidden="true"
data-icon="ellipsis"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M176 511a56 56 0 10112 0 56 56 0 10-112 0zm280 0a56 56 0 10112 0 56 56 0 10-112 0zm280 0a56 56 0 10112 0 56 56 0 10-112 0z"
/>
</svg>
</span>
</div>
</div>
<div
class="ant-actions-list-item"
>
<div
class="ant-actions-list-item-icon"
>
<span
aria-label="reload"
class="anticon anticon-reload"
role="img"
>
<svg
aria-hidden="true"
data-icon="reload"
fill="currentColor"
focusable="false"
height="1em"
viewBox="64 64 896 896"
width="1em"
>
<path
d="M909.1 209.3l-56.4 44.1C775.8 155.1 656.2 92 521.9 92 290 92 102.3 279.5 102 511.5 101.7 743.7 289.8 932 521.9 932c181.3 0 335.8-115 394.6-276.1 1.5-4.2-.7-8.9-4.9-10.3l-56.7-19.5a8 8 0 00-10.1 4.8c-1.8 5-3.8 10-5.9 14.9-17.3 41-42.1 77.8-73.7 109.4A344.77 344.77 0 01655.9 829c-42.3 17.9-87.4 27-133.8 27-46.5 0-91.5-9.1-133.8-27A341.5 341.5 0 01279 755.2a342.16 342.16 0 01-73.7-109.4c-17.9-42.4-27-87.4-27-133.9s9.1-91.5 27-133.9c17.3-41 42.1-77.8 73.7-109.4 31.6-31.6 68.4-56.4 109.3-73.8 42.3-17.9 87.4-27 133.8-27 46.5 0 91.5 9.1 133.8 27a341.5 341.5 0 01109.3 73.8c9.9 9.9 19.2 20.4 27.8 31.4l-60.2 47a8 8 0 003 14.1l175.6 43c5 1.2 9.9-2.6 9.9-7.7l.8-180.9c-.1-6.6-7.8-10.3-13-6.2z"
/>
</svg>
</span>
</div>
</div>
</div>
</div>
`;
44 changes: 44 additions & 0 deletions components/actions/__tests__/actionMenu.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { fireEvent, render } from '@testing-library/react';
import React from 'react';
import ActionMenu, { findItem } from '../ActionMenu'; // Adjust the import according to your file structure
import { ActionItemType } from '../interface';

describe('findItem function', () => {
const items: ActionItemType[] = [
{ key: '1', label: 'Action 1' },
{
key: '2',
label: 'Action 2',
children: [
{ key: '2-1', label: 'Sub Action 1' },
{ key: '2-2', label: 'Sub Action 2' },
],
},
{ key: '3', label: 'Action 3' },
];

it('should return the item if it exists at the root level', () => {
const result = findItem(['1'], items);
expect(result).toEqual(items[0]);
});

it('should return the item if it exists at a deeper level', () => {
const result = findItem(['2', '2-1'], items);
expect(result).toEqual(items[1].children![0]);
});
Comment on lines +25 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

优化类型断言处理

在访问嵌套子项时使用了非空断言操作符 !,这可能会在运行时导致潜在的类型安全问题。

建议修改为以下实现:

 it('should return the item if it exists at a deeper level', () => {
   const result = findItem(['2', '2-1'], items);
-  expect(result).toEqual(items[1].children![0]);
+  const children = items[1].children;
+  expect(children).toBeDefined();
+  expect(result).toEqual(children?.[0]);
 });
📝 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('should return the item if it exists at a deeper level', () => {
const result = findItem(['2', '2-1'], items);
expect(result).toEqual(items[1].children![0]);
});
it('should return the item if it exists at a deeper level', () => {
const result = findItem(['2', '2-1'], items);
const children = items[1].children;
expect(children).toBeDefined();
expect(result).toEqual(children?.[0]);
});


it('should return null if the item does not exist', () => {
const result = findItem(['4'], items);
expect(result).toBeNull();
});

it('should return null when searching a non-existent sub-item', () => {
const result = findItem(['2', '2-3'], items);
expect(result).toBeNull();
});

it('should handle an empty keyPath gracefully', () => {
const result = findItem([], items);
expect(result).toBeNull();
});
});
55 changes: 55 additions & 0 deletions components/actions/__tests__/actions.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { fireEvent, render, waitFor } from '@testing-library/react';
import React from 'react';
import Actions, { ActionsProps } from '../index'; // Adjust the import according to your file structure

describe('Actions Component', () => {
const consoleSpy = jest.spyOn(console, 'log'); // 监视 console.log
const mockOnClick = jest.fn();
const items = [
{ key: '1', label: 'Action 1', icon: <span>icon1</span> },
{
key: '2',
label: 'Action 2',
icon: <span>icon2</span>,
onClick: () => console.log('Action 2 clicked'),
},
{
key: 'sub',
children: [{ key: 'sub-1', label: 'Sub Action 1', icon: <span>⚙️</span> }],
},
];

it('renders correctly', () => {
const { getByText } = render(<Actions items={items} onClick={mockOnClick} />);

expect(getByText('icon1')).toBeInTheDocument();
expect(getByText('icon2')).toBeInTheDocument();
});

it('calls onClick when an action item is clicked', () => {
const onClick: ActionsProps['onClick'] = ({ keyPath }) => {
console.log(`You clicked ${keyPath.join(',')}`);
};
const { getByText } = render(<Actions items={items} onClick={onClick} />);

fireEvent.click(getByText('icon1'));
expect(consoleSpy).toHaveBeenCalledWith('You clicked 1');
});

it('calls individual item onClick if provided', () => {
const consoleSpy = jest.spyOn(console, 'log');
const { getByText } = render(<Actions items={items} onClick={mockOnClick} />);

fireEvent.click(getByText('icon2'));
expect(consoleSpy).toHaveBeenCalledWith('Action 2 clicked');
consoleSpy.mockRestore();
});

it('renders sub-menu items', async () => {
const { getByText, container } = render(<Actions items={items} onClick={mockOnClick} />);

fireEvent.mouseOver(container.querySelector('.ant-dropdown-trigger')!); // Assuming the dropdown opens on hover

await waitFor(() => expect(getByText('Sub Action 1')).toBeInTheDocument());
});
Comment on lines +48 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

改进子菜单测试的可靠性

当前的子菜单测试存在以下问题:

  1. 使用 CSS 选择器 .ant-dropdown-trigger 不够可靠,建议使用 data-testid
  2. 缺少超时处理

建议修改如下:

-  it('renders sub-menu items', async () => {
-    const { getByText, container } = render(<Actions items={items} onClick={mockOnClick} />);
-
-    fireEvent.mouseOver(container.querySelector('.ant-dropdown-trigger')!);
-
-    await waitFor(() => expect(getByText('Sub Action 1')).toBeInTheDocument());
-  });
+  it('renders sub-menu items', async () => {
+    const { getByText, getByTestId } = render(
+      <Actions items={items} onClick={mockOnClick} data-testid="actions-dropdown" />
+    );
+
+    fireEvent.mouseOver(getByTestId('actions-dropdown'));
+
+    await waitFor(
+      () => expect(getByText('Sub Action 1')).toBeInTheDocument(),
+      { timeout: 3000 }
+    );
+  });

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

});
7 changes: 7 additions & 0 deletions components/actions/demo/basic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## zh-CN

基础用法。

## en-US

Basic usage.
26 changes: 26 additions & 0 deletions components/actions/demo/basic.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { CopyOutlined, RedoOutlined } from '@ant-design/icons';
import { Actions, ActionsProps } from '@ant-design/x';
import { message } from 'antd';
import React from 'react';

const actionItems = [
{
key: 'retry',
icon: <RedoOutlined />,
label: '重试',
},
{
key: 'copy',
icon: <CopyOutlined />,
label: '复制',
},
];

const Demo: React.FC = () => {
const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => {
message.success(`you click ${keyPath.join(',')}`);
};
return <Actions items={actionItems} onClick={onClick} />;
};
Comment on lines +19 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

需要改进的几个方面

  1. 成功提示消息使用英文,而界面标签使用中文,存在语言不一致的问题
  2. 缺少错误处理机制

建议按照以下方式修改:

 const Demo: React.FC = () => {
+  const intl = useIntl();
+
   const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => {
-    message.success(`you click ${keyPath.join(',')}`);
+    try {
+      // 处理点击事件的逻辑
+      message.success(intl.formatMessage(
+        { id: 'actions.clicked' },
+        { path: keyPath.join(',') }
+      ));
+    } catch (error) {
+      message.error(intl.formatMessage({ id: 'actions.error' }));
+    }
   };
   return <Actions items={actionItems} onClick={onClick} />;
 };

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


export default Demo;
7 changes: 7 additions & 0 deletions components/actions/demo/sub.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## zh-CN

更多菜单项。

## en-US

Basic usage.
Loading
Loading