fix: reliable window drag region in tab bar#69
Conversation
Includes scroll improvements: - Scroll to bottom on session open and live auto-scroll - Make auto-scroll StrictMode-safe via needsInitialScrollRef - Add floating scroll-to-bottom button in chat view Window drag fix: - Apply drag region to leftmost pane TabBar regardless of sidebar state - Cap tab list at 75% so drag spacer always has room - Add explicit flex-1 drag spacer between tabs and action buttons - Set WebkitAppRegion: no-drag on tab items for reordering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello @proxikal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by addressing two key areas: window dragging and chat auto-scrolling. It resolves an issue where window dragging on macOS was unreliable and ensures that tab reordering works correctly on Windows. Additionally, it refines the auto-scroll behavior in chat, making it more robust and introducing a convenient scroll-to-bottom button for improved navigation. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully addresses the unreliable window drag regions on macOS and improves the auto-scroll behavior in the chat history. The introduction of a drag spacer in the tab bar and the use of double-requestAnimationFrame for auto-scrolling are solid improvements. I have a few suggestions regarding maintainability (using constants for magic numbers) and ensuring consistency in the drag region logic across different pane states.
| const [showScrollButton, setShowScrollButton] = useState(false); | ||
|
|
||
| const checkScrollButton = useCallback(() => { | ||
| const container = scrollContainerRef.current; | ||
| if (!container) return; | ||
| const { scrollTop, scrollHeight, clientHeight } = container; | ||
| setShowScrollButton(!isNearBottom(scrollTop, scrollHeight, clientHeight, 300)); | ||
| }, []); |
There was a problem hiding this comment.
The scroll threshold 300 is used in multiple places within this component. It is recommended to define this as a constant (e.g., SCROLL_THRESHOLD) to ensure consistency and make it easier to maintain.
| const [showScrollButton, setShowScrollButton] = useState(false); | |
| const checkScrollButton = useCallback(() => { | |
| const container = scrollContainerRef.current; | |
| if (!container) return; | |
| const { scrollTop, scrollHeight, clientHeight } = container; | |
| setShowScrollButton(!isNearBottom(scrollTop, scrollHeight, clientHeight, 300)); | |
| }, []); | |
| const SCROLL_THRESHOLD = 300; | |
| // Scroll-to-bottom button visibility | |
| const [showScrollButton, setShowScrollButton] = useState(false); | |
| const checkScrollButton = useCallback(() => { | |
| const container = scrollContainerRef.current; | |
| if (!container) return; | |
| const { scrollTop, scrollHeight, clientHeight } = container; | |
| setShowScrollButton(!isNearBottom(scrollTop, scrollHeight, clientHeight, SCROLL_THRESHOLD)); | |
| }, []); |
| useAutoScrollBottom([conversation], { | ||
| threshold: 150, | ||
| const { scrollToBottom } = useAutoScrollBottom([conversation], { | ||
| threshold: 300, |
There was a problem hiding this comment.
Use the constant here as well to keep the threshold in sync with the scroll button visibility logic.
| threshold: 300, | |
| threshold: SCROLL_THRESHOLD, |
| style={{ | ||
| right: | ||
| isContextPanelVisible && allContextInjections.length > 0 | ||
| ? 'calc(320px + 1rem)' |
There was a problem hiding this comment.
The 320px value matches the width of the context panel (w-80). It would be safer to use a constant for the panel width to avoid layout issues if the sidebar width is ever adjusted.
| Gives users a reliable window-drag target regardless of how many tabs are open. */} | ||
| <div | ||
| className="flex-1 self-stretch" | ||
| style={{ WebkitAppRegion: 'drag' } as React.CSSProperties} |
There was a problem hiding this comment.
To maintain consistency with the background drag logic defined at line 271, the spacer's drag region should also be conditional on isLeftmostPane. This ensures that only the primary title bar area acts as a window drag handle, preventing unexpected behavior in split-pane layouts.
| style={{ WebkitAppRegion: 'drag' } as React.CSSProperties} | |
| style={{ WebkitAppRegion: isElectronMode() && isLeftmostPane ? 'drag' : undefined } as React.CSSProperties} |
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds a bottom-scroll button to the chat history with scroll visibility detection and a larger auto-scroll threshold (150→300). Updates auto-scroll hook to track resetKey and use a double requestAnimationFrame flow. Adjusts Electron drag-region styling and adds a drag spacer in the tab bar. Changes
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/components/chat/ChatHistory.tsx (1)
1-10: Reorder imports to follow external → alias → relative grouping.
This keeps the file aligned with the project's import-order convention.♻️ Proposed import order
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; - -import { isNearBottom, useAutoScrollBottom } from '@renderer/hooks/useAutoScrollBottom'; -import { useTabNavigationController } from '@renderer/hooks/useTabNavigationController'; -import { useTabUI } from '@renderer/hooks/useTabUI'; -import { useVisibleAIGroup } from '@renderer/hooks/useVisibleAIGroup'; -import { useStore } from '@renderer/store'; -import { useVirtualizer } from '@tanstack/react-virtual'; -import { ChevronsDown } from 'lucide-react'; -import { useShallow } from 'zustand/react/shallow'; - -import { SessionContextPanel } from './SessionContextPanel/index'; -import { ChatHistoryEmptyState } from './ChatHistoryEmptyState'; -import { ChatHistoryItem } from './ChatHistoryItem'; -import { ChatHistoryLoadingState } from './ChatHistoryLoadingState'; - -import type { ContextInjection } from '@renderer/types/contextInjection'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useVirtualizer } from '@tanstack/react-virtual'; +import { ChevronsDown } from 'lucide-react'; +import { useShallow } from 'zustand/react/shallow'; + +import { isNearBottom, useAutoScrollBottom } from '@renderer/hooks/useAutoScrollBottom'; +import { useTabNavigationController } from '@renderer/hooks/useTabNavigationController'; +import { useTabUI } from '@renderer/hooks/useTabUI'; +import { useVisibleAIGroup } from '@renderer/hooks/useVisibleAIGroup'; +import { useStore } from '@renderer/store'; +import type { ContextInjection } from '@renderer/types/contextInjection'; + +import { SessionContextPanel } from './SessionContextPanel/index'; +import { ChatHistoryEmptyState } from './ChatHistoryEmptyState'; +import { ChatHistoryItem } from './ChatHistoryItem'; +import { ChatHistoryLoadingState } from './ChatHistoryLoadingState';As per coding guidelines "Organize imports in order: external packages, path aliases (
@main,@renderer,@shared,@preload), then relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/ChatHistory.tsx` around lines 1 - 10, The imports in ChatHistory.tsx are not grouped per project convention; reorder them so external packages come first (react, `@tanstack/react-virtual`, lucide-react, zustand), then path aliases (`@renderer/hooks`, `@renderer/store`), then any relative imports, preserving existing imported symbols (e.g., useCallback/useEffect/useMemo/useRef/useState, useVirtualizer, ChevronsDown, useShallow, isNearBottom, useAutoScrollBottom, useTabNavigationController, useTabUI, useVisibleAIGroup, useStore) and add single blank lines between each group to match the external → alias → relative ordering rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/chat/ChatHistory.tsx`:
- Around line 1-10: The imports in ChatHistory.tsx are not grouped per project
convention; reorder them so external packages come first (react,
`@tanstack/react-virtual`, lucide-react, zustand), then path aliases
(`@renderer/hooks`, `@renderer/store`), then any relative imports, preserving
existing imported symbols (e.g., useCallback/useEffect/useMemo/useRef/useState,
useVirtualizer, ChevronsDown, useShallow, isNearBottom, useAutoScrollBottom,
useTabNavigationController, useTabUI, useVisibleAIGroup, useStore) and add
single blank lines between each group to match the external → alias → relative
ordering rule.
- Extract SCROLL_THRESHOLD (300px) constant so auto-scroll hook and scroll-button visibility logic stay in sync - Extract CONTEXT_PANEL_WIDTH_PX (320px) constant to avoid layout drift if the context panel width is ever adjusted - Gate drag spacer on isElectronMode() && isLeftmostPane to match the TabBar drag region logic and prevent unintended drag regions in split-pane layouts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Problem
On macOS, dragging the window by the tab bar was unreliable — roughly a 20% success rate. The drag region was only applied when the sidebar was collapsed and the pane was leftmost, meaning any other state left users with no draggable area. After two days of testing this was confirmed fully resolved on macOS.
On Windows, tab items were not marked
no-drag, interfering with tab reordering via DnD.Changes
Window drag fix (
TabBar.tsx,SortableTab.tsx)WebkitAppRegion: dragto the leftmost pane's tab bar regardless of sidebar state — previously gated behindsidebarCollapsed75%width so the drag spacer always has room to the rightflex-1drag spacer between the tab list and action buttons, giving a reliable drag target no matter how many tabs are openWebkitAppRegion: no-dragonSortableTabso tab DnD reordering doesn't conflictAuto-scroll improvements (
useAutoScrollBottom.ts,ChatHistory.tsx)requestAnimationFramewith double-RAF + cleanup (cancelAnimationFrameon both IDs) to prevent React StrictMode's double-invoke from firing the scroll twiceneedsInitialScrollRefto force scroll-to-bottom on first load after a tab/session switch, even ifwasAtBottomBeforeUpdateRefhasn't settled yetTesting
Summary by CodeRabbit
New Features
UI Improvements