fix: resolve double scrollbar in conversation detail and console page - #9382
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The hard-coded
calc(100vh - 67px)in.console-pagecouples the layout to a specific header height; consider deriving this from a shared layout variable or using a container-based height to avoid brittle magic numbers. - After removing
onContainerWheeland the related scroll handling, theref="messagesContainer"on the preview container is now unused and can be dropped to reduce dead code and potential confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded `calc(100vh - 67px)` in `.console-page` couples the layout to a specific header height; consider deriving this from a shared layout variable or using a container-based height to avoid brittle magic numbers.
- After removing `onContainerWheel` and the related scroll handling, the `ref="messagesContainer"` on the preview container is now unused and can be dropped to reduce dead code and potential confusion.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/ConsolePage.vue" line_range="113" />
<code_context>
- height: 100%;
+ display: flex;
+ flex-direction: column;
+ height: calc(100vh - 67px);
margin: 0 auto;
max-width: 1400px;
</code_context>
<issue_to_address>
**suggestion:** Using a hard-coded `67px` offset for the page height can be brittle if header/toolbars change.
This height depends on the header/footer staying exactly 67px tall, so any layout change (responsive variants, new toolbars, design tweaks) could cause extra whitespace or clipping. Consider relying on flex layout for sizing or using a shared CSS variable/token instead of a literal `67px` to make the dependency explicit and easier to maintain.
Suggested implementation:
```
.console-page {
display: flex;
flex-direction: column;
min-height: calc(100vh - var(--app-header-height, 67px));
margin: 0 auto;
max-width: 1400px;
padding: 24px;
}
.console-header {
```
To make this fully robust:
1. Define the `--app-header-height` CSS variable in a shared/global stylesheet (e.g. on `:root` or your main layout wrapper) so it reflects the actual header/toolbars height.
2. If the header height is responsive, update `--app-header-height` via media queries or layout breakpoints instead of changing this page component.
3. Verify parent layout allows `.console-page` to grow (e.g. parent uses flex and lets this view stretch) so `min-height` behaves as expected.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| height: 100%; | ||
| display: flex; | ||
| flex-direction: column; | ||
| height: calc(100vh - 67px); |
There was a problem hiding this comment.
suggestion: Using a hard-coded 67px offset for the page height can be brittle if header/toolbars change.
This height depends on the header/footer staying exactly 67px tall, so any layout change (responsive variants, new toolbars, design tweaks) could cause extra whitespace or clipping. Consider relying on flex layout for sizing or using a shared CSS variable/token instead of a literal 67px to make the dependency explicit and easier to maintain.
Suggested implementation:
.console-page {
display: flex;
flex-direction: column;
min-height: calc(100vh - var(--app-header-height, 67px));
margin: 0 auto;
max-width: 1400px;
padding: 24px;
}
.console-header {
To make this fully robust:
- Define the
--app-header-heightCSS variable in a shared/global stylesheet (e.g. on:rootor your main layout wrapper) so it reflects the actual header/toolbars height. - If the header height is responsive, update
--app-header-heightvia media queries or layout breakpoints instead of changing this page component. - Verify parent layout allows
.console-pageto grow (e.g. parent uses flex and lets this view stretch) somin-heightbehaves as expected.
WebUI 中"对话数据"详情弹窗和"平台日志"页面出现套娃滚动条。
Closes #9361
Modifications / 改动点
ConversationPage.vue — 对话详情对话框:
v-card-text内移至标题区下方,保持固定可见.conversation-messages-container的max-height和overflow-y: autov-dialog[scrollable],由 Vuetify 统一管理滚动,消除双层滚动条ConsolePage.vue — 平台日志页面:
将
.console-page改为 flex 列布局,使用calc(100vh - 67px)约束高度.console-display改为flex: 1; min-height: 0自适应剩余空间.console-term保留overflow-y: auto作为唯一滚动层This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Resolve double scrollbar issues in the conversation detail dialog and console logs page by relying on a single scroll container in each view.
Bug Fixes:
Enhancements: