Restore inactive-window glass tint (revert #180) - #182
Conversation
#180 removed the tint overlay on the assumption it only ever *added* opacity on unfocus. That was wrong: the overlay masks NSGlassEffectView's own inactive-window desaturation, which is native liquid-glass behavior with no API to disable. Without the overlay the raw system dimming makes a translucent window read as markedly *more* translucent when unfocused. Restore the fade-in tint. It keeps the Macterm-specific divergence from Ghostty of scaling the inactive tint by the window opacity, so a very translucent window doesn't jump to a near-opaque unfocused state and the inactive appearance stays proportional to the opacity slider. Focused, the tint is at alpha 0, so the focused glass still matches the chosen opacity exactly. Doc comments now record why the overlay is load-bearing so it isn't removed again.
📝 WalkthroughWalkthroughChangesThe window glass appearance now includes an opacity-scaled inactive tint derived from background color properties. Glass configuration receives the initial key-window state, and window delegate callbacks synchronize tint changes when key status changes. Inactive tint rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MainWindowCoordinator
participant WindowAppearance
participant MactermGlassView
MainWindowCoordinator->>WindowAppearance: window key status changes
WindowAppearance->>MactermGlassView: sync key status
MactermGlassView->>MactermGlassView: update inactive tint overlay
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Macterm/Views/WindowAppearance.swift`:
- Around line 163-170: Update updateKeyStatus so key-status changes animate
tintOverlay opacity through NSAnimationContext and
tintOverlay.animator().alphaValue, while preserving a direct alphaValue
assignment during the initial configuration path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 59398735-f28e-428a-b2c2-8a4b7af974be
📒 Files selected for processing (2)
Macterm/Views/MainWindow.swiftMacterm/Views/WindowAppearance.swift
| func updateKeyStatus(_ isKeyWindow: Bool, backgroundColor: NSColor) { | ||
| let tint = tintProperties(for: backgroundColor) | ||
| tintOverlay.layer?.backgroundColor = tint.color.cgColor | ||
| // Scale by the window opacity so the inactive tint stays within the | ||
| // translucency the user chose — otherwise an unfocused window reads as | ||
| // near-opaque regardless of the opacity slider. | ||
| tintOverlay.alphaValue = isKeyWindow ? 0 : tint.opacity * backgroundOpacity | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file, then inspect the relevant section and nearby call sites.
ast-grep outline Macterm/Views/WindowAppearance.swift --view expanded || true
printf '\n--- lines 120-210 ---\n'
sed -n '120,210p' Macterm/Views/WindowAppearance.swift
printf '\n--- call sites of updateKeyStatus ---\n'
rg -n "updateKeyStatus\(" Macterm/Views/WindowAppearance.swift Macterm -g '*.swift'Repository: thdxg/macterm
Length of output: 6453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the implementation and any surrounding animation helpers.
sed -n '1,220p' Macterm/Views/WindowAppearance.swiftRepository: thdxg/macterm
Length of output: 10588
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file outline ---\n'
ast-grep outline Macterm/Views/WindowAppearance.swift --view expanded || true
printf '\n--- relevant lines ---\n'
nl -ba Macterm/Views/WindowAppearance.swift | sed -n '140,190p'
printf '\n--- call sites ---\n'
rg -n "updateKeyStatus\(" Macterm/Views/WindowAppearance.swift Macterm -g '*.swift' || trueRepository: thdxg/macterm
Length of output: 242
🌐 Web query:
AppKit NSView alphaValue animator animation context documentation
💡 Result:
To animate an NSView's alphaValue in AppKit, you must use the view's animator proxy [1][2] within an NSAnimationContext [3]. Crucially, the view must be layer-backed for this animation to function [4][5][6]. Key Requirements and Usage 1. Layer-Backing: You must enable layer-backing for the view (or its superview if appropriate) by setting its wantsLayer property to true [5][6]. Without this, alphaValue changes will typically occur instantly rather than animating [5]. 2. Animator Proxy: Use the animator method to obtain a proxy object [1][2]. Any property changes made to this proxy are automatically animated if they are animatable [1]. 3. Animation Context: Wrap the property change in an NSAnimationContext to control parameters such as duration and timing functions [3][7]. Example (Swift) NSAnimationContext.runAnimationGroup({ context in context.duration = 1.0 // Set animation duration myView.animator.alphaValue = 0.0 // Animate to transparent }) { // Optional: Actions to perform after animation completes myView.isHidden = true } When executing these animations, ensure the view's wantsLayer property is set beforehand [6]: myView.wantsLayer = true // Then proceed with the animation block as shown above. Behavioral Notes - The NSAnimatablePropertyContainer protocol defines this proxy-based animation behavior [1][2]. - If you set a new value while an animation is in progress, the system will animate from the current interpolated value to the new target [1][2]. - You can nest NSAnimationContexts to apply different durations or configurations to specific groups of animations [3].
Citations:
- 1: https://sosumi.ai/documentation/appkit/nsanimatablepropertycontainer
- 2: https://developer.apple.com/documentation/appkit/nsanimatablepropertycontainer?changes=la%2Cla
- 3: https://developer.apple.com/documentation/appkit/nsanimationcontext
- 4: https://stackoverflow.com/questions/4149596/animator-of-nsview-does-not-animate-alpha-value-change
- 5: https://stackoverflow.com/questions/6335517/cant-get-id-animator-setalphavalue-0-0-to-work
- 6: https://stackoverflow.com/questions/26203139/animate-a-views-alpha-from-0-to-0-5
- 7: https://www.advancedswift.com/nsview-animations-guide/
Animate the key-status opacity change. alphaValue is assigned directly here, so the inactive tint snaps instead of fading. Wrap this in NSAnimationContext and set tintOverlay.animator().alphaValue for key changes; keep the initial configure path direct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Macterm/Views/WindowAppearance.swift` around lines 163 - 170, Update
updateKeyStatus so key-status changes animate tintOverlay opacity through
NSAnimationContext and tintOverlay.animator().alphaValue, while preserving a
direct alphaValue assignment during the initial configuration path.
github-actions
Bot
added
benchmark:regression
Window-state benchmark
|
1 participant
What this reverts
PR #180 removed the
MactermGlassViewtint overlay, believing it only ever made unfocused windows more opaque. That was a misread. This PR restores it.Why #180 was wrong
NSGlassEffectView(macOS 26 liquid glass) desaturates itself when its window is not key — native OS behavior, and the macOS 26 SDK header exposes no property to opt out (onlytintColor/style/cornerRadius/effectIsInteractive). The tint overlay's real job was to mask that system dimming with a fade-in tint.With the overlay gone, the raw system dimming showed through: on the glass path a translucent window read as markedly more translucent when unfocused. This only affected the liquid-glass appearance (glass enabled + opacity < 1.0), which is why it slipped through — the plain
backgroundColor+blur path never varies with focus, and a glass-off build looks fine.Ghostty itself keeps this overlay for the same reason (its
TerminalGlassViewfades the tint0 → tint.opacityon resign-key); it does not — and cannot cleanly — suppress the system dim.What's restored
tintOverlay+backgroundOpacity+updateKeyStatusinMactermGlassViewWindowAppearance.syncKeyStatusand itswindowDidBecomeKey/windowDidResignKeycall sitesNSColor.luminance/isLightColor/adjustingSaturationhelpersBehavior
Functionally identical to the pre-#180 code, with improved doc comments explaining why the overlay is load-bearing (so it isn't removed again).
(0.35 light / 0.85 dark) * backgroundOpacity. The* backgroundOpacityscaling is a deliberate Macterm divergence from Ghostty: Macterm has a full-range opacity slider, and the raw tint would jump a very-translucent window to a near-opaque unfocused state, ignoring the slider. Scaling keeps the inactive look proportional to the setting.Testing
mise run testgreen; format/lint clean.🤖 Generated with Claude Code
Summary by CodeRabbit