fix: improve message navigation scroll position to show content from top #777
Conversation
WalkthroughChanged scroll alignment in MessageList.vue: scrollIntoView now uses block: 'start' instead of 'center'. Smooth scrolling and post-scroll highlight remain unchanged. No public API or other logic modifications. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant MessageList
participant DOM as DOM Element
participant UI as UI Highlight
User->>MessageList: navigate to message
MessageList->>DOM: scrollIntoView({ behavior: "smooth", block: "start" })
note over MessageList,DOM: Changed from block: "center" to "start"
DOM-->>MessageList: scroll completes
MessageList->>UI: apply temporary highlight
UI-->>User: highlight fades
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/components/message/MessageList.vue (2)
313-314: Pass the ref itself to useElementBounding, not its value.useElementBounding expects a ref so it can react to mount/size changes. Passing messageList.value (likely undefined during setup) breaks reactivity.
Apply this diff:
- const { height } = useElementBounding(messageList.value) + const { height } = useElementBounding(messageList)
269-276: Replace invalid ScrollBehavior ‘instant’ with ‘auto’The ScrollBehavior value
'instant'is not supported by the DOM API and fails TypeScript checks inlib.dom.d.ts. To perform an immediate scroll without animation, use'auto'instead.Locations to update:
- src/renderer/src/components/message/MessageList.vue: line 272
Recommended change:
nextTick(() => { scrollAnchor.value?.scrollIntoView({ - behavior: 'instant', + behavior: 'auto', block: 'end' }) })
🧹 Nitpick comments (6)
src/renderer/src/components/message/MessageList.vue (6)
281-289: Top-aligned navigation is correct; consider minor UX hardening.Changing scrollIntoView to block: 'start' meets the PR goal of showing the navigated message from the top. To avoid unwanted horizontal scrolling on narrow layouts, add inline: 'nearest'.
Apply this diff:
- messageElement.scrollIntoView({ - behavior: 'smooth', - block: 'start' - }) + messageElement.scrollIntoView({ + behavior: 'smooth', + block: 'start', + inline: 'nearest' + })Optional: if a sticky header ever overlays the container, set a top offset via scroll-padding-top on the container or scroll-margin-top on each message element.
101-103: Type the messages prop as a heterogeneous array.Messages are mixed user/assistant items. Using UserMessage[] | AssistantMessage[] implies the array is homogeneous. Prefer a union element type.
Apply this diff:
-const props = defineProps<{ - messages: UserMessage[] | AssistantMessage[] -}>() +const props = defineProps<{ + messages: Array<UserMessage | AssistantMessage> +}>()
119-129: Avoid any for component refs; type exposed API.Replace any with a minimal typed surface for the assistant refs. This improves safety when calling handleAction.
Apply this diff:
-// Store refs as Record to avoid type checking issues -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const assistantRefs = reactive<Record<number, any>>({}) +type AssistantItemExposed = { handleAction?: (action: 'retry') => void } +// Store refs as Record to avoid type checking issues +const assistantRefs = reactive<Record<number, AssistantItemExposed | null>>({}) // Helper function to set refs -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const setAssistantRef = (index: number) => (el: any) => { - if (el) { - assistantRefs[index] = el - } -} +const setAssistantRef = (index: number) => (el: AssistantItemExposed | null) => { + assistantRefs[index] = el ?? null +}
264-266: Logs should be in English per guidelines.Change the error message to English to align with the repo’s logging guideline.
Apply this diff:
- if (!success) { - console.error('截图复制失败') - } + if (!success) { + console.error('Failed to copy screenshot') + }
135-141: Escape CSS attribute selectors built from IDs.If message IDs can contain special CSS characters (e.g., quotes, brackets), document.querySelector with a raw template string can break. Use CSS.escape when available.
Example adjustment:
const byMessageIdSelector = (id: string) => `[data-message-id="${typeof CSS !== 'undefined' && typeof CSS.escape === 'function' ? CSS.escape(id) : id}"]` // usage const userMessageSelector = byMessageIdSelector(parentId) const messageElement = document.querySelector(byMessageIdSelector(messageId))Also applies to: 283-285
398-407: Consider Tailwind utilities for the highlight style.The highlight styles are simple and could be expressed with Tailwind classes (e.g., bg-blue-500/10, border-l-4, border-blue-500). Not required, but it would reduce custom CSS and keep styling consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/renderer/src/components/message/MessageList.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.vue: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/message/MessageList.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.
Files:
src/renderer/src/components/message/MessageList.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
improve message navigation scroll position to show content from top
Summary by CodeRabbit