Repository navigation
Refactor to using Window interface - #4471
Conversation
dosubot
Bot
added
the
size:L
WalkthroughThis change generalizes window-related types throughout the codebase by replacing concrete Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Window (interface)
participant WebviewWindow (impl)
participant PlatformDialog
participant SystemTray
App->>Window: Create or manage window (via interface)
App->>Window: Register key binding (func(Window))
Window->>WebviewWindow: (if concrete) method calls
App->>PlatformDialog: AttachToWindow(Window)
PlatformDialog->>Window: NativeWindow()
App->>SystemTray: AttachWindow(Window)
SystemTray->>Window: PositionWindow(Window, offset)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested labels
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (34)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
dosubot
Bot
added
Enhancement
Deploying wails with
|
| Latest commit: |
30545bc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e57fe048.wails.pages.dev |
| Branch Preview URL: | https://v3-alpha-chore-refactor-wind.wails.pages.dev |
![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/pkg/application/webview_window_close_darwin.go (1)
17-18: Consider adding type safety for the assertion.The direct type assertion
window.(*WebviewWindow)could panic if the window is not actually a*WebviewWindow. Consider adding a type check or documenting the assumption that all windows in this context are WebviewWindows.- webviewWindow := window.(*WebviewWindow) + webviewWindow, ok := window.(*WebviewWindow) + if !ok { + globalApplication.debug("windowShouldUnconditionallyClose: window is not a WebviewWindow", "windowId", windowId) + return C.bool(false) + }The method call to
shouldUnconditionallyClose()is a good improvement for encapsulation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
v3/examples/build/main.go(3 hunks)v3/examples/events-bug/main.go(3 hunks)v3/examples/keybindings/main.go(3 hunks)v3/examples/systray-basic/main.go(1 hunks)v3/examples/systray-menu/main.go(1 hunks)v3/examples/window-menu/main.go(2 hunks)v3/examples/window/main.go(6 hunks)v3/internal/commands/appimage_testfiles/main.go(1 hunks)v3/pkg/application/application.go(2 hunks)v3/pkg/application/application_options.go(1 hunks)v3/pkg/application/dialogs.go(8 hunks)v3/pkg/application/dialogs_darwin.go(3 hunks)v3/pkg/application/dialogs_linux.go(2 hunks)v3/pkg/application/dialogs_windows.go(2 hunks)v3/pkg/application/key_binding_manager.go(3 hunks)v3/pkg/application/linux_cgo.go(3 hunks)v3/pkg/application/linux_purego.go(1 hunks)v3/pkg/application/messageprocessor_dialog.go(2 hunks)v3/pkg/application/native_window_darwin.go(1 hunks)v3/pkg/application/native_window_linux.go(1 hunks)v3/pkg/application/native_window_windows.go(1 hunks)v3/pkg/application/systemtray.go(4 hunks)v3/pkg/application/systemtray_darwin.go(1 hunks)v3/pkg/application/systemtray_linux.go(1 hunks)v3/pkg/application/systemtray_windows.go(1 hunks)v3/pkg/application/webview_window.go(4 hunks)v3/pkg/application/webview_window_close_darwin.go(1 hunks)v3/pkg/application/webview_window_darwin.go(1 hunks)v3/pkg/application/webview_window_linux.go(1 hunks)v3/pkg/application/webview_window_options.go(1 hunks)v3/pkg/application/webview_window_windows.go(1 hunks)v3/pkg/application/window.go(1 hunks)v3/pkg/application/window_manager.go(1 hunks)v3/pkg/services/badge/badge_windows.go(4 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/pkg/application/webview_window_bindings_darwin.h:0-0
Timestamp: 2024-09-21T13:34:24.145Z
Learning: In this codebase, typedefs for window references (like replacing `void *` with `WindowRef`) are not preferred; using `void *` is acceptable.
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:0-0
Timestamp: 2025-03-24T06:54:22.127Z
Learning: popaprozac prefers to focus on getting the Notifications API functionality working first and may consider code cleanup/refactoring for payload encoding logic in notifications_windows.go at a later time.
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/examples/keybindings/main.go:16-17
Timestamp: 2024-09-20T23:34:29.841Z
Learning: In the codebase, `application.Options.KeyBindings` uses the `application.Window` type, whereas `application.WebviewWindowOptions.KeyBindings` uses `*application.WebviewWindow`. This is intentional and acceptable.
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/pkg/application/application_options.go:93-93
Timestamp: 2024-10-08T22:11:37.054Z
Learning: The `KeyBindings` property of `application.Option` uses the `application.Window` type, while `application.WebviewWindowOptions` still accepts `*application.WebviewWindow`, and this is intentional.
📚 Learning: in the codebase, `application.options.keybindings` uses the `application.window` type, whereas `appl...
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/examples/keybindings/main.go:16-17
Timestamp: 2024-09-20T23:34:29.841Z
Learning: In the codebase, `application.Options.KeyBindings` uses the `application.Window` type, whereas `application.WebviewWindowOptions.KeyBindings` uses `*application.WebviewWindow`. This is intentional and acceptable.
Applied to files:
v3/pkg/application/messageprocessor_dialog.gov3/examples/keybindings/main.gov3/pkg/application/native_window_windows.gov3/pkg/application/native_window_darwin.gov3/pkg/application/native_window_linux.gov3/pkg/application/window_manager.gov3/pkg/application/webview_window_linux.gov3/internal/commands/appimage_testfiles/main.gov3/pkg/application/webview_window_darwin.gov3/pkg/application/dialogs_linux.gov3/pkg/application/webview_window_windows.gov3/examples/systray-menu/main.gov3/pkg/application/systemtray_windows.gov3/pkg/application/webview_window_close_darwin.gov3/examples/events-bug/main.gov3/pkg/application/application_options.gov3/pkg/application/webview_window_options.gov3/examples/build/main.gov3/pkg/application/application.gov3/pkg/application/systemtray_linux.gov3/examples/systray-basic/main.gov3/pkg/application/systemtray_darwin.gov3/examples/window-menu/main.gov3/pkg/application/linux_cgo.gov3/pkg/application/linux_purego.gov3/pkg/application/window.gov3/pkg/application/key_binding_manager.gov3/examples/window/main.gov3/pkg/application/dialogs.gov3/pkg/application/systemtray.gov3/pkg/application/webview_window.go
📚 Learning: the `keybindings` property of `application.option` uses the `application.window` type, while `applic...
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/pkg/application/application_options.go:93-93
Timestamp: 2024-10-08T22:11:37.054Z
Learning: The `KeyBindings` property of `application.Option` uses the `application.Window` type, while `application.WebviewWindowOptions` still accepts `*application.WebviewWindow`, and this is intentional.
Applied to files:
v3/pkg/application/messageprocessor_dialog.gov3/examples/keybindings/main.gov3/pkg/application/webview_window_darwin.gov3/pkg/application/dialogs_linux.gov3/pkg/application/webview_window_windows.gov3/examples/systray-menu/main.gov3/pkg/application/systemtray_windows.gov3/pkg/application/webview_window_close_darwin.gov3/examples/events-bug/main.gov3/pkg/application/application_options.gov3/pkg/application/webview_window_options.gov3/examples/build/main.gov3/pkg/application/application.gov3/examples/systray-basic/main.gov3/pkg/application/systemtray_darwin.gov3/examples/window-menu/main.gov3/pkg/application/linux_cgo.gov3/pkg/application/key_binding_manager.gov3/examples/window/main.gov3/pkg/application/dialogs.gov3/pkg/application/systemtray.gov3/pkg/application/webview_window.go
📚 Learning: in `v3/examples/window/main.go`, `time.sleep` is used within a goroutine and does not block the ui t...
Learnt from: leaanthony
PR: wailsapp/wails#3763
File: v3/examples/window/main.go:472-475
Timestamp: 2024-09-30T06:13:46.595Z
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/pkg/application/messageprocessor_dialog.gov3/pkg/services/badge/badge_windows.gov3/pkg/application/dialogs_darwin.gov3/examples/keybindings/main.gov3/pkg/application/native_window_windows.gov3/pkg/application/native_window_darwin.gov3/pkg/application/native_window_linux.gov3/internal/commands/appimage_testfiles/main.gov3/pkg/application/dialogs_linux.gov3/examples/systray-menu/main.gov3/pkg/application/systemtray_windows.gov3/pkg/application/webview_window_close_darwin.gov3/examples/events-bug/main.gov3/examples/build/main.gov3/pkg/application/dialogs_windows.gov3/examples/systray-basic/main.gov3/examples/window-menu/main.gov3/pkg/application/linux_purego.gov3/pkg/application/window.gov3/examples/window/main.gov3/pkg/application/systemtray.go
📚 Learning: in this codebase, typedefs for window references (like replacing `void *` with `windowref`) are not ...
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/pkg/application/webview_window_bindings_darwin.h:0-0
Timestamp: 2024-09-21T13:34:24.145Z
Learning: In this codebase, typedefs for window references (like replacing `void *` with `WindowRef`) are not preferred; using `void *` is acceptable.
Applied to files:
v3/pkg/application/messageprocessor_dialog.gov3/pkg/services/badge/badge_windows.gov3/pkg/application/dialogs_darwin.gov3/examples/keybindings/main.gov3/pkg/application/native_window_windows.gov3/pkg/application/native_window_darwin.gov3/pkg/application/webview_window_linux.gov3/pkg/application/webview_window_darwin.gov3/pkg/application/dialogs_linux.gov3/pkg/application/webview_window_windows.gov3/pkg/application/webview_window_close_darwin.gov3/examples/events-bug/main.gov3/pkg/application/application_options.gov3/examples/build/main.gov3/pkg/application/systemtray_darwin.gov3/examples/window-menu/main.gov3/pkg/application/linux_cgo.gov3/examples/window/main.gov3/pkg/application/dialogs.gov3/pkg/application/systemtray.gov3/pkg/application/webview_window.go
📚 Learning: in the notifications package, initialization of the `service` struct is handled through platform-spe...
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications.go:46-55
Timestamp: 2025-03-24T20:22:56.233Z
Learning: In the notifications package, initialization of the `Service` struct is handled through platform-specific `New()` functions in each implementation file (darwin, windows, linux) rather than a generic constructor in the main package file.
Applied to files:
v3/pkg/application/native_window_darwin.go
📚 Learning: in the notifications package, initialization of the `service` struct is handled through platform-spe...
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications.go:46-55
Timestamp: 2025-03-24T20:22:56.233Z
Learning: In the notifications package, initialization of the `Service` struct is handled through platform-specific `New()` functions in each implementation file (darwin, windows, linux) rather than a generic constructor in the main package file. Each platform implementation follows a singleton pattern using `notificationServiceOnce.Do()` and creates a global `NotificationService` variable that's accessed through a thread-safe `getNotificationService()` function.
Applied to files:
v3/pkg/application/native_window_darwin.go
📚 Learning: in `v3/internal/commands/appimage_testfiles/main.go`, `time.sleep` is used within a goroutine and do...
Learnt from: leaanthony
PR: wailsapp/wails#3763
File: v3/internal/commands/appimage_testfiles/main.go:295-299
Timestamp: 2024-09-30T06:14:32.602Z
Learning: In `v3/internal/commands/appimage_testfiles/main.go`, `time.Sleep` is used within a goroutine and does not block the UI thread.
Applied to files:
v3/internal/commands/appimage_testfiles/main.gov3/examples/build/main.gov3/pkg/application/linux_purego.gov3/examples/window/main.go
📚 Learning: in the wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally d...
Learnt from: leaanthony
PR: wailsapp/wails#4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Applied to files:
v3/examples/systray-menu/main.gov3/examples/build/main.gov3/examples/window-menu/main.gov3/pkg/application/window.gov3/examples/window/main.go
📚 Learning: safety checks for `p.nspanel` are performed in the `setfloating` method of `webviewpanel`, following...
Learnt from: nixpare
PR: wailsapp/wails#3763
File: v3/pkg/application/webview_panel_darwin.go:88-89
Timestamp: 2024-09-21T09:56:48.126Z
Learning: Safety checks for `p.nsPanel` are performed in the `SetFloating` method of `WebviewPanel`, following the `WebviewWindow` and `macosWebviewWindow` implementations and code style.
Applied to files:
v3/pkg/application/linux_cgo.gov3/pkg/application/linux_purego.go
📚 Learning: in wails v2, unlike v3-alpha which has a `serviceshutdown` method for services, there is no standard...
Learnt from: popaprozac
PR: wailsapp/wails#4256
File: v2/internal/frontend/desktop/linux/notifications.go:27-28
Timestamp: 2025-04-29T23:54:07.488Z
Learning: In Wails v2, unlike v3-alpha which has a `ServiceShutdown` method for services, there is no standardized teardown pattern for frontend implementations. When implementing features that require cleanup (like goroutines or resources), add explicit cleanup methods (e.g., `CleanupNotifications()`) that handle resource release, context cancellation, and connection closure.
Applied to files:
v3/pkg/application/webview_window.go
📚 Learning: when dealing with fixed-length arrays in go structs, encoding/json automatically handles bounds chec...
Learnt from: fbbdev
PR: wailsapp/wails#4001
File: v3/internal/generator/testcases/no_bindings_here/person.go:9-9
Timestamp: 2025-01-15T22:30:08.796Z
Learning: When dealing with fixed-length arrays in Go structs, encoding/json automatically handles bounds checking during unmarshaling, returning an error if the JSON array length exceeds the Go array length. No additional runtime validation is needed.
Applied to files:
v3/pkg/application/webview_window.go
📚 Learning: in the wails v3 codebase, panic is used to catch invalid invocations that should never occur in prod...
Learnt from: fbbdev
PR: wailsapp/wails#4024
File: v3/pkg/application/services.go:38-51
Timestamp: 2025-01-18T21:19:47.778Z
Learning: In the Wails v3 codebase, panic is used to catch invalid invocations that should never occur in production (programming errors), while error returns are reserved for recoverable error conditions (runtime errors). For example, the `NewService` function in `v3/pkg/application/services.go` uses panic to handle the case where more than one `ServiceOptions` instance is provided, as this is considered an unrecoverable programming error.
Applied to files:
v3/pkg/application/webview_window.go
📚 Learning: in the wails v3 project, internal functions are designed to panic on nil parameters as they represen...
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
Applied to files:
v3/pkg/application/webview_window.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (62)
v3/pkg/application/messageprocessor_dialog.go (1)
105-105: LGTM! Clean interface generalization.The removal of type assertions aligns perfectly with the PR objective to use the Window interface instead of concrete WebviewWindow types. Both dialog option assignments now use the interface directly.
Also applies to: 147-147
v3/pkg/application/webview_window_darwin.go (1)
1329-1331: LGTM! Improved native window handle abstraction.The method rename from
nativeWindowHandle()tonativeWindow()with the newNativeWindowreturn type eliminates unnecessary uintptr conversions and aligns with the unified native window abstraction across platforms.v3/pkg/application/native_window_linux.go (1)
1-13: LGTM! Clean platform-specific abstraction.The
NativeWindowtype alias provides a clean abstraction for Linux GTK window handles, consistent with the unified native window handling approach across platforms.v3/pkg/application/native_window_windows.go (1)
1-9: LGTM! Consistent platform abstraction.The Windows
NativeWindowtype alias mirrors the approach used for other platforms, providing a unified interface for native window handles across the codebase.v3/pkg/application/dialogs_darwin.go (1)
410-410: LGTM! Simplified native window access.The replacement of
NativeWindowHandle()calls with directNativeWindow()calls eliminates unnecessary error handling and simplifies native window pointer retrieval across all dialog types.Also applies to: 475-475, 555-555
v3/pkg/application/webview_window_linux.go (1)
379-381: LGTM! Improved type safety and consistency.The method refactoring from
nativeWindowHandle() uintptrtonativeWindow() NativeWindowimproves type safety by returning the platform-specificNativeWindowtype instead of a genericuintptr. This change standardizes native window handle access across platforms and aligns with the broader Window interface refactor.v3/pkg/application/native_window_darwin.go (1)
1-12: LGTM! Well-structured platform-specific type definition.The
NativeWindowtype alias is properly implemented with:
- Correct build constraint for Darwin platform
- Clear documentation indicating it represents NSWindow*
- Appropriate use of
unsafe.Pointerfor native handles- Proper cgo import for Cocoa framework
This aligns with the codebase pattern where
unsafe.Pointeris acceptable for native window references.v3/internal/commands/appimage_testfiles/main.go (1)
35-35: LGTM! Appropriate type assertion for concrete type requirements.The explicit type assertion to
*application.WebviewWindowis correct here since the callback function parameter requires the concrete type rather than theWindowinterface. This maintains compatibility while allowing the broader codebase to use interface-based window handling.v3/examples/keybindings/main.go (1)
16-20: LGTM! Perfect alignment with Window interface pattern.The parameter type changes from
*application.WebviewWindowtoapplication.Windowalign perfectly with the retrieved learnings that indicateapplication.Options.KeyBindingsshould use theapplication.Windowtype. The callback methods (Center(),OpenDevTools()) are part of the Window interface, so this change improves type consistency without breaking functionality.Also applies to: 27-31, 38-42
v3/pkg/application/systemtray_windows.go (1)
50-50: LGTM! Clean interface abstraction.The method signature update from
*WebviewWindowtoWindowinterface aligns perfectly with the broader refactoring effort to use interface-based window handling throughout the codebase. The method implementation remains unchanged since it only uses methods available through theWindowinterface.v3/pkg/application/webview_window_windows.go (1)
237-239: LGTM! Improved type safety for native window handles.The method replacement from
nativeWindowHandle() uintptrtonativeWindow() NativeWindowenhances type safety by using a platform-specific type alias instead of a raw integer. Theunsafe.Pointercast is appropriate for converting the Windows HWND handle to the abstractedNativeWindowtype.v3/pkg/application/window_manager.go (1)
73-82: LGTM! Interface-based return type with simplified implementation.The method signature change from
*WebviewWindowtoWindowinterface aligns with the broader refactoring effort. The simplified implementation correctly returns the window directly from the map - if the window ID doesn't exist, returningnilis appropriate for theWindowinterface type.v3/pkg/application/application.go (1)
335-335: LGTM! Consistent interface refactor.The keyBindings field type change from
*WebviewWindowtoWindowinterface aligns perfectly with the PR objective to standardize on the Window interface. The initialization on line 418 correctly matches the new type signature.Also applies to: 418-418
v3/examples/systray-menu/main.go (1)
40-44: LGTM! Example updated correctly for interface refactor.The KeyBindings function signature change from
*application.WebviewWindowtoapplication.Windowproperly aligns with the interface refactor. The function body remains unchanged, which is correct since the Window interface provides the necessary abstraction.v3/examples/systray-basic/main.go (1)
36-40: LGTM! Consistent example interface update.The KeyBindings function signature change from
*application.WebviewWindowtoapplication.Windowis consistent with the interface refactor pattern seen across the codebase. The function body correctly remains unchanged.v3/pkg/application/webview_window_options.go (1)
134-134: LGTM! Interface standardization improves API consistency.This change updates WebviewWindowOptions.KeyBindings to use the Window interface, making it consistent with Options.KeyBindings. This addresses the previous API inconsistency where different option structs used different types for key binding callbacks.
v3/pkg/application/application_options.go (1)
69-69: LGTM! Completing the interface standardization.The KeyBindings field type change to use the Window interface completes the standardization across all configuration structs. This ensures consistent callback signatures throughout the key binding system.
v3/pkg/application/systemtray_linux.go (1)
227-227: LGTM! Successful generalization to Window interface.The parameter type change from
*WebviewWindowtoWindowinterface aligns perfectly with the PR's refactor objective while maintaining identical functionality.v3/pkg/application/dialogs_windows.go (2)
42-45: LGTM! Improved native window handle retrieval.The change from
NativeWindowHandle()toNativeWindow()with nil checking simplifies the code while maintaining the same functionality. The error handling removal is appropriate since we can directly check for nil.
114-117: LGTM! Consistent pattern applied.Same improvement as in the message dialog - replacing error-prone
NativeWindowHandle()with simplerNativeWindow()nil checking pattern.v3/pkg/application/linux_purego.go (1)
1104-1109: LGTM! Excellent type safety improvement.The explicit type assertions with
okchecks prevent potential runtime panics and follow Go best practices for safe type casting. This defensive programming approach is much safer than direct type assertions.v3/pkg/application/dialogs_linux.go (2)
7-10: LGTM! Consistent native window access pattern.The change from
NativeWindowHandle()toNativeWindow()with nil checking matches the pattern used in other platform implementations, improving consistency across the codebase.
33-36: LGTM! Consistent implementation.Same improvement applied consistently - using the standardized
NativeWindow()approach with proper nil checking.v3/pkg/services/badge/badge_windows.go (4)
8-8: LGTM! Added necessary import for error handling.The
errorsimport is needed for the new error creation pattern used when native window is unavailable.
90-94: LGTM! Consistent native window handling with proper error messaging.The change from
NativeWindowHandle()toNativeWindow()follows the same pattern as dialog implementations. The custom error message "window native handle unavailable" provides clear feedback when the native window is not available.
131-135: LGTM! Consistent pattern applied.Same improvement as in
SetBadge- using the standardized native window access approach with appropriate error handling.
189-193: LGTM! Complete consistency across all badge methods.All three badge methods now use the same native window access pattern, ensuring consistent behavior throughout the badge service.
v3/pkg/application/systemtray_darwin.go (1)
104-115: LGTM! Excellent interface abstraction with improved error handling.The refactor from
*WebviewWindowtoWindowinterface follows good design principles. The addition of the nil check for the native window pointer and proper error handling makes this implementation more robust.v3/pkg/application/linux_cgo.go (3)
1281-1284: LGTM! Good defensive programming with type assertion checks.The addition of explicit type assertion checks with early returns prevents potential panics and makes the code more robust when working with the Window interface abstraction.
1468-1471: LGTM! Consistent type safety pattern.The type assertion pattern is consistent with other functions in the file and provides necessary safety when working with the Window interface.
1509-1512: LGTM! Maintains consistent type safety across event handlers.The type assertion pattern is consistent with the other event handlers in this file, ensuring robust handling of the Window interface abstraction.
v3/examples/events-bug/main.go (3)
16-17: LGTM! Proper interface abstraction in key bindings.The change from
*application.WebviewWindowtoapplication.Windowfollows the interface abstraction pattern established throughout the codebase, improving modularity while maintaining functionality.
35-36: LGTM! Consistent interface usage across window-specific key bindings.The interface abstraction is consistently applied to window-specific key bindings, maintaining the same pattern as the global application key bindings.
46-47: LGTM! Complete consistency in interface abstraction.All key binding callbacks consistently use the
application.Windowinterface, demonstrating proper application of the interface abstraction pattern throughout the example.v3/examples/build/main.go (2)
32-35: LGTM! Clean interface abstraction with improved readability.The refactor from
*application.WebviewWindowtoapplication.Windowinterface follows the established pattern. The local variable assignment before the nil check slightly improves readability while maintaining the same safety guarantees.
122-262: LGTM! Comprehensive and consistent interface refactor.All usages of
currentWindowhave been consistently updated to work with theapplication.Windowinterface. The refactor is thorough and maintains all existing functionality while improving abstraction.v3/pkg/application/window.go (1)
84-107: LGTM! Comprehensive interface expansion supporting the refactor objectives.The addition of these methods significantly enhances the Window interface capabilities:
- State management:
IsVisible(),SetEnabled(),Flash()improve window control- Bounds handling:
Bounds()andSetBounds()provide comprehensive positioning support- Platform integration:
NativeWindow()offers safe access to native handles- Functionality:
Print()andRegisterHook()add document and event capabilities- Editing support: Complete set of editing operations for text manipulation
These additions effectively support the refactor from concrete
*WebviewWindowtypes to the interface-based approach, providing all necessary functionality through the abstraction layer.v3/examples/window-menu/main.go (3)
47-57: LGTM: Proper interface abstraction in key bindings.The refactoring correctly updates the KeyBindings map to use the
Windowinterface instead of the concrete*WebviewWindowtype. This improves abstraction while maintaining functionality through the key binding callbacks.
31-31: Appropriate type assertion for concrete method access.The type assertion to
*WebviewWindowis correctly used to access theHideMenuBar()method, which is specific to the concrete implementation and not available on theWindowinterface.
36-36: Correct type assertion pattern for WebviewWindow-specific methods.The type assertion follows the established pattern for accessing concrete methods like
SetURL()when working with the abstractWindowinterface.v3/pkg/application/key_binding_manager.go (3)
16-16: LGTM: Interface abstraction improves flexibility.The
Addmethod now accepts a callback withWindowinterface parameter instead of concrete*WebviewWindow, improving the API's flexibility while maintaining backward compatibility.
30-30: Consistent interface usage in Process method.The
Processmethod parameter type correctly updated to match the interface abstraction pattern used throughout the refactoring.
45-45: KeyBinding struct properly updated.The
Callbackfield type correctly reflects the interface abstraction, maintaining consistency with the method signatures.v3/examples/window/main.go (4)
91-91: LGTM: Variable type updated for interface consistency.The
hiddenWindowsslice now uses theWindowinterface type, which is consistent with the broader refactoring and provides better abstraction.
93-99: Correct helper function signature update.The
currentWindowhelper function parameter type correctly updated to accept functions that work with theWindowinterface. The nil check and debug message appropriately updated.
277-277: Proper variable declaration with interface type.The explicit variable declaration
var w application.Windowcorrectly uses the interface type while allowing assignment from the concrete window creation method.
479-481: Consistent callback parameter types.All the callback functions throughout the file properly use
Windowinterface parameters, maintaining consistency with the refactoring pattern.v3/pkg/application/systemtray.go (4)
38-38: Interface method signature properly updated.The
positionWindowmethod in thesystemTrayImplinterface correctly uses theWindowinterface parameter, maintaining consistency with the refactoring.
124-131: PositionWindow method correctly generalized.The public
PositionWindowmethod properly accepts theWindowinterface, improving API flexibility while maintaining the same functionality.
260-260: WindowAttachConfig struct field updated appropriately.The
Windowfield inWindowAttachConfigstruct correctly uses the interface type, allowing attachment of any window implementation.
281-284: AttachWindow method properly abstracted.The
AttachWindowmethod parameter correctly updated to use theWindowinterface, providing better abstraction for system tray window attachment.v3/pkg/application/dialogs.go (6)
78-78: MessageDialogOptions struct field properly updated.The
windowfield correctly uses theWindowinterface type, allowing message dialogs to be attached to any window implementation.
135-137: AttachToWindow method parameter correctly generalized.The
AttachToWindowmethod for MessageDialog properly accepts theWindowinterface, improving API flexibility.
181-181: OpenFileDialogOptions Window field updated consistently.The
Windowfield inOpenFileDialogOptionsstruct correctly uses the interface type, maintaining consistency with other dialog options.
207-207: OpenFileDialogStruct window field properly abstracted.The internal
windowfield correctly uses theWindowinterface, allowing the dialog implementation to work with any window type.
248-250: File dialog AttachToWindow method correctly updated.The method parameter properly uses the
Windowinterface, maintaining consistency with the MessageDialog implementation.
375-375: SaveFileDialog consistently updated with interface types.All SaveFileDialog-related structs and methods properly use the
Windowinterface for window fields and attachment methods, maintaining consistency across all dialog types.Also applies to: 392-392, 454-455
v3/pkg/application/webview_window.go (5)
80-80: LGTM: Interface method signature updated consistently.The change from
nativeWindowHandle()returninguintptrtonativeWindow()returningNativeWindowaligns with the broader refactor to use platform-specific type aliases for better type safety.
152-152: LGTM: Key binding callback signature updated to use Window interface.The change from
func(*WebviewWindow)tofunc(Window)is consistent with the refactor to use the Window interface instead of concrete types, improving abstraction.
244-250: LGTM: New native window method follows established patterns.The
NativeWindow()method implementation correctly:
- Checks for nil implementation and destroyed state before proceeding
- Returns
nilfor uninitialized/destroyed windows- Delegates to the platform-specific implementation
- Follows the same defensive programming pattern used throughout the file
253-255: LGTM: Good encapsulation of atomic field access.The
shouldUnconditionallyClose()method provides a clean interface for checking the unconditional close flag, replacing direct atomic field access with proper encapsulation.
292-294: LGTM: Function signature updated consistently with interface refactor.The
processKeyBindingOptionsfunction signature changes from concrete*WebviewWindowtoWindowinterface are consistent with the broader refactor, maintaining the same functionality while improving abstraction.
|
8 similar comments
|
|
|
|
|
|
|
|
|
Tested and working on Windows |
|
1 similar comment
|
github-actions
Bot
added
the
Documentation
…de maintainability
f12370f to
30545bc
Compare
|
Refactor window interface to eliminate type assertions and improve code maintainability
Description
Removes hard-coded usage of WebviewWindow where appropriate and makes sure it all conforms with the Window interface.
Summary by CodeRabbit
New Features
Refactor
Documentation