feat(dashboard): 优化前端在手机端的表现#8447
Conversation
dosubot
Bot
added
size:XL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In OverlayScrollbar.vue's onMounted, you call ResizeObserver.observe and MutationObserver.observe on viewport.value without guarding against it being null; add a null check before creating/using the observers to avoid runtime errors when the component mounts before the ref is set.
- OverlayScrollbar sets document.body.style.userSelect = 'none' on drag start but only resets it in onThumbPointerUp; consider also restoring userSelect in onUnmounted in case the component is destroyed mid-drag to avoid leaving the page in a non-selectable state.
- The responsive tab styling for narrow screens is duplicated in AddNewProvider.vue and ProviderPage.vue; you could centralize these rules into a shared class or SCSS partial to reduce duplication and keep behavior consistent across future changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In OverlayScrollbar.vue's onMounted, you call ResizeObserver.observe and MutationObserver.observe on viewport.value without guarding against it being null; add a null check before creating/using the observers to avoid runtime errors when the component mounts before the ref is set.
- OverlayScrollbar sets document.body.style.userSelect = 'none' on drag start but only resets it in onThumbPointerUp; consider also restoring userSelect in onUnmounted in case the component is destroyed mid-drag to avoid leaving the page in a non-selectable state.
- The responsive tab styling for narrow screens is duplicated in AddNewProvider.vue and ProviderPage.vue; you could centralize these rules into a shared class or SCSS partial to reduce duplication and keep behavior consistent across future changes.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/chat/Chat.vue" line_range="777-780" />
<code_context>
loadingSessions.value = false;
}
+ // Bind messagesContainer to OverlayScrollbar viewport
+ nextTick(() => {
+ if (messagesScrollbar.value?.viewport) {
+ messagesContainer.value = messagesScrollbar.value.viewport;
+ messagesContainer.value.addEventListener('scroll', handleMessagesScroll);
+ }
+ });
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The scroll listener attached in `nextTick` is never explicitly removed, which can lead to subtle leaks across remounts.
Because the handler is manually attached to `messagesContainer` in `nextTick`, there should be a matching `removeEventListener` in `onBeforeUnmount` (or use a composable that handles setup/teardown). This prevents leaks or unexpected behavior if `messagesContainer` is reused or the component is mounted/unmounted often.
Suggested implementation:
```
onBeforeUnmount(() => {
if (messagesContainer.value) {
messagesContainer.value.removeEventListener("scroll", handleMessagesScroll);
}
transform: rotate(180deg);
}
```
The snippet you provided shows `transform: rotate(180deg);` inside `onBeforeUnmount`, which looks like a truncated or mis-placed style declaration. If this is not actually part of the unmount hook in your real file, you should instead:
1. Locate the real `onBeforeUnmount` hook (or create one if it doesn’t exist).
2. Add only the cleanup logic inside it:
```ts
onBeforeUnmount(() => {
if (messagesContainer.value) {
messagesContainer.value.removeEventListener("scroll", handleMessagesScroll);
}
});
```
3. Ensure any existing logic in `onBeforeUnmount` (if present) is preserved alongside this cleanup.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| nextTick(() => { | ||
| if (messagesScrollbar.value?.viewport) { | ||
| messagesContainer.value = messagesScrollbar.value.viewport; | ||
| messagesContainer.value.addEventListener('scroll', handleMessagesScroll); |
There was a problem hiding this comment.
suggestion (bug_risk): The scroll listener attached in nextTick is never explicitly removed, which can lead to subtle leaks across remounts.
Because the handler is manually attached to messagesContainer in nextTick, there should be a matching removeEventListener in onBeforeUnmount (or use a composable that handles setup/teardown). This prevents leaks or unexpected behavior if messagesContainer is reused or the component is mounted/unmounted often.
Suggested implementation:
onBeforeUnmount(() => {
if (messagesContainer.value) {
messagesContainer.value.removeEventListener("scroll", handleMessagesScroll);
}
transform: rotate(180deg);
}
The snippet you provided shows transform: rotate(180deg); inside onBeforeUnmount, which looks like a truncated or mis-placed style declaration. If this is not actually part of the unmount hook in your real file, you should instead:
- Locate the real
onBeforeUnmounthook (or create one if it doesn’t exist). - Add only the cleanup logic inside it:
onBeforeUnmount(() => { if (messagesContainer.value) { messagesContainer.value.removeEventListener("scroll", handleMessagesScroll); } });
- Ensure any existing logic in
onBeforeUnmount(if present) is preserved alongside this cleanup.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a custom OverlayScrollbar component to replace native scrollbars across the chat sidebar, messages panel, main layout, and vertical sidebar, alongside layout adjustments to prevent double scrollbars. The review feedback highlights critical issues: using v-if/v-else in the main layout destroys the Chat component and loses state during navigation; binding the scroll listener in onMounted fails if the chat panel is not initially rendered; unmounting the scrollbar during a drag permanently locks text selection on the page; and using ResizeObserver on the first child of the slot is fragile. Solutions are provided to use dynamic watchers, preserve the chat component mounting, reset body styles on unmount, and wrap slot content in a stable container.
| // Bind messagesContainer to OverlayScrollbar viewport | ||
| nextTick(() => { | ||
| if (messagesScrollbar.value?.viewport) { | ||
| messagesContainer.value = messagesScrollbar.value.viewport; | ||
| messagesContainer.value.addEventListener('scroll', handleMessagesScroll); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
在 onMounted 中使用单次 nextTick 来绑定滚动事件监听器存在一个严重的 Bug。
由于 messagesScrollbar 位于 v-else 分支中（当 selectedProject 为真时，渲染的是 ProjectView，此时 messagesScrollbar 为 null），如果组件初始化时 selectedProject 为真，onMounted 执行时 messagesScrollbar.value 将为 null，导致滚动事件监听器永远无法被绑定。当用户随后切换到普通会话（selectedProject 变为假）时，滚动监听器依然缺失，这会导致“滚动到底部”、“滚动到指定消息”等核心聊天功能失效。
解决方案：
建议移除 onMounted 中的单次绑定逻辑，改为使用 Vue 3 的 watch 动态监听 messagesScrollbar 的变化。这样无论是初始化还是后续视图切换，只要 messagesScrollbar 被挂载，就能正确绑定/解绑事件监听器，同时也能避免潜在的内存泄漏。
});
// 动态绑定 messagesContainer 到 OverlayScrollbar 视口，支持视图切换
watch(messagesScrollbar, (newVal, oldVal) => {
if (oldVal?.viewport) {
oldVal.viewport.removeEventListener('scroll', handleMessagesScroll);
}
if (newVal?.viewport) {
messagesContainer.value = newVal.viewport;
newVal.viewport.addEventListener('scroll', handleMessagesScroll);
} else {
messagesContainer.value = null;
}
});
| <v-container | ||
| v-else | ||
| fluid | ||
| class="page-wrapper" | ||
| :class="{ 'chat-mode-container': isCurrentChatRoute }" | ||
| :style="{ | ||
| height: isCurrentChatRoute ? '100%' : 'calc(100% - 8px)', | ||
| padding: isCurrentChatRoute ? '0' : undefined, | ||
| minHeight: isCurrentChatRoute ? 'unset' : undefined, | ||
| }" | ||
| class="page-wrapper chat-mode-container" | ||
| style="height: 100%; padding: 0; min-height: unset;" | ||
| > | ||
| <div | ||
| :style="{ | ||
| height: '100%', | ||
| width: '100%', | ||
| overflow: isCurrentChatRoute ? 'hidden' : undefined, | ||
| }" | ||
| > | ||
| <div style="height: 100%; width: 100%; overflow: hidden;"> | ||
| <div | ||
| v-if="shouldMountChat" | ||
| v-show="isCurrentChatRoute" | ||
| style="height: 100%; width: 100%; overflow: hidden" | ||
| > | ||
| <Chat :active="isCurrentChatRoute" /> | ||
| </div> | ||
| <RouterView v-if="!isCurrentChatRoute" /> | ||
| </div> | ||
| </v-container> |
There was a problem hiding this comment.
在重构中将 v-main 内部拆分为 v-if="!isCurrentChatRoute" 和 v-else 分支，会导致一个严重的体验和性能退化。
在原先的设计中，Chat 组件是通过 v-if="shouldMountChat" 和 v-show="isCurrentChatRoute" 组合实现类似 keep-alive 的效果。这样当用户从聊天页面导航到其他页面（如设置、插件）再返回时，Chat 组件不会被销毁，从而保留了当前的会话状态、输入框草稿、滚动位置等，并避免了重复请求会话列表和消息历史。
而重构后，一旦 isCurrentChatRoute 为假，v-else 分支会被完全销毁，导致 Chat 组件被彻底卸载。再次返回聊天时，组件需要重新挂载和初始化，所有临时状态都会丢失。
解决方案：
建议不要使用 v-if/v-else 互斥拆分，而是保持 Chat 容器的独立挂载，通过 v-show 控制其显隐，仅对非聊天路由的 OverlayScrollbar 使用 v-if。
<v-container
v-if="shouldMountChat"
v-show="isCurrentChatRoute"
fluid
class="page-wrapper chat-mode-container"
style="height: 100%; padding: 0; min-height: unset;"
>
<div style="height: 100%; width: 100%; overflow: hidden;">
<div style="height: 100%; width: 100%; overflow: hidden">
<Chat :active="isCurrentChatRoute" />
</div>
</div>
</v-container>
| onUnmounted(() => { | ||
| if (hideTimer) clearTimeout(hideTimer); | ||
| if (resizeObserver) resizeObserver.disconnect(); | ||
| if (mutationObserver) mutationObserver.disconnect(); | ||
| window.removeEventListener('pointermove', onThumbPointerMove); | ||
| window.removeEventListener('pointerup', onThumbPointerUp); | ||
| }); |
There was a problem hiding this comment.
如果在拖拽滚动条的过程中组件被突然卸载（例如由于路由切换或父组件状态改变），onUnmounted 虽然移除了事件监听器，但并没有重置 document.body.style.userSelect。
这会导致整个页面的 user-select: none 样式被永久锁定，用户将无法在整个网站上选择任何文本，直到刷新页面。
解决方案：
在 onUnmounted 中重置 document.body.style.userSelect。
onUnmounted(() => {
if (hideTimer) clearTimeout(hideTimer);
if (resizeObserver) resizeObserver.disconnect();
if (mutationObserver) mutationObserver.disconnect();
window.removeEventListener('pointermove', onThumbPointerMove);
window.removeEventListener('pointerup', onThumbPointerUp);
document.body.style.userSelect = '';
});
| onMounted(() => { | ||
| nextTick(updateThumb); | ||
| if (window.ResizeObserver) { | ||
| resizeObserver = new ResizeObserver(() => updateThumb()); | ||
| resizeObserver.observe(viewport.value); | ||
| if (viewport.value.firstElementChild) { | ||
| resizeObserver.observe(viewport.value.firstElementChild); | ||
| } | ||
| } |
There was a problem hiding this comment.
使用 ResizeObserver 监听 viewport.value.firstElementChild 存在健壮性问题。
- 动态内容替换：由于组件内部使用
<slot />，插槽内的子元素可能会动态改变（例如从加载状态切换到内容列表）。一旦firstElementChild被替换，ResizeObserver仍会监听已被销毁的旧元素，而无法感知新元素的大小变化。 - 空插槽：如果初始化时插槽为空，
firstElementChild为null，则后续插入内容时将完全无法触发监听。
解决方案：
建议在 <slot /> 外层包裹一个固定的内容容器 div（例如 ref="content"），并让 ResizeObserver 始终监听这个容器。这样无论插槽内容如何变化，容器本身都不会被销毁，且能完美捕获所有子元素的大小变化。
onMounted(() => {
nextTick(updateThumb);
if (window.ResizeObserver) {
resizeObserver = new ResizeObserver(() => updateThumb());
resizeObserver.observe(viewport.value);
if (content.value) {
resizeObserver.observe(content.value);
}
}
| const viewport = ref(null); // scrollable element | ||
| const thumb = ref(null); |
There was a problem hiding this comment.
声明用于包裹插槽内容容器的 content 模板引用，以配合 ResizeObserver 进行更健壮的尺寸监听。
const viewport = ref(null); // scrollable element
const thumb = ref(null);
const content = ref(null);
| <div ref="viewport" class="overlay-scrollbar__viewport" @scroll="onScroll"> | ||
| <slot /> | ||
| </div> |
There was a problem hiding this comment.
在 <slot /> 外层包裹一个固定的内容容器 div，并绑定 ref="content"，以便 ResizeObserver 能够稳定地监听插槽内容的高度变化。
<div ref="viewport" class="overlay-scrollbar__viewport" @scroll="onScroll">
<div ref="content" class="overlay-scrollbar__content">
<slot />
</div>
</div>
1 participant
Motivation / 动机
前端 Dashboard 在窄屏/手机端存在多个体验问题：
Modifications / 改动点
This is NOT a breaking change. / 这不是一个破坏性变更。
Checklist / 检查清单
Summary by Sourcery
Improve dashboard scrolling behavior and mobile responsiveness across sidebar, chat, provider, and main layout views by introducing a reusable overlay scrollbar component and refining layout breakpoints.
New Features:
Enhancements: