-
Notifications
You must be signed in to change notification settings - Fork 614
feat: implement floating chat window system with performance optimization #724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement floating chat window system with performance optimization #724
Conversation
WalkthroughAdds a floating chat window feature with always-on-top BrowserWindow, positioning relative to the floating button, and lifecycle management. Introduces right-click handling on the floating button to show a context menu. Extends presenters and shared interfaces for floating window registration, visibility control, IPC broadcasting, and app-quit awareness. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Renderer as FloatingButton.vue
participant Preload as floating-preload
participant Main as FloatingButtonPresenter
participant Menu as Electron.Menu
participant WP as WindowPresenter
User->>Renderer: Right-click button
Renderer->>Preload: floatingButtonAPI.onRightClick()
Preload->>Main: IPC RIGHT_CLICKED
Main->>Menu: build context menu
alt Button window exists
Main->>Menu: popup(buttonWindow)
else Main window available
Main->>Menu: popup(mainWindow)
else
Main->>Menu: popup()
end
Menu->>WP: Open main / Exit app (menu selection)
sequenceDiagram
participant User
participant Renderer as FloatingButton.vue
participant Preload as floating-preload
participant Main as FloatingButtonPresenter
participant WP as WindowPresenter
participant FCW as FloatingChatWindow
User->>Renderer: Left-click button
Renderer->>Preload: floatingButtonAPI.onClick()
Preload->>Main: IPC CLICKED
Main->>Main: get button bounds
Main->>WP: toggleFloatingChatWindow(bounds?)
WP->>FCW: create if needed
WP->>FCW: toggle(show/hide) with positioning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/renderer/floating/FloatingButton.vue (1)
30-72: Extract common animation logic to reduce duplicationBoth
handleClickandhandleRightClickcontain identical animation code. Consider extracting this into a reusable function:+// Animation helper +const triggerButtonAnimation = () => { + if (floatingButton.value) { + floatingButton.value.style.transform = 'scale(0.9)' + setTimeout(() => { + if (floatingButton.value) { + floatingButton.value.style.transform = '' + } + }, 150) + } +} // 点击处理 - 专注于唤起主窗口 const handleClick = () => { - // 点击反馈动画 - if (floatingButton.value) { - floatingButton.value.style.transform = 'scale(0.9)' - setTimeout(() => { - if (floatingButton.value) { - floatingButton.value.style.transform = '' - } - }, 150) - } + triggerButtonAnimation() if (window.floatingButtonAPI) { try { window.floatingButtonAPI.onClick() } catch (error) { console.error('=== FloatingButton: Error calling onClick API ===:', error) } } else { console.error('=== FloatingButton: floatingButtonAPI not available ===') } } const handleRightClick = (event: MouseEvent) => { event.preventDefault() - if (floatingButton.value) { - floatingButton.value.style.transform = 'scale(0.9)' - setTimeout(() => { - if (floatingButton.value) { - floatingButton.value.style.transform = '' - } - }, 150) - } + triggerButtonAnimation() if (window.floatingButtonAPI) { try { window.floatingButtonAPI.onRightClick() } catch (error) { console.error('=== FloatingButton: Error calling onRightClick API ===:', error) } } else { console.error('=== FloatingButton: floatingButtonAPI not available ===') } }
🧹 Nitpick comments (5)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (3)
116-148: Simplify show method logicThe current implementation has redundant
show()andfocus()calls across different branches. Consider consolidating the logic:public show(floatingButtonPosition?: FloatingButtonPosition): void { if (!this.window) { return } if (floatingButtonPosition) { const position = this.calculatePosition(floatingButtonPosition) this.window.setPosition(position.x, position.y) } - if (!this.window.isVisible()) { - if (this.window.webContents.isLoading() === false) { - this.window.show() - this.window.focus() - this.refreshWindowData() - } else { - this.window.show() - this.window.focus() - this.shouldShowWhenReady = true - this.window.webContents.once('did-finish-load', () => { - if (this.shouldShowWhenReady) { - this.refreshWindowData() - this.shouldShowWhenReady = false - } - }) - } - } else { - this.window.show() - this.window.focus() - this.refreshWindowData() - } + this.window.show() + this.window.focus() + + if (this.window.webContents.isLoading()) { + this.shouldShowWhenReady = true + this.window.webContents.once('did-finish-load', () => { + if (this.shouldShowWhenReady) { + this.refreshWindowData() + this.shouldShowWhenReady = false + } + }) + } else { + this.refreshWindowData() + } this.isVisible = true logger.debug('FloatingChatWindow shown') }
258-258: Consider making the gap configurableThe gap value is hardcoded to 15 pixels. Consider making this configurable through the
FloatingChatConfiginterface for better flexibility.const windowHeight = this.config.size.height -const gap = 15 +const gap = this.config.gap ?? 15Also add the
gapproperty to theFloatingChatConfiginterface:interface FloatingChatConfig { // ... existing properties gap?: number }
340-344: Consider making the dev server URL configurableThe development server URL is hardcoded to
http://localhost:5173/. Consider making this configurable through environment variables to support different development setups.if (isDev) { - await this.window.loadURL('http://localhost:5173/') + const devServerUrl = process.env.ELECTRON_RENDERER_URL || 'http://localhost:5173/' + await this.window.loadURL(devServerUrl) } else {src/main/presenter/floatingButtonPresenter/index.ts (1)
155-167: Pre-create floating chat window: consider deferring to idle to reduce startup contentionPre-warming improves first-toggle latency, but can compete with startup work. Optionally schedule after a short idle timeout or on first hover/focus of the floating button.
Example:
- this.preCreateFloatingChatWindow() + setTimeout(() => this.preCreateFloatingChatWindow(), 800)src/main/presenter/windowPresenter/index.ts (1)
177-198: Preview target selection enhanced (includes floating window)Good fallback order; macOS-only previewFile usage is correct. Consider logging/handling the string return value from shell.openPath for error messages on non-macOS if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/events.ts(1 hunks)src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts(1 hunks)src/main/presenter/floatingButtonPresenter/index.ts(4 hunks)src/main/presenter/tabPresenter.ts(1 hunks)src/main/presenter/windowPresenter/FloatingChatWindow.ts(1 hunks)src/main/presenter/windowPresenter/index.ts(6 hunks)src/preload/floating-preload.ts(2 hunks)src/renderer/floating/FloatingButton.vue(2 hunks)src/renderer/floating/env.d.ts(1 hunks)src/shared/presenter.d.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/floating/FloatingButton.vuesrc/main/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/shared/presenter.d.tssrc/renderer/floating/env.d.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.tssrc/preload/floating-preload.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/floating/FloatingButton.vuesrc/renderer/floating/env.d.ts
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.vuesrc/renderer/floating/env.d.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/floating/FloatingButton.vuesrc/renderer/floating/env.d.ts
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/floating/FloatingButton.vuesrc/renderer/floating/env.d.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Strict type checking enabled for TypeScript
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/shared/presenter.d.tssrc/renderer/floating/env.d.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.tssrc/preload/floating-preload.ts
src/main/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Main to Renderer: Use EventBus to broadcast events via mainWindow.webContents.send()
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/shared/presenter.d.tssrc/renderer/floating/env.d.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.tssrc/preload/floating-preload.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/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/renderer/floating/env.d.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/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/events.tssrc/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/main/presenter/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
One presenter per functional domain
Files:
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.tssrc/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.tssrc/main/presenter/windowPresenter/FloatingChatWindow.ts
src/shared/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Shared types in src/shared/
Files:
src/shared/presenter.d.ts
src/shared/*.d.ts
📄 CodeRabbit Inference Engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/presenter.d.ts
src/preload/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Context isolation enabled with preload scripts for security
Files:
src/preload/floating-preload.ts
🧠 Learnings (3)
📚 Learning: 2025-07-21T01:45:54.229Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:45:54.229Z
Learning: Applies to src/renderer/src/composables/usePresenter.ts : The IPC in the renderer process is implemented in usePresenter.ts, allowing direct calls to the presenter-related interfaces exposed by the main process
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/floatingButtonPresenter/index.ts
📚 Learning: 2025-07-21T01:45:54.229Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:45:54.229Z
Learning: Applies to src/main/presenter/index.ts : The IPC messages from the main process to notify the view mainly rely on the EventBus index.ts to listen for events that need to be notified and then send them to the renderer through the mainWindow
Applied to files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.ts
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/main/presenter/configPresenter/**/*.ts : Centralize configuration in configPresenter/
Applied to files:
src/main/presenter/windowPresenter/index.tssrc/main/presenter/floatingButtonPresenter/index.ts
🧬 Code Graph Analysis (3)
src/main/presenter/windowPresenter/index.ts (1)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (1)
FloatingChatWindow(44-398)
src/main/presenter/windowPresenter/FloatingChatWindow.ts (3)
src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
TAB_EVENTS(156-164)src/main/presenter/index.ts (1)
presenter(184-184)
src/preload/floating-preload.ts (1)
src/main/events.ts (1)
FLOATING_BUTTON_EVENTS(178-184)
⏰ 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 (16)
src/main/events.ts (1)
180-180: LGTM!The new
RIGHT_CLICKEDevent follows the established naming convention and integrates well with the existing event structure.src/renderer/floating/env.d.ts (1)
14-14: LGTM!The
onRightClickcallback follows the same pattern as the existingonClickhandler and properly extends the floating button API.src/shared/presenter.d.ts (2)
181-181: LGTM!The
isApplicationQuitting()method provides essential state information for proper window lifecycle management during app shutdown.
218-219: LGTM!The floating window registration methods properly integrate the floating window with the tab management system using appropriate Electron types.
src/main/presenter/windowPresenter/FloatingChatWindow.ts (2)
1-10: LGTM!Imports are well-organized and all necessary dependencies are included for the floating window implementation.
357-397: LGTM!The window event handling is well-implemented, properly managing the window lifecycle and preventing accidental closure when the app is not quitting. This aligns perfectly with the PR objective of maintaining an always-on-top floating window.
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (1)
173-175: Accessor looks good; minimal, non-breaking additionNo concerns with exposing a nullable getter for the floating button BrowserWindow.
src/preload/floating-preload.ts (2)
20-26: Right-click IPC path is correct and safeThe try/catch and contextIsolation exposure are appropriate. No issues.
5-7: Event name consistency confirmedThe
RIGHT_CLICKEDevent name'floating-button:right-clicked'is defined identically in bothsrc/preload/floating-preload.tsandsrc/main/events.ts, and is used consistently throughout the IPC handlers in the presenters. No mismatches found—no changes required.src/main/presenter/floatingButtonPresenter/index.ts (2)
51-52: Cleanup completenessRemoving both CLICKED and RIGHT_CLICKED handlers on destroy prevents duplicate bindings. Good.
111-112: Idempotent rebind of IPC handlersRemoving and re-registering both handlers before creating the window is a safe pattern to prevent multiple listeners. Good.
src/main/presenter/windowPresenter/index.ts (5)
16-16: Importing FloatingChatWindow is correct and scopedNo issues.
42-42: FloatingChatWindow state holder addedAddition is straightforward and contained.
59-63: Clean shutdown of floating chat on before-quitExplicitly destroying the floating chat window on app quit is good to prevent leaks.
504-513: Broadcast includes floating chat windowIncluding the floating chat window webContents in broadcasts is correct and guarded against destruction.
1178-1180: Expose application quitting flagUseful for cooperating presenters (e.g., FloatingChatWindow close handler). Looks good.
| ipcMain.on(FLOATING_BUTTON_EVENTS.CLICKED, async () => { | ||
| try { | ||
| // 触发内置事件处理器 | ||
| handleShowHiddenWindow(true) | ||
| } catch {} | ||
| let floatingButtonPosition: { x: number; y: number; width: number; height: number } | null = | ||
| null | ||
| if (this.floatingWindow && this.floatingWindow.exists()) { | ||
| const buttonWindow = this.floatingWindow.getWindow() | ||
| if (buttonWindow && !buttonWindow.isDestroyed()) { | ||
| const bounds = buttonWindow.getBounds() | ||
| floatingButtonPosition = { | ||
| x: bounds.x, | ||
| y: bounds.y, | ||
| width: bounds.width, | ||
| height: bounds.height | ||
| } | ||
| } | ||
| } | ||
| if (floatingButtonPosition) { | ||
| await presenter.windowPresenter.toggleFloatingChatWindow(floatingButtonPosition) | ||
| } else { | ||
| await presenter.windowPresenter.toggleFloatingChatWindow() | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to handle floating button click:', error) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Pass-through of button bounds is good; consider multi-display correctness
You pass the floating button’s bounds to help position the chat window near the button. However, FloatingChatWindow.calculatePosition() uses screen.getPrimaryDisplay(), which can misplace the window when the button is on a non-primary monitor. Prefer deriving the display nearest the button center and using that display’s workArea.
Additional change needed in FloatingChatWindow (for reference):
- const primaryDisplay = screen.getPrimaryDisplay()
- const { workArea } = primaryDisplay
+ const buttonCenter = floatingButtonPosition
+ ? { x: floatingButtonPosition.x + floatingButtonPosition.width / 2,
+ y: floatingButtonPosition.y + floatingButtonPosition.height / 2 }
+ : undefined
+ const display = buttonCenter
+ ? screen.getDisplayNearestPoint(buttonCenter)
+ : screen.getPrimaryDisplay()
+ const { workArea } = displayCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/index.ts around lines 113 to 137,
the presenter passes button bounds but the downstream
FloatingChatWindow.calculatePosition() still uses screen.getPrimaryDisplay(),
which will misplace the chat on multi-monitor setups; update the code so the
display nearest the button center is used (use screen.getDisplayNearestPoint or
equivalent with the button center x/y) and pass that display’s workArea (or pass
the display object) into calculatePosition(), and modify
FloatingChatWindow.calculatePosition() to prefer the provided display/workArea
over screen.getPrimaryDisplay().
| private showContextMenu(): void { | ||
| const template = [ | ||
| { | ||
| label: '打开主窗口', | ||
| click: () => { | ||
| this.openMainWindow() | ||
| } | ||
| }, | ||
| { | ||
| type: 'separator' as const | ||
| }, | ||
| { | ||
| label: '退出应用', | ||
| click: () => { | ||
| this.exitApplication() | ||
| } | ||
| } | ||
| ] | ||
|
|
||
| const contextMenu = Menu.buildFromTemplate(template) | ||
|
|
||
| if (this.floatingWindow && this.floatingWindow.exists()) { | ||
| const buttonWindow = this.floatingWindow.getWindow() | ||
| if (buttonWindow && !buttonWindow.isDestroyed()) { | ||
| contextMenu.popup({ window: buttonWindow }) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| const mainWindow = presenter.windowPresenter.mainWindow | ||
| if (mainWindow) { | ||
| contextMenu.popup({ window: mainWindow }) | ||
| } else { | ||
| contextMenu.popup() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Localize context menu labels and reuse existing i18n infra
Hardcoded Chinese labels break localization. Reuse your i18n helpers (like getContextMenuLabels) or add keys for “Open main window” and “Exit application” to keep UX consistent.
Example:
- const template = [
- { label: '打开主窗口', click: () => this.openMainWindow() },
- { type: 'separator' as const },
- { label: '退出应用', click: () => this.exitApplication() }
- ]
+ const lang = presenter?.configPresenter?.getLanguage?.() ?? 'en'
+ const labels = { openMain: /* t('menu.openMain') by lang */, exitApp: /* t('menu.exitApp') */ }
+ const template = [
+ { label: labels.openMain, click: () => this.openMainWindow() },
+ { type: 'separator' as const },
+ { label: labels.exitApp, click: () => this.exitApplication() }
+ ]I can wire this to your existing i18n utilities on request.
Committable suggestion skipped: line range outside the PR's diff.
| private exitApplication(): void { | ||
| try { | ||
| console.log('Exiting application from floating button context menu') | ||
| app.quit() | ||
| } catch (error) { | ||
| console.error('Failed to exit application from floating button:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Route app exit through WindowPresenter to set isQuitting before window closes
Direct app.quit() can race with close handlers that rely on isQuitting. Emit the FORCE_QUIT_APP event instead, which your WindowPresenter already handles to set isQuitting then quit.
- private exitApplication(): void {
- try {
- console.log('Exiting application from floating button context menu')
- app.quit()
- } catch (error) {
- console.error('Failed to exit application from floating button:', error)
- }
- }
+ private exitApplication(): void {
+ try {
+ console.log('Exiting application from floating button context menu')
+ // Ensure centralized quit path sets isQuitting before windows receive 'close'
+ const { eventBus } = require('@/eventbus')
+ const { WINDOW_EVENTS } = require('@/events')
+ eventBus.sendToMain(WINDOW_EVENTS.FORCE_QUIT_APP)
+ } catch (error) {
+ console.error('Failed to exit application from floating button:', error)
+ }
+ }Note: add missing imports if your bundler supports them at top-level instead of require().
📝 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.
| private exitApplication(): void { | |
| try { | |
| console.log('Exiting application from floating button context menu') | |
| app.quit() | |
| } catch (error) { | |
| console.error('Failed to exit application from floating button:', error) | |
| } | |
| } | |
| private exitApplication(): void { | |
| try { | |
| console.log('Exiting application from floating button context menu') | |
| // Ensure centralized quit path sets isQuitting before windows receive 'close' | |
| const { eventBus } = require('@/eventbus') | |
| const { WINDOW_EVENTS } = require('@/events') | |
| eventBus.sendToMain(WINDOW_EVENTS.FORCE_QUIT_APP) | |
| } catch (error) { | |
| console.error('Failed to exit application from floating button:', error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/index.ts around lines 228 to 235,
replace the direct app.quit() call with emitting the existing FORCE_QUIT_APP
event that WindowPresenter listens for so isQuitting is set before windows
close; keep the try/catch and logs but remove app.quit(), import the
FORCE_QUIT_APP constant (and the app/event emitter or WindowPresenter helper
used to emit it) at top-level, and call the emitter/WindowPresenter to emit
FORCE_QUIT_APP instead of invoking app.quit() directly.
| registerFloatingWindow(webContentsId: number, webContents: Electron.WebContents): void { | ||
| try { | ||
| console.log(`TabPresenter: Registering floating window as virtual tab, ID: ${webContentsId}`) | ||
| if (this.tabs.has(webContentsId)) { | ||
| console.warn(`TabPresenter: Tab ${webContentsId} already exists, skipping registration`) | ||
| return | ||
| } | ||
| const virtualView = { | ||
| webContents: webContents, | ||
| setVisible: () => {}, | ||
| setBounds: () => {}, | ||
| getBounds: () => ({ x: 0, y: 0, width: 400, height: 600 }) | ||
| } as any | ||
| this.webContentsToTabId.set(webContentsId, webContentsId) | ||
| this.tabs.set(webContentsId, virtualView) | ||
| console.log( | ||
| `TabPresenter: Virtual tab registered successfully for floating window ${webContentsId}` | ||
| ) | ||
| } catch (error) { | ||
| console.error('TabPresenter: Failed to register floating window:', error) | ||
| } | ||
| } | ||
|
|
||
| unregisterFloatingWindow(webContentsId: number): void { | ||
| try { | ||
| console.log(`TabPresenter: Unregistering floating window virtual tab, ID: ${webContentsId}`) | ||
| this.webContentsToTabId.delete(webContentsId) | ||
| this.tabs.delete(webContentsId) | ||
| console.log( | ||
| `TabPresenter: Virtual tab unregistered successfully for floating window ${webContentsId}` | ||
| ) | ||
| } catch (error) { | ||
| console.error('TabPresenter: Failed to unregister floating window:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid storing non-WebContentsView “virtual tabs” in Map<number, WebContentsView>
Using a plain object cast as any risks runtime/typing issues if later code assumes a real WebContentsView (e.g., addChildView, setBorderRadius). Recommend isolating virtual entries in a separate map or introducing a safe abstraction instead of widening via any.
Proposed direction:
- Keep tabs: Map<number, WebContentsView> strictly for real views.
- Add virtualTabs: Map<number, { webContents: Electron.WebContents }> for floating windows.
- Update getTabIdByWebContentsId/getWindowIdByWebContentsId to consult both maps, or keep only webContentsToTabId as source of truth.
This avoids accidental operations (attach/detach/bring-to-front/bounds updates) on non-view objects.
🤖 Prompt for AI Agents
In src/main/presenter/tabPresenter.ts around lines 946 to 980, the current
registerFloatingWindow/unregisterFloatingWindow functions insert a plain object
cast to any into tabs: Map<number, WebContentsView>, which can break code that
assumes real WebContentsView methods; instead create a new private virtualTabs:
Map<number, { webContents: Electron.WebContents }>, stop inserting virtual
entries into tabs, set webContentsToTabId as before to preserve lookup, and move
the virtual object into virtualTabs in registerFloatingWindow and remove it from
virtualTabs in unregisterFloatingWindow; also update any tab-lookup helpers
(getTabIdByWebContentsId/getWindowIdByWebContentsId) to consult
webContentsToTabId and then check tabs first and virtualTabs second (or rely
solely on webContentsToTabId) and ensure callers that operate on WebContentsView
guard against virtual entries so real view-only operations aren’t invoked on
virtualTabs.
| webPreferences: { | ||
| nodeIntegration: false, | ||
| contextIsolation: true, | ||
| preload: path.join(__dirname, '../preload/index.mjs'), | ||
| webSecurity: false, | ||
| devTools: isDev, | ||
| sandbox: false | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Consider enabling web security and sandbox
The current configuration disables important security features:
webSecurity: falseallows cross-origin requests without restrictionssandbox: falsedisables process isolation
These settings expose the application to potential security vulnerabilities. Consider enabling these security features unless they are absolutely necessary for the floating window functionality.
If these settings are required, document the specific reasons and ensure proper input validation and content sanitization are implemented.
🤖 Prompt for AI Agents
In src/main/presenter/windowPresenter/FloatingChatWindow.ts around lines 89 to
96, the BrowserWindow options disable key security features (webSecurity: false,
sandbox: false); change them to webSecurity: true and sandbox: true unless there
is a documented, justified need to disable them; if disabling is absolutely
required, add an inline comment referencing the specific justification,
implement strict input validation and content sanitization in the preload and
renderer, enforce a Content Security Policy and origin checks before loading
remote content, restrict exposed APIs in the preload script, and include a short
note in the PR describing why these settings are necessary and how risks are
mitigated.
| public async createFloatingChatWindow(): Promise<void> { | ||
| if (this.floatingChatWindow) { | ||
| console.log('FloatingChatWindow already exists') | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| this.floatingChatWindow = new FloatingChatWindow() | ||
| await this.floatingChatWindow.create() | ||
| console.log('FloatingChatWindow created successfully') | ||
| } catch (error) { | ||
| console.error('Failed to create FloatingChatWindow:', error) | ||
| this.floatingChatWindow = null | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Guard against concurrent createFloatingChatWindow calls
pre-create plus first-click can race. Add a creation promise sentinel to ensure only one create runs and subsequent callers await it.
+ private floatingChatInitPromise: Promise<void> | null = null
public async createFloatingChatWindow(): Promise<void> {
- if (this.floatingChatWindow) {
+ if (this.floatingChatWindow) {
console.log('FloatingChatWindow already exists')
return
}
- try {
- this.floatingChatWindow = new FloatingChatWindow()
- await this.floatingChatWindow.create()
- console.log('FloatingChatWindow created successfully')
- } catch (error) {
- console.error('Failed to create FloatingChatWindow:', error)
- this.floatingChatWindow = null
- throw error
- }
+ if (!this.floatingChatInitPromise) {
+ this.floatingChatInitPromise = (async () => {
+ try {
+ this.floatingChatWindow = new FloatingChatWindow()
+ await this.floatingChatWindow.create()
+ console.log('FloatingChatWindow created successfully')
+ } catch (error) {
+ console.error('Failed to create FloatingChatWindow:', error)
+ this.floatingChatWindow = null
+ throw error
+ } finally {
+ this.floatingChatInitPromise = null
+ }
+ })()
+ }
+ await this.floatingChatInitPromise
}📝 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.
| public async createFloatingChatWindow(): Promise<void> { | |
| if (this.floatingChatWindow) { | |
| console.log('FloatingChatWindow already exists') | |
| return | |
| } | |
| try { | |
| this.floatingChatWindow = new FloatingChatWindow() | |
| await this.floatingChatWindow.create() | |
| console.log('FloatingChatWindow created successfully') | |
| } catch (error) { | |
| console.error('Failed to create FloatingChatWindow:', error) | |
| this.floatingChatWindow = null | |
| throw error | |
| } | |
| } | |
| // sentinel to prevent concurrent creations | |
| private floatingChatInitPromise: Promise<void> | null = null | |
| public async createFloatingChatWindow(): Promise<void> { | |
| if (this.floatingChatWindow) { | |
| console.log('FloatingChatWindow already exists') | |
| return | |
| } | |
| if (!this.floatingChatInitPromise) { | |
| this.floatingChatInitPromise = (async () => { | |
| try { | |
| this.floatingChatWindow = new FloatingChatWindow() | |
| await this.floatingChatWindow.create() | |
| console.log('FloatingChatWindow created successfully') | |
| } catch (error) { | |
| console.error('Failed to create FloatingChatWindow:', error) | |
| this.floatingChatWindow = null | |
| throw error | |
| } finally { | |
| this.floatingChatInitPromise = null | |
| } | |
| })() | |
| } | |
| await this.floatingChatInitPromise | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/windowPresenter/index.ts around lines 1106 to 1121,
concurrent calls to createFloatingChatWindow can race (pre-create + first-click)
so add a creation promise sentinel (e.g., this.creatingFloatingChatWindow:
Promise<void> | null) that is set before starting creation and returned/awaited
by subsequent callers; implement: if sentinel exists return await sentinel,
otherwise set sentinel = (async () => { try { this.floatingChatWindow = new
FloatingChatWindow(); await this.floatingChatWindow.create(); } catch (err) {
this.floatingChatWindow = null; throw err } finally {
this.creatingFloatingChatWindow = null } })(); await sentinel; this ensures only
one create runs, handles success/error cleanup, and clears the sentinel when
done.
* wip: add modelscope provider * feat: add mcp sync to modelscope * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * chore: i18n and format * feat: better style * fix: mcp tool display --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com>
* fix: add AlertDialogDescription to resolve accessibility warning (#706) * fix: resolve focus flicker when creating new windows with Ctrl+Shift+N (#707) * feat: enhance window management by implementing main window ID handling (#709) * docs: update zhipu developer doc website link (#715) Co-authored-by: gongchao <chao.gong@aminer.cn> * refactor: better translate (#716) * chore: en-us i18n * chore(i18n): polish ja-JP translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fr-FR translations; keep chat.input.placeholder unchanged * chore(i18n): refine fr-FR MCP & Settings copy; idiomatic, concise, brand-consistent * chore(i18n): polish ru-RU translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fa-IR translations across UI; keep chat.input.placeholder unchanged * chore: fix format * chore: fix i18n * chore: lock rolldown-vite version * feat: add GPT-5 series model support (#717) * ci(vite): Bundle the main file into a single file to speed up loading. (#718) * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps (#721) * chore: bump deps * fix: rolldown-vite 7.1.0 and duckdb bundle issue * chore: back to vite * chore: update electron * chore: update versions * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps --------- Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * feat: add mcp sync and modelscope provider #615 (#723) * wip: add modelscope provider * feat: add mcp sync to modelscope * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * chore: i18n and format * feat: better style * fix: mcp tool display --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com> * fix: move_files newPath parse issue (#725) * fix: move_files newPath 参数计算规则 * fix: move_files 移动前需要判断dest是目录还是文件 * feat: add Claude Opus 4.1 to anthropic default model list (#726) * feat: Add mcprouter's MCP marketplace api support (#727) * wip: add mcp market * feat: mcp market install * wip: mcp install status sync * feat: mcp server config mask * chore: remove working doc * chore: add translate * feat: add ESC key to close floating chat window (#728) * feat: add floating button position persistence with boundary validation (#729) * feat: add floating button position persistence with boundary validation * feat: refactor floating button to use electron-window-state * chore: bump to 0.3.0 * feat: add reasoning_effort parameter support for gpt-oss models (#731) * feat: add reasoning_effort parameter support for gpt-oss models - add reasoning effort UI support across all components * fix: preserve user reasoning effort settings and improve display logic * fix: artifacts code not streaming (#732) * fix: artifact react load failed * chore: remove log * fix: artifacts code not stream * fix: format --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com> Co-authored-by: hllshiro <40970081+hllshiro@users.noreply.github.com> Co-authored-by: tomsun28 <tomsun28@outlook.com> Co-authored-by: gongchao <chao.gong@aminer.cn> Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> Co-authored-by: wanna <wanna.w@binarywalk.com>
* fix: add AlertDialogDescription to resolve accessibility warning (#706) * fix: resolve focus flicker when creating new windows with Ctrl+Shift+N (#707) * feat: enhance window management by implementing main window ID handling (#709) * docs: update zhipu developer doc website link (#715) Co-authored-by: gongchao <chao.gong@aminer.cn> * refactor: better translate (#716) * chore: en-us i18n * chore(i18n): polish ja-JP translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fr-FR translations; keep chat.input.placeholder unchanged * chore(i18n): refine fr-FR MCP & Settings copy; idiomatic, concise, brand-consistent * chore(i18n): polish ru-RU translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fa-IR translations across UI; keep chat.input.placeholder unchanged * chore: fix format * chore: fix i18n * chore: lock rolldown-vite version * feat: add GPT-5 series model support (#717) * ci(vite): Bundle the main file into a single file to speed up loading. (#718) * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps (#721) * chore: bump deps * fix: rolldown-vite 7.1.0 and duckdb bundle issue * chore: back to vite * chore: update electron * chore: update versions * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps --------- Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * feat: add mcp sync and modelscope provider #615 (#723) * wip: add modelscope provider * feat: add mcp sync to modelscope * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * chore: i18n and format * feat: better style * fix: mcp tool display --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com> * fix: move_files newPath parse issue (#725) * fix: move_files newPath 参数计算规则 * fix: move_files 移动前需要判断dest是目录还是文件 * feat: add Claude Opus 4.1 to anthropic default model list (#726) * feat: Add mcprouter's MCP marketplace api support (#727) * wip: add mcp market * feat: mcp market install * wip: mcp install status sync * feat: mcp server config mask * chore: remove working doc * chore: add translate * feat: add ESC key to close floating chat window (#728) * feat: add floating button position persistence with boundary validation (#729) * feat: add floating button position persistence with boundary validation * feat: refactor floating button to use electron-window-state * chore: bump to 0.3.0 * feat: add reasoning_effort parameter support for gpt-oss models (#731) * feat: add reasoning_effort parameter support for gpt-oss models - add reasoning effort UI support across all components * fix: preserve user reasoning effort settings and improve display logic * fix: artifacts code not streaming (#732) * fix: artifact react load failed * chore: remove log * fix: artifacts code not stream * fix: format * feat: disable automatic model enabling for better UX (#734) * feat: sync provider sorting from settings to model selection (#736) * feat: sync provider sorting from settings to model selection * feat: refactor ModelSelect to use computed providers for better reactivity --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com> Co-authored-by: hllshiro <40970081+hllshiro@users.noreply.github.com> Co-authored-by: tomsun28 <tomsun28@outlook.com> Co-authored-by: gongchao <chao.gong@aminer.cn> Co-authored-by: Simon He <57086651+Simon-He95@users.noreply.github.com> Co-authored-by: wanna <wanna.w@binarywalk.com>
close #719
Summary by CodeRabbit