refactor(BubbleList): 重构布局与自动滚动逻辑 #284
Conversation
WalkthroughThe BubbleList component now renders its list in reverse order and updates all scroll, index, and rendering logic to match. Scrolling and auto-scroll behaviors are reworked using IntersectionObserver and new helper functions. Styles are updated for column-reverse flex direction. A story file adds dynamic message content extension for demonstration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BubbleList
participant DOM
participant IntersectionObserver
User->>BubbleList: Add new message to list
BubbleList->>BubbleList: wrapList (reverse list)
BubbleList->>DOM: Render bubbles in reversed order
BubbleList->>IntersectionObserver: Observe checkViewRef at bottom
IntersectionObserver-->>BubbleList: checkViewRef not visible
BubbleList->>DOM: Scroll checkViewRef into view (autoScroll)
IntersectionObserver-->>BubbleList: checkViewRef visible
BubbleList->>BubbleList: Stop autoScroll
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/components/BubbleList/index.vue (1)
70-70: Fix ESLint style violations.Please address the ESLint warnings by adding newlines after
ifstatements and fixing brace styles for better code consistency.Also applies to: 89-89, 96-96, 100-100, 119-119, 178-178, 133-133, 213-213
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/components/BubbleList/index.vue(6 hunks)packages/core/src/components/BubbleList/style.scss(1 hunks)packages/core/src/stories/BubbleList/CustomSolt.vue(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/core/src/components/BubbleList/style.scss (1)
packages/core/src/components/BubbleList/types.d.ts (4)
BubbleListInstance(31-35)BubbleListProps(12-25)BubbleListItemProps(5-5)BackButtonPosition(7-10)
packages/core/src/components/BubbleList/index.vue (1)
packages/core/src/components/BubbleList/types.d.ts (5)
BubbleListInstance(31-35)BubbleListProps(12-25)BubbleListEmits(27-29)BubbleListItemProps(5-5)BackButtonPosition(7-10)
🪛 ESLint
packages/core/src/components/BubbleList/index.vue
[error] 70-70: Expect newline after if
(antfu/if-newline)
[error] 89-89: Expect newline after if
(antfu/if-newline)
[error] 96-96: Expect newline after if
(antfu/if-newline)
[error] 100-100: Expect newline after if
(antfu/if-newline)
[error] 119-119: Expect newline after if
(antfu/if-newline)
[error] 133-133: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 178-178: Expect newline after if
(antfu/if-newline)
[error] 213-213: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🔇 Additional comments (5)
packages/core/src/components/BubbleList/style.scss (1)
3-3: LGTM!The
flex-direction: column-reversechange correctly implements the reversed layout for the bubble list, ensuring new items appear at the bottom of the container.packages/core/src/components/BubbleList/index.vue (4)
27-44: Well-implemented list reversal logic.The
wrapListcomputed property correctly creates a reversed copy without mutating the original prop, andgetTrueIndexproperly maps indices between the reversed and original lists.
75-84: Correct implementation for reversed container.Setting
scrollTopto a negative value is the correct approach for scrolling to the top in acolumn-reverseflex container.
176-226: Well-implemented scroll handling for reversed container.The scroll calculations correctly account for the reversed layout, and the threshold-based user interaction detection provides good UX.
243-243: Smart implementation of scroll detection.The 1px invisible element for intersection observation is an elegant solution, and the template correctly uses the reversed list.
Also applies to: 274-274
| let intersectionObserver!: IntersectionObserver; | ||
| // 组件内部触发方法，跟随打字器滚动，滚动底部 | ||
| function autoScroll() { | ||
| if (scrollContainer.value) { | ||
| const listBubbles = scrollContainer.value!.querySelectorAll( | ||
| '.el-bubble-content-wrapper' | ||
| ); | ||
| // 如果页面上有监听节点，先移除 | ||
| if (resizeObserver.value) { | ||
| resizeObserver.value.disconnect(); | ||
| } | ||
| const lastItem = listBubbles[listBubbles.length - 1]; | ||
| if (lastItem) { | ||
| resizeObserver.value = new ResizeObserver(() => { | ||
| if (!stopAutoScrollToBottom.value) { | ||
| scrollToBottom(); | ||
| const container = checkViewRef.value; | ||
| if (!container) return; | ||
| if (intersectionObserver) { | ||
| intersectionObserver.unobserve(container); | ||
| } | ||
| container.scrollIntoView({ | ||
| behavior: 'smooth' | ||
| }); | ||
| intersectionObserver = new IntersectionObserver( | ||
| entries => { | ||
| if (stopAutoScrollToBottom.value) | ||
| return intersectionObserver.unobserve(container); | ||
| entries.forEach(entry => { | ||
| if (entry.isIntersecting) { | ||
| intersectionObserver.unobserve(container); | ||
| } else { | ||
| intersectionObserver.unobserve(container); | ||
| intersectionObserver.disconnect(); | ||
| autoScroll(); | ||
| } | ||
| }); | ||
| resizeObserver.value.observe(lastItem); | ||
| }, | ||
| { | ||
| threshold: 1 | ||
| } | ||
| } | ||
| ); | ||
| intersectionObserver.observe(container); | ||
| } |
There was a problem hiding this comment.
Potential memory leak with IntersectionObserver.
The global intersectionObserver variable could lead to memory leaks if multiple observers are created without proper cleanup.
Consider this improvement:
-let intersectionObserver!: IntersectionObserver;
// 组件内部触发方法，跟随打字器滚动，滚动底部
function autoScroll() {
const container = checkViewRef.value;
if (!container) return;
- if (intersectionObserver) {
- intersectionObserver.unobserve(container);
- }
container.scrollIntoView({
behavior: 'smooth'
});
- intersectionObserver = new IntersectionObserver(
+ const observer = new IntersectionObserver(
entries => {
if (stopAutoScrollToBottom.value)
- return intersectionObserver.unobserve(container);
+ return observer.disconnect();
entries.forEach(entry => {
if (entry.isIntersecting) {
- intersectionObserver.unobserve(container);
+ observer.disconnect();
} else {
- intersectionObserver.unobserve(container);
- intersectionObserver.disconnect();
+ observer.disconnect();
autoScroll();
}
});
},
{
threshold: 1
}
);
- intersectionObserver.observe(container);
+ observer.observe(container);
}Also consider adding a cleanup in onBeforeUnmount to ensure any active observer is disconnected.
📝 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.
| let intersectionObserver!: IntersectionObserver; | |
| // 组件内部触发方法，跟随打字器滚动，滚动底部 | |
| function autoScroll() { | |
| if (scrollContainer.value) { | |
| const listBubbles = scrollContainer.value!.querySelectorAll( | |
| '.el-bubble-content-wrapper' | |
| ); | |
| // 如果页面上有监听节点，先移除 | |
| if (resizeObserver.value) { | |
| resizeObserver.value.disconnect(); | |
| } | |
| const lastItem = listBubbles[listBubbles.length - 1]; | |
| if (lastItem) { | |
| resizeObserver.value = new ResizeObserver(() => { | |
| if (!stopAutoScrollToBottom.value) { | |
| scrollToBottom(); | |
| const container = checkViewRef.value; | |
| if (!container) return; | |
| if (intersectionObserver) { | |
| intersectionObserver.unobserve(container); | |
| } | |
| container.scrollIntoView({ | |
| behavior: 'smooth' | |
| }); | |
| intersectionObserver = new IntersectionObserver( | |
| entries => { | |
| if (stopAutoScrollToBottom.value) | |
| return intersectionObserver.unobserve(container); | |
| entries.forEach(entry => { | |
| if (entry.isIntersecting) { | |
| intersectionObserver.unobserve(container); | |
| } else { | |
| intersectionObserver.unobserve(container); | |
| intersectionObserver.disconnect(); | |
| autoScroll(); | |
| } | |
| }); | |
| resizeObserver.value.observe(lastItem); | |
| }, | |
| { | |
| threshold: 1 | |
| } | |
| } | |
| ); | |
| intersectionObserver.observe(container); | |
| } | |
| // 组件内部触发方法，跟随打字器滚动，滚动底部 | |
| function autoScroll() { | |
| const container = checkViewRef.value; | |
| if (!container) return; | |
| container.scrollIntoView({ | |
| behavior: 'smooth' | |
| }); | |
| const observer = new IntersectionObserver( | |
| entries => { | |
| if (stopAutoScrollToBottom.value) | |
| return observer.disconnect(); | |
| entries.forEach(entry => { | |
| if (entry.isIntersecting) { | |
| observer.disconnect(); | |
| } else { | |
| observer.disconnect(); | |
| autoScroll(); | |
| } | |
| }); | |
| }, | |
| { | |
| threshold: 1 | |
| } | |
| ); | |
| observer.observe(container); | |
| } |
🧰 Tools
🪛 ESLint
[error] 119-119: Expect newline after if
(antfu/if-newline)
[error] 133-133: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🤖 Prompt for AI Agents
In packages/core/src/components/BubbleList/index.vue around lines 115 to 145,
the global intersectionObserver variable may cause memory leaks by creating
multiple observers without proper cleanup. To fix this, ensure that before
assigning a new IntersectionObserver to the variable, any existing observer is
disconnected and unobserved. Additionally, add a cleanup step in the component's
onBeforeUnmount lifecycle hook to disconnect the observer if it exists,
preventing lingering observers after the component is destroyed.
| let num = 50; | ||
| const T = setInterval(() => { | ||
| if (num < 1) { | ||
| clearInterval(T); | ||
| } | ||
| bubbleItems.value[bubbleItems.value.length - 1].content += | ||
| '欢迎使用 Element Plus X .'; | ||
| num--; | ||
| }, 100); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Variable shadowing and potential performance issues.
The local variable num shadows the ref declared at line 17. Additionally, updating the message content 50 times in 5 seconds could cause performance issues.
Consider these improvements:
- let num = 50;
- const T = setInterval(() => {
- if (num < 1) {
- clearInterval(T);
+ let counter = 50;
+ const intervalId = setInterval(() => {
+ if (counter < 1 || bubbleItems.value.length === 0) {
+ clearInterval(intervalId);
+ return;
}
bubbleItems.value[bubbleItems.value.length - 1].content +=
'欢迎使用 Element Plus X .';
- num--;
+ counter--;
}, 100);This addresses variable shadowing, follows naming conventions, and adds a safety check for empty arrays.
📝 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.
| let num = 50; | |
| const T = setInterval(() => { | |
| if (num < 1) { | |
| clearInterval(T); | |
| } | |
| bubbleItems.value[bubbleItems.value.length - 1].content += | |
| '欢迎使用 Element Plus X .'; | |
| num--; | |
| }, 100); | |
| let counter = 50; | |
| const intervalId = setInterval(() => { | |
| if (counter < 1 || bubbleItems.value.length === 0) { | |
| clearInterval(intervalId); | |
| return; | |
| } | |
| bubbleItems.value[bubbleItems.value.length - 1].content += | |
| '欢迎使用 Element Plus X .'; | |
| counter--; | |
| }, 100); |
🤖 Prompt for AI Agents
In packages/core/src/stories/BubbleList/CustomSolt.vue around lines 46 to 54,
the local variable 'num' shadows a ref declared earlier, which can cause
confusion and bugs. Rename the local variable to avoid shadowing, follow
consistent naming conventions, and add a check to ensure 'bubbleItems.value' is
not empty before updating its last item's content to prevent runtime errors.
Also, consider reducing the update frequency or batching updates to mitigate
potential performance issues from frequent DOM updates.
1 participant
Summary by CodeRabbit
New Features
Improvements
Style