Make menus to be displayed on Windows OS in v3\examples\dialogs#4928
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughMenus are now attached per-platform in the dialogs example: macOS uses Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "Run example"
participant OS as "Runtime / OS"
participant App as "App"
participant Window as "Window"
participant Menu as "Menu"
CLI->>OS: start example process
OS->>App: initialize
App->>Menu: build menus (File, Edit, Window, Services, Help)
App->>Window: app.Window.New()
alt OS is darwin
App->>App: app.Menu.Set(menu)
else other OS
App->>Window: window.SetMenu(menu)
end
App->>Window: show / run
Window->>User: window displayed with attached menu
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![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
🤖 Fix all issues with AI agents
In `@v3/examples/dialogs/main.go`:
- Around line 343-351: Handle the error returned by os.UserHomeDir() instead of
ignoring it: after calling os.UserHomeDir() capture the error and, inside the
saveMenu.Add(...).OnClick handler (or before calling SetDirectory), if the call
failed or returned an empty string replace userHomeDir with a safe fallback
(e.g., os.TempDir() or "."), and then pass that non-empty fallback to
SetDirectory to avoid unexpected behavior; reference userHomeDir,
os.UserHomeDir(), saveMenu.Add(...).OnClick and SetDirectory when making the
change.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v3/examples/dialogs/main.go (1)
366-378:⚠️ Potential issue | 🔴 CriticalWindow must be displayed with
.Show()beforeapp.Run().The platform-specific menu attachment logic correctly addresses issue
#4927, but the code has a critical issue: the window created on line 366 is never displayed.While the correct Wails v3 window creation API (
app.Window.New()) is used, the window must be explicitly shown before callingapp.Run(). All other examples in the codebase consistently call.Show()after creating a window:
v3/examples/window/main.goline 120v3/examples/window-call/main.goline 43v3/examples/dialogs-basic/main.goline 181v3/examples/panic-handling/main.goline 55Add
.Show()after line 372 (the menu setup):window.Show() err = app.Run()
|
The menu situation has been bothering me for a bit. I'm going to push a better way but merging this for now. Thank you for making it 🙏 |
|
2 participants
Description
This PR applies platform-specific API calls to make menus to be displayed in
v3/examples/dialogs.Fixes #4927
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Bug Fixes
New Features