fix Ghostty background opacity toggle action handling#225
fix Ghostty background opacity toggle action handling#225vadimi wants to merge 2 commits intosupabitapp:mainfrom
Conversation
Handle Ghostty toggle-background-opacity action in the surface bridge and add a per-window opaque override in GhosttySurfaceView. This lets users temporarily force an opaque terminal background when transparency is enabled, while preserving fullscreen behavior.
sbertix
left a comment
There was a problem hiding this comment.
Thanks for the contribution 🙇♂️
| case GHOSTTY_ACTION_TOGGLE_BACKGROUND_OPACITY: | ||
| surfaceView?.toggleBackgroundOpacity() | ||
| return true |
There was a problem hiding this comment.
isBackgroundOpaque is per-view, but applyWindowBackgroundAppearance() modifies window-level properties. With splits, another surface's call to applyWindowBackgroundAppearance() would revert the toggle silently cause its own isBackgroundOpaque is still false. Could we move this state to window-level instead? Wdyt?
There was a problem hiding this comment.
ah yes, that totally makes sense. I've checked ghostty and it does it per tab, but I like per window much better. I've added isBackgroundOpaque on the NSWindow level. I haven't found any other custom state properties in the repo, so please let me know if you prefer another approach. Ghostty does it on the windowController level, but it's not used in this repo.
| private var lastSurfaceFocus: Bool? | ||
| private var eventMonitor: Any? | ||
| private var notificationObservers: [NSObjectProtocol] = [] | ||
| private var isBackgroundOpaque = false |
There was a problem hiding this comment.
Are we considering resetting isBackgroundOpaque when ghosttyRuntimeConfigDidChange fires? Cause if the user toggles opaque then changes the config, the flag persists and they'd need to toggle twice 🤔
There was a problem hiding this comment.
I've checked ghostty and it looks like it doesn't do it when I reload the config.
| guard runtime.backgroundOpacity() < 1 else { return } | ||
| guard let window, !window.styleMask.contains(.fullScreen) else { return } |
There was a problem hiding this comment.
NIT, both guards are re-checked inside applyWindowBackgroundAppearance(). Could we add a short comment explaining they prevent toggling isBackgroundOpaque when it'd have no visible effect? Otherwise it reads like duplication at first glance.
- define isBackgroundOpaque on NSWindow level - add comments inside toggleBackgroundOpacity function - change GHOSTTY_ACTION_TOGGLE_BACKGROUND_OPACITY return statement to use similar style as other actions
Handle Ghostty toggle-background-opacity action in the surface bridge and add a per-window opaque override in GhosttySurfaceView. This lets users temporarily force an opaque terminal background when transparency is enabled, while preserving fullscreen behavior.
This should fix #180