Skip to content

Fix/app titlebar windows#6437

Merged
bijin-bruno merged 4 commits intousebruno:mainfrom
abhishek-bruno:fix/app-titlebar-windows
Dec 17, 2025
Merged

Fix/app titlebar windows#6437
bijin-bruno merged 4 commits intousebruno:mainfrom
abhishek-bruno:fix/app-titlebar-windows

Conversation

@abhishek-bruno
Copy link
Member

@abhishek-bruno abhishek-bruno commented Dec 17, 2025

Description

App Titlebar ui issues in windows fixed.
JIRA

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

Summary by CodeRabbit

  • New Features

    • Native window controls for Windows (minimize, maximize/restore, close) with IPC-driven maximize state and OS-aware titlebar behavior.
  • Style

    • Refined titlebar visuals: spacing, no-drag regions, dedicated actions area, window control button interactions (hover/active) and Windows-specific sizing.
    • Added top separator in the main wrapper and increased a layout toggle icon size for improved appearance.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Walkthrough

Adds Windows-native window controls and OS-specific classing to the AppTitleBar, introduces renderer↔main IPC channels for minimize/maximize/close and maximize-state synchronization, adjusts titlebar styling/no-drag regions, and makes small UI tweaks in ResponseLayoutToggle and the Bruno page Wrapper.

Changes

Cohort / File(s) Summary
AppTitleBar UI & Styling
packages/bruno-app/src/components/AppTitleBar/index.js, packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
Add OS detection and os-* class; introduce .titlebar-actions and Windows-only .window-controls with minimize/maximize/close buttons; add -webkit-app-region: no-drag for action areas; adjust padding/margins, remove bottom border, and add hover/active styles (close turns red).
Electron main (IPC handlers)
packages/bruno-electron/src/index.js
Remove titleBarOverlay option; add IPC handlers: renderer:window-minimize, renderer:window-maximize (toggle), renderer:window-close, renderer:window-is-maximized; emit main:window-maximized/main:window-unmaximized and forward fullscreen enter/leave events.
Small UI tweaks
packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js, packages/bruno-app/src/pages/Bruno/StyledWrapper.js
Increase default ActionIcon size to lg in ResponseLayoutToggle; add border-top: 1px solid using theme color to Bruno page Wrapper.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Renderer as App (renderer)
  participant Main as Electron Main
  participant OS as Window Manager

  User->>Renderer: Click minimize / maximize / close
  Renderer->>Main: ipcRenderer.send('renderer:window-minimize' / 'renderer:window-maximize' / 'renderer:window-close')
  Main->>Main: BrowserWindow.minimize()/toggleMaximize()/close()
  Main->>OS: Request window action
  OS-->>Main: Action completed
  Main-->>Renderer: emit main:window-maximized / main:window-unmaximized (on change)
  Renderer->>Main: ipcRenderer.invoke('renderer:window-is-maximized') (initial / query)
  Main-->>Renderer: return isMaximized boolean
  Renderer->>Renderer: update isMaximized UI state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus areas:
    • IPC channel correctness, platform gating (isWindows/isMac), and potential races during startup.
    • Maximize toggle logic and proper cleanup of listeners.
    • CSS -webkit-app-region scoping to ensure drag regions remain functional across platforms.
    • Z-index/spacing and hover/active states for window-control buttons (close red-on-hover).

Possibly related PRs

Suggested reviewers

  • helloanoop
  • lohit-bruno
  • naman-bruno

Poem

Tiny controls at the rim of the app,
Renderer whispers, main answers the tap,
Minimize, restore, or close on cue,
Windows-aware, the titlebar sings true,
A little cross-process dance — neatly wrapped. 🎵

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix/app titlebar windows' directly matches the PR's main objective—addressing app titlebar UI issues on Windows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (1)

227-233: Simplify duplicate background styles.

The hover and active states have identical backgrounds. Consider consolidating them.

