[V3 Alpha] Improves macOS Window menu support - #4588
Conversation
WalkthroughAdds macOS Windows menu support by setting NSApp.WindowsMenu when a submenu has the WindowMenu role, updates the menu example to use NewWithOptions with Name/Title, and documents the change in the unreleased changelog. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant Menu as Menu Builder
participant NSApp as macOS NSApp
participant C as C Bridge
App->>Menu: Build menus
loop For each menu/submenu item
Menu->>Menu: Check role
alt role == WindowMenu
Menu->>C: addWindowsMenu(nsSubmenu)
C->>NSApp: set WindowsMenu = nsSubmenu
Note over NSApp: Windows menu registered
else role == ServicesMenu
Menu->>C: setServicesMenu(nsSubmenu)
else
Menu->>Menu: No special handling
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 |
dosubot
Bot
added
the
size:S
|
Related Documentation Checked 1 published document(s). No updates required. You have 2 draft document(s). Publish docs to keep them always up-to-date How did I do? Any feedback? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v3/examples/menu/main.go (1)
149-152: Remove duplicate background color assignment.Line 152 contains a redundant call to
SetBackgroundColourwith identical parameters. This doesn't affect functionality but adds unnecessary code.Apply this diff to remove the duplicate:
- window := app.Window.NewWithOptions(application.WebviewWindowOptions{ - Name: "menu-example", - Title: "Menu Example", - }).SetBackgroundColour(application.NewRGB(33, 37, 41)).SetBackgroundColour(application.NewRGB(33, 37, 41)) + window := app.Window.NewWithOptions(application.WebviewWindowOptions{ + Name: "menu-example", + Title: "Menu Example", + }).SetBackgroundColour(application.NewRGB(33, 37, 41))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/examples/menu/main.go(1 hunks)v3/pkg/application/menu_darwin.go(2 hunks)v3/pkg/application/webview_window_darwin.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T22:11:37.054Z
Learnt from: leaanthony
PR: wailsapp/wails#3763
File: v3/examples/window/main.go:472-475
Timestamp: 2024-10-08T22:11:37.054Z
Learning: In `v3/examples/window/main.go`, `time.Sleep` is used within a goroutine and does not block the UI thread.
Applied to files:
v3/examples/menu/main.go
🧬 Code graph analysis (2)
v3/pkg/application/menu_darwin.go (1)
v3/pkg/application/roles.go (1)
WindowMenu(17-17)
v3/examples/menu/main.go (1)
v3/pkg/application/webview_window_options.go (1)
WebviewWindowOptions(32-143)
🔇 Additional comments (4)
v3/pkg/application/menu_darwin.go (2)
56-60: LGTM!The Windows menu implementation follows the established pattern for macOS menu roles. The
addWindowsMenufunction correctly uses the bridge cast and setsNSApp.windowsMenuto enable window management features.
105-107: LGTM!The Windows menu role processing follows the same pattern as the existing ServicesMenu handling. This will properly wire up the Windows menu when the
WindowMenurole is specified in the menu structure.v3/pkg/application/webview_window_darwin.go (1)
48-49: LGTM! Appropriate fix for macOS tabbing behavior.Setting
NSWindowTabbingModeDisallowedafter the delegate assignment is the correct approach. This prevents unwanted window tabbing and associated tab controls from appearing in the Window menu, which aligns with the PR's goal of improving macOS Window menu support.v3/examples/menu/main.go (1)
149-151: Well-structured window configuration with meaningful metadata.The use of
NewWithOptionswith explicitNameandTitlefields provides good window identification for the macOS Window menu integration. The naming convention using "menu-example" is clear and descriptive for this example application.
|
You legend! Thanks! 🙏 Could we split out the tab mode to a different setting? I think this would be better and give the developer more control? Please could you also add an entry to the changelog located at |
So we'd be removing it from this PR and adding it in its own PR? Update: Did remove the hardcoded disabling of tab controls. So it can be implemented in a future PR as an optional config by developers. |
github-actions
Bot
added
the
Documentation
Nice one! It's still a good thing to add mind! You ok opening a PR for that? |
Head branch was pushed to by a user without write access
|
* feat: Binds window menu to app windows * chore: disables tab feature & controls from menu by default * docs: Adds title to example for it to show in the window menu item * style: removes redundant SetBackgroundColor call * chore: rolls back disabling tab controls for a future optional config * docs: adds change description to unreleased notes --------- Co-authored-by: Lea Anthony <lea.anthony@gmail.com>
3 participants
Description
This PR improves macOS integration by:
Evidence 1
Windows binding to MacOS's Window menu
https://share.cleanshot.com/swdZxgrY
Evidence 2
Window controls done by external tools (Raycast > center window)
https://share.cleanshot.com/lYp8pZ7h
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Tested locally in MacOS 15.6.1
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit