Skip to content

fix(desktop): prevent sidebar buttons from shifting under macOS traffic lights on zoom - #45266

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-fullscreen-zoom-layout
Open

fix(desktop): prevent sidebar buttons from shifting under macOS traffic lights on zoom#45266
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-fullscreen-zoom-layout

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the sidebar and session-swap buttons shifting underneath macOS traffic-light controls when the window is maximized via "zoom" (double-click titlebar). The viewport fullscreen fallback incorrectly treated zoom as fullscreen, pinning the titlebar controls to the left edge where they collided with the still-visible traffic lights.

Related Issue

Fixes #45264

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/src/app/shell/app-shell.tsx: Updated viewportIsFullscreen() to distinguish true macOS fullscreen (menu bar hidden, screen.availHeight === screen.height) from zoom/maximize (menu bar visible, availHeight < screen.height). Exported the function for testing.
  • apps/desktop/src/app/shell/titlebar.test.ts: Added 4 tests for viewportIsFullscreen covering windowed, true fullscreen, macOS zoom ([Bug]: Sidebar and session swap buttons move when app is full screen on macOS #45264 regression), and Windows/Linux cases.

How to Test

  1. Open Hermes Desktop on macOS in a windowed state
  2. Observe sidebar/swap buttons are to the right of the red/yellow/green traffic lights
  3. Double-click the titlebar to zoom (maximize) the window
  4. Verify the buttons stay to the right of the traffic lights (do NOT shift underneath them)
  5. Enter true fullscreen (green button) — buttons should pin to the left edge (traffic lights hidden)
  6. Run: cd apps/desktop && npx vitest run src/app/shell/titlebar.test.ts — all 8 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run npx vitest run src/app/shell/titlebar.test.ts and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

⚠️ GitNexus unavailable (stale index) — grep-based fallback used.

  • Checked files: apps/desktop/src/app/shell/app-shell.tsx, apps/desktop/src/app/shell/titlebar.ts, apps/desktop/src/app/shell/titlebar.test.ts
  • Blast radius: LOW — isolated to viewport fullscreen detection, no cross-module impact
  • Related patterns: titlebarControlsPosition() unchanged; only the viewportIsFullscreen fallback refined

…ic lights on zoom

The viewportIsFullscreen() fallback returned true when the window filled
the screen (innerWidth >= screen.width && innerHeight >= screen.height),
but on macOS "zoom" (double-click titlebar to maximize) the traffic-light
buttons are still visible at the top-left. This caused titlebarControls
to pin to TITLEBAR_EDGE_INSET (14px), placing the sidebar/swap buttons
directly underneath the traffic lights.

Fix by checking screen.availHeight vs screen.height: true macOS fullscreen
hides the menu bar (availHeight === height), while zoom keeps it visible
(availHeight < height). Only treat the viewport as fullscreen when the
menu bar is also hidden.

Fixes NousResearch#45264
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this to the renderer fallback. The premise is still present on current main: apps/desktop/src/app/shell/app-shell.tsx:62-63 derives fullscreen only from viewport dimensions, then passes it into the edge-positioning path at :83-89.

Problems

  • The proposed screen.availHeight < screen.height exclusion is unconditionally cross-platform. The existing layout contract explicitly treats macOS traffic lights separately from Windows/Linux controls in apps/desktop/src/app/shell/titlebar.ts:36-41, but the new predicate has no platform gate. The new Windows/Linux test covers only availHeight === height, so it does not exercise this changed branch for a reduced available work area.

Suggested changes

  • Apply the availHeight distinction only on macOS, following the existing renderer platform-detection pattern in apps/desktop/src/lib/keybinds/combo.ts:13; preserve the current dimension fallback elsewhere.
  • Add a non-mac reduced-available-height regression test alongside the macOS zoom test.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Thanks for the PR — your diagnosis of the viewportIsFullscreen() heuristic was correct.

However, the target file (app-shell.tsx) was deleted entirely in commit 0f398f8e9 ("feat(desktop): focused-session-aware titlebar + statusbar", 2026-07-13), which rewrote the titlebar chrome to rely exclusively on Electron's native win.isFullScreen() API. This API correctly distinguishes macOS zoom from true fullscreen, resolving #45264.

Since the file this PR modifies no longer exists and the underlying issue is fixed on main, this PR can be closed as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Sidebar and session swap buttons move when app is full screen on macOS

4 participants