-    &:hover {
-      background: ${(props) => props.theme.sidebar.collection.item.hoverBg};
-    }
-
-    &:active {
-      background: ${(props) => props.theme.sidebar.collection.item.hoverBg};
-    }
+    &:hover,
+    &:active {
+      background: ${(props) => props.theme.sidebar.collection.item.hoverBg};
+    }
packages/bruno-app/src/components/AppTitleBar/index.js (1)

278-299: Verify unused CSS classes.

The window control buttons include class names like minimize and maximize, but StyledWrapper.js only defines styles for the generic .window-control-btn and .close classes. Confirm whether these additional classes are intentional for future styling or can be removed.

If unused, simplify to:

               <button
-                className="window-control-btn minimize"
+                className="window-control-btn"
                 onClick={handleMinimize}
                 aria-label="Minimize"
               >
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c1fba6 and 7e84a69.

📒 Files selected for processing (5)
  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (3 hunks)
  • packages/bruno-app/src/components/AppTitleBar/index.js (5 hunks)
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (1 hunks)
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js (1 hunks)
  • packages/bruno-electron/src/index.js (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/components/AppTitleBar/index.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
  • packages/bruno-electron/src/index.js
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js
🧠 Learnings (2)
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Applied to files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Applied to files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/AppTitleBar/index.js (2)
packages/bruno-electron/src/index.js (1)
  • isWindows (87-87)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
  • window (18-18)
packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (1)
packages/bruno-app/src/ui/ActionIcon/index.js (1)
  • ActionIcon (22-56)
⏰ 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). (6)
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: Unit Tests
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: CLI Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Windows
🔇 Additional comments (9)
packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (1)

94-94: LGTM: Size consistency improvement.

The addition of size="lg" aligns with the ActionIcon instances in the title bar, improving visual consistency.

packages/bruno-app/src/pages/Bruno/StyledWrapper.js (1)

8-8: LGTM: Theme-aware border addition.

The border-top addition using theme colors maintains visual consistency with the rest of the UI.

packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (3)

24-24: LGTM: Fullscreen padding adjustment.

The increased padding provides better visual spacing when traffic lights are hidden.


105-112: LGTM: Interactive region and actions container.

The no-drag region on titlebar-right ensures window controls and action buttons remain interactive, and the new titlebar-actions container improves layout organization.


211-211: Verify z-index 1000 necessity.

The window controls have z-index: 1000 to stay above modal overlays. Confirm this value doesn't conflict with other high z-index elements (modals, dropdowns, tooltips) in the app.

#!/bin/bash
# Search for z-index usage to check for conflicts
rg -nP 'z-index:\s*\d+' packages/bruno-app/src/ | gawk -F':' '{print $1":"$2" -> "$3}' | sort -t'>' -k2 -n
packages/bruno-app/src/components/AppTitleBar/index.js (3)

2-2: LGTM: Icon imports for Windows controls.

All imported icons are correctly used in the Windows window controls. The IconCopy icon represents the "restore" state when maximized, which is a common Windows pattern.


35-37: LGTM: Window state and OS detection setup.

The state initialization and OS class derivation are clean and appropriate for the Windows-specific UI logic.


252-274: LGTM: Structured title bar actions.

The restructured right section with the titlebar-actions container improves organization and maintains all existing functionality while adding Windows-specific controls.

packages/bruno-electron/src/index.js (1)

16-16: nativeTheme import is actively used. The nativeTheme module is referenced in packages/bruno-electron/src/ipc/preferences.js (line 43) where it's set to control the application theme source. The import is necessary and should be kept.

Likely an incorrect or invalid review comment.

