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: Add index to roles function - for BubbleList #500

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

chenluda
Copy link

@chenluda chenluda commented Jan 24, 2025

link to: [RFC] Add index to roles function - for BubbleList #498
fix:#475

Summary: 为 BubbleList 的 roles 属性扩展 index,通过 index 可以个性化渲染列表中任意 Bubble。

  • feat(BubbleList.tsx, useListData.ts):更新 RolesType 类型以包含索引 index 参数

Summary by CodeRabbit

Summary by CodeRabbit

  • 新功能

    • 更新了气泡列表组件的角色类型定义,允许在角色属性计算中使用索引信息。
    • 修改了 Bubble.List API 中的 roles 属性,支持传递索引以增强角色分配的灵活性。
    • 在调试示例中新增了状态管理和交互功能,允许动态生成气泡内容。
    • 引入了新的角色函数,增强了角色定义的灵活性和自定义能力。
  • 改进

    • 增强了 useListData 钩子的功能,使角色属性生成可以基于气泡数据和其在列表中的位置。
    • UI布局进行了调整,新增了切换组件以动态选择角色表示方式。

feat(useListData.ts):更新getRoleBubbleProps函数以接受索引参数
Copy link
Contributor

coderabbitai bot commented Jan 24, 2025

📝 Walkthrough

概述

遍历

此次更改主要修改了 BubbleList.tsxuseListData.ts 中的类型和函数签名,为相关函数添加了 index 参数。这允许在处理气泡列表数据时,能够获取和使用每个气泡项目的索引位置信息。

变更

文件 变更摘要
components/bubble/BubbleList.tsx 更新 RolesType 类型定义,在函数签名中添加 index: number 参数
components/bubble/hooks/useListData.ts 修改 getRoleBubbleProps 函数签名,增加 index 参数并在调用 roles 函数时传递索引
components/bubble/index.en-US.md 更新 roles 属性类型,增加 index 参数
components/bubble/index.zh-CN.md 更新 roles 属性类型,增加 index 参数
components/bubble/demo/debug.tsx App 组件中添加新的状态和函数,增强气泡列表的交互性和动态内容生成
components/bubble/demo/list.tsx 重命名变量并添加新功能,以支持基于索引的角色配置

可能相关的 PR

建议的审阅者

  • afc163

诗歌

🐰 气泡列表,索引飞舞
代码如诗,位置有据
一个数字,解锁新境
灵活编程,兔子欢笑 🎉


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Jan 24, 2025

Preview is ready

Copy link

Walkthrough

此PR为BubbleList组件的roles属性添加了index参数,使得可以通过index个性化渲染列表中的任意Bubble。

Changes

文件 概要
components/bubble/BubbleList.tsx, components/bubble/hooks/useListData.ts 更新RolesType类型以包含索引index参数,并在相关函数中传递该参数。

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
components/bubble/hooks/useListData.ts (1)

12-14: 新增参数 index 的逻辑检查

这里为 getRoleBubbleProps 新增 index 参数,并在内部调用 roles(bubble, index)。此改动符合需求,允许在外部根据气泡所在位置进行更多自定义操作。建议在测试用例中补充基于索引的用例,以确保逻辑正确性。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6b0cc and 34cf16e.

📒 Files selected for processing (2)
  • components/bubble/BubbleList.tsx (1 hunks)
  • components/bubble/hooks/useListData.ts (2 hunks)
🔇 Additional comments (2)
components/bubble/hooks/useListData.ts (1)

32-32: 在返回合并对象时传入索引

使用 getRoleBubbleProps(bubbleData, i) 合并属性,可以针对每个气泡做更灵活的处理。逻辑清晰且与新增的 index 参数保持一致,建议在单元测试中关联验证。

components/bubble/BubbleList.tsx (1)

30-30: 为 roles 类型添加 index 参数

此处更新 RolesType,将原先的 (bubbleDataP: BubbleDataType) => RoleType 扩展为 (bubbleDataP: BubbleDataType, index: number) => RoleType,与 useListData 中的修改保持一致。此增强能让开发者基于索引自定义气泡渲染。建议结合测试或文档,提醒使用者注意该位置索引的潜在用途与限制。

@chenluda chenluda changed the title feat:Add index to roles function - for BubbleList feat: Add index to roles function - for BubbleList Jan 24, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
components/bubble/index.zh-CN.md (1)

64-64: 文档更新正确,建议添加示例说明

roles 属性的类型定义更新准确反映了新增的 index 参数。为了帮助用户更好地理解如何使用这个新参数,建议添加一个使用示例。

建议在文档中添加如下示例代码:

<Bubble.List
  items={messages}
  roles={(bubble, index) => ({
    // 根据索引自定义气泡样式
    variant: index % 2 === 0 ? 'filled' : 'outlined',
    // 其他属性...
  })}
/>
components/bubble/index.en-US.md (1)

63-63: 建议补充 index 参数的说明

