Add Custom Window Controls for Linux and Windows#68
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces comprehensive window control functionality across the application. It adds IPC handlers, interface methods, and UI components to support minimizing, maximizing, closing, and querying the window state. The Sidebar UI now features interactive window control buttons for Windows and Linux, and window state changes are propagated throughout the app. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Preload
participant MainProcess
participant BrowserWindow
Renderer->>Preload: flow.interface.minimizeWindow()
Preload->>MainProcess: IPC minimizeWindow
MainProcess->>BrowserWindow: minimize()
MainProcess->>MainProcess: fireWindowStateChanged()
MainProcess->>Renderer: Send window state changed event
Renderer->>Preload: flow.interface.maximizeWindow()
Preload->>MainProcess: IPC maximizeWindow
MainProcess->>BrowserWindow: maximize()/unmaximize()
MainProcess->>MainProcess: fireWindowStateChanged()
MainProcess->>Renderer: Send window state changed event
Renderer->>Preload: flow.interface.getWindowState()
Preload->>MainProcess: IPC getWindowState
MainProcess->>BrowserWindow: get state
MainProcess->>Preload: Return state
Preload->>Renderer: Return state
Renderer->>Preload: flow.interface.onWindowStateChanged(callback)
MainProcess-->>Renderer: Emit window state changed events on state changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/ipc/browser/interface.ts (1)
67-90: Well-implemented window control handlers with good safety checks.The IPC handlers properly check for window existence before performing operations and implement intuitive toggle behavior for the maximize function. The implementation follows the existing code patterns in the file.
Consider adding try-catch blocks around the window operations to handle potential exceptions:
ipcMain.on("interface:minimize-window", (event) => { const win = browser?.getWindowFromWebContents(event.sender); if (win) { + try { win.window.minimize(); + } catch (error) { + console.error("Failed to minimize window:", error); + } } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/browser/window.ts(3 hunks)src/main/ipc/browser/interface.ts(2 hunks)src/preload/index.ts(2 hunks)src/renderer/src/components/browser-ui/main.tsx(1 hunks)src/renderer/src/components/browser-ui/sidebar/header/window-controls.tsx(2 hunks)src/shared/flow/interfaces/browser/interface.ts(2 hunks)src/shared/flow/types.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/main/ipc/browser/interface.ts (2)
src/main/index.ts (1)
browser(14-14)src/main/browser/window.ts (1)
TabbedBrowserWindow(30-284)
src/main/browser/window.ts (1)
src/main/ipc/browser/interface.ts (1)
fireWindowStateChanged(107-109)
src/shared/flow/interfaces/browser/interface.ts (1)
src/shared/flow/types.ts (2)
WindowState(8-11)IPCListener(13-13)
src/renderer/src/components/browser-ui/sidebar/header/window-controls.tsx (1)
src/renderer/src/lib/utils.ts (1)
cn(5-7)
src/preload/index.ts (1)
src/shared/flow/types.ts (1)
WindowState(8-11)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-24.04-arm)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (16)
src/shared/flow/types.ts (1)
8-11: Clean and well-defined type structure.The
WindowStatetype provides a clear contract for window state communication across the application. The boolean properties are appropriately named and typed.src/renderer/src/components/browser-ui/main.tsx (1)
89-89:Details
❓ Verification inconclusive
Appropriate removal of Windows titlebar padding.
Removing the Windows-specific titlebar padding aligns with the custom window controls implementation. Since custom controls are being added, the native titlebar spacing is no longer needed.
Please verify that the layout remains correct on Windows after this change, especially ensuring that:
- Content doesn't overlap with the custom window controls
- The drag area functions properly
- No visual gaps or spacing issues occur
🏁 Script executed:
#!/bin/bash # Description: Check for other references to titlebar area calculations that might need updating # Search for other uses of titlebar area environment variables rg -A 3 -B 3 "titlebar-area|env\(titlebar"Length of output: 1436
Approve removal of Windows titlebar padding in main UI
The Windows-specific
pt-[calc(env(titlebar-area-y)+env(titlebar-area-height))]padding was intentionally removed from
src/renderer/src/components/browser-ui/main.tsxto support the new custom window controls. A project-wide search found no other references to those titlebar environment variables in the main UI (only the onboarding drag-disabler still uses it by design).Please manually verify on Windows that:
- Main window content and custom controls remain correctly spaced
- The drag region still functions as expected
- No unintended gaps or overlaps appear in both the main UI and the onboarding flow
src/main/browser/window.ts (2)
14-14: Good platform-specific titlebar configuration.The conditional
titleBarStylesetting is appropriate - hiding the titlebar on macOS while keeping it undefined (default) on other platforms aligns with the PR objective of adding custom controls for Linux and Windows specifically.Also applies to: 54-54
93-93: Comprehensive window state change broadcasting.The
fireWindowStateChangedcalls are properly placed after state changes occur and cover all relevant window state transitions (fullscreen enter/exit, maximize/unmaximize). This ensures consistent state synchronization across the application.Also applies to: 99-99, 103-106
src/main/ipc/browser/interface.ts (3)
3-3: Necessary import for window state broadcasting.The import of
sendMessageToListenersInWindowis required for implementing the window state change notification system.
92-105: Clean window state query implementation.The
getWindowStatehelper function correctly maps to theWindowStatetype, and the IPC handler properly handles edge cases by returningfalsewhen no window is found. The implementation is straightforward and reliable.
107-109: Effective window state broadcasting implementation.The
fireWindowStateChangedfunction correctly uses the existing IPC messaging infrastructure to broadcast window state changes to all listeners in the window. This provides real-time state synchronization for UI components.src/shared/flow/interfaces/browser/interface.ts (2)
1-1: LGTM: Clean import addition.The WindowState type import is correctly added to support the new window management methods.
59-82: LGTM: Well-designed window management interface.The new window control methods are well-designed with:
- Clear, descriptive method names following existing conventions
- Appropriate return types (void for actions, Promise for async operations)
- Consistent JSDoc documentation
- Proper use of the IPCListener pattern for state change events
The interface additions integrate seamlessly with the existing API structure.
src/preload/index.ts (2)
16-16: LGTM: Proper type import.The WindowState type import is correctly added to support the new window state callback typing.
312-327: LGTM: Solid IPC implementation of window controls.The implementation correctly follows established patterns:
- Uses
ipcRenderer.sendfor fire-and-forget window control commands- Uses
ipcRenderer.invokefor async window state queries- Uses
listenOnIPCChannelhelper for event subscription- IPC channel names follow consistent
interface:*convention- Proper typing with WindowState for the callback parameter
The integration with the existing interfaceAPI structure is seamless.
src/renderer/src/components/browser-ui/sidebar/header/window-controls.tsx (5)
2-3: LGTM: Proper imports for enhanced functionality.Good addition of necessary imports for utility functions and React hooks to support the window controls implementation.
5-6: LGTM: Well-defined shared styling constant.The shared button classes provide consistent styling and the
remove-app-dragclass correctly prevents drag behavior on interactive buttons.
8-61: LGTM: Clean and accessible icon components.The SVG icon implementations are well-designed:
- Consistent sizing and styling across all icons
- Proper responsive design with dark mode support
- WindowsMaximize correctly toggles between maximize/restore states
- Clean, semantic SVG markup
The icons follow Windows design conventions and will work well for Linux too as intended.
107-117: LGTM: Simple and correct event handlers.The event handlers are clean and straightforward, directly calling the appropriate interface methods without unnecessary complexity.
119-167: LGTM: Well-structured responsive UI with good UX.The component rendering is well-designed:
- Proper platform-specific styling with Tailwind classes
- Good responsive behavior (hidden on fullscreen for macOS)
- Appropriate hover states and visual feedback
- Semantic button elements with proper titles for accessibility
- Clean conditional rendering for Windows/Linux vs macOS
The UI follows platform conventions and provides a polished user experience.
Summary by CodeRabbit
New Features
Style