perf(conversations): #446 Tooltip性能优化#464
perf(conversations): #446 Tooltip性能优化#464SmartBigFeng wants to merge 1 commit intoelement-plus-x:mainfrom
Conversation
使用 Range API 替代临时span标签创建 添加 MutationObserver 监听文本变化 将 v-if 改为 :disabled 属性控制
📝 WalkthroughWalkthroughRefactored text overflow detection in a conversation item component, replacing a synchronous computed function that repeatedly measures text via temporary DOM elements with a reactive, observer-driven approach using the Range API and MutationObserver for efficient, event-driven measurement updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/components/Conversations/components/item.vue`:
- Line 91: The single-line conditional "if (!icon) return null;" violates
newline and brace style rules and causes HTML indentation issues elsewhere;
replace the single-line if with a properly formatted block (e.g., if (!icon) {
return null; }) and ensure the surrounding template/JSX/template HTML in the
Conversations item component (item.vue) is indented to match the project's
vue/html-indent settings—also apply the same brace/newline formatting to the
other occurrences referenced around the render logic (the blocks at the regions
corresponding to lines ~130-145 and ~355-356) so they use braces, newlines, and
consistent indentation.
- Around line 129-167: The measure() logic can produce stale isTextOverflow
because it only reacts to label text/class/style changes; update the watchers so
layout/visibility changes retrigger measurement: in the labelRef watcher (the
existing observer setup) also create a ResizeObserver that observes
labelRef.value and its closest row container (or parentElement) and call measure
on resize; add window 'resize' and document 'visibilitychange' listeners that
call measure; when measure() early-returns because !labelRef.value.offsetParent,
register an IntersectionObserver (or MutationObserver on the row) to retry
measure when the element becomes visible, and ensure all observers/listeners are
cleaned up when observer is disconnected or labelRef changes; keep using
isMeasuring/isTextOverflow as before and disconnect observers on teardown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0f53ed2-3c86-4348-ba8f-75e0724a5f63
📒 Files selected for processing (1)
packages/core/src/components/Conversations/components/item.vue
| function renderIcon(icon: Component | null | undefined) { | ||
| if (!icon) | ||
| return null; | ||
| if (!icon) return null; |
There was a problem hiding this comment.
Please fix the ESLint errors in this hunk.
Static analysis is already flagging antfu/if-newline, style/brace-style, and vue/html-indent violations here, so this will not merge cleanly until the formatting is normalized.
Also applies to: 130-145, 355-356
🧰 Tools
🪛 ESLint
[error] 91-91: Expect newline after if
(antfu/if-newline)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/components/Conversations/components/item.vue` at line 91,
The single-line conditional "if (!icon) return null;" violates newline and brace
style rules and causes HTML indentation issues elsewhere; replace the
single-line if with a properly formatted block (e.g., if (!icon) { return null;
}) and ensure the surrounding template/JSX/template HTML in the Conversations
item component (item.vue) is indented to match the project's vue/html-indent
settings—also apply the same brace/newline formatting to the other occurrences
referenced around the render logic (the blocks at the regions corresponding to
lines ~130-145 and ~355-356) so they use braces, newlines, and consistent
indentation.
| function measure() { | ||
| if (isMeasuring) return; | ||
| if (!labelMaxWidth.value || !labelRef.value) { | ||
| isTextOverflow.value = false; | ||
| return; | ||
| } | ||
| isMeasuring = true; | ||
| try { | ||
| if (!labelRef.value.offsetParent) return; | ||
| const range = document.createRange(); | ||
| range.selectNodeContents(labelRef.value); | ||
| isTextOverflow.value = | ||
| range.getBoundingClientRect().width > labelMaxWidth.value; | ||
| } catch { | ||
| isTextOverflow.value = false; | ||
| } finally { | ||
| isMeasuring = false; | ||
| } | ||
| } | ||
|
|
||
| watch([labelMaxWidth, () => item.value?.label], measure, { flush: 'post' }); | ||
|
|
||
| watch( | ||
| labelRef, | ||
| (newVal, _) => { | ||
| if (observer) { | ||
| observer.disconnect(); | ||
| observer = null; | ||
| } | ||
| span.style.visibility = 'hidden'; | ||
| span.style.position = 'absolute'; | ||
| span.style.whiteSpace = 'nowrap'; | ||
| span.style.fontSize = '14px'; // 与CSS中定义的字体大小一致 | ||
| span.textContent = label; | ||
|
|
||
| document.body.appendChild(span); | ||
| const textWidth = span.offsetWidth; | ||
| document.body.removeChild(span); | ||
|
|
||
| // 如果文本宽度大于最大宽度，则返回true表示溢出 | ||
| return textWidth > labelMaxWidth.value; | ||
| }; | ||
| }); | ||
| if (newVal) { | ||
| observer = new MutationObserver(measure); | ||
| observer.observe(newVal, { | ||
| attributes: true, | ||
| attributeFilter: ['style', 'class'] | ||
| }); | ||
| measure(); | ||
| } | ||
| }, | ||
| { flush: 'post', immediate: true } |
There was a problem hiding this comment.
Overflow state can go stale after layout-only changes.
This watcher path only reacts to labelMaxWidth, item.label, and the label node’s own style/class. But Line 189 and Line 371-Line 378 can change the row layout on hover/active/menu-open, which can reduce the label’s available width without touching any of those sources. Line 137 also exits early for hidden rows and nothing retries when they become visible. In both cases isTextOverflow can stay stale, so the tooltip stays disabled while the text is actually truncated. Please remeasure on layout/visibility changes as well, not just text changes.
🧰 Tools
🪛 ESLint
[error] 130-130: Expect newline after if
(antfu/if-newline)
[error] 137-137: Expect newline after if
(antfu/if-newline)
[error] 142-142: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
[error] 144-144: Closing curly brace appears on the same line as the subsequent block.
(style/brace-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/components/Conversations/components/item.vue` around lines
129 - 167, The measure() logic can produce stale isTextOverflow because it only
reacts to label text/class/style changes; update the watchers so
layout/visibility changes retrigger measurement: in the labelRef watcher (the
existing observer setup) also create a ResizeObserver that observes
labelRef.value and its closest row container (or parentElement) and call measure
on resize; add window 'resize' and document 'visibilitychange' listeners that
call measure; when measure() early-returns because !labelRef.value.offsetParent,
register an IntersectionObserver (or MutationObserver on the row) to retry
measure when the element becomes visible, and ensure all observers/listeners are
cleaned up when observer is disconnected or labelRef changes; keep using
isMeasuring/isTextOverflow as before and disconnect observers on teardown.
1 participant
使用 Range API 替代临时span标签创建
MutationObserver 监听
将 v-if 改为 :disabled 属性控制，避免切换
Summary by CodeRabbit