roles 属性的文档已更新以包含新的 index 参数,但缺少对该参数的具体说明。建议在描述中添加 index 参数的用途说明,以帮助用户更好地理解和使用这个新功能。

示例说明文本:

-| roles | Set the default properties of the bubble. The `role` in `items` will be automatically matched. | Record<string, BubbleProps> \| (bubble, index) => BubbleProps | - |  |
+| roles | Set the default properties of the bubble. The `role` in `items` will be automatically matched. The `index` parameter represents the position of the bubble in the list. | Record<string, BubbleProps> \| (bubble, index) => BubbleProps | - |  |
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34cf16e and 816e518.

📒 Files selected for processing (2)
  • components/bubble/index.en-US.md (1 hunks)
  • components/bubble/index.zh-CN.md (1 hunks)

@YumoImer YumoImer linked an issue Jan 24, 2025 that may be closed by this pull request
Copy link

codecov bot commented Jan 24, 2025

Bundle Report

Bundle size has no change ✅

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.45%. Comparing base (9c6b0cc) to head (84a0ea5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #500   +/-   ##
=======================================
  Coverage   91.45%   91.45%           
=======================================
  Files          67       67           
  Lines        1462     1462           
  Branches      370      385   +15     
=======================================
  Hits         1337     1337           
  Misses        125      125           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YumoImer
Copy link
Collaborator

可以在当前 气泡列表 demo 中秀一下这个能力。以帮助 #475 快速上手。

同时新增的修改会被 demo 测试覆盖。

@chenluda
Copy link
Author

可以在当前 气泡列表 demo 中秀一下这个能力。以帮助 #475 快速上手。

同时新增的修改会被 demo 测试覆盖。

OK

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 816e518 and e950ef8.

📒 Files selected for processing (1)
  • components/bubble/demo/debug.tsx (4 hunks)
🔇 Additional comments (5)
components/bubble/demo/debug.tsx (5)

1-3: 导入语句使用恰当,符合预期。
本段添加的 UserOutlinedButtonDividerFlex 等导入在当前文件中能够正确使用或后续可用,保持一致性无碍。


37-39: 通过 React 状态与引用管理,提高组件扩展性。
新增的 countmarkedIndex 状态,以及 listRef 引用,更好地支持动态生成与定位 Bubble,逻辑明晰,无明显问题。


82-94: 为角色分配基础属性时,增强了可交互性与可读性。
这里定义的 commonProps 提供了对 avatarfooterheaderstyles 与点击事件等统一处理,方便后续在角色差异化中复用。用 markedIndex 来控制高亮,交互友好。


160-187: 确认索引越界情况的处理机制。
此处在点击滚动时会依据 markedIndex 将列表滚动到对应位置,若索引超出当前数据范围可能会无效或引发问题。建议检查边界或考虑无效索引时的错误提示,确保交互体验更稳健。


188-204: 动态生成 Bubble 列表展示效果良好。
基于 count 循环创建数据项,并配合 roles 函数丰富展示方式,整体实现符合 PR 目标,易读易扩展。

Comment on lines 95 to 112
switch (bubbleData.role) {
case 'ai':
return {
placement: 'start',
avatar: customizationProps.avatar,
typing: { step: 5, interval: 20 },
style: {
maxWidth: 600,
},
...commonProps,
};
case 'user':
return {
placement: 'end',
avatar: customizationProps.avatar,
...commonProps,
};
}
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

为未知角色添加安全兜底逻辑。
当前的 switch 只处理 aiuser,若出现其他角色,函数没有返回值会导致后续调用潜在错误。建议在 switch 末尾添加默认分支:

  switch (bubbleData.role) {
    case 'ai':
      ...
    case 'user':
      ...
+   default:
+     // 添加兜底逻辑或抛出异常提示
+     return {
+       ...commonProps,
+       placement: 'end',
+     };
  }
📝 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
switch (bubbleData.role) {
case 'ai':
return {
placement: 'start',
avatar: customizationProps.avatar,
typing: { step: 5, interval: 20 },
style: {
maxWidth: 600,
},
...commonProps,
};
case 'user':
return {
placement: 'end',
avatar: customizationProps.avatar,
...commonProps,
};
}
switch (bubbleData.role) {
case 'ai':
return {
placement: 'start',
avatar: customizationProps.avatar,
typing: { step: 5, interval: 20 },
style: {
maxWidth: 600,
},
...commonProps,
};
case 'user':
return {
placement: 'end',
avatar: customizationProps.avatar,
...commonProps,
};
default:
// 添加兜底逻辑或抛出异常提示
return {
...commonProps,
placement: 'end',
};
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
components/bubble/demo/debug.tsx (3)

82-113: 建议增强类型安全性

建议对 placement 类型使用更严格的类型定义,避免使用 as const 类型断言:

- placement: 'start' as const,
+ placement: 'start',

同时可以考虑将通用的类型提取为接口:

interface RoleConfig {
  placement: 'start' | 'end';
  typing?: {
    step: number;
    interval: number;
  };
  style?: React.CSSProperties;
}

161-187: 建议优化用户体验和可访问性

  1. 建议为按钮添加 aria-label 属性以提升可访问性
  2. 建议添加加载状态处理,避免在滚动操作时出现重复点击
 <Button
+ aria-label="添加新的对话气泡"
+ loading={isScrolling}
  onClick={() => {
    setCount((i) => i + 1);
  }}
>
  Add Bubble
</Button>

188-204: 建议优化列表性能

对于大量数据的情况,建议:

  1. 考虑使用虚拟滚动优化性能
  2. 使用 React.memo 包装列表项组件
  3. 添加错误边界处理异常情况
const MemoizedBubbleList = React.memo(({ items, roles }) => {
  return (
    <ErrorBoundary fallback={<ErrorFallback />}>
      <VirtualList
        data={items}
        renderItem={(item) => (
          <Bubble.List
            key={item.key}
            roles={roles}
            items={[item]}
          />
        )}
      />
    </ErrorBoundary>
  );
});
components/bubble/demo/list.tsx (2)

23-51: 建议优化 rolesAsFunction 的实现

当前实现有以下几点可以改进:

  1. switch 语句可以使用对象映射来简化
  2. 重复的 messageRender 逻辑可以提取出来

建议按如下方式重构:

 const rolesAsFunction = (bubbleData: BubbleProps, index: number) => {
   const RenderIndex: BubbleProps['messageRender'] = (content) => (
     <Flex>
       #{index}: {content}
     </Flex>
   );
-  switch (bubbleData.role) {
-    case 'ai':
-      return {
-        placement: 'start' as const,
-        avatar: { icon: <UserOutlined />, style: { background: '#fde3cf' } },
-        typing: { step: 5, interval: 20 },
-        style: {
-          maxWidth: 600,
-        },
-        messageRender: RenderIndex,
-      };
-    case 'user':
-      return {
-        placement: 'end' as const,
-        avatar: { icon: <UserOutlined />, style: { background: '#87d068' } },
-        messageRender: RenderIndex,
-      };
-    default:
-      return {
-        messageRender: RenderIndex,
-      };
-  }
+  const roleConfigs = {
+    ai: {
+      placement: 'start' as const,
+      avatar: { icon: <UserOutlined />, style: { background: '#fde3cf' } },
+      typing: { step: 5, interval: 20 },
+      style: {
+        maxWidth: 600,
+      },
+    },
+    user: {
+      placement: 'end' as const,
+      avatar: { icon: <UserOutlined />, style: { background: '#87d068' } },
+    },
+  };
+  
+  return {
+    ...(roleConfigs[bubbleData.role as keyof typeof roleConfigs] ?? {}),
+    messageRender: RenderIndex,
+  };
 };

61-67: 建议增加无障碍支持

Switch 组件缺少必要的无障碍属性,建议添加 aria-label 来提高可访问性。

 <Switch
   checked={useRolesAsFunction}
   onChange={(checked) => setUseRolesAsFunction(checked)}
   checkedChildren="Function"
   unCheckedChildren="Object"
   style={{ alignSelf: 'center' }}
+  aria-label="切换角色配置模式"
 />
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e950ef8 and eb0e710.

📒 Files selected for processing (2)
  • components/bubble/demo/debug.tsx (4 hunks)
  • components/bubble/demo/list.tsx (2 hunks)
🔇 Additional comments (3)
components/bubble/demo/debug.tsx (1)

2-2: 导入和状态管理实现得当!

新增的状态变量和引用对象能够很好地支持新功能的实现。

Also applies to: 37-39

components/bubble/demo/list.tsx (2)

3-4: 导入语句看起来不错!

新添加的导入语句恰当地支持了新功能的实现。


91-91: roles 属性的使用非常恰当!

通过三元运算符优雅地实现了 roles 的条件切换,完美符合需求。

@YumoImer
Copy link
Collaborator

› 4 snapshots failed from 2 test suites. Inspect your code changes or run npm test -- -u to update them.

checked={useRolesAsFunction}
onChange={(checked) => setUseRolesAsFunction(checked)}
checkedChildren="Function"
unCheckedChildren="Object"
Copy link
Collaborator

Choose a reason for hiding this comment

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

checkedChildren、unCheckedChildren

这里感觉用户看文档时可能会不清楚这个 Switch 的用途及含义。

Copy link
Author

Choose a reason for hiding this comment

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

原本的 roles 示例只展示了作为对象的用法,index 是加在函数里的,switch 可以同时展示两种用法,这里有什么建议吗

Copy link
Collaborator

Choose a reason for hiding this comment

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

建议是文案可以修改下,或者加个 label ,明确的表明这个 Switch 是控制 roles 的用法。你觉得呢~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bubble messageRender 能否添加 index 参数
2 participants