-
Notifications
You must be signed in to change notification settings - Fork 611
fix: macos full screen error #1251
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
Conversation
📝 WalkthroughWalkthroughThe changes refactor tab movement to new windows and window creation. They remove automatic DevTools opening for tabs, add a Changes
Sequence DiagramsequenceDiagram
actor User
participant AppBar as Renderer: AppBar
participant TabPresenter as Main: tabPresenter
participant WindowPresenter as Main: windowPresenter
participant Shell as Shell Window
User->>AppBar: Drag tab outside window
AppBar->>AppBar: handleDragEnd triggered
AppBar->>TabPresenter: moveTabToNewWindow(tabId, screenX, screenY)
TabPresenter->>WindowPresenter: createShellWindow({<br/>forMovedTab: true,<br/>x: screenX, y: screenY<br/>})
rect rgb(200, 220, 255)
Note over WindowPresenter: Window Creation
WindowPresenter->>WindowPresenter: shouldAutoShow = true<br/>(forMovedTab bypasses type check)
WindowPresenter->>Shell: Create window at screen position
Shell->>WindowPresenter: ready-to-show
end
rect rgb(220, 240, 220)
Note over WindowPresenter: macOS Overlay Setup
WindowPresenter->>WindowPresenter: Configure tooltip overlay<br/>(hidden in Mission Control,<br/>visible on all workspaces)
end
WindowPresenter->>Shell: showInactive() if needed
Shell-->>User: New window with tab visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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. Comment |
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: 1
Fix all issues with AI Agents 🤖
In @src/main/presenter/tabPresenter.ts:
- Line 219: Translate the Chinese comment "// DevTools 不再自动打开(避免在 macOS
全屏时产生额外窗口/空间的异常体验)" to clear English, e.g. "// DevTools no longer open
automatically to avoid creating extra windows/spaces when macOS is fullscreen",
and replace the original comment in tabPresenter.ts; also add a short follow-up
note (same comment block) that developers can open DevTools manually via the
menu or shortcut (F12 / Cmd+Option+I) to ensure the development workflow is
clear.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.tssrc/renderer/shell/components/AppBar.vue
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments in TypeScript/JavaScript code
Files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict type checking enabled
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/presenter/**/*.ts: Use EventBus to broadcast events from main to renderer viamainWindow.webContents.send()
Implement one presenter per functional domain in the main process
Files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/**/*.ts: Use EventBus fromsrc/main/eventbus.tsfor decoupled inter-process communication
Context isolation must be enabled with preload scripts for secure IPC communicationElectron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/main/presenter/tabPresenter.tssrc/renderer/shell/components/AppBar.vuesrc/main/presenter/windowPresenter/index.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/main/presenter/tabPresenter.tssrc/renderer/shell/components/AppBar.vuesrc/main/presenter/windowPresenter/index.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/main/presenter/tabPresenter.tssrc/renderer/shell/components/AppBar.vuesrc/main/presenter/windowPresenter/index.ts
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: Use Vue 3 Composition API for all components
Use Tailwind CSS for styling with scoped styles
All user-facing strings must use i18n keys via vue-i18n
Files:
src/renderer/shell/components/AppBar.vue
src/renderer/shell/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Shell UI code should be located in
src/renderer/shell/
Files:
src/renderer/shell/components/AppBar.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Vue components must be named in PascalCase (e.g.,
ChatInput.vue) and use Vue 3 Composition API with Pinia for state management and Tailwind for styling
Files:
src/renderer/shell/components/AppBar.vue
🧠 Learnings (6)
📚 Learning: 2025-06-21T15:48:29.950Z
Learnt from: neoragex2002
Repo: ThinkInAIXYZ/deepchat PR: 550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:250-252
Timestamp: 2025-06-21T15:48:29.950Z
Learning: In the meeting server implementation (src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts), when multiple tabs have the same title, the user prefers to let the code silently select the first match without adding warnings or additional ambiguity handling.
Applied to files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
📚 Learning: 2026-01-05T02:41:31.619Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.619Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use `usePresenter` composable for main process communication instead of direct IPC calls
Applied to files:
src/main/presenter/tabPresenter.ts
📚 Learning: 2026-01-05T02:40:52.831Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-05T02:40:52.831Z
Learning: Applies to src/main/presenter/**/*.ts : Use EventBus to broadcast events from main to renderer via `mainWindow.webContents.send()`
Applied to files:
src/main/presenter/tabPresenter.tssrc/main/presenter/windowPresenter/index.ts
📚 Learning: 2026-01-05T02:40:52.831Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-05T02:40:52.831Z
Learning: Applies to src/main/presenter/**/*.ts : Implement one presenter per functional domain in the main process
Applied to files:
src/main/presenter/tabPresenter.ts
📚 Learning: 2026-01-05T02:40:52.831Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-05T02:40:52.831Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use `usePresenter.ts` composable for renderer-to-main IPC communication via direct presenter method calls
Applied to files:
src/main/presenter/tabPresenter.ts
📚 Learning: 2026-01-05T02:41:45.204Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.204Z
Learning: Applies to src/main/**/*.ts : Electron main process code should reside in `src/main/`, with presenters organized in `presenter/` subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed via `eventbus.ts`
Applied to files:
src/main/presenter/windowPresenter/index.ts
⏰ 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 (7)
src/main/presenter/windowPresenter/index.ts (6)
676-687: LGTM: Well-documented new parameter for moved tab behavior.The
forMovedTabflag is a clean solution to distinguish user-initiated tab moves from programmatic browser window creation, ensuring proper visibility behavior.
774-791: LGTM: Correct auto-show logic for moved tabs.The updated
shouldAutoShowlogic correctly prevents browser windows from stealing focus except when explicitly moved by the user via drag-and-drop. This addresses the focus management issues mentioned in the PR.
79-110: LGTM: Defensive visibility checks prevent redundant overlay operations.The added checks ensure
overlay.showInactive()is only called when the overlay is not already visible, preventing potential flickering or race conditions on macOS.
112-124: LGTM: Consistent visibility checks for hide operations.The visibility guards before calling
overlay.hide()mirror the pattern used forshowInactive(), ensuring symmetric and safe overlay state management.
1105-1108: LGTM: macOS-specific overlay configuration addresses fullscreen issues.The
setHiddenInMissionControl(true)andsetVisibleOnAllWorkspaces(true, { visibleOnFullScreen: true })settings ensure tooltip overlays work correctly across workspaces and in fullscreen mode on macOS, which directly addresses the PR's objective.
1177-1185: LGTM: Consistent visibility management in clearTooltipOverlay.The visibility check before hiding the overlay follows the established pattern and ensures clean tooltip lifecycle management.
src/renderer/shell/components/AppBar.vue (1)
528-565: LGTM: Correct coordinate system for new window positioning.The change from
event.clientX, event.clientYtoevent.screenX, event.screenYis correct. When dragging a tab outside the window to create a new window, screen-absolute coordinates are needed to position the new window at the drop location, not viewport-relative coordinates.
fix #1243
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.