Repository navigation
Conversation
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Refactored the macOS traffic-light inset into two pure helpers —
macTrafficLightsWidthandshouldShowMacTrafficLightPad— insrc/renderer/src/lib/desktop-window-chrome.ts, both gated on!isWebClient. MAC_TRAFFIC_LIGHTS_WIDTHinapp-window-chrome.tsnow derives frommacTrafficLightsWidth(...), so a paired Mac web client gets0pxinstead of80px.TitlebarLeftControls.tsxrenders the pad viashouldShowMacTrafficLightPad, falling through to thepl-2branch for a Mac web client (same as desktop fullscreen).- Added unit tests covering desktop macOS, Mac web client, and Windows/Linux.
Behavior is unchanged for desktop macOS, and the web client now starts at its real top-left. Both the width token and the pad element key off the same isPairedWebClientWindow() global that hasCustomTitleBar already depends on, so there is no new module-load timing risk. The added tests are exact-value assertions that would fail if either predicate regressed, and the full file passes (6 tests).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between dde585c2964c91046f045fc33ef383d6a2721d76 and d34dabb. 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe app shell now sets the macOS traffic-light inset to 0px for paired web clients and 80px for desktop clients. It renders traffic-light padding only for non-fullscreen macOS desktop windows. Tests cover these cases, along with Windows and Linux behavior. Priority: ⬇️ Low Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The change removes the empty macOS titlebar gap for paired web clients and leaves desktop macOS, Windows and Linux spacing as before. The tests cover these cases. The remaining uncertainty is that the author did not check a live page in Chrome on macOS, and that is not a merge blocker. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
A browser tab has no native window buttons, but a Mac user agent still reserved that left gap. Desktop macOS keeps the inset. Co-authored-by: Cursor <cursoragent@cursor.com>
The inset token and the pad were each passed isWebClient by their own call site, so a test of the helpers could stay green while one site stopped reading the flag. Co-authored-by: Cursor <cursoragent@cursor.com>
dde585c to
d34dabb
Compare
Sync update (
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Unified the paired-web read:
macTrafficLightChromeForWindow({ isMac, isFullScreen })indesktop-window-chrome.tsnow reads__ORCA_WEB_CLIENT__once and returns both the'80px' | '0px'width and theshowPadflag, replacing two separate helper calls. MAC_TRAFFIC_LIGHTS_WIDTHinapp-window-chrome.tsnow derives from the combined helper (stillisFullScreen: false, so the width token is unchanged in fullscreen).TitlebarLeftControls.tsxrenders the pad from the combined helper'sshowPad, so a Mac web client falls through to the existingpl-2branch.- Added a test that stubs
__ORCA_WEB_CLIENT__and asserts the width and pad together, closing the wiring gap greptile flagged.
Centralizing the single __ORCA_WEB_CLIENT__ read means the width token and the pad can no longer drift — the exact gap the previous two-independent-call-sites shape left. Desktop macOS is unchanged (80px plus the pad until fullscreen); the paired Mac web client gets 0px and no pad. The focused suite passes (7 tests).
ℹ️ Nitpicks
docs/STYLEGUIDE.md:304still states macOS reserves an 80px gutter withtitlebar-traffic-light-padas unconditional. That is now desktop-app-only; a short qualifier noting the paired web client reserves none would keep the guide accurate.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

ELI5
Opening Orca in a browser on a Mac left an empty gap at the top left, where the desktop app's red, yellow, and green window buttons would be. The browser has no such buttons, so that gap is gone. The desktop Mac app still leaves room for them.
What Changed
Before, any Mac user agent reserved an 80px traffic-light inset and rendered
titlebar-traffic-light-pad, including the paired web client. After, that inset is reserved only for the desktop Mac window. Fullscreen still hides the pad element and still keeps the width token, which is what the desktop already did.Why
The web client already skips Windows and Linux custom window controls. The Mac inset was the remaining desktop-only gap. Gating the existing chrome helper is smaller than a separate web layout.
Linked Issue
Fixes #19299
Visual Proof
N/A. This session did not open a paired browser client. The change is which chrome helper returns a width, covered by unit tests below.
Testing
vitest run --config config/vitest.config.ts src/renderer/src/lib/desktop-window-chrome.test.ts— 6 passed.Covered: desktop macOS keeps 80px and shows the pad until fullscreen; the paired web client on a Mac gets 0px and no pad, fullscreen or not; Windows and Linux stay at 0px.
Not measured: a live
orca servepage in Chrome on macOS, or the desktop Mac window next to it.AI Disclosure
Cursor Grok 4.7 drafted this change.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Desktop macOS traffic lights are unchanged. SSH and mobile are not involved.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)