[v3 alpha test] X and Y in WebviewWindowOptions do nothing
#3857
Conversation
WalkthroughThe changes introduced across multiple files involve the addition of a new field, Changes
Suggested reviewers
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
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (22)
v3/examples/frameless/main.go (1)
29-29: Consider enhancing this example to demonstrate window positioning options.Since this PR introduces explicit window positioning behavior, this example could be more educational by:
- Adding a comment explaining that
Centered: truewill override any X/Y coordinates- Demonstrating both centered and positioned windows (perhaps with multiple windows)
Example enhancement:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Frameless: true, - Centered: true, + // Centered: true will override X/Y coordinates if specified + Centered: true, }) +// Create a second window with specific coordinates +app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ + Frameless: true, + X: 100, + Y: 100, +})v3/examples/ignore-mouse/main.go (2)
22-22: Document the window positioning behavior.Since this PR introduces breaking changes to window positioning, it would be helpful to add comments explaining that:
- Setting
Centered: truewill override any X/Y coordinates- To position the window at specific coordinates, set
Centered: falseand provide X/Y valueswindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Width: 800, Height: 600, + // Window will be centered on screen. To position at specific coordinates, + // set Centered: false and provide X/Y values Centered: true, Title: "Ignore Mouse Example",
Line range hint
19-26: Consider adding X/Y positioning example.Since this example focuses on
IgnoreMouseEvents, the window positioning behavior might be better demonstrated in a separate example or by adding alternative positioning code (commented out) to show both centered and X/Y positioning options.window := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Width: 800, Height: 600, Centered: true, Title: "Ignore Mouse Example", URL: "https://wails.io", IgnoreMouseEvents: false, }) +// Alternative positioning example (commented out): +// window := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ +// Width: 800, +// Height: 600, +// Centered: false, // Disable centering to use X/Y coordinates +// X: 100, // Position window 100 pixels from left +// Y: 100, // Position window 100 pixels from top +// Title: "Ignore Mouse Example", +// URL: "https://wails.io", +// IgnoreMouseEvents: false, +// })v3/examples/window-api/main.go (1)
27-30: Add comments explaining the window positioning behavior.Since this is a demo file, it would be helpful to add comments explaining the new window positioning behavior, particularly how
Centeredinteracts with X/Y coordinates.Add explanatory comments:
+// Window positioning example: +// When Centered is true, X and Y coordinates are ignored +// When Centered is false, X and Y coordinates determine the window position app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "JS Window API Demo", Width: 1280, Height: 1024, Centered: true,v3/examples/environment/main.go (1)
28-31: Consider adding environment-specific window positioning documentation.Since this is the environment demo, it would be helpful to add comments explaining any platform-specific behaviors or limitations regarding window positioning.
Add documentation comments above the window creation:
+ // Note: Window positioning behavior may vary by platform + // - macOS: X/Y positioning works when Centered is false + // - Other platforms: Document specific behaviors here app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{v3/examples/raw-message/main.go (1)
37-37: Add a comment explaining the window positioning behavior.Since this is a breaking change in window positioning behavior, it would be helpful to add a comment explaining that:
- Setting
Centered: truewill center the window regardless of X/Y values- To position the window at specific coordinates,
Centeredshould befalseand X/Y values should be provided+ // Window will be centered on screen. To position at specific coordinates, + // set Centered: false and provide X/Y values Centered: true,v3/examples/drag-n-drop/main.go (1)
36-36: Enhance example to demonstrate window positioning options.While setting
Centered: trueworks, this example could better demonstrate the new window positioning capabilities introduced in this PR. Consider either:
- Adding comments to explain that X/Y coordinates can be used instead of centering:
window := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "Drag-n-drop Demo", Mac: application.MacWindow{ Backdrop: application.MacBackdropTranslucent, TitleBar: application.MacTitleBarHiddenInsetUnified, InvisibleTitleBarHeight: 50, }, EnableDragAndDrop: true, + // Window can be either centered or positioned using X/Y coordinates + // Centered: true, // Center the window on screen + // X: 100, Y: 100, // Or position it at specific coordinates Centered: true, })
- Or creating a second window to demonstrate both behaviors:
// Create a centered window mainWindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "Centered Window", Centered: true, }) // Create a positioned window secondWindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "Positioned Window", X: 100, Y: 100, })Would you like me to help implement either of these suggestions?
v3/examples/keybindings/main.go (2)
44-44: Add comment explaining centering behaviorSince Window 2 demonstrates centered positioning, it would be helpful to add a comment explaining this is now an explicit opt-in feature.
- Centered: true, + Centered: true, // Explicitly center the window, overriding any X,Y values if provided
Line range hint
15-19: Consider enhancing the key binding exampleSince this example demonstrates both positioning methods (explicit coordinates and centering), consider adding a key binding to demonstrate moving the window to specific coordinates.
KeyBindings: map[string]func(window *application.WebviewWindow){ "shift+ctrl+c": func(window *application.WebviewWindow) { window.Center() }, + "shift+ctrl+p": func(window *application.WebviewWindow) { + // Demonstrate moving window to specific position + window.SetPosition(200, 200) + }, },v3/examples/events-bug/main.go (1)
40-40: Enhance example to demonstrate window positioning options.Since this PR introduces significant changes to window positioning behavior, this example could better demonstrate the various positioning options. Consider adding comments or additional examples showing:
- How X/Y coordinates work
- The precedence of Centered over X/Y coordinates
Consider adding comments above the window creation:
+ // Window will be centered on screen, ignoring any X/Y coordinates app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Name: "Window 1", Title: "Window 1", URL: "https://wails.io", KeyBindings: map[string]func(window *application.WebviewWindow){ "F12": func(window *application.WebviewWindow) { window.OpenDevTools() }, }, Centered: true, })v3/examples/show-macos-toolbar/main.go (3)
30-30: Document the window positioning behavior.Since this example demonstrates toolbar behavior and the
Centeredoption is part of a breaking change in v3, consider adding a comment explaining that windows can be positioned using eitherCentered: trueor explicitX/Ycoordinates.+ // Center the window on screen. Alternatively, you can use X/Y coordinates + // for specific positioning, but not both as Centered takes precedence Centered: true,
45-45: Document the window positioning behavior.Similar to the first window, add positioning documentation here as well.
+ // Center the window on screen. Alternatively, you can use X/Y coordinates + // for specific positioning, but not both as Centered takes precedence Centered: true,
Line range hint
1-53: Consider enhancing this example to demonstrate window positioning options.Since this PR introduces significant changes to window positioning behavior, consider either:
- Adding a third window that demonstrates explicit X/Y positioning, or
- Adding code comments at the top of the file explaining the available window positioning options in v3.
This would help developers understand the new positioning behavior introduced in v3.
v3/examples/contextmenus/main.go (1)
28-31: Add code comments to document the breaking change.Since this PR introduces a breaking change in window positioning behavior, it would be helpful to add comments explaining the behavior to users referencing this example.
Add a comment block explaining the window positioning behavior:
mainWindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ + // Window Positioning: + // - When Centered is true, the window will be centered regardless of X/Y values + // - When Centered is false, the window will be positioned at the specified X/Y coordinates + // - Default behavior is to center the window (Centered: true) Title: "Context Menu Demo", Width: 1024, Height: 1024, Centered: true,v3/examples/services/main.go (2)
58-60: Verify window positioning behavior documentation.The addition of
Centered: truealigns with the PR's objective to fix window positioning. However, since this is an example file that developers will reference, it would be beneficial to add a comment explaining the window positioning behavior, specifically:
- When
Centered: trueis used- How it interacts with X/Y coordinates
- That centering takes precedence over X/Y when both are specified
Add a comment above the window options:
+ // Window positioning: Setting Centered: true will center the window on screen. + // If both X/Y coordinates and Centered: true are specified, centering takes precedence. app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Width: 1024, Height: 768, Centered: true, })
58-60: Consider demonstrating X/Y positioning.Since this PR specifically addresses the X/Y positioning functionality, it might be valuable to create an additional example that demonstrates non-centered window positioning using X/Y coordinates.
Would you like me to help create a separate example that demonstrates window positioning with X/Y coordinates?
v3/examples/plain/main.go (1)
Line range hint
1-73: Add comments explaining window positioning behavior.As this is an example application and the PR introduces significant changes to window positioning behavior, consider:
- Adding code comments explaining the window positioning options (
X,Y,Centered)- Including examples of different positioning scenarios
- Documenting the precedence rules (e.g.,
CenteredoverridingX/Ywhen true)This will help users understand and correctly implement the new window positioning system.
v3/examples/systray/main.go (1)
27-27: Consider enhancing the example to demonstrate window positioning options.Since this PR introduces significant changes to window positioning behavior, it would be helpful to demonstrate both centered and positioned windows in this example. This would help developers understand when to use
CenteredvsX/Ycoordinates.Consider adding a comment explaining the positioning behavior and showing both options:
_ = app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Width: 500, Height: 500, - Centered: true, + // Window can either be centered or positioned using X/Y coordinates + // Option 1: Center the window + Centered: true, + // Option 2: Position the window (uncomment to test) + // Centered: false, + // X: 100, + // Y: 100, Name: "Systray Demo Window",v3/examples/events/main.go (1)
65-67: Add comments explaining window positioning behaviorSince this PR introduces breaking changes around window positioning, it would be helpful to add comments explaining that:
- Windows can be either centered or positioned using X/Y coordinates
- When
Centered: true, any X/Y values are ignoredwin1 := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "Window 1", + // Example of positioned window using X/Y coordinates + // Note: When Centered is true, X/Y values are ignored Centered: true, win2 := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ Title: "Window 2", + // Example of centered window + // Window will be centered on screen regardless of any X/Y values Centered: true,Also applies to: 87-89
v3/examples/window/main.go (3)
Line range hint
389-402: Update frameless window example to demonstrate X/Y positioning.The frameless window example uses X/Y coordinates but doesn't explicitly set
Centered: false. With the new changes, this might lead to unexpected behavior.Apply this diff to make the positioning behavior explicit:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ X: rand.Intn(1000), Y: rand.Intn(800), BackgroundColour: application.NewRGB(33, 37, 41), Frameless: true, + Centered: false, Mac: application.MacWindow{ InvisibleTitleBarHeight: 50, }, }).Show()
Line range hint
403-417: Update mouse events example to demonstrate X/Y positioning.Similar to the frameless window example, this window creation uses X/Y coordinates without explicitly setting
Centered: false.Apply this diff to make the positioning behavior explicit:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ HTML: "<div style='width: 100%; height: 95%; border: 3px solid red; background-color: \"0000\";'></div>", X: rand.Intn(1000), Y: rand.Intn(800), IgnoreMouseEvents: true, BackgroundType: application.BackgroundTypeTransparent, + Centered: false, Mac: application.MacWindow{ InvisibleTitleBarHeight: 50, }, }).Show()
Line range hint
516-520: Consider updating position menu examples.The position menu demonstrates both absolute and relative positioning. With the new
Centeredoption, it would be helpful to add examples showing how to:
- Disable centering when setting position
- Re-enable centering
Add new menu items to demonstrate the interaction between
Centeredand positioning:positionMenu := menu.AddSubmenu("Position") +positionMenu.Add("Enable Centering").OnClick(func(ctx *application.Context) { + currentWindow(func(w *application.WebviewWindow) { + w.SetOptions(application.WebviewWindowOptions{Centered: true}) + }) +}) +positionMenu.Add("Disable Centering").OnClick(func(ctx *application.Context) { + currentWindow(func(w *application.WebviewWindow) { + w.SetOptions(application.WebviewWindowOptions{Centered: false}) + }) +})Also applies to: 526-530
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (22)
- v3/examples/binding/main.go (1 hunks)
- v3/examples/contextmenus/main.go (1 hunks)
- v3/examples/dev/main.go (1 hunks)
- v3/examples/drag-n-drop/main.go (1 hunks)
- v3/examples/environment/main.go (1 hunks)
- v3/examples/events-bug/main.go (2 hunks)
- v3/examples/events/main.go (2 hunks)
- v3/examples/frameless/main.go (1 hunks)
- v3/examples/hide-window/main.go (1 hunks)
- v3/examples/ignore-mouse/main.go (1 hunks)
- v3/examples/keybindings/main.go (2 hunks)
- v3/examples/plain/main.go (2 hunks)
- v3/examples/raw-message/main.go (1 hunks)
- v3/examples/screen/main.go (1 hunks)
- v3/examples/services/main.go (1 hunks)
- v3/examples/show-macos-toolbar/main.go (2 hunks)
- v3/examples/systray/main.go (1 hunks)
- v3/examples/video/main.go (1 hunks)
- v3/examples/window-api/main.go (1 hunks)
- v3/examples/window/main.go (1 hunks)
- v3/examples/wml/main.go (1 hunks)
- v3/pkg/application/webview_window_darwin.go (2 hunks)
🔇 Additional comments (9)
v3/examples/dev/main.go (1)
35-36: Verify documentation references.Since this is a breaking change affecting window positioning behavior, let's verify if this example is referenced in the documentation.
✅ Verification successful
No documentation update needed for window centering behavior.
The window centering behavior is already well documented in multiple places:
- The
Centeredproperty is documented in the "What's New" section which explains window positioning capabilities- The window positioning APIs are thoroughly documented across runtime references
- There are multiple examples demonstrating window positioning and centering behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to this example in documentation rg -g "*.md" -i "examples/dev|window position|centered window"Length of output: 64
Script:
#!/bin/bash # Let's try a broader search in markdown files for window-related documentation rg -g "*.md" -i "window.*center|center.*window" # Also search for any references to the dev example rg -g "*.md" -i "dev.*example|example.*dev" # Check if there are any window positioning docs in specific doc directories fd -e md -e mdx . docs v3/docs 2>/dev/null | xargs rg -i "window.*position|position.*window"Length of output: 172754
v3/examples/raw-message/main.go (1)
37-37: Verify the example's behavior on macOS.Since this PR fixes a macOS-specific issue with window positioning, we should ensure this example works as expected.
✅ Verification successful
Let me analyze the results and generate additional verification scripts to ensure we have a complete picture.
The previous results show consistent usage of
Centered: trueacross examples, and the X/Y coordinates are only used in screen/assets/main.js which appears to be for drawing rectangles rather than window positioning. However, we need to see the specific changes made in this PR and the complete window configuration in the raw-message example to make a final determination.
Let me try one more verification to check for any macOS-specific window positioning changes in the codebase.
The current results show that the raw-message example has a standard window configuration with
Centered: trueand proper macOS-specific options. However, we need to verify if there are any related test files or documentation that might provide more context about the macOS-specific window positioning fix mentioned in the review comment.
Let's check the macOS-specific window implementation since it was found in the results.
We found the macOS-specific window implementation file and need to examine it to understand the window positioning behavior and any recent fixes that might affect the example.
Let me check one final aspect to verify the window positioning behavior.
The current results show that window centering is properly implemented in the macOS window implementation, and the raw-message example uses standard window positioning with
Centered: true. We need to check if there were any recent changes to this example and see other examples using similar window positioning for consistency.
Based on the complete analysis, I can now generate the final response:
The window positioning behavior is correctly implemented and consistent
The
Centered: trueoption was recently added to the raw-message example as part of a broader change to standardize window positioning across examples. The macOS window implementation properly handles window centering through the nativewindowCenterfunction. No macOS-specific issues were found in the window positioning code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the example properly demonstrates the new window positioning behavior # and that similar changes are consistent across all example files. # Test 1: Check if other example files have consistent window positioning options echo "Checking consistency of window positioning options across examples..." rg -l "Centered: true" "v3/examples/" # Test 2: Verify no examples are using X/Y without explicitly setting Centered: false echo "Checking for potential incorrect usage of X/Y coordinates..." rg "X: \d+" "v3/examples/" -C 2 rg "Y: \d+" "v3/examples/" -C 2Length of output: 2146
Script:
#!/bin/bash # Let's verify if there are any window positioning related changes in the PR # and check the specific changes in raw-message example # Check the PR diff for window positioning related changes git diff origin/main -- 'v3/examples/raw-message/main.go' # Check if there are any window positioning related changes across the codebase git diff origin/main | rg -i "window.*position|center" # Check the complete window configuration in raw-message example cat v3/examples/raw-message/main.goLength of output: 1448
Script:
#!/bin/bash # Search for recent changes or discussions about macOS window positioning # Look for any related test files or documentation # Check for any macOS window positioning related files fd -e md -e go . | xargs rg -l -i "macos.*window.*position|center.*window.*macos" # Look for any test files related to window positioning fd -e test.go -e test.js . | xargs rg -l -i "window.*position|center" # Check for any recent changes in documentation fd README.md | xargs rg -l -i "macos.*window|center.*window"Length of output: 296
Script:
#!/bin/bash # Check the macOS-specific window implementation cat v3/pkg/application/webview_window_darwin.go # Also check the keybindings README that might have relevant information cat v3/examples/keybindings/README.md # Look for any recent changes in the macOS window implementation rg "func.*SetPosition|func.*Center" v3/pkg/application/webview_window_darwin.go -C 5Length of output: 42148
Script:
#!/bin/bash # Check for any recent changes or issues related to window positioning git log -p -- v3/examples/raw-message/main.go # Check if there are any other examples using similar window positioning rg -C 3 "Centered: true" v3/examples/Length of output: 2079
v3/examples/video/main.go (1)
38-38: LGTM: HTML formatting change looks good.The HTML string formatting adjustment improves readability while maintaining the same content.
v3/examples/screen/main.go (1)
63-66: LGTM! The changes align with the PR objectives.The explicit
Centered: truesetting follows the new behavior where centering must be explicitly requested.v3/examples/plain/main.go (1)
40-44: LGTM!The centered positioning is appropriate for this test window demonstrating HTML/CSS/JS injection capabilities.
v3/examples/events/main.go (1)
88-88: LGTM!The centered configuration for the second window is appropriate and works well with the event demonstration focus of this example.
v3/examples/window/main.go (1)
665-665: Verify the impact ofCenteredoption on window positioning.The addition of
Centered: truealigns with the PR's objective to make window centering explicit. However, this example should demonstrate both centered and positioned windows to showcase the new behavior.Consider adding comments or creating additional examples to demonstrate:
- Window creation with explicit X/Y positioning
- Window creation with
Centered: true- The precedence of
Centeredover X/Y coordinatesThis will help users understand the breaking changes in the window positioning behavior.
v3/pkg/application/webview_window_darwin.go (2)
1169-1170: LGTM: Window position initialization is correctly implemented.The addition of
w.setPosition(options.X, options.Y)during window initialization properly addresses the issue where X and Y options were not being respected.
1234-1236: LGTM: Window centering logic is correctly implemented.The conditional centering based on
options.Centeredensures that:
- Window is only centered when explicitly requested
- Centering takes precedence over X/Y coordinates when both are specified
This implementation aligns with the PR objectives and provides the expected behavior.
| @@ -26,6 +26,7 @@ func main() { | |||
| app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{ | |||
| URL: "/", | |||
| DevToolsEnabled: true, | |||
| Centered: true, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the example to demonstrate both centered and positioned windows.
Since this PR introduces explicit window positioning capabilities, this example could better serve developers by demonstrating both centering and X/Y positioning behaviors. Consider either:
- Adding comments explaining the positioning options, or
- Creating another window with explicit X/Y coordinates to showcase both behaviors.
Here's a suggested enhancement:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
URL: "/",
DevToolsEnabled: true,
+ // Explicitly center the window. When Centered is true, X/Y coordinates are ignored
Centered: true,
})
+// Example of positioning a window at specific coordinates
+app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ URL: "/",
+ DevToolsEnabled: true,
+ // Position window at specific coordinates when Centered is false (default)
+ X: 100,
+ Y: 100,
+})Committable suggestion was skipped due to low confidence.
| Title: "JS Window API Demo", | ||
| Width: 1280, | ||
| Height: 1024, | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance the example to demonstrate both centered and positioned windows.
Since this is a Window API demo and the PR introduces significant changes to window positioning behavior, consider expanding the example to showcase both centered and explicitly positioned windows. This would help users understand the new behavior where Centered: true takes precedence over X/Y coordinates.
Here's a suggested implementation that demonstrates both behaviors:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "JS Window API Demo",
Width: 1280,
Height: 1024,
Centered: true,
Mac: application.MacWindow{
Backdrop: application.MacBackdropTranslucent,
TitleBar: application.MacTitleBarHiddenInsetUnified,
InvisibleTitleBarHeight: 50,
},
})
+// Example of a positioned window
+app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ Title: "Positioned Window Demo",
+ Width: 800,
+ Height: 600,
+ Centered: false,
+ X: 100,
+ Y: 100,
+ Mac: application.MacWindow{
+ Backdrop: application.MacBackdropTranslucent,
+ TitleBar: application.MacTitleBarHiddenInsetUnified,
+ InvisibleTitleBarHeight: 50,
+ },
+})Committable suggestion was skipped due to low confidence.
| Title: "Environment Demo", | ||
| Width: 800, | ||
| Height: 600, | ||
| Centered: true, |
There was a problem hiding this comment.
Example should demonstrate both centered and positioned window options.
Given that this PR introduces changes to window positioning behavior where X/Y coordinates work when Centered is false, this example should demonstrate both behaviors to help developers understand the options available.
Consider updating the example to show both behaviors:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Environment Demo",
Width: 800,
Height: 600,
- Centered: true,
+ // Window will be positioned at coordinates when Centered is false
+ X: 100,
+ Y: 100,
+ Centered: false,
Mac: application.MacWindow{Additionally, consider creating a second window to demonstrate centered positioning:
// Create a centered window
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Centered Window Demo",
Width: 800,
Height: 600,
Centered: true, // This will override any X/Y values
})| URL: "/", | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance example to demonstrate window positioning options.
Since this PR introduces breaking changes to window positioning behavior, consider enhancing this example to demonstrate both centered and positioned windows. This would help users understand the new behavior.
Here's a suggested implementation that shows both cases:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Plain Bundle",
CSS: `body { background-color: rgba(255, 255, 255, 0); } .main { color: white; margin: 20%; }`,
Mac: application.MacWindow{
InvisibleTitleBarHeight: 50,
Backdrop: application.MacBackdropTranslucent,
TitleBar: application.MacTitleBarHiddenInset,
},
URL: "/",
- Centered: true,
})
+// Example of a positioned window
+app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ Title: "Positioned Window",
+ URL: "/",
+ X: 100,
+ Y: 100,
+})
+
+// Example of a centered window
+app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ Title: "Centered Window",
+ URL: "/",
+ Centered: true,
+ // X and Y are ignored when Centered is true
+ X: 200,
+ Y: 200,
+})Committable suggestion was skipped due to low confidence.
| @@ -34,6 +34,7 @@ func main() { | |||
| TitleBar: application.MacTitleBarHiddenInsetUnified, | |||
| InvisibleTitleBarHeight: 50, | |||
| }, | |||
| Centered: true, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing this example to demonstrate both centered and positioned windows.
Since this example demonstrates the new window positioning behavior, it would be more helpful to show both centering and explicit positioning use cases.
Consider creating two windows to demonstrate both behaviors:
Centered: true,
})
+ // Create a second window with explicit positioning
+ app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ Title: "Window 2",
+ Name: "Window 2",
+ Mac: application.MacWindow{
+ Backdrop: application.MacBackdropTranslucent,
+ TitleBar: application.MacTitleBarHiddenInsetUnified,
+ InvisibleTitleBarHeight: 50,
+ },
+ Centered: false,
+ X: 100,
+ Y: 100,
+ })Committable suggestion was skipped due to low confidence.
| Title: "Context Menu Demo", | ||
| Width: 1024, | ||
| Height: 1024, | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider demonstrating both centered and positioned window options.
Since this is an example application and the PR introduces significant changes to window positioning behavior, it would be valuable to demonstrate both centered and positioned windows. This would help users understand the new behavior where Centered: true takes precedence over X and Y coordinates.
Consider updating the window options to show both behaviors:
mainWindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Context Menu Demo",
Width: 1024,
Height: 1024,
- Centered: true,
+ // Example: Window will be centered regardless of X/Y when Centered is true
+ X: 100,
+ Y: 100,
+ Centered: true, // Takes precedence over X/Y coordinatesAdditionally, consider adding a second window to demonstrate explicit positioning:
// Example: Window will be positioned at specific coordinates when Centered is false
positionedWindow := app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Positioned Window Demo",
Width: 800,
Height: 600,
X: 200,
Y: 200,
Centered: false, // Window will be positioned at X,Y coordinates
})| Title: "Screen Demo", | ||
| Width: 800, | ||
| Height: 600, | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing this example to demonstrate window positioning.
Since this is a Screen Demo and the PR introduces functional X/Y positioning, consider creating additional windows to showcase both centered and positioned windows. This would help users understand the new behavior.
Here's a suggested enhancement:
app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
Title: "Screen Demo",
Width: 800,
Height: 600,
Centered: true,
Mac: application.MacWindow{
Backdrop: application.MacBackdropTranslucent,
TitleBar: application.MacTitleBarHiddenInsetUnified,
InvisibleTitleBarHeight: 50,
},
})
+
+// Example of positioned window
+app.NewWebviewWindowWithOptions(application.WebviewWindowOptions{
+ Title: "Positioned Window Demo",
+ Width: 400,
+ Height: 300,
+ X: 100, // Explicit X position
+ Y: 100, // Explicit Y position
+ Centered: false,
+ Mac: application.MacWindow{
+ Backdrop: application.MacBackdropTranslucent,
+ TitleBar: application.MacTitleBarHiddenInsetUnified,
+ InvisibleTitleBarHeight: 50,
+ },
+})Committable suggestion was skipped due to low confidence.
| URL: "/", | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider demonstrating both centered and positioned windows.
Since this is an example application and the PR introduces explicit window positioning control, it would be valuable to demonstrate both centered and positioned windows. Consider modifying this window to use specific X/Y coordinates instead of centering, to showcase the new positioning capabilities.
- URL: "/",
- Centered: true,
+ URL: "/",
+ X: 100,
+ Y: 100,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| URL: "/", | |
| Centered: true, | |
| URL: "/", | |
| X: 100, | |
| Y: 100, |
| Title: "Plain Bundle new Window from GoRoutine", | ||
| Width: 500, | ||
| Height: 500, | ||
| Centered: true, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider demonstrating dynamic window positioning.
Since this window is created dynamically after a delay, it presents an excellent opportunity to demonstrate runtime window positioning. Consider positioning it relative to the main window or using calculated coordinates.
- Title: "Plain Bundle new Window from GoRoutine",
- Width: 500,
- Height: 500,
- Centered: true,
+ Title: "Plain Bundle new Window from GoRoutine",
+ Width: 500,
+ Height: 500,
+ X: 200, // Offset from main window
+ Y: 200, // Offset from main windowCommittable suggestion was skipped due to low confidence.
| @@ -63,6 +63,7 @@ func main() { | |||
| TitleBar: application.MacTitleBarHiddenInsetUnified, | |||
| InvisibleTitleBarHeight: 50, | |||
| }, | |||
| Centered: true, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider demonstrating both centered and positioned windows
Since this PR introduces significant changes to window positioning behavior, this example could be more educational by demonstrating both centered and positioned windows. Consider modifying win1 to use explicit X/Y coordinates to showcase the new positioning capabilities.
- Centered: true,
+ X: 100,
+ Y: 100,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Centered: true, | |
| X: 100, | |
| Y: 100, |
Description
In
v3/pkg/application/webview_window_darwin.gothere's no logic that involvesoptions.Xandoptions.Yand window is always centered, soXandYin WebviewWindowOptions do nothing.This PR fixes
XYissue and requires to setCentered: trueexclusively to launch window centered. When bothXYandCentered: trueare set the latter one is prioritized.Type of change
How Has This Been Tested?
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
Centeredoption to the webview window configuration, ensuring that windows open centered on the screen across various examples.eventsexample, enhancing interactivity.