feat(darwin): Add NSPanel support for Spotlight-like windows#5024
feat(darwin): Add NSPanel support for Spotlight-like windows#5024Grantmartin2002 wants to merge 21 commits intowailsapp:v3-alphafrom
Conversation
This adds support for creating NSPanel-based windows instead of NSWindow, enabling Spotlight-like auxiliary windows that can appear over fullscreen apps without yanking the user out of fullscreen. Key changes: - Add UsePanel and PanelOptions to MacWindow configuration - Implement WebviewPanel class (NSPanel subclass) with same functionality as WebviewWindow - Add panelNew() function to create panels with NonactivatingPanel style - Override sendEvent: on both WebviewWindow and WebviewPanel to ensure KeyBindings work regardless of WKWebView first responder state - Add collection behavior and window level configuration options The NSPanel with NSWindowStyleMaskNonactivatingPanel allows the panel to receive keyboard input without activating the owning application, which is the key feature for Spotlight/Alfred-like interfaces. Fixes wailsapp#5023 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds macOS NSPanel support: new Changes
Sequence Diagram(s)sequenceDiagram
participant App as Go App
participant Router as Window Creation Router
participant PanelC as panelNew (C)
participant Panel as WebviewPanel
participant Delegate as Window Delegate
participant WebKit as WKWebView
App->>Router: Request window with WindowClass=Panel
Router->>PanelC: Call panelNew with PanelPreferences + WebviewPreferences
PanelC->>Panel: Instantiate WebviewPanel with styleMask & panel flags
PanelC->>WebKit: Create WKWebView and apply WebviewPreferences
Panel->>Panel: Embed WKWebView into content view
PanelC->>Delegate: Set delegate and register drag types
PanelC->>App: Return panel/window identifier
Note over Panel: Keyboard event flow
App->>Panel: User keydown
Panel->>Panel: sendEvent -> intercept keydown
Panel->>Panel: keyDown -> keyStringFromEvent
Panel->>Delegate: processWindowKeyDownEvent(mod+key)
Delegate->>App: Route to Go-side handler
sequenceDiagram
participant Comparison as Creation Paths
rect rgba(100,150,200,0.5)
Note over Comparison: windowNew (NSWindow)
participant WindowC as windowNew (C)
participant Window as WebviewWindow
WindowC->>Window: Create NSWindow, set WKWebView, delegate
end
rect rgba(150,100,200,0.5)
Note over Comparison: panelNew (NSPanel) — NEW
participant PanelC2 as panelNew (C)
participant Panel2 as WebviewPanel
PanelC2->>Panel2: Create NSPanel with panel-specific flags, set WKWebView, delegate
end
Note over Comparison: Both converge on delegate, WKWebView setup, and keyboard interception
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Grantmartin2002
force-pushed
the
v3-alpha-feature/issues-5023
branch
from
d8ef125 to
ffe6e91
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
v3/pkg/application/webview_window_darwin.m (1)
279-411: Extract shared key mapping/serialization for window + panel.
WebviewPanelandWebviewWindownow duplicate the full key parsing map. Consider a shared helper to avoid future divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.m` around lines 279 - 411, Both WebviewPanel and WebviewWindow duplicate the full key parsing/map logic; extract the mapping and serialization into a shared helper to prevent divergence. Create a shared utility (e.g., KeyMapper or WebviewKeyHelper) that exports a method like keyStringFromEvent:(NSEvent*)event and a serializer that builds the "modifier+key" string (used by keyDown:); replace the local implementations in WebviewWindow (keyStringFromEvent and keyDown:) and the analogous methods in WebviewPanel to call the shared helper, keeping existing method names (keyDown:, keyStringFromEvent:) as thin wrappers so callers/delegates (e.g., WebviewWindowDelegate, processWindowKeyDownEvent) continue to work unchanged.v3/pkg/application/webview_window_darwin.go (1)
183-289:panelNew()andwindowNew()are now heavily duplicated.This is likely to drift over time (preferences, delegates, drag-drop, and webview setup). Consider extracting a shared builder/helper for common setup and keep only style-mask/class-specific bits separate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.go` around lines 183 - 289, panelNew() and windowNew() contain nearly identical webview/delegate/preferences setup (WebviewWindowDelegate, WKWebViewConfiguration config, WKUserContentController, WKWebView webView, drag/drop wiring), so extract the common logic into a shared helper (eg. configureWebviewContent or buildWebviewForContainer) that accepts the container view/panel/window, id, WebviewPreferences, fraudulentWebsiteWarningEnabled, enableDragAndDrop and returns or assigns the configured WKWebView and delegate; leave only the style mask and panel-specific bits (frameless, floatingPanel, WebviewPanel creation) in panelNew(), and call the new helper to set preferences, set URL scheme handler, userContentController setup, assign navigation/UIDelegate, and attach drag view when requested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 202-203: The panel's floating state set in panelNew() via [panel
setFloatingPanel:YES] is being overridden later by an unconditional [window
setLevel:MacWindowLevelNormal] call; update the logic that sets default window
level (the code paths around setWindowLevel / setLevel and MacWindowLevelNormal)
to skip applying a default level when WindowClass == NSPanel and
PanelOptions.FloatingPanel is true (or alternatively detect when
setFloatingPanel was explicitly set and avoid calling setLevel), ensuring
panelNew's setFloatingPanel remains effective; look for functions/methods named
panelNew, setFloatingPanel, setWindowLevel/setLevel, and references to
PanelOptions.FloatingPanel, WindowClass/NSPanel and MacWindowLevelNormal to
apply the conditional.
In `@v3/pkg/application/webview_window_darwin.m`:
- Around line 33-41: sendEvent: is causing duplicate key handling by
unconditionally invoking [self keyDown:event] before [super sendEvent:event];
update the sendEvent: implementations in WebviewWindow and WebviewPanel to only
call [self keyDown:event] when the window's firstResponder is not the WKWebView
(or a subview of it) — i.e., obtain the firstResponder from the window and skip
the manual keyDown dispatch if it isKindOfClass: WKWebView (or resides within
the WKWebView), otherwise call [self keyDown:event] and then [super
sendEvent:event] as before.
In `@website/src/pages/changelog.mdx`:
- Line 18: Fix the spelling typo in the changelog entry: replace the misspelled
word "auxillary" with "auxiliary" in the sentence describing NSPanel support
(the line mentioning "Panels serve as auxillary windows..."); ensure the rest of
the text (references to `NSWindowStyleMaskNonactivatingPanel`,
`Mac.WindowClass`, and `Mac.PanelOptions`) remains unchanged.
---
Nitpick comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 183-289: panelNew() and windowNew() contain nearly identical
webview/delegate/preferences setup (WebviewWindowDelegate,
WKWebViewConfiguration config, WKUserContentController, WKWebView webView,
drag/drop wiring), so extract the common logic into a shared helper (eg.
configureWebviewContent or buildWebviewForContainer) that accepts the container
view/panel/window, id, WebviewPreferences, fraudulentWebsiteWarningEnabled,
enableDragAndDrop and returns or assigns the configured WKWebView and delegate;
leave only the style mask and panel-specific bits (frameless, floatingPanel,
WebviewPanel creation) in panelNew(), and call the new helper to set
preferences, set URL scheme handler, userContentController setup, assign
navigation/UIDelegate, and attach drag view when requested.
In `@v3/pkg/application/webview_window_darwin.m`:
- Around line 279-411: Both WebviewPanel and WebviewWindow duplicate the full
key parsing/map logic; extract the mapping and serialization into a shared
helper to prevent divergence. Create a shared utility (e.g., KeyMapper or
WebviewKeyHelper) that exports a method like keyStringFromEvent:(NSEvent*)event
and a serializer that builds the "modifier+key" string (used by keyDown:);
replace the local implementations in WebviewWindow (keyStringFromEvent and
keyDown:) and the analogous methods in WebviewPanel to call the shared helper,
keeping existing method names (keyDown:, keyStringFromEvent:) as thin wrappers
so callers/delegates (e.g., WebviewWindowDelegate, processWindowKeyDownEvent)
continue to work unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
v3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_darwin.hv3/pkg/application/webview_window_darwin.mv3/pkg/application/webview_window_options.gowebsite/src/pages/changelog.mdx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/content/docs/features/windows/options.mdx (1)
831-864:⚠️ Potential issue | 🔴 CriticalUnescaped
{}in prose will break MDX parsing (lines 831, 864).In Astro Starlight
.mdxfiles, a left curly brace{starts a JavaScript expression. Wrap these code references in backticks to render them literally.Proposed fix
-**Note:** NSWindow fullscreen overlay requires application.Options{ Mac: application.MacOptions{ ActivationPolicy: application.ActivationPolicyAccessory } } +**Note:** NSWindow fullscreen overlay requires `application.Options{ Mac: application.MacOptions{ ActivationPolicy: application.ActivationPolicyAccessory } }` -**Note:** Unlike NSWindow, NSPanel fullscreen overlay does NOT require application.Options{ Mac: application.MacOptions{ ActivationPolicy: application.ActivationPolicyAccessory } } +**Note:** Unlike NSWindow, NSPanel fullscreen overlay does **not** require `application.Options{ Mac: application.MacOptions{ ActivationPolicy: application.ActivationPolicyAccessory } }`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/content/docs/features/windows/options.mdx` around lines 831 - 864, The MDX contains unescaped curly-brace expressions that break parsing; wrap the literal references containing braces in inline code ticks (e.g., `application.Options{ Mac: application.MacOptions{ ActivationPolicy: application.ActivationPolicyAccessory } }`) and similarly backtick the examples and type names that include braces or look like struct literals (references such as `Mac: application.MacWindow{ ... }`, `application.MacWindowCollectionBehaviorCanJoinAllSpaces | application.MacWindowCollectionBehaviorFullScreenAuxiliary`, `application.MacWindowLevelFloating`, `MacWindowClass`, `NSWindow`, `NSPanel`, `MacPanelOptions`, and the panel option names) so the `{}` are rendered literally and MDX parsing is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/src/content/docs/features/windows/options.mdx`:
- Around line 831-864: The MDX contains unescaped curly-brace expressions that
break parsing; wrap the literal references containing braces in inline code
ticks (e.g., `application.Options{ Mac: application.MacOptions{
ActivationPolicy: application.ActivationPolicyAccessory } }`) and similarly
backtick the examples and type names that include braces or look like struct
literals (references such as `Mac: application.MacWindow{ ... }`,
`application.MacWindowCollectionBehaviorCanJoinAllSpaces |
application.MacWindowCollectionBehaviorFullScreenAuxiliary`,
`application.MacWindowLevelFloating`, `MacWindowClass`, `NSWindow`, `NSPanel`,
`MacPanelOptions`, and the panel option names) so the `{}` are rendered
literally and MDX parsing is preserved.
- Fix FloatingPanel level override: skip setting default WindowLevel when NSPanel has FloatingPanel=true, preserving setFloatingPanel behavior - Clean up sendEvent override comments - Fix typo in changelog (auxillary -> auxiliary) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 1365-1371: Calls to w.setAlwaysOnTop(options.AlwaysOnTop) are
overwriting a floating panel's level (setting NSNormalWindowLevel when
AlwaysOnTop=false) and undoing PanelOptions.FloatingPanel; update the logic so
setAlwaysOnTop does not force MacWindowLevelNormal for windows that are panels
with PanelOptions.FloatingPanel==true (i.e., when macOptions.WindowClass ==
NSPanel && macOptions.PanelOptions.FloatingPanel). Concretely, change the places
that call w.setAlwaysOnTop (the two sites that currently force normal level) to
check the same guard used for setWindowLevel (or update setAlwaysOnTop to
early-return when the window is a floating panel) so floating panels retain
NSFloatingWindowLevel.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/src/content/docs/features/windows/options.mdxv3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_darwin.mwebsite/src/pages/changelog.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- website/src/pages/changelog.mdx
- docs/src/content/docs/features/windows/options.mdx
leaanthony
left a comment
leaanthony
left a comment
There was a problem hiding this comment.
Hey @Grantmartin2002 👋
Thanks for this! NSPanel support is something people have been asking about and this is a solid start. Really nice job on the docs too — the Spotlight-like panel example and the note about ActivationPolicyAccessory not being needed for panels is great 🙏
I've got some feedback before we can merge this:
1. sendEvent: override on WebviewWindow is a breaking change
This is the big one. The sendEvent: override is being added to both WebviewPanel and WebviewWindow. On WebviewWindow this changes behavior for all existing users — every keystroke will now fire Go keybindings regardless of first responder state, which means typing in a <input> field would also trigger keybindings. You called this out yourself in the PR description (good instinct!).
This needs to be opt-in, not default. Please remove the sendEvent: override from WebviewWindow (keep the existing keyDown: behavior). For WebviewPanel it's fine since it's new, but ideally add an option for it there too so users can control it.
2. Double-dispatch of key events
With sendEvent: calling [self keyDown:event] and then [super sendEvent:event], the normal event dispatch chain will call keyDown: again when the window is first responder — so processWindowKeyDownEvent fires twice per keystroke.
If you keep sendEvent: (for panels), don't call [super sendEvent:event] after handling key-down events — that should prevent the double-fire.
3. Inconsistent memory management
In windowNew() the delegate gets autoreleased, but WebviewPanel has a custom setDelegate: that does retain/release and a dealloc cleanup. These two patterns conflict — the autorelease in panelNew() combined with the retain in setDelegate: will over-retain.
I'd prefer we stick with the autorelease pattern consistently unless you have a specific reason for the explicit retain/release approach? Also worth noting that WebviewWindow is missing the userContentController cleanup that you added to WebviewPanel.dealloc — that's likely a pre-existing leak, so nice catch if intentional!
4. Rename NSWindow/NSPanel constants
Using NSWindow and NSPanel as Go constant names masks the actual Cocoa class names, which gets confusing. Please rename these — something like MacWindowClassWindow and MacWindowClassPanel would work, or whatever feels right as long as they don't shadow the Cocoa types.
5. Code duplication in .m — consider a protocol or macro
The WebviewPanel implementation is nearly identical to WebviewWindow (init, responder methods, etc). The refactoring of keyStringFromEvent → keyStringFromKeyEvent and dispatchKeyDownEvent as static helpers was a good move. Could you look into doing something similar for the remaining duplicated bits? Even just a comment acknowledging the duplication with a TODO would be fine if it's tricky to extract cleanly.
6. panelNew has a lot of bool parameters
void* panelNew(unsigned int id, int width, int height, bool fraudulentWebsiteWarningEnabled,
bool frameless, bool enableDragAndDrop, struct WebviewPreferences preferences,
bool floatingPanel, bool becomesKeyOnlyIfNeeded, bool nonactivatingPanel, bool utilityWindow)4 trailing bools is easy to get wrong at call sites. Could you wrap the panel-specific ones in a struct PanelPreferences (similar to the existing WebviewPreferences)? That way the call site is self-documenting.
7. Docs
Great job here overall! Will just need a minor update once the constant names change (point 4).
Happy to answer any questions on these. The core feature is really useful — just want to make sure we get the details right before it lands. Thanks again for the contribution! 🙌
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
v3/pkg/application/webview_window_darwin.go (1)
1467-1479:⚠️ Potential issue | 🟠 MajorFloating NSPanel is still reset to normal level after first navigation.
Line 1469 and Line 1479 call
w.setAlwaysOnTop(options.AlwaysOnTop)unconditionally. WhenAlwaysOnTopis false,windowSetAlwaysOnTopforcesNSNormalWindowLevel, which undoes a previously configured floating panel level.🛠️ Suggested fix
// We need to wait for the HTML to load before we can execute the javascript + shouldApplyAlwaysOnTop := !(macOptions.WindowClass == MacWindowClassPanel && + macOptions.PanelPreferences.FloatingPanel.IsSet() && + macOptions.PanelPreferences.FloatingPanel.Get() && + !options.AlwaysOnTop) + w.parent.OnWindowEvent(events.Mac.WebViewDidFinishNavigation, func(_ *WindowEvent) { InvokeAsync(func() { @@ if !options.Hidden { w.parent.Show() w.setHasShadow(!options.Mac.DisableShadow) - w.setAlwaysOnTop(options.AlwaysOnTop) + if shouldApplyAlwaysOnTop { + w.setAlwaysOnTop(options.AlwaysOnTop) + } } else { @@ } w.setHasShadow(!options.Mac.DisableShadow) - w.setAlwaysOnTop(options.AlwaysOnTop) + if shouldApplyAlwaysOnTop { + w.setAlwaysOnTop(options.AlwaysOnTop) + } cancel() }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.go` around lines 1467 - 1479, The floating NSPanel level is being reset because w.setAlwaysOnTop(options.AlwaysOnTop) is called unconditionally; when options.AlwaysOnTop is false the underlying windowSetAlwaysOnTop forces NSNormalWindowLevel and overwrites a floating panel level. Fix by only invoking w.setAlwaysOnTop(...) when options.AlwaysOnTop is true (i.e., guard both calls at the top-level and inside the OnWindowEvent cancel handler), leaving the window level untouched when AlwaysOnTop is false so previously configured floating panel levels are preserved; update references around the calls to w.setAlwaysOnTop and any logic that depends on windowSetAlwaysOnTop accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_options.go`:
- Around line 492-507: The comments use the term "PanelOptions" but the exported
type/field is named "PanelPreferences" (see MacPanelPreferences and any
references in the MacWindowClass comment block); update all comment text that
mentions "PanelOptions" to "PanelPreferences" so the documentation matches the
actual exported symbol names (e.g., change "PanelOptions contains options for
NSPanel windows" to "PanelPreferences contains options for NSPanel windows" and
any other occurrences).
---
Duplicate comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 1467-1479: The floating NSPanel level is being reset because
w.setAlwaysOnTop(options.AlwaysOnTop) is called unconditionally; when
options.AlwaysOnTop is false the underlying windowSetAlwaysOnTop forces
NSNormalWindowLevel and overwrites a floating panel level. Fix by only invoking
w.setAlwaysOnTop(...) when options.AlwaysOnTop is true (i.e., guard both calls
at the top-level and inside the OnWindowEvent cancel handler), leaving the
window level untouched when AlwaysOnTop is false so previously configured
floating panel levels are preserved; update references around the calls to
w.setAlwaysOnTop and any logic that depends on windowSetAlwaysOnTop accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb44af40-07ad-41ad-88d6-82c534a6ca52
📒 Files selected for processing (2)
v3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_options.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
v3/pkg/application/webview_window_darwin.go (1)
1379-1385:⚠️ Potential issue | 🟠 MajorFloating panels can still be downgraded during initial show flow.
The guard here helps, but
w.setAlwaysOnTop(options.AlwaysOnTop)at Line 1455 and Line 1465 still sets normal level whenAlwaysOnTop == false, which cancelsPanelPreferences.FloatingPanel == true.Suggested fix
@@ - w.parent.OnWindowEvent(events.Mac.WebViewDidFinishNavigation, func(_ *WindowEvent) { + shouldApplyAlwaysOnTop := !(macOptions.WindowClass == MacWindowClassPanel && + macOptions.PanelPreferences.FloatingPanel && + !options.AlwaysOnTop) + + w.parent.OnWindowEvent(events.Mac.WebViewDidFinishNavigation, func(_ *WindowEvent) { @@ if !options.Hidden { w.parent.Show() w.setHasShadow(!options.Mac.DisableShadow) - w.setAlwaysOnTop(options.AlwaysOnTop) + if shouldApplyAlwaysOnTop { + w.setAlwaysOnTop(options.AlwaysOnTop) + } } else { @@ } w.setHasShadow(!options.Mac.DisableShadow) - w.setAlwaysOnTop(options.AlwaysOnTop) + if shouldApplyAlwaysOnTop { + w.setAlwaysOnTop(options.AlwaysOnTop) + } cancel() }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.go` around lines 1379 - 1385, w.setAlwaysOnTop currently forces the window level to normal when options.AlwaysOnTop is false, which overrides a floating panel set earlier; change the logic in the code that calls w.setAlwaysOnTop (the branch handling macOptions.AlwaysOnTop) so it does not downgrade a panel with macOptions.WindowClass == MacWindowClassPanel and macOptions.PanelPreferences.FloatingPanel == true (or when the current level is already a floating level). Instead, only raise the level when AlwaysOnTop is true, and skip setting the normal level when AlwaysOnTop is false for floating panels — keep using setWindowLevel only when a change is actually needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 1379-1385: w.setAlwaysOnTop currently forces the window level to
normal when options.AlwaysOnTop is false, which overrides a floating panel set
earlier; change the logic in the code that calls w.setAlwaysOnTop (the branch
handling macOptions.AlwaysOnTop) so it does not downgrade a panel with
macOptions.WindowClass == MacWindowClassPanel and
macOptions.PanelPreferences.FloatingPanel == true (or when the current level is
already a floating level). Instead, only raise the level when AlwaysOnTop is
true, and skip setting the normal level when AlwaysOnTop is false for floating
panels — keep using setWindowLevel only when a change is actually needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5fde02ee-d1ac-4858-96df-43b1add07fd7
📒 Files selected for processing (4)
docs/src/content/docs/features/windows/options.mdxv3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_options.gowebsite/src/pages/changelog.mdx
|
@leaanthony Thank you for the thorough review! Few things: 1. sendEvent: override on WebviewWindow is a breaking changeAgreed. Did it originally to keep behavior consistent and considered an opt in option, but honestly not necessary. will remove 2. Double-dispatch of key events
3. Inconsistent memory managementcan you point me to the exact lines you are referring to? Memory management looks identical to me 4. Rename NSWindow/NSPanel constantsDone 5. Code duplication in .m — consider a protocol or macroI should be able to do something for consolodiating panelNew and windowNew (EDIT: DONE). For the interface implementation, I could use a macro, but honestly its not that much duplicated code here. Probably not worth it due to readibility issues imo. What do you think? Will do what you think best :) 6. panelNew has a lot of bool parametersDone. Also renamed "options" to "preferences" to pattern match. 7. DodcsDone. Thank you again!!!! |
leaanthony
left a comment
leaanthony
left a comment
There was a problem hiding this comment.
Thanks for your patience on this 🙏
I've added a couple of comments on filenames but not seeing any major issues here!
|
|
||
| @end | ||
|
|
||
| @interface WebviewPanel : NSPanel |
There was a problem hiding this comment.
I think this should live in webview_panel_darwin.h
| } | ||
| @end | ||
|
|
||
| @implementation WebviewPanel |
There was a problem hiding this comment.
I think this should live in webview_panel_darwin.h
There was a problem hiding this comment.
hmmm, we will have to duplicate dispatchKeyDownEvent in that case. are you ok with that?
There was a problem hiding this comment.
also, i consolidated windowPanel and windowNew to just use windowNew with a flag check to avoid having to maintain two methods if/when we add properties to general window behavior. With the change you propose, I'd also have to make a webview_panel_darwin.go right? So i'd have to separate the methods again and duplicate more logic.
There was a problem hiding this comment.
Not having the dupes makes sense to me
There was a problem hiding this comment.
If they are using the same build flags then it won't be dupes? I'm not sure I understand the issue here. I was only suggesting moving some of the code to different named files.
Split WebviewPanel interface and implementation out of
webview_window_darwin.{h,m} into new webview_panel_darwin.{h,m}.
Remove `static` from the shared key event helpers so they can be
called from the panel file without duplication — one definition in
webview_window_darwin.m, declared in the shared header.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hey @Grantmartin2002 — the "dupes" concern can be resolved without any code duplication. The two helper functions ( I've pushed a branch showing the approach: Changes:
Feel free to pull those commits into your fork if you want to keep this PR going, or I can merge from there. |
3 participants
This adds support for creating NSPanel-based windows instead of NSWindow, enabling auxiliary windows that can appear over fullscreen apps without activating the application.
Key changes
WindowClass MacWindowClasstoMacWindowconfiguration withMacWindowClassWindow(default) andMacWindowClassPaneloptionsMacPanelPreferencesstruct with panel-specific options:FloatingPanel- float above other windowsBecomesKeyOnlyIfNeeded- become key only when neededNonactivatingPanel- receive input without activating the appUtilityWindow- utility window styleWebviewPanelclass (NSPanel subclass) with same functionality asWebviewWindowpanelNew()function to create panels with configurable style masksThe
NSWindowStyleMaskNonactivatingPanelstyle allows the panel to receive keyboard input without activating the owning application.Fixes #5023
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
Documentation