Comment on lines +235 to 238
&.close:hover {
background: #e81123;
color: white;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hardcoded color breaks theming.

The close button hover uses a hardcoded red (#e81123) instead of a theme color. This won't adapt to custom themes and breaks the theme consistency.

Consider adding a theme color for the close button hover state:

     &.close:hover {
-      background: #e81123;
+      background: ${(props) => props.theme.windowControls?.closeBtnHoverBg || '#e81123'};
       color: white;
     }

Alternatively, if maintaining Windows system colors is intentional for native feel, document this decision with a comment.

🤖 Prompt for AI Agents
In packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js around lines
235-238 the close button hover uses a hardcoded color (#e81123) which breaks
theming; replace the literal with a theme token (e.g. use
props.theme.colors.closeHover or props.theme.semantic.error ||
props.theme.colors.danger) and update the theme(s) to include that token (with a
Windows-native red as the default) so custom themes can override it;
alternatively, if the hardcoded Windows color is intentional, add a clear
comment above the rule noting that the native Windows red is required and should
not be changed.

Comment on lines +171 to +189
ipcMain.on('renderer:window-minimize', () => {
mainWindow.minimize();
});

ipcMain.on('renderer:window-maximize', () => {
if (mainWindow.isMaximized()) {
mainWindow.unmaximize();
} else {
mainWindow.maximize();
}
});

ipcMain.on('renderer:window-close', () => {
mainWindow.close();
});

ipcMain.handle('renderer:window-is-maximized', () => {
return mainWindow.isMaximized();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add mainWindow existence checks.

The IPC handlers don't validate that mainWindow exists before calling methods on it. If a handler is triggered before the window is created or after it's destroyed, this will cause a runtime error.

Apply this diff to add safety checks:

   ipcMain.on('renderer:window-minimize', () => {
+    if (!mainWindow) return;
     mainWindow.minimize();
   });
 
   ipcMain.on('renderer:window-maximize', () => {
+    if (!mainWindow) return;
     if (mainWindow.isMaximized()) {
       mainWindow.unmaximize();
     } else {
       mainWindow.maximize();
     }
   });
 
   ipcMain.on('renderer:window-close', () => {
+    if (!mainWindow) return;
     mainWindow.close();
   });
 
   ipcMain.handle('renderer:window-is-maximized', () => {
+    if (!mainWindow) return false;
     return mainWindow.isMaximized();
   });
📝 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.

Suggested change
ipcMain.on('renderer:window-minimize', () => {
mainWindow.minimize();
});
ipcMain.on('renderer:window-maximize', () => {
if (mainWindow.isMaximized()) {
mainWindow.unmaximize();
} else {
mainWindow.maximize();
}
});
ipcMain.on('renderer:window-close', () => {
mainWindow.close();
});
ipcMain.handle('renderer:window-is-maximized', () => {
return mainWindow.isMaximized();
});
ipcMain.on('renderer:window-minimize', () => {
if (!mainWindow) return;
mainWindow.minimize();
});
ipcMain.on('renderer:window-maximize', () => {
if (!mainWindow) return;
if (mainWindow.isMaximized()) {
mainWindow.unmaximize();
} else {
mainWindow.maximize();
}
});
ipcMain.on('renderer:window-close', () => {
if (!mainWindow) return;
mainWindow.close();
});
ipcMain.handle('renderer:window-is-maximized', () => {
if (!mainWindow) return false;
return mainWindow.isMaximized();
});
🤖 Prompt for AI Agents
In packages/bruno-electron/src/index.js around lines 171 to 189, the IPC
handlers call methods on mainWindow without checking it exists, causing runtime
errors if invoked before creation or after destruction; add guard checks in each
handler/handle to verify mainWindow is non-null (e.g., if (!mainWindow ||
mainWindow.isDestroyed()) return or return false as appropriate) before calling
minimize/maximize/unmaximize/close/isMaximized, and ensure the
'renderer:window-is-maximized' handler returns a boolean when mainWindow is
absent.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
packages/bruno-app/src/components/AppTitleBar/index.js (2)

2-2: Consider using a dedicated restore icon instead of IconCopy.

IconCopy for the restore button is unconventional. Windows typically uses overlapping rectangles (IconWindowRestore or similar). If @tabler/icons doesn't have one, IconCopy works visually but may confuse users expecting standard Windows controls.


64-67: Avoid variable shadowing - rename the callback parameter.

The callback parameter isMaximized shadows the state variable of the same name. This can cause confusion during maintenance.

     ipcRenderer.invoke('renderer:window-is-maximized')
-      .then((isMaximized) => {
-        setIsMaximized(isMaximized);
+      .then((maximized) => {
+        setIsMaximized(maximized);
       })
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e84a69 and 8d3182f.

📒 Files selected for processing (2)
  • packages/bruno-app/src/components/AppTitleBar/index.js (5 hunks)
  • packages/bruno-electron/src/index.js (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/bruno-electron/src/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/AppTitleBar/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/AppTitleBar/index.js (4)
packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (2)
  • dispatch (55-55)
  • ResponseLayoutToggle (74-105)
packages/bruno-electron/src/index.js (1)
  • isWindows (87-87)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
  • window (18-18)
packages/bruno-app/src/pages/Bruno/index.js (1)
  • isConsoleOpen (61-61)
⏰ 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). (6)
  • GitHub Check: CLI Tests
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: SSL Tests - macOS
🔇 Additional comments (5)
packages/bruno-app/src/components/AppTitleBar/index.js (5)

23-29: Previous feedback addressed - platform utilities now used correctly.

The OS detection now properly uses the existing isMacOS and isWindowsOS utilities from utils/common/platform, which is consistent with the rest of the codebase.


34-36: LGTM - state and OS class setup is clean.

The derivation of osClass and isWindows is straightforward. While getOsClass() runs on each render, it's a trivial synchronous call with no performance concern.


75-87: Previous feedback addressed - window handlers are correctly implemented.

The race condition fix using functional state update setIsMaximized((prev) => !prev) is properly applied. All handlers use optional chaining for safety.


218-218: LGTM - className composition is clean.


257-279: LGTM - titlebar actions restructured cleanly.

Grouping the action buttons in a dedicated container makes sense for layout control and potential no-drag region styling on Windows.

Copy link
Collaborator

@bijin-bruno bijin-bruno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix the toggle issues pointed by coderabbit

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/bruno-electron/src/index.js (1)

170-193: Add mainWindow null checks to IPC handlers.

The handlers guard against non-Windows platforms but don't check if mainWindow exists. While unlikely, if the window is destroyed before handlers fire, this could cause runtime errors.

Apply this diff:

   ipcMain.on('renderer:window-minimize', () => {
-    if (!isWindows) return;
+    if (!isWindows || !mainWindow) return;
     mainWindow.minimize();
   });
 
   ipcMain.on('renderer:window-maximize', () => {
-    if (!isWindows) return;
+    if (!isWindows || !mainWindow) return;
     if (mainWindow.isMaximized()) {
       mainWindow.unmaximize();
     } else {
       mainWindow.maximize();
     }
   });
 
   ipcMain.on('renderer:window-close', () => {
-    if (!isWindows) return;
+    if (!isWindows || !mainWindow) return;
     mainWindow.close();
   });
 
   ipcMain.handle('renderer:window-is-maximized', () => {
-    if (!isWindows) return false;
+    if (!isWindows || !mainWindow) return false;
     return mainWindow.isMaximized();
   });
🧹 Nitpick comments (2)
packages/bruno-app/src/components/AppTitleBar/index.js (1)

2-2: Unused import: IconCopy may not be semantically correct for restore.

IconCopy is being used for the "Restore" window control (line 307), but it's typically a copy/duplicate icon. Consider using a more semantically appropriate icon for window restore, if available in the icon library.

packages/bruno-electron/src/index.js (1)

16-16: Unused import: nativeTheme.

nativeTheme is imported but not used anywhere in this file. Remove it to keep imports clean.

-const { BrowserWindow, app, session, Menu, globalShortcut, ipcMain, nativeTheme } = require('electron');
+const { BrowserWindow, app, session, Menu, globalShortcut, ipcMain } = require('electron');
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d3182f and ba15011.

📒 Files selected for processing (2)
  • packages/bruno-app/src/components/AppTitleBar/index.js (5 hunks)
  • packages/bruno-electron/src/index.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-electron/src/index.js
  • packages/bruno-app/src/components/AppTitleBar/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/AppTitleBar/index.js (2)
packages/bruno-electron/src/index.js (1)
  • isWindows (87-87)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
  • window (18-18)
🔇 Additional comments (6)
packages/bruno-app/src/components/AppTitleBar/index.js (5)

16-16: Unused import: nativeTheme.

nativeTheme is imported on line 16 of packages/bruno-electron/src/index.js but doesn't appear to be used anywhere in the file. This should be removed unless there's a planned use.


25-29: Good fix: OS detection now uses shared platform utilities.

This addresses the previous review feedback. Using isMacOS() and isWindowsOS() from the shared utilities is more reliable than navigator.platform.


57-85: Well-structured IPC setup with proper cleanup.

The effect correctly:

  • Guards for non-Windows platforms
  • Handles initial state retrieval with error handling
  • Sets up event listeners for external maximize/unmaximize events
  • Returns cleanup functions

This addresses previous review concerns about state synchronization.


87-99: Window control handlers look clean.

Handlers use optional chaining for safety. Note that handleMaximize no longer optimistically sets state—it relies on IPC events from main process, which is the correct pattern.


293-317: Windows window controls implementation looks good.

The controls are conditionally rendered only on Windows, with proper accessibility labels that update based on isMaximized state. The icon toggle between IconCopy and IconSquare reflects the window state correctly.

packages/bruno-electron/src/index.js (1)

231-238: Good: Renderer notification on maximize/unmaximize events.

This enables the renderer to stay in sync when the window state changes via external actions (Win+Arrow keys, snap, double-click titlebar). This addresses the previous review concern about state synchronization.

…; adjust ActionIcon size in ResponseLayoutToggle for better UI consistency; enhance title bar color handling in Electron app
…ppTitleBar; enhance state management in Electron main process
@abhishek-bruno abhishek-bruno force-pushed the fix/app-titlebar-windows branch from ba15011 to a973b63 Compare December 17, 2025 15:22
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (1)

235-238: Hardcoded color breaks theming.

The close button hover uses #e81123 instead of a theme color. This was flagged in a previous review.

If maintaining Windows system colors is intentional for native feel, add a comment documenting this decision.

🧹 Nitpick comments (2)
packages/bruno-app/src/pages/Bruno/StyledWrapper.js (1)

8-8: Consider using a more semantically appropriate theme color.

Using hoverBg for a static border is unconventional—hover colors typically indicate interactive states. If there's a dedicated border or separator color in the theme, that would be more appropriate. Otherwise, this works and maintains visual consistency.

packages/bruno-app/src/components/AppTitleBar/index.js (1)

116-123: Move WorkspaceName outside the component or memoize it.

Defining a forwardRef component inside another component causes it to be recreated on every render, which can lead to unnecessary re-renders and potential focus/state loss issues in child components.

+const WorkspaceName = forwardRef((props, ref) => {
+  const { workspaceName } = props;
+  return (
+    <div ref={ref} className="workspace-name-container" {...props}>
+      <span className="workspace-name">{workspaceName}</span>
+      <IconChevronDown size={14} stroke={1.5} className="chevron-icon" />
+    </div>
+  );
+});
+
 const AppTitleBar = () => {
   // ... existing code ...
-  const WorkspaceName = forwardRef((props, ref) => {
-    return (
-      <div ref={ref} className="workspace-name-container" {...props}>
-        <span className="workspace-name">{toTitleCase(activeWorkspace?.name) || 'Default Workspace'}</span>
-        <IconChevronDown size={14} stroke={1.5} className="chevron-icon" />
-      </div>
-    );
-  });

Then pass workspaceName={toTitleCase(activeWorkspace?.name) || 'Default Workspace'} as a prop.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba15011 and a973b63.

📒 Files selected for processing (5)
  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (3 hunks)
  • packages/bruno-app/src/components/AppTitleBar/index.js (5 hunks)
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (1 hunks)
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js (1 hunks)
  • packages/bruno-electron/src/index.js (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/bruno-electron/src/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/components/AppTitleBar/index.js
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
🧠 Learnings (3)
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Applied to files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Applied to files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
  • packages/bruno-app/src/pages/Bruno/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Applied to files:

  • packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/AppTitleBar/index.js (2)
packages/bruno-electron/src/index.js (1)
  • isWindows (87-87)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js (1)
  • window (18-18)
⏰ 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). (6)
  • GitHub Check: Unit Tests
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: CLI Tests
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: SSL Tests - Windows
🔇 Additional comments (5)
packages/bruno-app/src/components/ResponsePane/ResponseLayoutToggle/index.js (1)

94-94: LGTM!

The size="lg" prop aligns with the updated titlebar UI sizing. Consistent with other ActionIcon usages in the title bar.

packages/bruno-app/src/components/AppTitleBar/index.js (2)

303-327: LGTM!

Window controls properly delegate state management to IPC events from the main process. The maximize/restore button correctly toggles icon based on synchronized state.


57-85: LGTM!

Proper IPC setup for Windows maximize state tracking with error handling and cleanup. The dependency array correctly includes isWindows to avoid setting up listeners on non-Windows platforms.

packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js (2)

189-202: LGTM!

Good use of scoped selectors (&:not(.os-mac) and &.os-windows) instead of body-based targeting. Cleaner and more maintainable approach.


214-234: LGTM!

Window control button styling properly uses theme props for colors and hover states. The -webkit-app-region: no-drag ensures buttons remain clickable within the draggable titlebar.

Comment on lines +204 to 212
/* Custom window control buttons for Windows - always interactive, above modal overlay */
.window-controls {
display: flex;
align-items: stretch;
height: 36px;
margin-left: 8px;
position: relative;
z-index: 1000;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for all z-index values in the bruno-app codebase
echo "=== Z-INDEX VALUES IN CODEBASE ==="
rg -n "z-index\s*:" packages/bruno-app/src --type css --type scss -A 0 | sort -t: -k3 -n -r | head -30

echo -e "\n=== Z-INDEX VALUES IN JS/TS FILES ==="
rg -n "zIndex|z-index" packages/bruno-app/src --type js --type ts -B 1 -A 1 | head -50

Repository: usebruno/bruno

Length of output: 3775


🏁 Script executed:

#!/bin/bash
# Read the specific file to confirm z-index: 1000
echo "=== StyledWrapper.js content (lines 200-220) ==="
sed -n '200,220p' packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js

# Search for modal, dialog, overlay, backdrop definitions
echo -e "\n=== Modal/Dialog/Overlay/Backdrop z-index ==="
rg -n "modal|dialog|overlay|backdrop|Modal|Dialog|Overlay" packages/bruno-app/src --type js -A 2 -B 2 | grep -E "(z-index|modal|dialog|overlay|backdrop)" | head -40

# Search for toast/notification definitions
echo -e "\n=== Toast/Notification z-index ==="
rg -n "toast|notification|Toast|Notification" packages/bruno-app/src --type js -A 2 -B 2 | grep -E "(z-index|toast|notification)" | head -40

# Search for tooltip definitions
echo -e "\n=== Tooltip z-index ==="
rg -n "tooltip|Tooltip" packages/bruno-app/src --type js -A 2 | grep -E "(z-index|tooltip)" | head -20

Repository: usebruno/bruno

Length of output: 11585


🏁 Script executed:

#!/bin/bash
# Search for CSS styling of modals, toasts, tooltips
echo "=== Search for bruno-modal CSS styling ==="
rg -n "bruno-modal|\.modal" packages/bruno-app/src --type js --type css -A 3 | grep -E "(z-index|backdrop|overlay)" | head -30

# Check globalStyles for modal/toast/tooltip styling
echo -e "\n=== globalStyles.js around modal/toast/tooltip sections ==="
grep -n "modal\|toast\|tooltip\|overlay\|backdrop" packages/bruno-app/src/globalStyles.js | head -20

# Search for any CSS files in the codebase
echo -e "\n=== All CSS/SCSS files ==="
find packages/bruno-app/src -name "*.css" -o -name "*.scss" | head -10

# Check for modal styling in globalStyles more broadly
echo -e "\n=== Checking for modal backdrop styling ==="
sed -n '280,350p' packages/bruno-app/src/globalStyles.js

Repository: usebruno/bruno

Length of output: 2779


🏁 Script executed:

#!/bin/bash
# Read Modal StyledWrapper to find backdrop z-index
echo "=== Modal/StyledWrapper.js - backdrop styling ==="
sed -n '115,140p' packages/bruno-app/src/components/Modal/StyledWrapper.js

# Search for any z-index on modal card or overlay
echo -e "\n=== Complete Modal StyledWrapper z-index definitions ==="
rg -n "z-index" packages/bruno-app/src/components/Modal/StyledWrapper.js

# Check GlobalSearchModal for z-index
echo -e "\n=== GlobalSearchModal z-index ==="
rg -n "z-index" packages/bruno-app/src/components/GlobalSearchModal/StyledWrapper.js

# Check if there's a modal z-index in globalStyles
echo -e "\n=== Search globalStyles for modal z-index ==="
grep -n "modal" packages/bruno-app/src/globalStyles.js | grep -i "z-index"

# Check for react-hot-toast styling or configuration
echo -e "\n=== Toast configuration ==="
cat packages/bruno-app/src/providers/Toaster/index.js

Repository: usebruno/bruno

Length of output: 1697


🏁 Script executed:

#!/bin/bash
# Verify react-hot-toast default z-index by checking package.json and searching for any explicit config
echo "=== Check for Toaster z-index configuration ==="
grep -r "Toaster\|toastOptions\|z-index" packages/bruno-app/src/providers/Toaster/ -A 5 -B 5

# Check if there's any other react-hot-toast config in the app
echo -e "\n=== Search for any toast z-index overrides ==="
rg -n "toast.*z-index|z-index.*toast" packages/bruno-app/src --type js

# Check the full Modal StyledWrapper to see all z-index values
echo -e "\n=== Full Modal StyledWrapper z-index context ==="
sed -n '20,45p' packages/bruno-app/src/components/Modal/StyledWrapper.js

# Verify window controls positioning context
echo -e "\n=== Window controls full context (lines 195-225) ==="
sed -n '195,225p' packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js

Repository: usebruno/bruno

Length of output: 4225


Reduce z-index: 1000 to avoid conflicts with react-hot-toast and simplify stacking hierarchy.

Window controls are set to z-index: 1000, but modals (the highest competing UI elements) only reach z-index: 20. The excessive z-index value creates an unnecessary stacking conflict: react-hot-toast defaults to z-index: 1000, meaning toasts and window controls will compete at the same level with unpredictable stacking order. Reduce window controls to z-index: 25–30 to maintain positioning above modals while allowing toasts to coexist below.

🤖 Prompt for AI Agents
In packages/bruno-app/src/components/AppTitleBar/StyledWrapper.js around lines
204 to 212, the .window-controls class uses z-index: 1000 which conflicts with
react-hot-toast (also 1000) and is unnecessary given modals only reach ~20;
change the z-index to a much lower value in the 25–30 range (e.g., 25 or 30) so
window controls remain above modals but below toasts, updating the CSS
declaration accordingly.

@bijin-bruno bijin-bruno merged commit 78ee99e into usebruno:main Dec 17, 2025
10 of 11 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 18, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants