feat(BubbleList): add index to slotProps & refactor slots #345
Conversation
WalkthroughIntroduces typed slot aliases for Bubble and BubbleList, imports BubbleSlots into BubbleList types, defines BubbleListSlots, and updates BubbleList to declare typed slots and render all provided slots dynamically per item and index. Runtime logic remains the same aside from slot rendering structure. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Consumer
participant BL as BubbleList
participant Slots as Provided Slots
App->>BL: Provide items and optional slots (avatar/header/content/footer/loading/backToBottom/…)
BL->>BL: Iterate items
loop For each item
BL->>Slots: For each available slot key<br/>invoke slot(props: { item, index })
Note right of Slots: Dynamic slot dispatch<br/>based on $slots keys
end
App-->>BL: Receives rendered content via slots
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/core/src/components/Bubble/types.d.ts (1)
30-30: Slot alias may widen; ensure Bubble’s slots are explicitly typed.
InstanceType<typeof Bubble>['$slots']is great if Bubble declares typed slots; otherwisekeyofcollapses tostring. Consider keeping as-is but ensure Bubble definesdefineSlots<...>()for narrow keys.packages/core/src/components/BubbleList/index.vue (2)
4-8: Avoid importing with .d.ts extension.TypeScript/bundlers prefer extensionless module specifiers. Switch to
./types.-} from './types.d.ts'; +} from './types';
252-254: Don’t forward non‑Bubble slots (e.g., backToBottom/default) to Bubble.Forwarding all
$slotsincludesbackToBottomanddefault, which Bubble likely ignores. Filter them to avoid confusion.- <template v-for="(_, slotName) in $slots" :key="slotName" #[slotName]> - <slot :name="slotName" :item="item" :index="index" /> - </template> + <template + v-for="(_, slotName) in $slots" + :key="slotName" + v-if="slotName !== 'backToBottom' && slotName !== 'default'" + #[slotName] + > + <slot :name="slotName" :item="item" :index="index" /> + </template>Note: Kept changes minimal per your preference for low‑touch fixes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/components/Bubble/types.d.ts(2 hunks)packages/core/src/components/BubbleList/index.vue(4 hunks)packages/core/src/components/BubbleList/types.d.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: imddc
PR: element-plus-x/Element-Plus-X#343
File: packages/core/src/components/BubbleList/index.vue:27-35
Timestamp: 2025-09-14T00:44:10.411Z
Learning: User imddc prefers minimal code modifications for bug fixes rather than comprehensive refactors, focusing on solving the immediate issue with the least amount of change to existing code.
🧬 Code graph analysis (1)
packages/core/src/components/BubbleList/types.d.ts (1)
packages/core/src/components/Bubble/types.d.ts (1)
BubbleSlots(30-30)
🔇 Additional comments (3)
packages/core/src/components/BubbleList/index.vue (2)
30-30: Typed slots declaration LGTM.
defineSlots<BubbleListSlots<T>>()aligns the template with the new slot surface.
77-77: Comment tweak acknowledged.No behavior change.
packages/core/src/components/Bubble/types.d.ts (1)
1-1: Bubble re‑export path verifiedpackages/core/src/components.ts re-exports Bubble (export { default as Bubble } from './components/Bubble/index.vue'), so
import type { Bubble } from '../../components'is valid.
| export type BubbleListSlots<T> = { | ||
| [K in keyof BubbleSlots]: (props: { item: T; index: number }) => void; | ||
| } & { | ||
| backToBottom: (props: object) => void; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make slots optional and return a VNode-ish (or any) instead of void.
Parents won’t provide every slot; returning void is atypical for Vue slots. Mark them optional and loosen return type. Minimal change diff:
-export type BubbleListSlots<T> = {
- [K in keyof BubbleSlots]: (props: { item: T; index: number }) => void;
-} & {
- backToBottom: (props: object) => void;
-};
+export type BubbleListSlots<T> = {
+ [K in keyof BubbleSlots]?: (props: { item: T; index: number }) => any;
+} & {
+ backToBottom?: () => any;
+};🤖 Prompt for AI Agents
In packages/core/src/components/BubbleList/types.d.ts around lines 32 to 36, the
slot function types are required and return void; change them to be optional and
return a VNode-ish type (e.g. any or VNode) instead of void. Update the mapped
slot type to make each slot optional (use Partial or add ? to the property) and
change the function signature from (props: {...}) => void to (props: {...}) =>
any (or an appropriate VNode type), and do the same for backToBottom so it is
optional and returns any.
1 participant
相关issue：
#272
#100
先前提交过一次pr， 由于未同步远程仓库pr信息比较混乱，遂重新提交
#344
Summary by CodeRabbit