-
-
Notifications
You must be signed in to change notification settings - Fork 215
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
base: main
Are you sure you want to change the base?
Conversation
feat(useListData.ts):更新getRoleBubbleProps函数以接受索引参数
📝 Walkthrough概述遍历此次更改主要修改了 变更
可能相关的 PR
建议的审阅者
诗歌
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 (
|
Walkthrough此PR为BubbleList组件的roles属性添加了index参数,使得可以通过index个性化渲染列表中的任意Bubble。 Changes
|
There was a problem hiding this 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
📒 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
中的修改保持一致。此增强能让开发者基于索引自定义气泡渲染。建议结合测试或文档,提醒使用者注意该位置索引的潜在用途与限制。
There was a problem hiding this 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
📒 Files selected for processing (2)
components/bubble/index.en-US.md
(1 hunks)components/bubble/index.zh-CN.md
(1 hunks)
Bundle ReportBundle size has no change ✅ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
There was a problem hiding this 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
📒 Files selected for processing (1)
components/bubble/demo/debug.tsx
(4 hunks)
🔇 Additional comments (5)
components/bubble/demo/debug.tsx (5)
1-3
: 导入语句使用恰当,符合预期。
本段添加的UserOutlined
、Button
、Divider
、Flex
等导入在当前文件中能够正确使用或后续可用,保持一致性无碍。
37-39
: 通过 React 状态与引用管理,提高组件扩展性。
新增的count
与markedIndex
状态,以及listRef
引用,更好地支持动态生成与定位 Bubble,逻辑明晰,无明显问题。
82-94
: 为角色分配基础属性时,增强了可交互性与可读性。
这里定义的commonProps
提供了对avatar
、footer
、header
、styles
与点击事件等统一处理,方便后续在角色差异化中复用。用markedIndex
来控制高亮,交互友好。
160-187
: 确认索引越界情况的处理机制。
此处在点击滚动时会依据markedIndex
将列表滚动到对应位置,若索引超出当前数据范围可能会无效或引发问题。建议检查边界或考虑无效索引时的错误提示,确保交互体验更稳健。
188-204
: 动态生成 Bubble 列表展示效果良好。
基于count
循环创建数据项,并配合roles
函数丰富展示方式,整体实现符合 PR 目标,易读易扩展。
components/bubble/demo/debug.tsx
Outdated
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, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
为未知角色添加安全兜底逻辑。
当前的 switch 只处理 ai
和 user
,若出现其他角色,函数没有返回值会导致后续调用潜在错误。建议在 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.
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', | |
}; | |
} |
There was a problem hiding this 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
: 建议优化用户体验和可访问性
- 建议为按钮添加
aria-label
属性以提升可访问性- 建议添加加载状态处理,避免在滚动操作时出现重复点击
<Button + aria-label="添加新的对话气泡" + loading={isScrolling} onClick={() => { setCount((i) => i + 1); }} > Add Bubble </Button>
188-204
: 建议优化列表性能对于大量数据的情况,建议:
- 考虑使用虚拟滚动优化性能
- 使用
React.memo
包装列表项组件- 添加错误边界处理异常情况
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 的实现当前实现有以下几点可以改进:
- switch 语句可以使用对象映射来简化
- 重复的 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
📒 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 的条件切换,完美符合需求。
› 4 snapshots failed from 2 test suites. Inspect your code changes or run |
checked={useRolesAsFunction} | ||
onChange={(checked) => setUseRolesAsFunction(checked)} | ||
checkedChildren="Function" | ||
unCheckedChildren="Object" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkedChildren、unCheckedChildren
这里感觉用户看文档时可能会不清楚这个 Switch 的用途及含义。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
原本的 roles 示例只展示了作为对象的用法,index 是加在函数里的,switch 可以同时展示两种用法,这里有什么建议吗
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议是文案可以修改下,或者加个 label ,明确的表明这个 Switch 是控制 roles 的用法。你觉得呢~
link to: [RFC] Add index to roles function - for BubbleList #498
fix:#475
Summary: 为 BubbleList 的 roles 属性扩展 index,通过 index 可以个性化渲染列表中任意 Bubble。
index
参数Summary by CodeRabbit
Summary by CodeRabbit
新功能
Bubble.List
API 中的roles
属性,支持传递索引以增强角色分配的灵活性。改进
useListData
钩子的功能,使角色属性生成可以基于气泡数据和其在列表中的位置。