Feat/actions #238
Open
Feat/actions #238
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthrough此次更改引入了多个新的 React 组件和相关的测试、文档以及样式配置，主要集中在 Changes
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (17)
components/actions/demo/basic.tsx (1)
6-17: 建议实现国际化支持当前的文本标签是硬编码的中文字符串。建议使用国际化解决方案来支持多语言。
建议按照以下方式修改：
+import { useIntl } from '@ant-design/x'; + +const Demo: React.FC = () => { + const intl = useIntl(); + const actionItems = [ { key: 'retry', icon: <RedoOutlined />, - label: '重试', + label: intl.formatMessage({ id: 'actions.retry' }), }, { key: 'copy', icon: <CopyOutlined />, - label: '复制', + label: intl.formatMessage({ id: 'actions.copy' }), }, ];components/actions/demo/variant.tsx (1)
1-4: 建议添加类型导入以增强类型安全性建议从
@ant-design/x中显式导入ActionsItem类型，以便更好地定义actionItems的类型。import { CopyOutlined, RedoOutlined } from '@ant-design/icons'; -import { Actions, ActionsProps } from '@ant-design/x'; +import { Actions, ActionsProps, ActionsItem } from '@ant-design/x'; import { message } from 'antd'; import React from 'react';components/actions/demo/sub.tsx (1)
1-10: 建议对导入进行分组整理建议将导入按照以下顺序分组：
- React 相关
- 第三方库（antd）
- 组件和图标
- 类型定义
+import React from 'react'; + +import { message } from 'antd'; +import { Actions, ActionsProps } from '@ant-design/x'; + import { CopyOutlined, DeleteOutlined, RedoOutlined, ReloadOutlined, ShareAltOutlined, } from '@ant-design/icons'; -import { Actions, ActionsProps } from '@ant-design/x'; -import { message } from 'antd'; -import React from 'react';components/actions/__tests__/actionMenu.test.tsx (2)
1-4: 清理未使用的导入并更新注释代码中存在以下问题：
- 导入了未使用的
fireEvent- 代码注释显示对文件结构存在不确定性
建议应用以下修改：
-import { fireEvent, render } from '@testing-library/react'; +import { render } from '@testing-library/react'; import React from 'react'; -import ActionMenu, { findItem } from '../ActionMenu'; // Adjust the import according to your file structure +import ActionMenu, { findItem } from '../ActionMenu'; import { ActionItemType } from '../interface';
20-43: 建议增加测试用例覆盖更多边界情况当前的测试用例覆盖了基本场景，但建议添加以下测试场景：
- 处理
children属性为undefined的情况- 处理
items参数为空数组的情况- 处理
items参数为undefined的情况是否需要我提供这些额外测试用例的实现代码？
components/actions/ActionMenu.tsx (2)
25-31: 建议添加属性验证组件的关键属性（如
item和onClick）缺少运行时验证。建议添加 PropTypes 或运行时检查以提早发现问题。建议添加以下验证：
if (!item) { throw new Error('ActionMenu: item prop is required'); }
60-62: 重新考虑 displayName 的设置时机当前代码仅在生产环境设置 displayName，这与通常的做法相反。displayName 在开发环境下更有用，因为它有助于调试。
建议修改为：
-if (process.env.NODE_ENV === 'production') { +if (process.env.NODE_ENV !== 'production') { ActionMenu.displayName = 'ActionMenu'; }components/actions/__tests__/actions.test.tsx (2)
5-20: 建议添加类型定义并清理测试间谍以下几点建议：
- 为测试数据添加类型定义
- 在
afterEach中清理 console spy建议添加以下代码：
+import { ActionItem } from '../types'; // 假设类型定义在这个位置 describe('Actions Component', () => { const consoleSpy = jest.spyOn(console, 'log'); const mockOnClick = jest.fn(); - const items = [ + const items: ActionItem[] = [ // ... existing items ... ]; + afterEach(() => { + consoleSpy.mockRestore(); + mockOnClick.mockReset(); + });
22-46: 建议增加错误场景测试当前测试用例主要覆盖了正常流程，建议添加以下场景的测试：
- 空数据处理
- 禁用状态的项目点击
- 嵌套子菜单的多层级点击
需要我为这些场景生成测试代码吗？
components/actions/style/index.ts (1)
73-82: 建议添加必要的代码注释为了提高代码的可维护性，建议为以下部分添加注释：
prepareComponentToken函数的用途和未来扩展计划mergeToken的合并策略说明- 样式钩子的使用说明
建议添加如下注释：
+// 准备组件令牌，预留扩展空间用于未来自定义主题 export const prepareComponentToken: GetDefaultToken<'Prompts'> = () => ({}); export default genStyleHooks( 'Actions', (token) => { + // 合并组件级别的主题令牌 const compToken = mergeToken<ActionsToken>(token, {}); return [genActionsStyle(compToken)]; }, prepareComponentToken, );components/actions/index.en-US.md (4)
1-12: 建议添加更具体的排序信息在 frontmatter 中，建议为组件本身添加
order属性，以便更好地控制组件在文档中的显示顺序。这样可以确保组件在导航中的位置更加合理。--- category: Components group: title: Common order: 0 title: Actions +order: 1 description: Used for quickly configuring required action buttons or features in some AI scenarios.
14-16: 建议扩充使用场景说明"When to Use" 部分的说明过于简单。建议添加以下内容：
- 具体的使用场景示例
- 与其他类似组件的对比
- 最佳实践建议
18-23: 建议为示例添加简短描述为了帮助用户更好地理解每个示例的用途，建议在每个示例链接下方添加简短的说明文字，解释该示例演示的具体功能和使用场景。
<code src="./demo/basic.tsx">Basic</code> +基础用法示例，展示 Actions 组件的基本功能和配置方式。 <code src="./demo/sub.tsx">More Menu Items</code> +展示如何配置带有子菜单的 Actions 组件。 <code src="./demo/variant.tsx">Using Variants</code> +展示不同的样式变体，包括无边框和带边框的显示方式。
25-62: 建议添加类型使用示例API 文档的结构清晰，但对于复杂类型（如
ActionItemType）建议添加具体的代码示例，以帮助开发者更好地理解如何使用这些类型。同时，建议在
onClick回调函数的参数说明中添加更详细的类型定义。建议添加如下示例代码：
// ActionItemType 使用示例 const actionItems: ActionItemType[] = [ { label: '编辑', key: 'edit', icon: <EditIcon />, onClick: () => console.log('编辑被点击') }, { label: '更多', key: 'more', icon: <MoreIcon />, children: [ { label: '删除', key: 'delete', onClick: () => console.log('删除被点击') } ] } ];components/actions/interface.ts (2)
4-47: 建议对类型定义进行优化建议进行以下改进：
- 建议将 variant 的类型定义提取为单独的类型常量，以提高可维护性和重用性
- onClick 回调函数的类型可以更具体化，便于使用者理解
建议应用以下改动：
+export type ActionVariant = 'borderless' | 'border'; + +export type OnClickCallback = (menuInfo: { + item: ActionItemType; + key: string; + keyPath: string[]; + domEvent: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>; +}) => void; + export interface ActionsProps { // ... other properties ... - onClick?: (menuInfo: { - item: ActionItemType; - key: string; - keyPath: string[]; - domEvent: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>; - }) => void; + onClick?: OnClickCallback; // ... other properties ... - variant?: 'borderless' | 'border'; + variant?: ActionVariant; }
49-70: 建议增强 key 属性的类型安全性当前
key属性使用string类型可能过于宽松，建议考虑使用更严格的类型定义来防止潜在的错误。建议考虑以下改进：
+export type ActionKey = string; + export interface ItemType extends Pick<MenuItemProps, 'danger'> { // ... other properties ... - key: string; + key: ActionKey; // ... other properties ... }components/actions/index.tsx (1)
13-44: 建议改进类型安全性和样式合并逻辑当前实现可以通过以下方式优化：
- rootClassName 的默认值类型应该更明确
- 样式合并逻辑可以更简洁
建议应用以下改进：
- rootClassName = {}, + rootClassName = '',- const mergedStyle = { - ...style, - ...contextConfig.style, - }; + const mergedStyle = { ...contextConfig.style, ...style };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
components/actions/__tests__/__snapshots__/demo.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
components/actions/ActionMenu.tsx(1 hunks)components/actions/__tests__/actionMenu.test.tsx(1 hunks)components/actions/__tests__/actions.test.tsx(1 hunks)components/actions/demo/basic.md(1 hunks)components/actions/demo/basic.tsx(1 hunks)components/actions/demo/sub.md(1 hunks)components/actions/demo/sub.tsx(1 hunks)components/actions/demo/variant.md(1 hunks)components/actions/demo/variant.tsx(1 hunks)components/actions/index.en-US.md(1 hunks)components/actions/index.tsx(1 hunks)components/actions/index.zh-CN.md(1 hunks)components/actions/interface.ts(1 hunks)components/actions/style/index.ts(1 hunks)components/index.ts(1 hunks)components/theme/components.ts(2 hunks)components/x-provider/context.ts(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/actions/demo/basic.md
- components/actions/demo/sub.md
- components/actions/demo/variant.md
🧰 Additional context used
🪛 LanguageTool
components/actions/index.zh-CN.md
[uncategorized] ~23-~23: A punctuation mark might be missing here.
Context: ... 更多菜单项 使用变体</co...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
🔇 Additional comments (17)
components/actions/demo/basic.tsx (2)
1-4: 导入声明看起来很好！
导入声明简洁明了，包含了所有必要的依赖项。
26-26: 导出语句正确！
组件导出方式符合 React 规范。
components/actions/demo/variant.tsx (1)
26-26: 导出语句正确且符合最佳实践
默认导出符合组件命名规范。
components/theme/components.ts (2)
1-1: 导入语句的位置符合字母顺序规范！
新增的 ActionsComponentToken 导入语句正确地放置在了按字母顺序排列的导入列表的开头。
20-20: 验证 Actions 组件的主题令牌定义
新增的 Actions 属性扩展了组件主题定制能力，建议确认相关的样式令牌是否完整定义。
✅ Verification successful
Let me gather more information about the Actions component and its style definitions.
Let me check for the Actions component implementation and its style definitions.
Actions 组件的主题令牌定义已完整实现
从代码分析结果来看，Actions 组件的样式令牌已在 components/actions/style/index.ts 中完整定义：
- 定义了
ActionsToken 接口继承自 FullToken<'Prompts'>
- 实现了
genActionsStyle 样式生成函数
- 通过
mergeToken 正确处理了组件令牌
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 Actions 组件的样式令牌定义是否完整
# 检查 actions/style 中的样式令牌定义
rg -A 10 "export interface ComponentToken" "../actions/style"
# 检查是否有其他组件在使用 Actions 的样式令牌
rg "ActionsComponentToken" --type ts
Length of output: 333
Script:
#!/bin/bash
# Search for Actions style related files
fd -e ts -e tsx actions/style
# Search for Actions component token definitions
rg -g '*.ts' -g '*.tsx' "interface.*Token.*Actions" -A 5
# Check imports of Actions style tokens
rg -g '*.ts' -g '*.tsx' "from.*actions/style"
Length of output: 250
Script:
#!/bin/bash
# Search for Actions component and its style implementation
fd -e ts -e tsx actions
# Check for any Actions related style or token definitions
rg -g '*.ts' -g '*.tsx' "Actions.*Token|Token.*Actions" -B 2 -A 5
# Look for Actions component implementation
ast-grep --pattern 'interface Actions {
$$$
}'
Length of output: 2166
components/index.ts (2)
38-40: 导出结构符合规范！
新增的 Actions 组件导出遵循了文件中既有的模式，包括：
- 组件默认导出和类型导出配对
- 保持了适当的空行间距
- 维持了字母顺序排序
39-40: 建议验证 Actions 组件的依赖关系
为确保组件能够正常工作，建议验证 Actions 组件的所有必要依赖是否都已正确导出。
✅ Verification successful
Actions 组件依赖关系验证完成
经过验证，Actions 组件的依赖关系完整且正确：
- 所有必要的类型和组件都已在
actions 目录中正确导出，包括 ActionsProps 接口和主组件
- 组件被其他模块正常引用，包括示例代码和测试
- 组件结构完整，包含了必要的样式、接口定义和测试文件
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 Actions 组件的依赖关系
# 检查 actions 目录中的导出
rg -t typescript "export" ./components/actions/
# 检查是否有其他组件依赖 Actions
rg -t typescript "import.*Actions.*from" -g '!index.ts'
Length of output: 175
Script:
#!/bin/bash
# 检查 actions 目录中的导出
rg "export" ./components/actions/
# 检查是否有其他组件依赖 Actions
rg "import.*Actions.*from" -g '!index.ts'
# 查找 actions 相关文件
fd actions ./components/
# 检查 actions 目录的内容
ls -la ./components/actions/ 2>/dev/null || echo "Directory not found"
Length of output: 3124
components/actions/__tests__/actionMenu.test.tsx (1)
7-18: 测试数据结构设计合理
测试数据结构完整，包含了根级别项目和嵌套项目，能够覆盖各种测试场景。
components/x-provider/context.ts (2)
4-4: 导入语句正确且符合规范！
新增的 ActionsProps 类型导入遵循了现有的导入模式，并且与其他组件类型导入保持一致。
37-37: 配置接口扩展合理！
新增的 actions 配置属性：
- 遵循了现有的组件配置模式
- 正确使用了
ComponentStyleConfig 泛型类型
- 保持了可选属性的一致性
components/actions/ActionMenu.tsx (2)
46-58: 代码实现规范，符合最佳实践
组件渲染逻辑清晰，正确使用了 Ant Design 的 Dropdown 组件，并保持了良好的样式命名规范。
1-64: 建议增加测试用例覆盖
该组件包含复杂的业务逻辑，建议添加以下测试场景：
- 菜单项点击事件处理
- 空值处理
- 自定义触发行为
- 样式类名生成
运行以下脚本检查现有测试覆盖情况：
components/actions/__tests__/actions.test.tsx (1)
1-4: 导入语句看起来不错！
导入了必要的测试工具和组件，结构清晰。
components/actions/style/index.ts (1)
1-8: 导入和接口定义看起来合理！
接口定义清晰，类型继承关系正确。空的 ComponentToken 接口为后续扩展预留了空间。
components/actions/interface.ts (2)
1-2: 导入声明看起来没有问题！
类型导入清晰明确，符合组件的需求。
100-100: 联合类型定义合理！
ActionItemType 的联合类型定义清晰且符合需求。
components/actions/index.tsx (1)
1-11: 导入语句组织良好！
导入声明按照类型分组，包括第三方库、React 核心、内部工具和本地模块，结构清晰。
Comment on lines
+19
to
+24
| const Demo: React.FC = () => { | ||
| const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => { | ||
| message.success(`you click ${keyPath.join(',')}`); | ||
| }; | ||
| return <Actions items={actionItems} onClick={onClick} />; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
需要改进的几个方面
- 成功提示消息使用英文，而界面标签使用中文，存在语言不一致的问题
- 缺少错误处理机制
建议按照以下方式修改：
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.
Comment on lines
+19
to
+24
| const Demo: React.FC = () => { | ||
| const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => { | ||
| message.success(`you click ${keyPath.join(',')}`); | ||
| }; | ||
| return <Actions items={actionItems} onClick={onClick} variant="border" />; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议完善错误处理和国际化支持
- 建议添加错误处理机制
- 成功提示信息需要支持国际化
- 考虑添加 loading 状态处理
const Demo: React.FC = () => {
const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => {
- message.success(`you click ${keyPath.join(',')}`);
+ try {
+ // 处理点击逻辑
+ message.success(t('actions.clickSuccess', { path: keyPath.join(',') }));
+ } catch (error) {
+ message.error(t('actions.clickError'));
+ console.error('Action click error:', error);
+ }
};
return <Actions items={actionItems} onClick={onClick} variant="border" />;
};Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+6
to
+17
| const actionItems = [ | ||
| { | ||
| key: 'retry', | ||
| icon: <RedoOutlined />, | ||
| label: '重试', | ||
| }, | ||
| { | ||
| key: 'copy', | ||
| icon: <CopyOutlined />, | ||
| label: '复制', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增加国际化支持并添加类型注解
- 建议使用国际化文案配置，而不是硬编码的中文文案
- 为
actionItems添加明确的类型注解
-const actionItems = [
+const actionItems: ActionsItem[] = [
{
key: 'retry',
icon: <RedoOutlined />,
- label: '重试',
+ label: t('actions.retry'),
},
{
key: 'copy',
icon: <CopyOutlined />,
- label: '复制',
+ label: t('actions.copy'),
},
];Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+12
to
+45
| const actionItems: ActionsProps['items'] = [ | ||
| { | ||
| key: 'retry', | ||
| label: '重试', | ||
| icon: <RedoOutlined />, | ||
| }, | ||
| { | ||
| key: 'copy', | ||
| label: '复制', | ||
| icon: <CopyOutlined />, | ||
| }, | ||
| { | ||
| key: 'more', | ||
| // icon: <EllipsisOutlined />, // 不传使用默认的icon | ||
| children: [ | ||
| { | ||
| key: 'share', | ||
| label: '分享', | ||
| icon: <ShareAltOutlined />, | ||
| children: [ | ||
| { key: 'qq', label: 'QQ' }, | ||
| { key: 'wechat', label: '微信' }, | ||
| ], | ||
| }, | ||
| { key: 'import', label: '引用' }, | ||
| { key: 'delete', label: '删除', icon: <DeleteOutlined />, onClick: () => {}, danger: true }, | ||
| ], | ||
| }, | ||
| { | ||
| key: 'clear', | ||
| label: '清空', | ||
| icon: <ReloadOutlined />, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
需要改进的几个关键点
- 建议移除注释掉的代码（第25行）
- 删除操作的
onClick处理器为空，可能会导致意外操作 - 建议添加国际化支持
- 建议为危险操作添加确认机制
建议进行如下修改：
const actionItems: ActionsProps['items'] = [
{
key: 'retry',
- label: '重试',
+ label: t('actions.retry'),
icon: <RedoOutlined />,
},
// ... other items
{
key: 'delete',
label: '删除',
icon: <DeleteOutlined />,
- onClick: () => {},
+ onClick: () => {
+ Modal.confirm({
+ title: t('actions.deleteConfirmTitle'),
+ content: t('actions.deleteConfirmContent'),
+ onOk: () => {
+ // 处理删除逻辑
+ },
+ });
+ },
danger: true
},
];Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+47
to
+54
| const Demo: React.FC = () => { | ||
| const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => { | ||
| message.success(`you click ${keyPath}`); | ||
| }; | ||
| return <Actions items={actionItems} onClick={onClick} />; | ||
| }; | ||
|
|
||
| export default Demo; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增强组件的健壮性和类型安全性
- 建议为不同的操作添加不同的处理逻辑
- 建议添加错误处理
- 建议使用更具体的类型定义
-const Demo: React.FC = () => {
+interface ActionHandler {
+ key: string;
+ handler: () => Promise<void>;
+}
+
+const Demo: React.FC = () => {
+ const handleAction = async ({ key, keyPath, item }: ActionsProps['onClick']) => {
+ try {
+ switch (key) {
+ case 'retry':
+ await handleRetry();
+ message.success(t('actions.retrySuccess'));
+ break;
+ case 'delete':
+ await handleDelete();
+ message.success(t('actions.deleteSuccess'));
+ break;
+ default:
+ message.success(`${keyPath} ${t('actions.actionSuccess')}`);
+ }
+ } catch (error) {
+ message.error(t('actions.actionError'));
+ console.error(error);
+ }
+ };
+
- const onClick: ActionsProps['onClick'] = ({ key, keyPath, item }) => {
- message.success(`you click ${keyPath}`);
- };
- return <Actions items={actionItems} onClick={onClick} />;
+ return <Actions items={actionItems} onClick={handleAction} />;
};Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+13
to
+71
| return { | ||
| [componentCls]: { | ||
| [`${componentCls}-list`]: { | ||
| display: 'inline-flex', | ||
| flexDirection: 'row', | ||
| gap: token.paddingXS, | ||
| color: '#8c8c8c', | ||
|
|
||
| '&-item, &-sub-item': { | ||
| cursor: 'pointer', | ||
| padding: token.paddingXXS, | ||
| borderRadius: token.borderRadius, | ||
| height: token.controlHeightSM, | ||
| width: token.controlHeightSM, | ||
| boxSizing: 'border-box', | ||
| display: 'inline-flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
|
|
||
| '&-icon': { | ||
| display: 'inline-flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| fontSize: token.fontSize, | ||
| width: '100%', | ||
| height: '100%', | ||
| }, | ||
|
|
||
| '&:hover': { | ||
| background: '#F6F6F6', | ||
| }, | ||
| }, | ||
| }, | ||
| '& .border': { | ||
| padding: `${token.paddingXS} ${token.paddingSM}`, | ||
| gap: token.paddingSM, | ||
| borderRadius: calc(token.borderRadiusLG).mul(1.5).equal(), | ||
| backgroundColor: '#F7F7F7', | ||
| color: token.colorTextSecondary, | ||
|
|
||
| [`${componentCls}-list-item, ${componentCls}-list-sub-item`]: { | ||
| padding: 0, | ||
| lineHeight: token.lineHeight, | ||
|
|
||
| '&-icon': { | ||
| fontSize: token.fontSizeLG, | ||
| }, | ||
|
|
||
| '&:hover': { | ||
| opacity: 0.8, | ||
| }, | ||
| }, | ||
| }, | ||
| '& .block': { | ||
| display: 'flex', | ||
| }, | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议使用主题令牌系统替代硬编码的颜色值
当前实现中存在以下需要优化的地方：
-
硬编码的颜色值应该使用主题令牌：
#8c8c8c建议使用token.colorTextSecondary#F6F6F6建议使用token.colorBgTextHover#F7F7F7建议使用token.colorFillTertiary
-
建议将重复的样式属性抽取为公共变量，提高可维护性。
建议按照以下方式修改：
[componentCls]: {
[`${componentCls}-list`]: {
display: 'inline-flex',
flexDirection: 'row',
gap: token.paddingXS,
- color: '#8c8c8c',
+ color: token.colorTextSecondary,
'&-item, &-sub-item': {
// ... other properties ...
'&:hover': {
- background: '#F6F6F6',
+ background: token.colorBgTextHover,
},
},
},
'& .border': {
// ... other properties ...
- backgroundColor: '#F7F7F7',
+ backgroundColor: token.colorFillTertiary,
}
}📝 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
| return { | |
| [componentCls]: { | |
| [`${componentCls}-list`]: { | |
| display: 'inline-flex', | |
| flexDirection: 'row', | |
| gap: token.paddingXS, | |
| color: '#8c8c8c', | |
| '&-item, &-sub-item': { | |
| cursor: 'pointer', | |
| padding: token.paddingXXS, | |
| borderRadius: token.borderRadius, | |
| height: token.controlHeightSM, | |
| width: token.controlHeightSM, | |
| boxSizing: 'border-box', | |
| display: 'inline-flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| '&-icon': { | |
| display: 'inline-flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| fontSize: token.fontSize, | |
| width: '100%', | |
| height: '100%', | |
| }, | |
| '&:hover': { | |
| background: '#F6F6F6', | |
| }, | |
| }, | |
| }, | |
| '& .border': { | |
| padding: `${token.paddingXS} ${token.paddingSM}`, | |
| gap: token.paddingSM, | |
| borderRadius: calc(token.borderRadiusLG).mul(1.5).equal(), | |
| backgroundColor: '#F7F7F7', | |
| color: token.colorTextSecondary, | |
| [`${componentCls}-list-item, ${componentCls}-list-sub-item`]: { | |
| padding: 0, | |
| lineHeight: token.lineHeight, | |
| '&-icon': { | |
| fontSize: token.fontSizeLG, | |
| }, | |
| '&:hover': { | |
| opacity: 0.8, | |
| }, | |
| }, | |
| }, | |
| '& .block': { | |
| display: 'flex', | |
| }, | |
| }, | |
| }; | |
| }; | |
| return { | |
| [componentCls]: { | |
| [`${componentCls}-list`]: { | |
| display: 'inline-flex', | |
| flexDirection: 'row', | |
| gap: token.paddingXS, | |
| color: token.colorTextSecondary, | |
| '&-item, &-sub-item': { | |
| cursor: 'pointer', | |
| padding: token.paddingXXS, | |
| borderRadius: token.borderRadius, | |
| height: token.controlHeightSM, | |
| width: token.controlHeightSM, | |
| boxSizing: 'border-box', | |
| display: 'inline-flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| '&-icon': { | |
| display: 'inline-flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| fontSize: token.fontSize, | |
| width: '100%', | |
| height: '100%', | |
| }, | |
| '&:hover': { | |
| background: token.colorBgTextHover, | |
| }, | |
| }, | |
| }, | |
| '& .border': { | |
| padding: `${token.paddingXS} ${token.paddingSM}`, | |
| gap: token.paddingSM, | |
| borderRadius: calc(token.borderRadiusLG).mul(1.5).equal(), | |
| backgroundColor: token.colorFillTertiary, | |
| color: token.colorTextSecondary, | |
| [`${componentCls}-list-item, ${componentCls}-list-sub-item`]: { | |
| padding: 0, | |
| lineHeight: token.lineHeight, | |
| '&-icon': { | |
| fontSize: token.fontSizeLG, | |
| }, | |
| '&:hover': { | |
| opacity: 0.8, | |
| }, | |
| }, | |
| }, | |
| '& .block': { | |
| display: 'flex', | |
| }, | |
| }, | |
| }; | |
| }; |
Comment on lines
+72
to
+99
| export interface SubItemType { | ||
| /** | ||
| * @desc 自定义操作的唯一标识 | ||
| * @descEN Unique identifier for the custom action. | ||
| */ | ||
| key: string; | ||
| /** | ||
| * @desc 自定义操作的显示标签 | ||
| * @descEN Display label for the custom action. | ||
| */ | ||
| label?: string; | ||
| /** | ||
| * @desc 自定义操作的图标 | ||
| * @descEN Icon for the custom action. | ||
| */ | ||
| icon?: ReactNode; | ||
| /** | ||
| * @desc 子操作项 | ||
| * @descEN Child action items. | ||
| */ | ||
| children?: ActionItemType[]; | ||
| triggerSubMenuAction?: MenuProps['triggerSubMenuAction']; | ||
| /** | ||
| * @desc 点击自定义操作按钮时的回调函数 | ||
| * @descEN Callback function when the custom action button is clicked. | ||
| */ | ||
| onClick?: () => void; | ||
| } |
There was a problem hiding this comment.
需要解决类型继承和一致性问题
SubItemType 和 ItemType 有大量重复的属性定义，这可能导致维护困难和不一致性。建议通过类型继承来优化结构。
建议应用以下改进：
-export interface SubItemType {
- key: string;
- label?: string;
- icon?: ReactNode;
- children?: ActionItemType[];
- triggerSubMenuAction?: MenuProps['triggerSubMenuAction'];
- onClick?: () => void;
+export interface SubItemType extends Omit<ItemType, 'danger'> {
+ children?: ActionItemType[];
+ triggerSubMenuAction?: MenuProps['triggerSubMenuAction'];
}Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+104
to
+108
| if (process.env.NODE_ENV === 'production') { | ||
| Actions.displayName = 'Actions'; | ||
| } | ||
|
|
||
| export default Actions; |
There was a problem hiding this comment.
修复 displayName 的环境判断逻辑
当前实现中 displayName 的设置逻辑似乎反了。通常我们希望在开发环境中设置 displayName 以便调试，而不是在生产环境。
建议修改为：
- if (process.env.NODE_ENV === 'production') {
+ if (process.env.NODE_ENV !== 'production') {
Actions.displayName = 'Actions';
}📝 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 (process.env.NODE_ENV === 'production') { | |
| Actions.displayName = 'Actions'; | |
| } | |
| export default Actions; | |
| if (process.env.NODE_ENV !== 'production') { | |
| Actions.displayName = 'Actions'; | |
| } | |
| export default Actions; |
Comment on lines
+57
to
+72
| const handleItemClick = ( | ||
| key: string, | ||
| item: ActionItemType, | ||
| domEvent: React.MouseEvent<HTMLElement, MouseEvent>, | ||
| ) => { | ||
| if (item.onClick) { | ||
| item.onClick(); | ||
| return; | ||
| } | ||
| onClick?.({ | ||
| key, | ||
| item, | ||
| keyPath: [key], | ||
| domEvent, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
优化事件处理函数的实现
handleItemClick 函数的实现可以更加优雅：
- 事件类型可以更具体
- 可以使用早期返回模式简化逻辑
建议按照以下方式重构：
const handleItemClick = (
key: string,
item: ActionItemType,
- domEvent: React.MouseEvent<HTMLElement, MouseEvent>,
+ domEvent: React.MouseEvent<HTMLDivElement>,
) => {
- if (item.onClick) {
- item.onClick();
- return;
- }
- onClick?.({
+ item.onClick?.();
+ onClick?.({
key,
item,
keyPath: [key],
domEvent,
});
};Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+74
to
+101
| const renderSingleItem = (item: ItemType) => { | ||
| const { icon, label, key } = item; | ||
|
|
||
| return ( | ||
| <div | ||
| className={classnames(`${prefixCls}-list-item`)} | ||
| onClick={(domEvent) => handleItemClick(key, item, domEvent)} | ||
| key={key} | ||
| > | ||
| {getTooltipNode(<div className={`${prefixCls}-list-item-icon`}>{icon}</div>, label)} | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| return wrapCSSVar( | ||
| <div className={mergedCls} style={mergedStyle}> | ||
| <div className={classnames(`${prefixCls}-list`, variant, block)}> | ||
| {items.map((item) => { | ||
| if ('children' in item) { | ||
| return ( | ||
| <ActionMenu key={item.key} item={item} prefixCls={prefixCls} onClick={onClick} /> | ||
| ); | ||
| } | ||
| return renderSingleItem(item); | ||
| })} | ||
| </div> | ||
| </div>, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
提升组件的可访问性和代码组织
渲染逻辑需要以下改进：
- 缺少必要的 ARIA 属性
- 可以提取一些 JSX 常量
- 建议添加键盘交互支持
建议进行以下改进：
<div
className={classnames(`${prefixCls}-list-item`)}
onClick={(domEvent) => handleItemClick(key, item, domEvent)}
key={key}
+ role="button"
+ tabIndex={0}
+ aria-label={label}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ handleItemClick(key, item, e as unknown as React.MouseEvent<HTMLDivElement>);
+ }
+ }}
>📝 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 renderSingleItem = (item: ItemType) => { | |
| const { icon, label, key } = item; | |
| return ( | |
| <div | |
| className={classnames(`${prefixCls}-list-item`)} | |
| onClick={(domEvent) => handleItemClick(key, item, domEvent)} | |
| key={key} | |
| > | |
| {getTooltipNode(<div className={`${prefixCls}-list-item-icon`}>{icon}</div>, label)} | |
| </div> | |
| ); | |
| }; | |
| return wrapCSSVar( | |
| <div className={mergedCls} style={mergedStyle}> | |
| <div className={classnames(`${prefixCls}-list`, variant, block)}> | |
| {items.map((item) => { | |
| if ('children' in item) { | |
| return ( | |
| <ActionMenu key={item.key} item={item} prefixCls={prefixCls} onClick={onClick} /> | |
| ); | |
| } | |
| return renderSingleItem(item); | |
| })} | |
| </div> | |
| </div>, | |
| ); | |
| const renderSingleItem = (item: ItemType) => { | |
| const { icon, label, key } = item; | |
| return ( | |
| <div | |
| className={classnames(`${prefixCls}-list-item`)} | |
| onClick={(domEvent) => handleItemClick(key, item, domEvent)} | |
| key={key} | |
| role="button" | |
| tabIndex={0} | |
| aria-label={label} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| handleItemClick(key, item, e as unknown as React.MouseEvent<HTMLDivElement>); | |
| } | |
| }} | |
| > | |
| {getTooltipNode(<div className={`${prefixCls}-list-item-icon`}>{icon}</div>, label)} | |
| </div> | |
| ); | |
| }; | |
| return wrapCSSVar( | |
| <div className={mergedCls} style={mergedStyle}> | |
| <div className={classnames(`${prefixCls}-list`, variant, block)}> | |
| {items.map((item) => { | |
| if ('children' in item) { | |
| return ( | |
| <ActionMenu key={item.key} item={item} prefixCls={prefixCls} onClick={onClick} /> | |
| ); | |
| } | |
| return renderSingleItem(item); | |
| })} | |
| </div> | |
| </div>, | |
| ); |
YumoImer
reviewed
Nov 25, 2024
| display: 'inline-flex', | ||
| flexDirection: 'row', | ||
| gap: token.paddingXS, | ||
| color: '#8c8c8c', |
There was a problem hiding this comment.
同类建议：最好不写固定颜色，比如在暗色主题下会存在显示问题
这个颜色看起来比较像 token.colorTextDescription ?
- color: '#8c8c8c',
+ color: token.colorTextDescription,
| }, | ||
|
|
||
| '&:hover': { | ||
| background: '#F6F6F6', |
There was a problem hiding this comment.
同类建议：最好不写固定颜色，比如在暗色主题下会存在显示问题
这个颜色看起来比较像 token.colorBgTextHover ?
- color: '#F6F6F6',
+ color: token.colorBgTextHover,
| padding: `${token.paddingXS} ${token.paddingSM}`, | ||
| gap: token.paddingSM, | ||
| borderRadius: calc(token.borderRadiusLG).mul(1.5).equal(), | ||
| backgroundColor: '#F7F7F7', |
There was a problem hiding this comment.
同类建议：最好不写固定颜色，比如在暗色主题下会存在显示问题
可以试试这几个：
| }; | ||
| }; | ||
|
|
||
| export const prepareComponentToken: GetDefaultToken<'Prompts'> = () => ({}); |
|
|
||
| const mergedStyle = { | ||
| ...style, | ||
| ...contextConfig.style, |
There was a problem hiding this comment.
这里顺序需要调整下，组件传入的 style 覆盖 contextConfig.style
| import useXComponentConfig from '../_util/hooks/use-x-component-config'; | ||
| import { useXProviderContext } from '../x-provider'; | ||
| import ActionMenu from './ActionMenu'; | ||
| import { ActionItemType, ActionsProps, ItemType } from './interface'; |
There was a problem hiding this comment.
import type { ActionItemType, ActionsProps, ItemType } from './interface';| import { ActionItemType, ActionsProps, ItemType } from './interface'; | ||
| import useStyle from './style'; | ||
|
|
||
| export { ActionsProps } from './interface'; |
|
|
||
| export { ActionsProps } from './interface'; | ||
|
|
||
| const Actions = (props: ActionsProps) => { |
There was a problem hiding this comment.
const Actions: React.FC<ActionsProps> = (props) => {...}| import type { MenuItemProps, MenuProps } from 'antd'; | ||
| import type { ReactNode } from 'react'; | ||
|
|
||
| export interface ActionsProps { |
There was a problem hiding this comment.
这里需要对 html 标签的属性进行继承，HTMLDivElement 取决于该组件的最外层容器的标签。
interface ActionsProps extends Omit<React.HTMLAttributes<HTMLDivElement>, 'onClick'> {...}| block = false, | ||
| onClick, | ||
| items = [], | ||
| } = props; |
There was a problem hiding this comment.
需要对 html 标签属性进行支持，可以参考下：/components/conversations/index.tsx [line: 98~105]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
ActionMenu组件，支持渲染带有操作项的下拉菜单。Actions组件，用于渲染一组操作项，并支持可选的工具提示。Demo组件，展示不同的操作项和用法。Actions组件及其用法。basic.md和sub.md文件，增加了中英文支持。findItem函数和Actions组件添加了单元测试，确保功能正常。