Skip to content

fix: wrong popup shadow rendering after #652 and #566#707

Merged
FuJacob merged 1 commit into
FuJacob:mainfrom
akramj13:codex/704-fix-popup-shadow
Jun 13, 2026
Merged

fix: wrong popup shadow rendering after #652 and #566#707
FuJacob merged 1 commit into
FuJacob:mainfrom
akramj13:codex/704-fix-popup-shadow

Conversation

@akramj13

@akramj13 akramj13 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

This fixes the shadow and double-outline regression introduced by the interaction between the fixes for #566 and #652 on macOS 26. The menu bar popover now binds to and configures the underlying host NSWindow, ensuring the native host window remains visually transparent while a single SwiftUI-rendered panel surface owns the visible background, border, and shadow.

Validation

  • Built and ran Cotabby on macOS 26.
  • Verified that the menu bar popover no longer displays an extra rounded outline around the popup.
  • Verified that the popup shadow appears correctly and matches the native popover shape.
  • Verified that light and dark mode appearance changes continue to render correctly.
  • Verified that existing popover dismissal behavior still works after binding to the host window.

Linked issues

Fixes #704

Risk / rollout notes

  • Changes are limited to the menu bar popover presentation path on macOS 26.
  • Introduces direct configuration of the underlying NSWindow used by MenuBarExtra(.window).
  • No data model, settings, migration, or persistence changes.
  • Older macOS versions continue to use their existing presentation paths unchanged.

Solves #704

Greptile Summary

This PR fixes a shadow and double-outline regression on macOS 26 where the MenuBarExtra(.window) popover rendered both the native host window's rounded chrome and the SwiftUI fill layer as separate visible surfaces. The fix clears the native NSWindow chrome via a new MenuBarWindowChromeConfigurator helper and transfers visual ownership of the background, border, and shadow entirely to a SwiftUI-rendered RoundedRectangle.

  • MenuBarWindowChromeConfigurator (macOS 26 only): sets window.isOpaque = false, backgroundColor = .clear, hasShadow = false, and strips layer chrome from contentView and its superview so the host window is fully transparent.
  • MenuBarWindowBackgroundModifier (macOS 26 branch): adds a SwiftUI .shadow() and .overlay stroke border to the fill rect, making this SwiftUI layer the sole visible rounded surface.
  • MenuBarPopoverDismisserBinder: gains an onWindowBind callback so MenuBarView can invoke configureMenuBarWindowIfNeeded the moment the host window becomes available.

Confidence Score: 4/5

Safe to merge for the targeted macOS 26 regression; older macOS paths are untouched.

The macOS 26 fix is carefully gated and the changed logic is well-reasoned. The two areas that warrant a second look are the window.invalidateShadow() call (harmless dead code after hasShadow = false) and the direct manipulation of contentView?.superview — AppKit's internal NSThemeFrame — which is undocumented and could quietly stop working in a future macOS 26 point release if the view hierarchy gains an intermediate layer.

MenuBarView.swift — specifically the MenuBarWindowChromeConfigurator.configure method and its access to contentView?.superview.

Important Files Changed

Filename Overview
Cotabby/UI/MenuBarView.swift Adds MenuBarWindowChromeConfigurator (macOS 26 only) that strips the host NSWindow's native chrome and shadow, then draws a single SwiftUI-rendered rounded panel with its own fill, border, and shadow. Two minor concerns: invalidateShadow() is a no-op after hasShadow = false, and accessing contentView?.superview (NSThemeFrame) is fragile undocumented API.
Cotabby/UI/MenuBarPopoverDismisser.swift Extends MenuBarPopoverDismisserBinder with an onWindowBind callback (defaulting to a no-op) so callers can react to the host window becoming available. The callback is correctly threaded through makeNSView and updateNSView, and fires in viewDidMoveToWindow guarded behind a non-nil window check. Straightforward and safe.

Sequence Diagram

sequenceDiagram
    participant SwiftUI
    participant Binder as MenuBarPopoverDismisserBinder
    participant WBV as WindowBindingView
    participant Configurator as MenuBarWindowChromeConfigurator
    participant NSWin as NSWindow

    SwiftUI->>Binder: makeNSView(context:)
    Binder->>WBV: create view, set onWindowBind
    SwiftUI->>WBV: add to window hierarchy
    WBV->>WBV: viewDidMoveToWindow()
    WBV->>Configurator: onWindowBind(window)
    Configurator->>NSWin: "isOpaque = false"
    Configurator->>NSWin: "backgroundColor = .clear"
    Configurator->>NSWin: "hasShadow = false"
    Configurator->>NSWin: contentView layer — clear bg, no border, no shadow
    Configurator->>NSWin: contentView.superview layer — clear bg, no border, no shadow
    Note over SwiftUI,NSWin: SwiftUI RoundedRectangle now owns fill + shadow + border
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(MenuBar): enhance popover window con..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Comment on lines +537 to +543
for backingView in [window.contentView, window.contentView?.superview].compactMap({ $0 }) {
backingView.wantsLayer = true
backingView.layer?.backgroundColor = NSColor.clear.cgColor
backingView.layer?.borderWidth = 0
backingView.layer?.shadowOpacity = 0
backingView.layer?.masksToBounds = false
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Modifying contentView?.superview (NSThemeFrame) is fragile internal API

window.contentView?.superview resolves to AppKit's private NSThemeFrame. While compactMap safely handles a nil superview, setting wantsLayer, borderWidth, shadowOpacity, and masksToBounds directly on that internal view relies on an undocumented, stable view-hierarchy assumption. A future macOS 26 point release could introduce an intermediate wrapper between NSWindow and its contentView, silently bypassing these property writes and re-exposing the double-outline. Consider logging a warning when contentView?.superview is the class name you expect so regressions surface early rather than silently.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code

Comment on lines +544 to +546
window.invalidateShadow()
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 window.invalidateShadow() is a no-op here because window.hasShadow was set to false on the line above — there is no native Cocoa shadow left for the window manager to redraw. The shadow is owned entirely by SwiftUI's .shadow() modifier on the fill rectangle. Remove or replace with an explanatory comment to avoid implying it has an effect.

Suggested change
window.invalidateShadow()
}
}
// Note: invalidateShadow() is intentionally omitted here — hasShadow is false so the
// native window manager shadow is disabled, and the visible shadow is owned by the
// SwiftUI .shadow() modifier on the fill rectangle.
}
}

Fix in Codex Fix in Claude Code

@FuJacob FuJacob merged commit 61475f5 into FuJacob:main Jun 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Wrong shadow around popup after fix #652, #566

2 participants