fix: resolve race condition in floating button drag-and-drop detection #813
Conversation
WalkthroughAdds stricter mouse-button guards and clears a pending drag-start timer in the renderer; restructures presenter drag-state to use a nullable state and an explicit wasVisible-before-drag flag, suppresses clicks during drag, and tightens drag move/end guards; simplifies FloatingChatWindow placement to a right/left/fallback algorithm with clamped coordinates. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FB as FloatingButton.vue
participant P as Presenter
U->>FB: mousedown (button === 0)
FB->>FB: start DRAG_DELAY timer
U->>FB: mousemove (small)
Note right of FB: still pending
U->>FB: mousemove (exceeds threshold)
rect rgba(200,240,255,0.25)
FB->>FB: clearDragTimer()
FB->>P: dispatch DRAG_START
end
P->>P: set isDuringDragSession = true\ninit dragState, hide FloatingChatWindow if visible
U->>FB: mousemove (while dragging)
FB->>P: dispatch DRAG_MOVE
P->>P: guard dragState && isDragging\nupdate window position
U->>FB: mouseup
FB->>P: dispatch DRAG_END
P->>P: guard dragState && isDragging\nclamp position, restore FloatingChatWindow if needed\nreset dragState, isDuringDragSession = false
sequenceDiagram
participant U as User
participant FB as FloatingButton.vue
participant P as Presenter
U->>FB: click
FB->>P: dispatch CLICKED
alt isDuringDragSession == true
P-->>FB: return early (suppress click)
else
P->>P: toggle/show FloatingChatWindow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 (
|
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/floating/FloatingButton.vue (2)
12-14: Avoid double mousemove handlingYou bind @mousemove on the element and also add a document-level mousemove on mousedown. During drag both will fire, doubling work and risking jitter. Rely on the document listener only.
- @mousemove="handleMouseMove"No other changes required since you already add/remove document listeners in handleMouseDown/Up.
Also applies to: 81-83
31-62: Comments must be in English per repo guidelinesSeveral comments are Chinese. Please translate to English to comply with src/**/*.{ts,tsx,vue} guideline. Example at Line 93:
- // 位移启动拖拽时，立即清除定时器，防止二次触发 + // When starting drag due to displacement, clear the pending timer to prevent a second startAlso applies to: 241-342
🧹 Nitpick comments (4)
src/renderer/floating/FloatingButton.vue (4)
94-95: Prettier failure: remove stray semicolon/trailing spaceLocal style appears to be no-semi. Also remove trailing whitespace. Apply:
- clearDragTimer(); + clearDragTimer()
138-141: Safer timer null-checkGuard against a falsy timer id (defensive) and be explicit:
- if (dragState.value.dragTimer) { - clearTimeout(dragState.value.dragTimer) + if (dragState.value.dragTimer !== null) { + window.clearTimeout(dragState.value.dragTimer) dragState.value.dragTimer = null }
205-210: Prevent stacked timeouts on mouse leaveRepeated enter/leave schedules multiple timers. Track and clear the previous one.
- // 延迟恢复脉冲动画 - setTimeout(() => { - isPulsing.value = true - }, 3000) + // Resume pulse after delay (deduplicated) + clearPulseTimer() + pulseTimer = window.setTimeout(() => { + isPulsing.value = true + pulseTimer = null + }, 3000)Add near the top (outside this hunk):
let pulseTimer: number | null = null const clearPulseTimer = () => { if (pulseTimer !== null) { window.clearTimeout(pulseTimer) pulseTimer = null } }And in onUnmounted(), call clearPulseTimer().
156-164: Prefer presenters over global window APIGuideline says: from renderer to main, call presenters via usePresenter.ts. If floatingButtonAPI is a bridge, consider wrapping it in a presenter/composable to keep renderer decoupled and testable.
Also applies to: 189-197, 221-224
📜 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/floating/FloatingButton.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/floating/FloatingButton.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/floating/FloatingButton.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/floating/FloatingButton.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.
src/renderer/**/*.{ts,vue}: Use Pinia for frontend state management
From renderer to main, call presenters via the usePresenter.ts composable
Files:
src/renderer/floating/FloatingButton.vue
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/floating/FloatingButton.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: Use Vue 3 Composition API for all components
Use Tailwind CSS with scoped styles for component styling
Files:
src/renderer/floating/FloatingButton.vue
🧠 Learnings (1)
📓 Common learnings
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:258-324
Timestamp: 2025-06-21T15:53:34.641Z
Learning: User neoragex2002 prefers concise, brief responses rather than chatty or verbose communication. Avoid lengthy explanations, excessive enthusiasm, or ASCII art in responses.
🪛 GitHub Actions: PR Check
src/renderer/floating/FloatingButton.vue
[error] 1-1: Prettier formatting check failed during 'pnpm run format:check' for this file. Run 'prettier --write' to fix.
🔇 Additional comments (3)
src/renderer/floating/FloatingButton.vue (3)
93-95: Good fix: timer race eliminatedClearing the pending DRAG_DELAY timer before immediate drag start prevents double onDragStart. This addresses the “dragging jumps/erratic” issue.
1-343: Formatting pipeline failureRun prettier to align with repo style:
- pnpm run format:write
Ensure CI passes after the small style tweak on Line 94.
235-237: Don't call removeAllListeners() on window.floatingButtonAPI — remove only the handler you addedRemoving all listeners on the shared API can break other components; unregister the specific event(s) you registered instead.
Suggested replacement:
- if (window.floatingButtonAPI) { - window.floatingButtonAPI.removeAllListeners() - } + window.floatingButtonAPI?.off?.('configUpdate', handleConfigUpdate) + // or: window.floatingButtonAPI?.removeListener?.('configUpdate', handleConfigUpdate)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (1)
100-101: Respect config.alwaysOnTop; don’t force true.You override the supplied config and always set on-top.
- this.window.setAlwaysOnTop(true, 'floating') + this.window.setAlwaysOnTop(this.config.alwaysOnTop, 'floating')src/main/presenter/floatingButtonPresenter/index.ts (2)
231-256: Clamp to current display’s workArea (uses 0-based coords now, breaks on multi-monitor).Current logic ignores workArea.x/y and uses require in a hot path. Import screen once and clamp against the matching display.
- ipcMain.on(FLOATING_BUTTON_EVENTS.DRAG_END, (_event, _data: { x: number; y: number }) => { + ipcMain.on(FLOATING_BUTTON_EVENTS.DRAG_END, (_event, _data: { x: number; y: number }) => { try { if ( dragState && dragState.isDragging && this.floatingWindow && this.floatingWindow.exists() ) { const buttonWindow = this.floatingWindow.getWindow() if (buttonWindow && !buttonWindow.isDestroyed()) { - // 简单边界检查 - const { screen } = require('electron') - const primaryDisplay = screen.getPrimaryDisplay() - const { workArea } = primaryDisplay - - const bounds = buttonWindow.getBounds() - - // 确保悬浮球完全在屏幕范围内 - const targetX = Math.max(0, Math.min(bounds.x, workArea.width - bounds.width)) - const targetY = Math.max(0, Math.min(bounds.y, workArea.height - bounds.height)) + // Bounds clamp within current display workArea + const bounds = buttonWindow.getBounds() + const display = screen.getDisplayMatching(bounds) + const { workArea } = display + const targetX = Math.max( + workArea.x, + Math.min(bounds.x, workArea.x + workArea.width - bounds.width) + ) + const targetY = Math.max( + workArea.y, + Math.min(bounds.y, workArea.y + workArea.height - bounds.height) + ) // 只有在越界时才调整位置 if (targetX !== bounds.x || targetY !== bounds.y) { buttonWindow.setPosition(targetX, targetY) } } }Additionally, import screen at the top:
-import { ipcMain, Menu, app } from 'electron' +import { ipcMain, Menu, app, screen } from 'electron'
159-166: Also suppress context menu during drag.Mirror the click guard to avoid menu popups mid-drag.
- ipcMain.on(FLOATING_BUTTON_EVENTS.RIGHT_CLICKED, () => { + ipcMain.on(FLOATING_BUTTON_EVENTS.RIGHT_CLICKED, () => { + if (isDuringDragSession) { + return + } try { this.showContextMenu() } catch (error) { console.error('Failed to handle floating button right click:', error) } })
🧹 Nitpick comments (1)
src/main/presenter/floatingButtonPresenter/index.ts (1)
208-229: Optional: sanity-check incoming coordinates.Validate that x/y are finite numbers before using them to move windows.
- ipcMain.on(FLOATING_BUTTON_EVENTS.DRAG_MOVE, (_event, { x, y }: { x: number; y: number }) => { + ipcMain.on(FLOATING_BUTTON_EVENTS.DRAG_MOVE, (_event, { x, y }: { x: number; y: number }) => { try { + if (!Number.isFinite(x) || !Number.isFinite(y)) return if ( dragState && dragState.isDragging && this.floatingWindow && this.floatingWindow.exists() ) {
📜 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 (3)
src/main/presenter/floatingButtonPresenter/index.ts(7 hunks)src/main/presenter/windowPresenter/FloatingChatWindow.ts(1 hunks)src/renderer/floating/FloatingButton.vue(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/floating/FloatingButton.vue
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
From main to renderer, broadcast events via EventBus using mainWindow.webContents.send()
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别Enable and adhere to strict TypeScript type checking
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain one presenter per functional domain in src/main/presenter/
Files:
src/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:258-324
Timestamp: 2025-06-21T15:53:34.641Z
Learning: User neoragex2002 prefers concise, brief responses rather than chatty or verbose communication. Avoid lengthy explanations, excessive enthusiasm, or ASCII art in responses.
🧬 Code graph analysis (1)
src/main/presenter/floatingButtonPresenter/index.ts (1)
src/main/events.ts (1)
FLOATING_BUTTON_EVENTS(180-189)
⏰ 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)
🔇 Additional comments (2)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (1)
90-96: Harden renderer security: enable webSecurity and sandbox.webSecurity and sandbox are currently disabled (
false), which weakens the renderer’s isolation—set both totrue(even in dev) unless there’s a documented requirement to disable them.src/main/presenter/floatingButtonPresenter/index.ts (1)
127-134: Good guard to suppress clicks during drag.Prevents spurious toggles while dragging.
| console.log('FloatingChatWindow hidden during drag start') | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Use structured logger and English logs; replace console.*.
Repo guidelines require English logs and structured logging. Replace console.* with the shared logger for consistency.
+import logger from '../../../shared/logger'
@@
- console.log('FloatingButtonPresenter.enable called, current enabled:', this.config.enabled, 'has window:', !!this.floatingWindow)
+ logger.info('FloatingButtonPresenter.enable called', { enabled: this.config.enabled, hasWindow: !!this.floatingWindow })
@@
- console.log('FloatingButton window already exists, showing it')
+ logger.info('FloatingButton window already exists, showing it')
@@
- console.log('FloatingChatWindow hidden during drag start')
+ logger.info('FloatingChatWindow hidden during drag start')
@@
- console.log('FloatingChatWindow shown after drag end')
+ logger.info('FloatingChatWindow shown after drag end')
@@
- console.error('Failed to handle drag end:', error)
+ logger.error('Failed to handle drag end', { error })Note: please migrate remaining Chinese comments/logs to English in a follow-up sweep.
Also applies to: 281-282, 289-292, 73-90
🏁 Script executed:
#!/bin/bash
set -e
# Locate the floatingButtonPresenter index file
echo "===== Locating floatingButtonPresenter/index.ts ====="
FILE_PATH=$(fd floatingButtonPresenter/index.ts)
# Report if the file was found
if [[ -z "$FILE_PATH" ]]; then
echo "ERROR: floatingButtonPresenter/index.ts not found"
exit 1
fi
echo "Found at: $FILE_PATH"
# Search for console.log and console.error usages
echo "===== console.log / console.error occurrences ====="
rg -n "console\.(log|error)" "$FILE_PATH" || true
# Check for existing import of shared logger
echo "===== Existing logger import ====="
rg -n "import .*logger" "$FILE_PATH" || true
# Identify any Chinese characters in comments or logs
echo "===== Chinese characters in file ====="
rg -nP "[\u4e00-\u9fff]" "$FILE_PATH" || trueLength of output: 309
🏁 Script executed:
#!/bin/bash
set -e
# Find the floatingButtonPresenter directory
echo "===== Finding floatingButtonPresenter directory ====="
DIR=$(fd floatingButtonPresenter -t d)
if [[ -z "$DIR" ]]; then
echo "ERROR: Directory floatingButtonPresenter not found"
exit 1
fi
echo "Directory found at: $DIR"
# Confirm index.ts exists
INDEX_FILE="$DIR/index.ts"
if [[ ! -f "$INDEX_FILE" ]]; then
echo "ERROR: index.ts not found in $DIR"
exit 1
fi
echo "===== Searching for console.log / console.error usages in $INDEX_FILE ====="
rg -n "console\.(log|error)" "$INDEX_FILE" || true
echo "===== Checking for existing logger import in $INDEX_FILE ====="
rg -n "import .*logger" "$INDEX_FILE" || true
echo "===== Identifying Chinese characters in $INDEX_FILE ====="
rg -nP "[\u4e00-\u9fff]" "$INDEX_FILE" || trueLength of output: 3106
🏁 Script executed:
#!/bin/bash
set -e
FILE="src/main/presenter/floatingButtonPresenter/index.ts"
echo "===== Identifying Chinese characters in $FILE ====="
rg -nP '\p{Han}' "$FILE" || trueLength of output: 918
Replace all console. calls with shared logger and translate Chinese comments to English*
• In src/main/presenter/floatingButtonPresenter/index.ts (19 occurrences): remove console.log/console.error, import logger from ../../../shared/logger, and use logger.info/logger.error with structured args.
• In the same file: migrate all Chinese comments (e.g. 初始化悬浮按钮功能, 销毁悬浮按钮功能, etc.) to English in a follow-up sweep.
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/index.ts around lines 191–193,
replace the console.log/console.error calls (there are 19 occurrences across the
file) with the shared logger: add import logger from '../../../shared/logger' at
the top, change console.log(...) to logger.info({ msg: 'FloatingChatWindow
hidden during drag start', context: 'floatingButtonPresenter', ...optionalFields
}) and console.error(...) to logger.error({ msg: '...', error, context:
'floatingButtonPresenter' }) using structured args; also convert all Chinese
comments in this file (e.g., 初始化悬浮按钮功能, 销毁悬浮按钮功能) to English equivalents in a
follow-up sweep for consistency.
| const primaryDisplay = screen.getPrimaryDisplay() | ||
| const { workArea } = primaryDisplay | ||
| let x: number, y: number | ||
| const windowWidth = this.window?.getBounds().width ?? this.config.size.width | ||
| const windowHeight = this.window?.getBounds().height ?? this.config.size.height | ||
|
|
||
| if (!floatingButtonPosition) { | ||
| x = workArea.x + workArea.width - this.config.size.width - 20 | ||
| y = workArea.y + workArea.height - this.config.size.height - 20 | ||
| const x = workArea.x + workArea.width - windowWidth - 20 | ||
| const y = workArea.y + workArea.height - windowHeight - 20 | ||
| return { x, y } | ||
| } | ||
|
|
||
| const buttonX = floatingButtonPosition.x | ||
| const buttonY = floatingButtonPosition.y | ||
| const buttonWidth = floatingButtonPosition.width | ||
| const buttonHeight = floatingButtonPosition.height | ||
| const windowWidth = this.config.size.width | ||
| const windowHeight = this.config.size.height | ||
| const gap = 15 | ||
| const buttonCenterX = buttonX + buttonWidth / 2 | ||
| const buttonCenterY = buttonY + buttonHeight / 2 | ||
| const screenCenterX = workArea.x + workArea.width / 2 | ||
| const screenCenterY = workArea.y + workArea.height / 2 | ||
|
|
||
| let positions: Array<{ x: number; y: number; priority: number }> = [] | ||
| if (buttonX + buttonWidth + gap + windowWidth <= workArea.x + workArea.width) { | ||
| positions.push({ | ||
| x: buttonX + buttonWidth + gap, | ||
| y: Math.max( | ||
| workArea.y, | ||
| Math.min( | ||
| buttonY + (buttonHeight - windowHeight) / 2, | ||
| workArea.y + workArea.height - windowHeight | ||
| ) | ||
| ), | ||
| priority: buttonCenterX < screenCenterX ? 1 : 3 | ||
| }) | ||
| } | ||
| const buttonBounds = floatingButtonPosition | ||
|
|
||
| if (buttonX - gap - windowWidth >= workArea.x) { | ||
| positions.push({ | ||
| x: buttonX - gap - windowWidth, | ||
| y: Math.max( | ||
| workArea.y, | ||
| Math.min( | ||
| buttonY + (buttonHeight - windowHeight) / 2, | ||
| workArea.y + workArea.height - windowHeight | ||
| ) | ||
| ), | ||
| priority: buttonCenterX >= screenCenterX ? 1 : 3 | ||
| }) | ||
| } | ||
| let finalX: number | ||
|
|
||
| if (buttonY + buttonHeight + gap + windowHeight <= workArea.y + workArea.height) { | ||
| positions.push({ | ||
| x: Math.max( | ||
| workArea.x, | ||
| Math.min( | ||
| buttonX + (buttonWidth - windowWidth) / 2, | ||
| workArea.x + workArea.width - windowWidth | ||
| ) | ||
| ), | ||
| y: buttonY + buttonHeight + gap, | ||
| priority: buttonCenterY < screenCenterY ? 2 : 4 | ||
| }) | ||
| // 1. Prioritize placing the window on the right side of the button | ||
| const rightPositionX = buttonBounds.x + buttonBounds.width + gap | ||
| if (rightPositionX + windowWidth <= workArea.x + workArea.width) { | ||
| finalX = rightPositionX | ||
| } else { | ||
| // 2. If right side has no space, try the left side | ||
| const leftPositionX = buttonBounds.x - windowWidth - gap | ||
| if (leftPositionX >= workArea.x) { | ||
| finalX = leftPositionX | ||
| } else { | ||
| // 3. Fallback: If both sides lack space, align window's right edge with screen's right edge. | ||
| finalX = workArea.x + workArea.width - windowWidth | ||
| } | ||
| } | ||
|
|
||
| if (buttonY - gap - windowHeight >= workArea.y) { | ||
| positions.push({ | ||
| x: Math.max( | ||
| workArea.x, | ||
| Math.min( | ||
| buttonX + (buttonWidth - windowWidth) / 2, | ||
| workArea.x + workArea.width - windowWidth | ||
| ) | ||
| ), | ||
| y: buttonY - gap - windowHeight, | ||
| priority: buttonCenterY >= screenCenterY ? 2 : 4 | ||
| }) | ||
| } | ||
| // Calculate vertical position: try to center with the button, but stay within screen bounds. | ||
| const idealY = buttonBounds.y + (buttonBounds.height - windowHeight) / 2 | ||
| const finalY = Math.max( | ||
| workArea.y, | ||
| Math.min(idealY, workArea.y + workArea.height - windowHeight) | ||
| ) | ||
|
|
||
| if (positions.length === 0) { | ||
| x = workArea.x + workArea.width - windowWidth - 20 | ||
| y = workArea.y + workArea.height - windowHeight - 20 | ||
| } else { | ||
| positions.sort((a, b) => a.priority - b.priority) | ||
| x = positions[0].x | ||
| y = positions[0].y | ||
| } | ||
| x = Math.max(workArea.x + 10, Math.min(x, workArea.x + workArea.width - windowWidth - 10)) | ||
| y = Math.max(workArea.y + 10, Math.min(y, workArea.y + workArea.height - windowHeight - 10)) | ||
| return { x, y } | ||
| return { x: Math.round(finalX), y: Math.round(finalY) } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix multi-display placement: choose display nearest to button/window, not primary.
Using the primary display misplaces/clamps when the button/window is on another monitor or when workArea has non-zero x/y. Select the display via getDisplayMatching and then derive workArea.
- const primaryDisplay = screen.getPrimaryDisplay()
- const { workArea } = primaryDisplay
+ const display = floatingButtonPosition
+ ? screen.getDisplayMatching({
+ x: floatingButtonPosition.x,
+ y: floatingButtonPosition.y,
+ width: floatingButtonPosition.width,
+ height: floatingButtonPosition.height
+ })
+ : (this.window && !this.window.isDestroyed()
+ ? screen.getDisplayMatching(this.window.getBounds())
+ : screen.getPrimaryDisplay())
+ const { workArea } = display📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const primaryDisplay = screen.getPrimaryDisplay() | |
| const { workArea } = primaryDisplay | |
| let x: number, y: number | |
| const windowWidth = this.window?.getBounds().width ?? this.config.size.width | |
| const windowHeight = this.window?.getBounds().height ?? this.config.size.height | |
| if (!floatingButtonPosition) { | |
| x = workArea.x + workArea.width - this.config.size.width - 20 | |
| y = workArea.y + workArea.height - this.config.size.height - 20 | |
| const x = workArea.x + workArea.width - windowWidth - 20 | |
| const y = workArea.y + workArea.height - windowHeight - 20 | |
| return { x, y } | |
| } | |
| const buttonX = floatingButtonPosition.x | |
| const buttonY = floatingButtonPosition.y | |
| const buttonWidth = floatingButtonPosition.width | |
| const buttonHeight = floatingButtonPosition.height | |
| const windowWidth = this.config.size.width | |
| const windowHeight = this.config.size.height | |
| const gap = 15 | |
| const buttonCenterX = buttonX + buttonWidth / 2 | |
| const buttonCenterY = buttonY + buttonHeight / 2 | |
| const screenCenterX = workArea.x + workArea.width / 2 | |
| const screenCenterY = workArea.y + workArea.height / 2 | |
| let positions: Array<{ x: number; y: number; priority: number }> = [] | |
| if (buttonX + buttonWidth + gap + windowWidth <= workArea.x + workArea.width) { | |
| positions.push({ | |
| x: buttonX + buttonWidth + gap, | |
| y: Math.max( | |
| workArea.y, | |
| Math.min( | |
| buttonY + (buttonHeight - windowHeight) / 2, | |
| workArea.y + workArea.height - windowHeight | |
| ) | |
| ), | |
| priority: buttonCenterX < screenCenterX ? 1 : 3 | |
| }) | |
| } | |
| const buttonBounds = floatingButtonPosition | |
| if (buttonX - gap - windowWidth >= workArea.x) { | |
| positions.push({ | |
| x: buttonX - gap - windowWidth, | |
| y: Math.max( | |
| workArea.y, | |
| Math.min( | |
| buttonY + (buttonHeight - windowHeight) / 2, | |
| workArea.y + workArea.height - windowHeight | |
| ) | |
| ), | |
| priority: buttonCenterX >= screenCenterX ? 1 : 3 | |
| }) | |
| } | |
| let finalX: number | |
| if (buttonY + buttonHeight + gap + windowHeight <= workArea.y + workArea.height) { | |
| positions.push({ | |
| x: Math.max( | |
| workArea.x, | |
| Math.min( | |
| buttonX + (buttonWidth - windowWidth) / 2, | |
| workArea.x + workArea.width - windowWidth | |
| ) | |
| ), | |
| y: buttonY + buttonHeight + gap, | |
| priority: buttonCenterY < screenCenterY ? 2 : 4 | |
| }) | |
| // 1. Prioritize placing the window on the right side of the button | |
| const rightPositionX = buttonBounds.x + buttonBounds.width + gap | |
| if (rightPositionX + windowWidth <= workArea.x + workArea.width) { | |
| finalX = rightPositionX | |
| } else { | |
| // 2. If right side has no space, try the left side | |
| const leftPositionX = buttonBounds.x - windowWidth - gap | |
| if (leftPositionX >= workArea.x) { | |
| finalX = leftPositionX | |
| } else { | |
| // 3. Fallback: If both sides lack space, align window's right edge with screen's right edge. | |
| finalX = workArea.x + workArea.width - windowWidth | |
| } | |
| } | |
| if (buttonY - gap - windowHeight >= workArea.y) { | |
| positions.push({ | |
| x: Math.max( | |
| workArea.x, | |
| Math.min( | |
| buttonX + (buttonWidth - windowWidth) / 2, | |
| workArea.x + workArea.width - windowWidth | |
| ) | |
| ), | |
| y: buttonY - gap - windowHeight, | |
| priority: buttonCenterY >= screenCenterY ? 2 : 4 | |
| }) | |
| } | |
| // Calculate vertical position: try to center with the button, but stay within screen bounds. | |
| const idealY = buttonBounds.y + (buttonBounds.height - windowHeight) / 2 | |
| const finalY = Math.max( | |
| workArea.y, | |
| Math.min(idealY, workArea.y + workArea.height - windowHeight) | |
| ) | |
| if (positions.length === 0) { | |
| x = workArea.x + workArea.width - windowWidth - 20 | |
| y = workArea.y + workArea.height - windowHeight - 20 | |
| } else { | |
| positions.sort((a, b) => a.priority - b.priority) | |
| x = positions[0].x | |
| y = positions[0].y | |
| } | |
| x = Math.max(workArea.x + 10, Math.min(x, workArea.x + workArea.width - windowWidth - 10)) | |
| y = Math.max(workArea.y + 10, Math.min(y, workArea.y + workArea.height - windowHeight - 10)) | |
| return { x, y } | |
| return { x: Math.round(finalX), y: Math.round(finalY) } | |
| // Choose the display that contains the floating button (or window), falling back to primary | |
| const display = floatingButtonPosition | |
| ? screen.getDisplayMatching({ | |
| x: floatingButtonPosition.x, | |
| y: floatingButtonPosition.y, | |
| width: floatingButtonPosition.width, | |
| height: floatingButtonPosition.height | |
| }) | |
| : (this.window && !this.window.isDestroyed() | |
| ? screen.getDisplayMatching(this.window.getBounds()) | |
| : screen.getPrimaryDisplay()) | |
| const { workArea } = display | |
| const windowWidth = this.window?.getBounds().width ?? this.config.size.width | |
| const windowHeight = this.window?.getBounds().height ?? this.config.size.height | |
| if (!floatingButtonPosition) { | |
| const x = workArea.x + workArea.width - windowWidth - 20 | |
| const y = workArea.y + workArea.height - windowHeight - 20 | |
| return { x, y } | |
| } | |
| const gap = 15 | |
| const buttonBounds = floatingButtonPosition | |
| let finalX: number | |
| // 1. Prioritize placing the window on the right side of the button | |
| const rightPositionX = buttonBounds.x + buttonBounds.width + gap | |
| if (rightPositionX + windowWidth <= workArea.x + workArea.width) { | |
| finalX = rightPositionX | |
| } else { | |
| // 2. If right side has no space, try the left side | |
| const leftPositionX = buttonBounds.x - windowWidth - gap | |
| if (leftPositionX >= workArea.x) { | |
| finalX = leftPositionX | |
| } else { | |
| // 3. Fallback: align the window’s right edge with the display’s right edge | |
| finalX = workArea.x + workArea.width - windowWidth | |
| } | |
| } | |
| // Calculate vertical position: center with the button but clamp to display | |
| const idealY = buttonBounds.y + (buttonBounds.height - windowHeight) / 2 | |
| const finalY = Math.max( | |
| workArea.y, | |
| Math.min(idealY, workArea.y + workArea.height - windowHeight) | |
| ) | |
| return { x: Math.round(finalX), y: Math.round(finalY) } |
🤖 Prompt for AI Agents
In src/main/presenter/windowPresenter/FloatingChatWindow.ts around lines
243-281, replace use of screen.getPrimaryDisplay() with logic that picks the
display containing the button or window using screen.getDisplayMatching, so
calculations use the correct workArea for that display: if
floatingButtonPosition is present, call getDisplayMatching with the button
bounds; else if this.window exists, call getDisplayMatching with
this.window.getBounds(); otherwise fallback to getPrimaryDisplay(); then derive
workArea from that selected display and continue the existing placement logic so
x/y are computed relative to the correct display.
解决悬浮按钮拖动乱飞问题
Summary by CodeRabbit