-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mac] fix always on top during window creation #3841
Conversation
Deploying wails with Cloudflare Pages
|
WalkthroughThe pull request introduces several updates to the project, including an expanded changelog that details various changes, fixes, and enhancements. Notable additions include a new menu item for creating a WebviewWindow that remains always on top and enhancements to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
mkdocs-website/docs/en/changelog.md (1)
25-25
: Remove redundant "Fixed" for better readability.There's a repetition of the word "Fixed" at the beginning of this line. To improve readability and maintain consistency with the changelog format, we should remove one instance.
Here's the suggested change:
-### Fixed -- Fixed `AlwaysOnTop` not working on Mac by [leaanthony](https://github.com/leaanthony) +### Fixed +- `AlwaysOnTop` not working on Mac by [leaanthony](https://github.com/leaanthony)🧰 Tools
🪛 LanguageTool
[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...thub.com//pull/3778) ### Fixed - FixedAlwaysOnTop
not working on Mac by [le...(ENGLISH_WORD_REPEAT_RULE)
v3/examples/window/main.go (1)
157-167
: LGTM! Consider a minor improvement for consistency.The implementation of the "Always on top" window creation looks good and aligns with the PR objectives. It follows the existing pattern for creating new window types.
For consistency with other similar menu items, consider extracting the window creation logic into a separate function. This would make the code more maintainable and easier to read.
Here's a suggested refactoring:
+func createAlwaysOnTopWindow(app *application.App, windowCounter int) { + app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ + AlwaysOnTop: true, + }). + SetTitle("WebviewWindow "+strconv.Itoa(windowCounter)). + SetRelativePosition(rand.Intn(1000), rand.Intn(800)). + SetURL("https://wails.io"). + Show() +} myMenu.Add("New WebviewWindow (Always on top)"). OnClick(func(ctx *application.Context) { - app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ - AlwaysOnTop: true, - }). - SetTitle("WebviewWindow "+strconv.Itoa(windowCounter)). - SetRelativePosition(rand.Intn(1000), rand.Intn(800)). - SetURL("https://wails.io"). - Show() + createAlwaysOnTopWindow(app, windowCounter) windowCounter++ })This refactoring would improve code organization and make it easier to maintain in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- mkdocs-website/docs/en/changelog.md (1 hunks)
- v3/examples/window/main.go (1 hunks)
- v3/pkg/application/webview_window_darwin.go (1 hunks)
🧰 Additional context used
🪛 LanguageTool
mkdocs-website/docs/en/changelog.md
[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...thub.com//pull/3778) ### Fixed - FixedAlwaysOnTop
not working on Mac by [le...(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (4)
mkdocs-website/docs/en/changelog.md (2)
25-27
: LGTM! This change directly addresses the PR objective.The addition of the "Fixed" section in the Unreleased changes, specifically addressing the
AlwaysOnTop
functionality on Mac, aligns perfectly with the PR objectives. The entry is well-formatted and provides clear information about the fix, including the contributor and the associated pull request.🧰 Tools
🪛 LanguageTool
[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...thub.com//pull/3778) ### Fixed - FixedAlwaysOnTop
not working on Mac by [le...(ENGLISH_WORD_REPEAT_RULE)
Line range hint
1-28
: Excellent changelog structure and content.The changelog is well-structured and follows the Keep a Changelog format consistently. It provides comprehensive information about various types of changes (Added, Changed, Fixed, etc.) across multiple versions and platforms. The inclusion of contributor information and links to pull requests enhances traceability.
The recent addition addressing the
AlwaysOnTop
functionality on Mac in the Unreleased section demonstrates active maintenance and aligns with the PR objectives.🧰 Tools
🪛 LanguageTool
[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...thub.com//pull/3778) ### Fixed - FixedAlwaysOnTop
not working on Mac by [le...(ENGLISH_WORD_REPEAT_RULE)
v3/pkg/application/webview_window_darwin.go (2)
1253-1253
: LGTM: Correctly implemented AlwaysOnTop featureThe addition of
w.setAlwaysOnTop(options.AlwaysOnTop)
after showing the window is correct and consistent with the expected behavior. This ensures that the AlwaysOnTop property is set appropriately when the window becomes visible.
1259-1259
: LGTM: Proper handling of AlwaysOnTop for initially hidden windowsThe addition of
w.setAlwaysOnTop(options.AlwaysOnTop)
within theWindowDidBecomeKey
event handler is a good approach. This ensures that the AlwaysOnTop property is correctly set even when the window is initially hidden and becomes visible later.
Description
This PR fixes an issue with AlwaysOnTop not being applied to the window correctly during creation.
Summary by CodeRabbit
New Features
Bug Fixes
AlwaysOnTop
issue on Mac.Documentation