feat: Floating window #553
Conversation
WalkthroughThis update introduces a floating button feature to an Electron application. It adds new main process presenters and window management for the floating button, integrates IPC communication, and provides a Vue-based renderer for the button UI. The Electron build configuration is updated for multiple entry points, and new TypeScript types and preload scripts are included for secure API exposure. Changes
Sequence Diagram(s)sequenceDiagram
participant ElectronMain as Electron Main Process
participant FloatingButtonPresenter
participant FloatingButtonWindow
participant PreloadScript
participant Renderer as Floating Button Renderer (Vue)
ElectronMain->>FloatingButtonPresenter: initialize()
FloatingButtonPresenter->>FloatingButtonWindow: create()
FloatingButtonWindow-->>FloatingButtonPresenter: window ready
FloatingButtonPresenter-->>ElectronMain: floating button ready
Renderer->>PreloadScript: window.floatingButtonAPI.onClick()
PreloadScript->>ElectronMain: IPC: FLOATING_BUTTON_EVENTS.CLICKED
ElectronMain->>FloatingButtonPresenter: handle click event
FloatingButtonPresenter->>ElectronMain: toggle main window visibility
ElectronMain->>FloatingButtonPresenter: onMainWindowVisibilityChanged()
FloatingButtonPresenter->>FloatingButtonWindow: show/hide as needed
FloatingButtonPresenter->>FloatingButtonWindow: updateConfig(newConfig)
FloatingButtonWindow-->>Renderer: send config update (via IPC)
Renderer->>PreloadScript: onConfigUpdate(callback)
PreloadScript-->>Renderer: callback(config)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 8
🧹 Nitpick comments (5)
src/preload/floating-preload.ts (1)
16-20: Consider using event constants for consistency.The hardcoded event name
'floating-button-config-update'should use a constant from the events module for consistency and maintainability.+import { FLOATING_BUTTON_EVENTS } from '@/events'; - ipcRenderer.on('floating-button-config-update', (_event, config) => { + ipcRenderer.on(FLOATING_BUTTON_EVENTS.CONFIG_UPDATE, (_event, config) => {- ipcRenderer.removeAllListeners('floating-button-config-update'); + ipcRenderer.removeAllListeners(FLOATING_BUTTON_EVENTS.CONFIG_UPDATE);src/renderer/floating/FloatingButton.vue (1)
28-49: Consider using Vue's reactivity instead of manual DOM manipulation.The click animation uses direct DOM manipulation which goes against Vue's reactive paradigm. Consider using a reactive state for better maintainability.
+const isClicked = ref(false) const handleClick = () => { - // 点击反馈动画 - if (floatingButton.value) { - floatingButton.value.style.transform = 'scale(0.9)' - setTimeout(() => { - if (floatingButton.value) { - floatingButton.value.style.transform = '' - } - }, 150) - } + // 点击反馈动画 + isClicked.value = true + setTimeout(() => { + isClicked.value = false + }, 150)And update the template:
- :class="{ 'floating-button-pulse': isPulsing }" + :class="{ + 'floating-button-pulse': isPulsing, + 'floating-button-clicked': isClicked + }"src/main/presenter/floatingButtonPresenter/index.ts (1)
110-112: Add parentheses for clarity in boolean expression.The current expression might be confusing due to operator precedence. Consider adding parentheses to make the intent clearer.
public isAvailable(): boolean { - return this.config.enabled && this.floatingWindow?.exists() || false; + return (this.config.enabled && this.floatingWindow?.exists()) || false; }Note: The
|| falseis redundant since the expression already returns a boolean.public isAvailable(): boolean { - return this.config.enabled && this.floatingWindow?.exists() || false; + return this.config.enabled && (this.floatingWindow?.exists() ?? false); }src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (2)
181-199: Remove redundant 'bottom-right' case.The 'bottom-right' case has identical logic to the default case, making it redundant.
case 'bottom-left': x = this.config.offset.x; y = workAreaSize.height - this.config.size.height - this.config.offset.y; break; - case 'bottom-right': default: x = workAreaSize.width - this.config.size.width - this.config.offset.x; y = workAreaSize.height - this.config.size.height - this.config.offset.y; break;
71-75: Consider making DevTools optional in development mode.Opening DevTools automatically on every launch in development mode can be disruptive, especially since the comment indicates it may interfere with dragging functionality.
if (isDev) { await this.window.loadURL('http://localhost:5173/floating/'); - // 开发模式下可选择性打开开发者工具（暂时禁用，避免影响拖拽） - this.window.webContents.openDevTools({ mode: 'detach' }); + // 开发模式下可通过环境变量控制是否打开开发者工具 + if (process.env.FLOATING_BUTTON_DEVTOOLS === 'true') { + this.window.webContents.openDevTools({ mode: 'detach' }); + } } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.vscode/settings.json(1 hunks)electron.vite.config.ts(2 hunks)src/main/events.ts(1 hunks)src/main/index.ts(8 hunks)src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts(1 hunks)src/main/presenter/floatingButtonPresenter/index.ts(1 hunks)src/main/presenter/floatingButtonPresenter/types.ts(1 hunks)src/main/presenter/index.ts(3 hunks)src/preload/floating-preload.ts(1 hunks)src/preload/index.d.ts(1 hunks)src/renderer/floating/FloatingButton.vue(1 hunks)src/renderer/floating/env.d.ts(1 hunks)src/renderer/floating/index.html(1 hunks)src/renderer/floating/main.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/renderer/floating/env.d.ts
[error] 6-6: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 6-6: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
src/main/presenter/floatingButtonPresenter/types.ts
[error] 40-40: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
[error] 208-210: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: build-check (x64)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
[failure] 3-3:
'fs' is declared but its value is never read.
🪛 GitHub Actions: PR Check
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
[error] 3-3: TypeScript error TS6133: 'fs' is declared but its value is never read.
🔇 Additional comments (23)
.vscode/settings.json (1)
6-7: Disable Vue-Vine telemetry in workspace settingsGood addition to turn off data tracking for the Vue-Vine extension. The comma after the previous entry is valid, and the JSON remains well-formed.
src/main/events.ts (1)
167-173: LGTM! Clean integration of floating button events.The new
FLOATING_BUTTON_EVENTSconstant follows the established naming convention and provides appropriate event types for the floating button feature. The events are well-defined and logically structured.src/renderer/floating/main.ts (1)
2-6: LGTM! Standard Vue application bootstrap.The Vue application setup is correctly implemented, creating an app instance with the FloatingButton component and mounting it to the DOM.
src/main/presenter/index.ts (3)
22-22: LGTM! Proper import of FloatingButtonPresenter.The import follows the established pattern and integrates cleanly with the existing presenter imports.
56-56: LGTM! Consistent property declaration.The property declaration follows the same pattern as other presenters in the class.
84-84: LGTM! Proper presenter initialization.The FloatingButtonPresenter is correctly initialized in the constructor following the established pattern. The initialization doesn't require dependencies, which is appropriate for this presenter.
src/renderer/floating/index.html (3)
1-12: LGTM! Proper HTML structure and meta configuration.The HTML structure is well-formed with appropriate meta tags and viewport settings for a floating button renderer.
8-21: LGTM! Excellent CSS styling for floating UI.The CSS reset and styling is perfect for a floating button:
- Global reset with box-sizing: border-box
- Transparent background for seamless desktop integration
- Hidden overflow to prevent scrollbars
- Full viewport dimensions
This ensures the floating button will render cleanly without visual artifacts.
24-25: LGTM! Proper Vue app mounting setup.The div element provides the correct mounting point for the Vue application, and the module script correctly references the main.ts entry point.
electron.vite.config.ts (2)
36-44: LGTM! Proper multi-entry configuration for Electron preload.The addition of multiple entry points in the preload build configuration correctly supports the floating button feature architecture.
88-90: LGTM! Renderer entry points properly configured.The floating renderer entry point is correctly added alongside existing entries, enabling proper build and loading of the floating button UI.
src/renderer/floating/env.d.ts (1)
10-19: LGTM! Well-defined API interface for floating button.The global window interface extension properly defines the floating button API with appropriate method signatures for IPC communication.
src/preload/floating-preload.ts (2)
4-27: LGTM! Secure API implementation with proper error handling.The floating button API is well-structured with appropriate error handling for IPC communication. The method implementations correctly use
ipcRendererfor communication with the main process.
29-42: LGTM! Proper security implementation for API exposure.The context isolation detection and fallback approach correctly handles both secure and legacy Electron contexts while maintaining appropriate error handling.
src/renderer/floating/FloatingButton.vue (2)
71-82: LGTM! Proper lifecycle management for API integration.The component correctly sets up and cleans up API listeners in the mounted/unmounted hooks, following Vue best practices.
85-168: LGTM! Excellent CSS implementation with smooth animations.The styling includes well-crafted animations (pulse, shine, hover effects) and a properly positioned tooltip. The gradient background and transitions create a polished user experience.
src/main/presenter/floatingButtonPresenter/types.ts (2)
1-20: LGTM! Comprehensive configuration interface.The
FloatingButtonConfiginterface provides well-defined properties with clear documentation, covering all necessary aspects of floating button configuration.
43-56: LGTM! Sensible default configuration values.The default configuration provides reasonable values for all properties, with the button positioned in the bottom-right corner and appropriate sizing and opacity settings.
src/main/index.ts (4)
109-115: LGTM! Good error handling for non-critical feature.The floating button initialization is properly wrapped in try-catch, preventing failures from affecting the main application flow.
128-158: Well-structured window visibility handling.The
handleShowHiddenWindowfunction properly handles window state transitions with appropriate checks for destroyed windows. ThemustShowparameter elegantly handles the different behaviors needed for tray clicks vs other triggers.
321-337: Correct handling of floating button in app lifecycle.The modified
window-all-closedhandler properly excludes floating button windows from the quit decision, ensuring the app behaves correctly when only the floating button remains.
45-47: ```shell
#!/bin/bashLocate the presenter index file and display its init() definition
INDEX_FILE=$(find src/main/presenter -maxdepth 1 -type f -name "index.ts")
echo "Presenter index file: $INDEX_FILE"
rg -A 5 -n "init\s*()" "$INDEX_FILE"</details> <details> <summary>src/main/presenter/floatingButtonPresenter/index.ts (1)</summary> `63-91`: **Well-implemented configuration update logic.** The `updateConfig` method properly handles state transitions between enabled/disabled states, with appropriate error handling and logging. The immediate show after enabling provides good user feedback. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @@ -9,6 +9,7 @@ declare global { | |||
| getPathForFile(file: File): string | |||
| getWindowId(): number | null | |||
| getWebContentsId(): number | |||
| } | |||
| }, | |||
| floatingButtonAPI: typeof floatingButtonAPI; | |||
There was a problem hiding this comment.
Fix the missing import for floatingButtonAPI type reference.
The type declaration typeof floatingButtonAPI references an undefined variable. This will cause a TypeScript compilation error since floatingButtonAPI is not imported or defined in this file.
You need to either import the floatingButtonAPI or define the type explicitly:
+import { floatingButtonAPI } from '../preload/floating-preload'
+
declare global {
interface Window {
electron: ElectronAPI
api: {
copyText(text: string): void
copyImage(image: string): void
getPathForFile(file: File): string
getWindowId(): number | null
getWebContentsId(): number
},
floatingButtonAPI: typeof floatingButtonAPI;
}
}📝 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.
| floatingButtonAPI: typeof floatingButtonAPI; | |
| import { floatingButtonAPI } from '../preload/floating-preload' | |
| declare global { | |
| interface Window { | |
| electron: ElectronAPI | |
| api: { | |
| copyText(text: string): void | |
| copyImage(image: string): void | |
| getPathForFile(file: File): string | |
| getWindowId(): number | null | |
| getWebContentsId(): number | |
| }, | |
| floatingButtonAPI: typeof floatingButtonAPI; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/preload/index.d.ts at line 13, the type reference `typeof
floatingButtonAPI` is used without importing or defining `floatingButtonAPI`,
causing a TypeScript error. Fix this by adding an import statement for
`floatingButtonAPI` from its module at the top of the file, or alternatively,
define the type explicitly in this file if importing is not suitable.
| @@ -0,0 +1,6 @@ | |||
| import '../src/assets/main.css' | |||
There was a problem hiding this comment.
Fix the incorrect CSS import path.
The CSS import path appears to be incorrect. From src/renderer/floating/main.ts, the path ../src/assets/main.css would resolve to src/src/assets/main.css, which likely doesn't exist.
Apply this fix:
-import '../src/assets/main.css'
+import '../../assets/main.css'📝 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.
| import '../src/assets/main.css' | |
| import '../../assets/main.css' |
🤖 Prompt for AI Agents
In src/renderer/floating/main.ts at line 1, the CSS import path
'../src/assets/main.css' is incorrect because it resolves to
'src/src/assets/main.css'. Update the import path to correctly point to the
assets directory relative to this file, likely by changing it to
'../../assets/main.css' or the correct relative path that leads to
'src/assets/main.css'.
| declare module '*.vue' { | ||
| import type { DefineComponent } from 'vue' | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-empty-object-type | ||
| const component: DefineComponent<{}, {}, any> | ||
| export default component | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix TypeScript type definition for Vue components.
The empty object type {} should be replaced with more specific types to improve type safety.
- const component: DefineComponent<{}, {}, any>
+ const component: DefineComponent<Record<string, never>, Record<string, never>, any>📝 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.
| declare module '*.vue' { | |
| import type { DefineComponent } from 'vue' | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-empty-object-type | |
| const component: DefineComponent<{}, {}, any> | |
| export default component | |
| } | |
| declare module '*.vue' { | |
| import type { DefineComponent } from 'vue' | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-empty-object-type | |
| const component: DefineComponent<Record<string, never>, Record<string, never>, any> | |
| export default component | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 6-6: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🤖 Prompt for AI Agents
In src/renderer/floating/env.d.ts between lines 3 and 8, the TypeScript type
definition for Vue components uses empty object types {} which lack specificity
and reduce type safety. Replace these empty object types with more precise types
such as Record<string, unknown> or appropriate generic parameters to better
represent props and other component options, thereby improving type safety in
the Vue component declarations.
| // 配置更新处理 | ||
| const handleConfigUpdate = (config: any) => { | ||
| // 根据配置更新按钮样式 | ||
| if (config.opacity !== undefined && document.body) { | ||
| document.body.style.opacity = config.opacity.toString() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for configuration parameter.
The config parameter uses any type, reducing type safety. Consider defining a proper interface.
-const handleConfigUpdate = (config: any) => {
+interface FloatingButtonConfig {
+ opacity?: number;
+ // Add other config properties as needed
+}
+
+const handleConfigUpdate = (config: FloatingButtonConfig) => {📝 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 handleConfigUpdate = (config: any) => { | |
| // 根据配置更新按钮样式 | |
| if (config.opacity !== undefined && document.body) { | |
| document.body.style.opacity = config.opacity.toString() | |
| } | |
| } | |
| // 配置更新处理 | |
| interface FloatingButtonConfig { | |
| opacity?: number; | |
| // Add other config properties as needed | |
| } | |
| const handleConfigUpdate = (config: FloatingButtonConfig) => { | |
| // 根据配置更新按钮样式 | |
| if (config.opacity !== undefined && document.body) { | |
| document.body.style.opacity = config.opacity.toString() | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/floating/FloatingButton.vue around lines 63 to 69, the config
parameter is typed as any, which reduces type safety. Define a proper TypeScript
interface for the config object specifying expected properties like opacity,
then update the handleConfigUpdate function to use this interface instead of any
to improve type safety and code clarity.
| export interface FloatingButtonEvents { | ||
| /** 悬浮按钮被点击 */ | ||
| 'floating-button-clicked': void; | ||
| /** 悬浮按钮显示状态改变 */ | ||
| 'floating-button-visibility-changed': { visible: boolean }; | ||
| /** 悬浮按钮位置改变 */ | ||
| 'floating-button-position-changed': { x: number; y: number }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix void type usage in event interface.
Using void outside of return types is confusing and should be replaced with undefined for better clarity.
export interface FloatingButtonEvents {
/** 悬浮按钮被点击 */
- 'floating-button-clicked': void;
+ 'floating-button-clicked': undefined;
/** 悬浮按钮显示状态改变 */
'floating-button-visibility-changed': { visible: boolean };
/** 悬浮按钮位置改变 */
'floating-button-position-changed': { x: number; y: number };
}🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/types.ts between lines 34 and 41,
replace the use of 'void' as the type for the 'floating-button-clicked' event
with 'undefined' to improve clarity and correctness, since 'void' is intended
for function return types and 'undefined' better represents the absence of a
value in this context.
| // 设置悬浮按钮点击事件的 IPC 处理器 | ||
| // 先移除可能存在的旧监听器 | ||
| ipcMain.removeAllListeners(FLOATING_BUTTON_EVENTS.CLICKED) | ||
| ipcMain.on(FLOATING_BUTTON_EVENTS.CLICKED, () => { | ||
| try { | ||
| // 触发内置事件处理器 | ||
| handleShowHiddenWindow(true) | ||
| } catch (error) { | ||
| } | ||
| }) |
There was a problem hiding this comment.
Add error logging in the empty catch block.
The empty catch block at line 125 could hide errors. Consider logging errors to help with debugging.
} catch (error) {
+ console.error('Error handling floating button click:', error)
}📝 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.
| // 设置悬浮按钮点击事件的 IPC 处理器 | |
| // 先移除可能存在的旧监听器 | |
| ipcMain.removeAllListeners(FLOATING_BUTTON_EVENTS.CLICKED) | |
| ipcMain.on(FLOATING_BUTTON_EVENTS.CLICKED, () => { | |
| try { | |
| // 触发内置事件处理器 | |
| handleShowHiddenWindow(true) | |
| } catch (error) { | |
| } | |
| }) | |
| // 设置悬浮按钮点击事件的 IPC 处理器 | |
| // 先移除可能存在的旧监听器 | |
| ipcMain.removeAllListeners(FLOATING_BUTTON_EVENTS.CLICKED) | |
| ipcMain.on(FLOATING_BUTTON_EVENTS.CLICKED, () => { | |
| try { | |
| // 触发内置事件处理器 | |
| handleShowHiddenWindow(true) | |
| } catch (error) { | |
| console.error('Error handling floating button click:', error) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In src/main/index.ts around lines 117 to 126, the catch block inside the
ipcMain.on event handler is empty, which can hide errors and make debugging
difficult. Modify the catch block to log the caught error using a suitable
logging method or console.error to ensure any exceptions thrown by
handleShowHiddenWindow(true) are recorded for troubleshooting.
| // 监听窗口显示/隐藏状态变化 | ||
| const handleWindowVisibilityChange = () => { | ||
| const allWindows = presenter.windowPresenter.getAllWindows() | ||
| const hasVisibleWindow = allWindows.some(win => !win.isDestroyed() && win.isVisible()) | ||
| floatingButtonPresenter.onMainWindowVisibilityChanged(hasVisibleWindow) | ||
| } | ||
|
|
||
| // 监听窗口显示事件 | ||
| app.on('browser-window-created', (_, window) => { | ||
| window.on('show', handleWindowVisibilityChange) | ||
| window.on('hide', handleWindowVisibilityChange) | ||
| window.on('closed', handleWindowVisibilityChange) | ||
| }) |
There was a problem hiding this comment.
Potential duplicate event listeners on windows.
The current implementation attaches event listeners every time a browser window is created. If a window is reused or if the same window triggers multiple browser-window-created events, this could lead to duplicate listeners and multiple calls to handleWindowVisibilityChange.
Consider tracking which windows already have listeners attached:
+ const windowsWithListeners = new WeakSet<BrowserWindow>();
app.on('browser-window-created', (_, window) => {
+ if (windowsWithListeners.has(window)) {
+ return;
+ }
+ windowsWithListeners.add(window);
window.on('show', handleWindowVisibilityChange)
window.on('hide', handleWindowVisibilityChange)
window.on('closed', handleWindowVisibilityChange)
})📝 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 handleWindowVisibilityChange = () => { | |
| const allWindows = presenter.windowPresenter.getAllWindows() | |
| const hasVisibleWindow = allWindows.some(win => !win.isDestroyed() && win.isVisible()) | |
| floatingButtonPresenter.onMainWindowVisibilityChanged(hasVisibleWindow) | |
| } | |
| // 监听窗口显示事件 | |
| app.on('browser-window-created', (_, window) => { | |
| window.on('show', handleWindowVisibilityChange) | |
| window.on('hide', handleWindowVisibilityChange) | |
| window.on('closed', handleWindowVisibilityChange) | |
| }) | |
| // 监听窗口显示/隐藏状态变化 | |
| const handleWindowVisibilityChange = () => { | |
| const allWindows = presenter.windowPresenter.getAllWindows() | |
| const hasVisibleWindow = allWindows.some(win => !win.isDestroyed() && win.isVisible()) | |
| floatingButtonPresenter.onMainWindowVisibilityChanged(hasVisibleWindow) | |
| } | |
| const windowsWithListeners = new WeakSet<BrowserWindow>(); | |
| // 监听窗口显示事件 | |
| app.on('browser-window-created', (_, window) => { | |
| if (windowsWithListeners.has(window)) { | |
| return; | |
| } | |
| windowsWithListeners.add(window); | |
| window.on('show', handleWindowVisibilityChange) | |
| window.on('hide', handleWindowVisibilityChange) | |
| window.on('closed', handleWindowVisibilityChange) | |
| }) |
🤖 Prompt for AI Agents
In src/main/index.ts around lines 160 to 172, the event listeners for 'show',
'hide', and 'closed' are attached every time a new browser window is created,
which can cause duplicate listeners if windows are reused or multiple
'browser-window-created' events occur for the same window. To fix this,
implement a mechanism to track windows that already have listeners attached,
such as using a WeakSet or a Map, and only add listeners if they have not been
added before, preventing duplicate event handling.
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (2)
193-194: Remove redundant case clause.The
'bottom-right'case is immediately followed by thedefaultcase that handles the same logic, making it redundant.case 'bottom-left': x = this.config.offset.x; y = workAreaSize.height - this.config.size.height - this.config.offset.y; break; - case 'bottom-right': default: x = workAreaSize.width - this.config.size.width - this.config.offset.x; y = workAreaSize.height - this.config.size.height - this.config.offset.y; break;
72-73: Make devTools opening configurable instead of automatic.Automatically opening devTools may not always be desired and could interfere with testing the floating window behavior.
if (isDev) { await this.window.loadURL('http://localhost:5173/floating/'); - // 开发模式下可选择性打开开发者工具（暂时禁用，避免影响拖拽） - this.window.webContents.openDevTools({ mode: 'detach' }); + // Optionally open devTools based on environment variable + if (process.env.FLOATING_DEVTOOLS === 'true') { + this.window.webContents.openDevTools({ mode: 'detach' }); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts(1 hunks)src/preload/floating-preload.ts(1 hunks)tsconfig.web.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- tsconfig.web.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/preload/floating-preload.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
[error] 207-209: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (1)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (1)
6-228: Well-designed class architecture for floating window management.The
FloatingButtonWindowclass provides good encapsulation with clear separation of concerns between window management, configuration, and state tracking. The event handling setup and cleanup logic are properly implemented.
| public show(): void { | ||
| if (!this.window) { | ||
| return; | ||
| } | ||
|
|
||
| this.window.show(); | ||
| this.state.isVisible = true; | ||
| logger.debug('FloatingButtonWindow shown'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve window state validation in show/hide methods.
The methods only check if window is null but don't validate if the window is still valid (not destroyed).
public show(): void {
- if (!this.window) {
+ if (!this.exists()) {
return;
}
this.window.show();
this.state.isVisible = true;
logger.debug('FloatingButtonWindow shown');
}
public hide(): void {
- if (!this.window) {
+ if (!this.exists()) {
return;
}
this.window.hide();
this.state.isVisible = false;
logger.debug('FloatingButtonWindow hidden');
}Also applies to: 104-112
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts around
lines 91 to 99 and 104 to 112, the show and hide methods only check if the
window object exists but do not verify if the window is still valid or has been
destroyed. Update these methods to also check if the window is not destroyed
before proceeding. This can be done by adding a condition to verify the window's
validity state, preventing operations on a destroyed window.
| webSecurity: false, // 开发模式下允许跨域 | ||
| devTools: true, // 开发模式下启用开发者工具 | ||
| sandbox: false // 禁用沙盒模式，确保预加载脚本能正常工作 |
There was a problem hiding this comment.
Address security vulnerabilities in webPreferences.
The current configuration disables important security features that should only be relaxed in development mode.
webPreferences: {
nodeIntegration: false,
contextIsolation: true,
preload: preloadPath,
- webSecurity: false, // 开发模式下允许跨域
- devTools: true, // 开发模式下启用开发者工具
- sandbox: false // 禁用沙盒模式，确保预加载脚本能正常工作
+ webSecurity: isDev ? false : true, // Only disable in development
+ devTools: isDev, // Only enable in development
+ sandbox: !isDev // Enable sandbox in production for better security
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts around
lines 60 to 62, the webPreferences settings disable key security features like
webSecurity and sandbox unconditionally. Modify the code to enable these
security features by default and only disable them when running in development
mode, ensuring production builds maintain proper security configurations.
| if (config.size) { | ||
| this.window.setSize(this.config.size.width, this.config.size.height); | ||
| this.state.bounds.width = this.config.size.width; | ||
| this.state.bounds.height = this.config.size.height; | ||
| } | ||
|
|
||
| if (config.position || config.offset) { | ||
| const position = this.calculatePosition(); | ||
| this.window.setPosition(position.x, position.y); | ||
| this.state.bounds.x = position.x; | ||
| this.state.bounds.y = position.y; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for window property updates.
State updates should only occur after successful window operations to maintain consistency.
if (config.size) {
- this.window.setSize(this.config.size.width, this.config.size.height);
- this.state.bounds.width = this.config.size.width;
- this.state.bounds.height = this.config.size.height;
+ try {
+ this.window.setSize(this.config.size.width, this.config.size.height);
+ this.state.bounds.width = this.config.size.width;
+ this.state.bounds.height = this.config.size.height;
+ } catch (error) {
+ logger.error('Failed to update window size:', error);
+ }
}
if (config.position || config.offset) {
- const position = this.calculatePosition();
- this.window.setPosition(position.x, position.y);
- this.state.bounds.x = position.x;
- this.state.bounds.y = position.y;
+ try {
+ const position = this.calculatePosition();
+ this.window.setPosition(position.x, position.y);
+ this.state.bounds.x = position.x;
+ this.state.bounds.y = position.y;
+ } catch (error) {
+ logger.error('Failed to update window position:', error);
+ }
}📝 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.
| if (config.size) { | |
| this.window.setSize(this.config.size.width, this.config.size.height); | |
| this.state.bounds.width = this.config.size.width; | |
| this.state.bounds.height = this.config.size.height; | |
| } | |
| if (config.position || config.offset) { | |
| const position = this.calculatePosition(); | |
| this.window.setPosition(position.x, position.y); | |
| this.state.bounds.x = position.x; | |
| this.state.bounds.y = position.y; | |
| } | |
| if (config.size) { | |
| try { | |
| this.window.setSize(this.config.size.width, this.config.size.height); | |
| this.state.bounds.width = this.config.size.width; | |
| this.state.bounds.height = this.config.size.height; | |
| } catch (error) { | |
| logger.error('Failed to update window size:', error); | |
| } | |
| } | |
| if (config.position || config.offset) { | |
| try { | |
| const position = this.calculatePosition(); | |
| this.window.setPosition(position.x, position.y); | |
| this.state.bounds.x = position.x; | |
| this.state.bounds.y = position.y; | |
| } catch (error) { | |
| logger.error('Failed to update window position:', error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts around
lines 134 to 145, the code updates the state properties immediately after
calling window methods without verifying if those calls succeeded. To fix this,
add error handling around the window.setSize and window.setPosition calls, and
only update the state.bounds properties if the respective window method
completes successfully. This ensures state consistency with the actual window
properties.
| const preloadPath = isDev | ||
| ? path.join(process.cwd(), 'out/preload/floating.mjs') | ||
| : path.join(__dirname, '../../preload/floating.mjs'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate preload script paths to prevent runtime failures.
The hardcoded paths for the preload script may not exist in all deployment scenarios. Consider adding path existence validation or using more robust path resolution.
const isDev = process.env.NODE_ENV === 'development';
- const preloadPath = isDev
- ? path.join(process.cwd(), 'out/preload/floating.mjs')
- : path.join(__dirname, '../../preload/floating.mjs');
+ const preloadPath = isDev
+ ? path.join(process.cwd(), 'out/preload/floating.mjs')
+ : path.join(__dirname, '../../preload/floating.mjs');
+
+ // Validate preload script exists
+ if (!require('fs').existsSync(preloadPath)) {
+ throw new Error(`Preload script not found: ${preloadPath}`);
+ }📝 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 preloadPath = isDev | |
| ? path.join(process.cwd(), 'out/preload/floating.mjs') | |
| : path.join(__dirname, '../../preload/floating.mjs'); | |
| const isDev = process.env.NODE_ENV === 'development'; | |
| const preloadPath = isDev | |
| ? path.join(process.cwd(), 'out/preload/floating.mjs') | |
| : path.join(__dirname, '../../preload/floating.mjs'); | |
| // Validate preload script exists | |
| if (!require('fs').existsSync(preloadPath)) { | |
| throw new Error(`Preload script not found: ${preloadPath}`); | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts around
lines 37 to 39, the preload script paths are hardcoded and may not exist in all
deployment environments, risking runtime failures. Add validation to check if
the resolved preloadPath exists on the filesystem before using it, and handle
the case where it does not (e.g., log an error or fallback to a default path).
Alternatively, improve path resolution by dynamically determining the correct
preload script location based on the environment or configuration.
Pull Request Description (中文)
你的功能请求是否与某个问题有关？请描述一下。
Link #244
桌面应用程序的 UI/UX 更改
deepchat floating window base infra
目前的功能：
20250623_185328.mp4
Summary by CodeRabbit
New Features
Chores