[V3] Drag-n-Drop Zones and improvements #4318
Conversation
the top 300px of the window are not dropabble for some reason i suspect it has to do with the drag enter/drag leave xy as the performOperation needed to use the ContentView for appropriate X/Y
WalkthroughThis update introduces structured support for drag-and-drop file operations targeting user-specified dropzones in the application window. It adds new event types, backend/frontend integration, and documentation for handling dropped files with metadata about the drop target. The changes span runtime, event system, Go backend, platform-specific code, and example/demo updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser (Frontend)
participant Go Backend
participant OS/Platform
User->>Browser (Frontend): Drag file over dropzone
Browser (Frontend)->>Browser (Frontend): Highlight dropzone with CSS class
User->>Browser (Frontend): Drop file onto dropzone
Browser (Frontend)->>OS/Platform: Native drop event (with coordinates)
OS/Platform->>Go Backend: processDragItems(windowID, files, x, y)
Go Backend->>Browser (Frontend): InitiateFrontendDropProcessing(files, x, y)
Browser (Frontend)->>Go Backend: HandlePlatformFileDrop(files, x, y)
Go Backend->>Go Backend: Extract dropzone element details
Go Backend->>Go Backend: Emit WindowDropZoneFilesDropped event (with files, dropzone metadata)
Go Backend->>Browser (Frontend): Emit frontend:FileDropInfo event (for demo)
Browser (Frontend)->>User: Show drop details in UI
Suggested reviewers
Poem
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. 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
|
github-actions
bot
added
Documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (11)
v3/pkg/application/messageprocessor_application.go (1)
20-26: Function signature formatting improved.The multi-line formatting enhances readability and follows Go conventions. However, this formatting change appears unrelated to the main drag-and-drop feature scope of this PR.
Consider whether this formatting change should be included in a separate code cleanup PR to maintain focused changesets.
v3/pkg/application/application_darwin.go (1)
342-346: Improve logging consistency and consider performance impact.The debug logging implementation has inconsistencies and potential performance concerns:
- Inconsistent logging approach: Mixing
globalApplication.Logger.Debugandfmt.Printfcreates inconsistent log formats and destinations.- Performance impact: Debug logging in a potentially high-frequency function like drag processing could impact performance.
Consider this improved approach:
- if globalApplication != nil && globalApplication.Logger != nil { - globalApplication.Logger.Debug("[DragDropDebug] processDragItems called", "windowID", windowID, "fileCount", len(filenames), "x", x, "y", y) - } else { - fmt.Printf("[DragDropDebug] processDragItems called - windowID: %d, fileCount: %d, x: %d, y: %d\n", windowID, len(filenames), x, y) - } + if globalApplication != nil && globalApplication.Logger != nil { + globalApplication.Logger.Debug("processDragItems called", "windowID", windowID, "fileCount", len(filenames), "x", x, "y", y) + }This approach:
- Removes the debug prefix tag for cleaner logs
- Eliminates the fmt.Printf fallback to maintain consistency
- Reduces verbosity while preserving essential information
docs/src/content/docs/learn/events.mdx (1)
291-295: Fix minor markdown formatting in bullet points.The static analysis tool identified loose punctuation in the bullet point formatting.
Apply this formatting fix:
-The `event.Context().DropZoneDetails()` method returns a pointer to an `application.DropZoneDetails` struct (or `nil` if details aren't available), containing: -- `ElementID string`: The `id` attribute of the HTML element that was the direct target of the drop. -- `ClassList []string`: The list of CSS classes of the HTML element. -- `X int`: The X-coordinate of the drop, relative to the window's content area. -- `Y int`: The Y-coordinate of the drop, relative to the window's content area. +The `event.Context().DropZoneDetails()` method returns a pointer to an `application.DropZoneDetails` struct (or `nil` if details aren't available), containing: +- `ElementID string`: The `id` attribute of the HTML element that was the direct target of the drop. +- `ClassList []string`: The list of CSS classes of the HTML element. +- `X int`: The X-coordinate of the drop, relative to the window's content area. +- `Y int`: The Y-coordinate of the drop, relative to the window's content area.🧰 Tools
🪛 LanguageTool
[uncategorized] ~292-~292: Loose punctuation mark.
Context: ...rget of the drop. -ClassList []string: The list of CSS classes of the HTML ele...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~293-~293: Loose punctuation mark.
Context: ...S classes of the HTML element. -X int: The X-coordinate of the drop, relative ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~294-~294: Loose punctuation mark.
Context: ... to the window's content area. -Y int: The Y-coordinate of the drop, relative ...(UNLIKELY_OPENING_PUNCTUATION)
v3/pkg/application/webview_window_darwin.m (2)
188-193: Consider making the log statement conditional for production builds.The implementation correctly registers for drag types when setting a WebviewWindowDelegate. However, the NSLog statement might be too verbose for production use.
Consider wrapping the log in a debug flag or removing it for production builds:
- NSLog(@"WebviewWindow: setDelegate - Registering window for dragged types (NSFilenamesPboardType) because WebviewWindowDelegate is being set."); + #ifdef DEBUG + NSLog(@"WebviewWindow: setDelegate - Registering window for dragged types (NSFilenamesPboardType) because WebviewWindowDelegate is being set."); + #endif
240-313: Consider reducing logging verbosity for production.While the extensive logging is excellent for debugging, it might be too verbose for production use. Consider using conditional compilation or a logging framework that supports different log levels.
v3/examples/drag-n-drop/assets/index.html (2)
47-51: Remove commented-out CSS to improve code clarity.The commented border-color property could cause confusion. Either implement it or remove it entirely.
.customDropzone { /* This is a custom BASE style variant */ background-color: #444; - /* border-color: #888; /* This would override .dropzone's border-color if also applied */ }
101-104: Fix class name for disabled drop zone.The disabled drop zone uses class
dropzone-not-so-muchwhich doesn't match any CSS rules. This appears to be intentional to disable drop functionality, but the styling won't match the other zones.If you want the disabled zone to look similar to the active zones but without drop functionality, consider:
- <div id="no-dropzone" class="dropzone-not-so-much"> + <div id="no-dropzone" class="dropzone" style="opacity: 0.5; cursor: not-allowed;">v3/pkg/application/webview_window_darwin_drag.m (1)
17-17: Consider making debug logging conditional.The NSLog statements throughout this file are helpful for debugging but should be conditionally compiled or controlled by a debug flag to avoid cluttering production logs.
Consider wrapping debug logs in a conditional compilation:
+#ifdef DEBUG NSLog(@"WebviewDrag: initWithFrame - Registering for dragged types. WindowID (at init if available, might be set later): %u", self.windowId); // self.windowId might not be set here yet. +#endifv3/pkg/application/application.go (1)
1121-1131: Consider returning an error for better debugging.The method silently returns nil on type assertion failure, which could make debugging difficult.
-func (a *App) getWindowByID(id uint) *WebviewWindow { +func (a *App) getWindowByID(id uint) (*WebviewWindow, error) { a.windowsLock.RLock() defer a.windowsLock.RUnlock() - w, ok := a.windows[id].(*WebviewWindow) + window, exists := a.windows[id] + if !exists { + return nil, fmt.Errorf("window with ID %d not found", id) + } + w, ok := window.(*WebviewWindow) if !ok { - // Or handle error appropriately, e.g., log and return nil - return nil + return nil, fmt.Errorf("window with ID %d is not a WebviewWindow", id) } - return w + return w, nil }v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (2)
536-564: Consider removing console.log statements from production code.The method correctly validates dropzones and sends drop details to the backend. However, the console.log statements (lines 543, 548, 551) should be removed or wrapped in a debug flag.
- console.log(`Wails Runtime: Drop on element (or no element) at ${x},${y} which is not a designated dropzone. Ignoring. Element:`, element); + if (window._wails?.environment?.Debug) { + console.log(`Wails Runtime: Drop on element (or no element) at ${x},${y} which is not a designated dropzone. Ignoring. Element:`, element); + } // No need to call backend if not a valid dropzone target return; } - console.log(`Wails Runtime: Drop on designated dropzone. Element at (${x}, ${y}):`, element, 'Effective dropzone:', dropzoneTarget); - // The 'element' variable is already defined from the line: const element = document.elementFromPoint(x, y); - // which should be before the dropzoneTarget check. The original console.log below also uses it. - console.log(`Window.HandlePlatformFileDrop: Original log - Dropped files at (${x}, ${y}) on element:`, element); + if (window._wails?.environment?.Debug) { + console.log(`Wails Runtime: Drop on designated dropzone. Element at (${x}, ${y}):`, element, 'Effective dropzone:', dropzoneTarget); + console.log(`Window.HandlePlatformFileDrop: Original log - Dropped files at (${x}, ${y}) on element:`, element); + }
577-630: Consider implementing optional chaining for cleaner code.The drag event handlers are well-implemented with proper state management. The static analysis tool correctly identifies opportunities for optional chaining.
Apply optional chaining for cleaner null checks:
- if (event.dataTransfer && event.dataTransfer.types.includes('Files')) { + if (event.dataTransfer?.types.includes('Files')) {This pattern can be applied at lines 579, 602, and 619.
🧰 Tools
🪛 Biome (1.9.4)
[error] 579-579: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 602-602: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 619-619: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
docs/src/content/docs/changelog.mdx(1 hunks)docs/src/content/docs/learn/events.mdx(1 hunks)v3/examples/drag-n-drop/assets/index.html(1 hunks)v3/examples/drag-n-drop/main.go(4 hunks)v3/internal/assetserver/bundledassets/runtime.js(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/system.ts(2 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts(3 hunks)v3/pkg/application/application.go(11 hunks)v3/pkg/application/application_darwin.go(2 hunks)v3/pkg/application/context_window_event.go(2 hunks)v3/pkg/application/linux_cgo.go(4 hunks)v3/pkg/application/messageprocessor_application.go(1 hunks)v3/pkg/application/messageprocessor_window.go(8 hunks)v3/pkg/application/webview_window.go(8 hunks)v3/pkg/application/webview_window_darwin.m(3 hunks)v3/pkg/application/webview_window_darwin_drag.m(1 hunks)v3/pkg/application/window.go(1 hunks)v3/pkg/events/events.go(6 hunks)v3/pkg/events/events.txt(1 hunks)v3/pkg/events/events_darwin.h(1 hunks)v3/pkg/events/events_linux.h(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
v3/pkg/application/messageprocessor_application.go (3)
v3/pkg/application/messageprocessor.go (1)
MessageProcessor(31-36)v3/pkg/application/window.go (1)
Window(14-89)v3/pkg/application/messageprocessor_params.go (1)
QueryParams(9-9)
v3/pkg/application/application_darwin.go (1)
v2/pkg/assetserver/assethandler.go (1)
Logger(19-22)
v3/pkg/application/window.go (1)
v3/pkg/application/application.go (1)
DropZoneDetails(229-234)
v3/pkg/application/context_window_event.go (1)
v3/pkg/application/application.go (1)
DropZoneDetails(229-234)
v3/pkg/application/messageprocessor_window.go (5)
v3/pkg/application/messageprocessor.go (1)
MessageProcessor(31-36)v3/pkg/application/window.go (1)
Window(14-89)v3/pkg/application/messageprocessor_params.go (1)
QueryParams(9-9)v3/pkg/application/application.go (1)
DropZoneDetails(229-234)v3/pkg/application/webview_window.go (1)
WebviewWindow(137-171)
v3/pkg/events/events.go (1)
v3/pkg/application/messageprocessor_window.go (12)
WindowHide(23-23)WindowMaximise(28-28)WindowUnMaximise(53-53)WindowMinimise(29-29)WindowUnMinimise(54-54)WindowShow(48-48)WindowZoomIn(57-57)WindowZoomOut(58-58)WindowZoomReset(59-59)WindowFullscreen(19-19)WindowRestore(35-35)WindowUnFullscreen(52-52)
🪛 LanguageTool
docs/src/content/docs/learn/events.mdx
[uncategorized] ~292-~292: Loose punctuation mark.
Context: ...rget of the drop. - ClassList []string: The list of CSS classes of the HTML ele...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~293-~293: Loose punctuation mark.
Context: ...S classes of the HTML element. - X int: The X-coordinate of the drop, relative ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~294-~294: Loose punctuation mark.
Context: ... to the window's content area. - Y int: The Y-coordinate of the drop, relative ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Biome (1.9.4)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
[error] 579-579: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 602-602: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 619-619: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
v3/internal/assetserver/bundledassets/runtime.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
🔇 Additional comments (35)
docs/src/content/docs/changelog.mdx (1)
36-36: LGTM! Well-formatted changelog entry.The changelog entry follows the established format and properly documents the new drag-and-drop dropzone feature with appropriate contributor attribution and PR reference.
v3/pkg/events/events.txt (1)
25-25: LGTM! Event identifier follows naming conventions.The new
WindowDropZoneFilesDroppedevent is properly named and placed in the common events section, consistent with other window-related events.v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts (1)
230-230: LGTM! Event type is consistent across the codebase.The new
WindowDropZoneFilesDroppedevent type matches the corresponding identifier inv3/pkg/events/events.txtand follows the established naming conventions.v3/pkg/application/linux_cgo.go (3)
80-80: LGTM: Function declaration properly updated for coordinate tracking.The addition of
xandyparameters to theonUriListfunction declaration correctly supports the enhanced drag-and-drop functionality with coordinate information.
233-233: LGTM: Coordinate forwarding implemented correctly.The modification properly passes the drag event coordinates (
x,y) from the GTK callback to the Go handler function.
1519-1519: LGTM: Go implementation properly updated for coordinate tracking.The function signature and implementation correctly incorporate the drag event coordinates into the
dragAndDropMessagestruct, enabling enhanced dropzone functionality.Also applies to: 1531-1532
v3/pkg/events/events_darwin.h (1)
9-142: LGTM: Systematic event ID increment is correct and complete.All Darwin-specific event constants have been consistently incremented by 1 to accommodate the new
WindowDropZoneFilesDroppedevent (ID 1048) in the common events. TheMAX_EVENTSconstant is properly updated to reflect the new maximum value.v3/pkg/application/window.go (2)
30-30: LGTM: Enhanced method signature supports rich dropzone context.The addition of the
dropZone *DropZoneDetailsparameter toHandleDragAndDropMessageenables passing detailed information about the drop target element, including coordinates, element ID, and CSS classes. The pointer type allows for flexible handling when dropzone details may not be available.
31-31: LGTM: New method enables backend-to-frontend drop coordination.The
InitiateFrontendDropProcessingmethod provides a clean interface for the backend to trigger frontend handling of dropped files at specific coordinates, enabling seamless integration between platform-native drop events and frontend dropzone processing.v3/internal/runtime/desktop/@wailsio/runtime/src/system.ts (2)
17-17: LGTM: New method constant properly defined.The
ApplicationFilesDroppedWithContextconstant is appropriately named and uses a distinct value (100) that avoids conflicts with existing method IDs.
158-188: LGTM: Well-implemented platform file drop handler.The
HandlePlatformFileDropfunction correctly:
- Uses
elementFromPointto identify the drop target element- Extracts relevant element properties (id, classList)
- Constructs a comprehensive payload with all necessary context
- Includes proper error handling and logging
- Uses appropriate async patterns for backend communication
The implementation provides seamless integration between platform-native drag-and-drop events and the enhanced dropzone system.
v3/pkg/application/application_darwin.go (2)
334-334: Function signature update looks correct.The addition of x and y coordinates to the function signature properly supports the enhanced drag-and-drop functionality with position information.
356-362:✅ Verification successful
Verify InitiateFrontendDropProcessing method exists.
The code calls
targetWindow.InitiateFrontendDropProcessing(filenames, int(x), int(y))but we should verify this method exists and has the correct signature.
🏁 Script executed:
#!/bin/bash # Description: Verify InitiateFrontendDropProcessing method exists with correct signature # Expected: Find the method definition with matching parameters ast-grep --pattern 'func ($receiver) InitiateFrontendDropProcessing($files, $x int, $y int) { $$$ }'Length of output: 104
🏁 Script executed:
#!/bin/bash # Search for the definition and references of InitiateFrontendDropProcessing rg -n "InitiateFrontendDropProcessing"Length of output: 818
Confirmed: InitiateFrontendDropProcessing exists with matching signature
Found the method definition and interface declaration:
- v3/pkg/application/webview_window.go:
func (w *WebviewWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) { … }- v3/pkg/application/window.go:
InitiateFrontendDropProcessing(filenames []string, x int, y int)No further changes required.
v3/pkg/application/context_window_event.go (4)
6-8: Good addition of constant for drop zone details key.The new constant follows the existing pattern and maintains consistency with the
droppedFilesconstant.
28-32: Good defensive programming improvement.The addition of nil check and map initialization in
setDroppedFilesprevents potential panic conditions and ensures the data map is always available.
39-45: Excellent implementation of setDropZoneDetails.The method properly handles both nil and non-nil cases, maintaining clear semantics for when drop zone details are unavailable.
47-63: Robust implementation of DropZoneDetails accessor.The method includes comprehensive error handling:
- Checks if key exists in map
- Handles nil values explicitly
- Performs safe type assertion
- Returns nil for any failure case
This follows defensive programming best practices and provides a reliable API.
v3/pkg/events/events_linux.h (1)
9-18: Event ID increments are correctly applied.All Linux-specific event IDs have been properly incremented by 1 to accommodate the new
WindowDropZoneFilesDroppedcommon event. This maintains consistency across the event system and ensures no ID conflicts.docs/src/content/docs/learn/events.mdx (1)
203-301: Excellent comprehensive documentation for the new dropzone feature.This documentation section provides:
- Clear explanations of how to define dropzones in HTML
- Practical CSS examples for visual feedback
- Complete Go code examples showing event handling
- Detailed explanation of the DropZoneDetails struct and its usage
- Reference to the example application
The documentation is well-structured and will help developers understand and implement the new drag-and-drop functionality effectively.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~292-~292: Loose punctuation mark.
Context: ...rget of the drop. -ClassList []string: The list of CSS classes of the HTML ele...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~293-~293: Loose punctuation mark.
Context: ...S classes of the HTML element. -X int: The X-coordinate of the drop, relative ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~294-~294: Loose punctuation mark.
Context: ... to the window's content area. -Y int: The Y-coordinate of the drop, relative ...(UNLIKELY_OPENING_PUNCTUATION)
v3/pkg/application/webview_window_darwin.m (1)
8-8: LGTM! Function signature correctly updated for coordinate support.The addition of
xandyparameters toprocessDragItemsaligns with the PR's objective to provide drop coordinates to the backend.v3/examples/drag-n-drop/main.go (1)
18-33: LGTM! Proper App struct implementation.The App struct and its methods follow standard Wails patterns correctly.
v3/pkg/application/webview_window_darwin_drag.m (2)
9-9: Function signature update looks good.The addition of x and y parameters to
processDragItemsproperly supports the new drop zone coordinate tracking feature.
69-81: Coordinate conversion implementation is correct.The logic properly converts from macOS's bottom-left origin coordinate system to the standard top-left origin system by using the content view's height. This addresses the macOS-specific positioning discrepancy mentioned in the PR objectives.
v3/pkg/application/messageprocessor_window.go (2)
115-121: Function signature change is appropriate.The addition of the
req *http.Requestparameter enables access to query parameters needed for the new drop zone functionality. Note that this is a breaking change for any code calling this method directly.
487-493: Well-structured payload definition.The
fileDropPayloadstruct properly captures all the necessary drop zone information including coordinates, element ID, and CSS classes.v3/pkg/application/application.go (1)
227-234: Well-designed drop zone details structure.The
DropZoneDetailsstruct is properly documented and includes all necessary information for identifying the drop target element.v3/pkg/events/events.go (3)
33-33: Well-structured event addition!The new
WindowDropZoneFilesDroppedevent is properly integrated into the common events structure with ID 1048.Also applies to: 62-62
493-677: Event-to-JS mapping correctly updated.The
eventToJSmap properly includes the new event at ID 1048 and all shifted IDs maintain their correct string mappings.
81-89:✅ Verification successful
Verify backward compatibility for event ID changes.
All platform-specific event IDs have been systematically incremented by 1. This is a breaking change if any external code relies on specific event IDs.
Run the following script to check for hardcoded event IDs in the codebase:
Also applies to: 231-363, 417-461
🏁 Script executed:
#!/bin/bash # Description: Search for potential hardcoded event IDs that might be affected by the increment # Look for numeric values in the range 1049-1232 (the shifted ID range) rg -A 3 -B 3 '\b(10[4-9][0-9]|11[0-9]{2}|12[0-2][0-9]|123[0-2])\b' --type go --type js --type ts | grep -v 'events.go' | grep -v 'events.txt'Length of output: 35705
No hardcoded event-ID literals detected
A search for numeric literals in the 1049–1232 range returned only:
- Windows error codes in v3/pkg/w32/constants.go (unrelated to event IDs)
- Pixel-coordinate values in examples/tests
No direct usage of event IDs as raw numbers was found in Go, JS or TS sources. Merging is safe, but if any external consumers relied on the old IDs by literal value, please update your code or document this shift in the CHANGELOG/API notes.
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (3)
14-18: Well-implemented dropzone constants and state management!The constants are clearly named and the hover state tracking with
currentHoveredDropzoneprovides good UX feedback.
69-75: Elegant dropzone detection implementation!The use of
closest()allows flexible dropzone nesting and provides a clean API for developers.
620-628: Robust drag leave handling!The counter-based approach correctly handles nested elements and the boundary condition checks prevent premature hover removal.
v3/internal/assetserver/bundledassets/runtime.js (1)
1-2: Skip review of minified runtime.jsThis is a minified build artifact. Code review should focus on the source files instead.
Verify that this file is properly generated from the source:
#!/bin/bash # Description: Verify runtime.js is a build artifact and check for source map # Check if this is listed in gitignore (build artifacts should be) rg "runtime\.js" .gitignore # Look for the build script that generates this file fd -t f "build|bundle|webpack|rollup|vite" -e json -e js -e ts | xargs rg "runtime\.js" # Check if there's a source map file fd "runtime\.js\.map" v3/internal/assetserver/bundledassets/🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
v3/pkg/application/webview_window.go (2)
4-4: LGTM!The addition of the
encoding/jsonimport is necessary for the newInitiateFrontendDropProcessingmethod.
211-214: LGTM!The method signature formatting changes improve code readability by placing parameters on separate lines.
Also applies to: 294-296, 319-327, 332-339, 344-351, 356-364, 776-779, 805-808
| targetWindow := globalApplication.getWindowByID(uint(windowID)) | ||
| if targetWindow == nil { | ||
| // Log an error if the window is not found. Consider using the application's logger. | ||
| // For now, a simple println to stderr or a log if available. | ||
| // TODO: Replace with proper logging if globalApplication.Logger is accessible and appropriate here. | ||
| println("Error: processDragItems could not find window with ID:", uint(windowID)) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling consistency.
The error handling uses println which is inconsistent with the rest of the codebase and may not be appropriate for production environments.
Apply this improvement for consistent error handling:
- targetWindow := globalApplication.getWindowByID(uint(windowID))
- if targetWindow == nil {
- // Log an error if the window is not found. Consider using the application's logger.
- // For now, a simple println to stderr or a log if available.
- // TODO: Replace with proper logging if globalApplication.Logger is accessible and appropriate here.
- println("Error: processDragItems could not find window with ID:", uint(windowID))
- return
- }
+ targetWindow := globalApplication.getWindowByID(uint(windowID))
+ if targetWindow == nil {
+ if globalApplication != nil && globalApplication.Logger != nil {
+ globalApplication.Logger.Error("processDragItems could not find window", "windowID", uint(windowID))
+ }
+ return
+ }This provides consistent logging and removes the TODO comment.
📝 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.
| targetWindow := globalApplication.getWindowByID(uint(windowID)) | |
| if targetWindow == nil { | |
| // Log an error if the window is not found. Consider using the application's logger. | |
| // For now, a simple println to stderr or a log if available. | |
| // TODO: Replace with proper logging if globalApplication.Logger is accessible and appropriate here. | |
| println("Error: processDragItems could not find window with ID:", uint(windowID)) | |
| return | |
| } | |
| targetWindow := globalApplication.getWindowByID(uint(windowID)) | |
| if targetWindow == nil { | |
| if globalApplication != nil && globalApplication.Logger != nil { | |
| globalApplication.Logger.Error("processDragItems could not find window", "windowID", uint(windowID)) | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
In v3/pkg/application/application_darwin.go around lines 347 to 354, replace the
println error message with a call to the application's logger (e.g.,
globalApplication.Logger) to log the error consistently with the rest of the
codebase. Remove the TODO comment after implementing this proper logging to
ensure production-appropriate error handling.
| func (c WindowEventContext) setCoordinates(x, y int) { | ||
| c.data["x"] = x | ||
| c.data["y"] = y | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add nil check for data map consistency.
The setCoordinates method doesn't check if c.data is nil before accessing it, which could cause a panic.
Apply this fix for consistency with other setter methods:
func (c WindowEventContext) setCoordinates(x, y int) {
+ if c.data == nil {
+ c.data = make(map[string]any)
+ }
c.data["x"] = x
c.data["y"] = y
}📝 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.
| func (c WindowEventContext) setCoordinates(x, y int) { | |
| c.data["x"] = x | |
| c.data["y"] = y | |
| } | |
| func (c WindowEventContext) setCoordinates(x, y int) { | |
| if c.data == nil { | |
| c.data = make(map[string]any) | |
| } | |
| c.data["x"] = x | |
| c.data["y"] = y | |
| } |
🤖 Prompt for AI Agents
In v3/pkg/application/context_window_event.go around lines 34 to 37, the
setCoordinates method accesses the c.data map without checking if it is nil,
which can cause a panic. Add a nil check for c.data at the start of the method
and initialize it as an empty map if it is nil before setting the "x" and "y"
keys. This will ensure consistency with other setter methods and prevent runtime
panics.
| char** cArray = (char**)malloc(count * sizeof(char*)); | ||
| for (NSUInteger i = 0; i < count; i++) { | ||
| NSString* str = files[i]; | ||
| NSLog(@"WebviewWindowDelegate: performDragOperation - File %lu: %@", (unsigned long)i, str); | ||
| cArray[i] = (char*)[str UTF8String]; | ||
| } |
There was a problem hiding this comment.
Add null check after malloc to prevent potential crash.
The malloc call could fail and return NULL, which would cause a crash when dereferencing.
Apply this fix to add proper error handling:
char** cArray = (char**)malloc(count * sizeof(char*));
+ if (cArray == NULL) {
+ NSLog(@"WebviewWindowDelegate: performDragOperation - Failed to allocate memory for file array.");
+ return NO;
+ }
for (NSUInteger i = 0; i < count; i++) {
NSString* str = files[i];
NSLog(@"WebviewWindowDelegate: performDragOperation - File %lu: %@", (unsigned long)i, str);
cArray[i] = (char*)[str UTF8String];
}📝 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.
| char** cArray = (char**)malloc(count * sizeof(char*)); | |
| for (NSUInteger i = 0; i < count; i++) { | |
| NSString* str = files[i]; | |
| NSLog(@"WebviewWindowDelegate: performDragOperation - File %lu: %@", (unsigned long)i, str); | |
| cArray[i] = (char*)[str UTF8String]; | |
| } | |
| char** cArray = (char**)malloc(count * sizeof(char*)); | |
| if (cArray == NULL) { | |
| NSLog(@"WebviewWindowDelegate: performDragOperation - Failed to allocate memory for file array."); | |
| return NO; | |
| } | |
| for (NSUInteger i = 0; i < count; i++) { | |
| NSString* str = files[i]; | |
| NSLog(@"WebviewWindowDelegate: performDragOperation - File %lu: %@", (unsigned long)i, str); | |
| cArray[i] = (char*)[str UTF8String]; | |
| } |
🤖 Prompt for AI Agents
In v3/pkg/application/webview_window_darwin.m around lines 286 to 291, the
malloc call for cArray does not check if the allocation succeeded, which can
lead to a crash if it returns NULL. Add a null check immediately after malloc to
verify cArray is not NULL before using it. If it is NULL, handle the error
appropriately, such as returning early or logging an error, to prevent
dereferencing a null pointer.
| wails.Events.On("frontend:FileDropInfo", (eventData) => { | ||
| console.log("JS: Received 'frontend:FileDropInfo' event with data:", eventData); | ||
| const { files, targetID, targetClasses, dropX, dropY } = eventData.data[0]; // Access the first element of the data array | ||
|
|
||
| let message = `Dropped on Element ID: ${targetID || 'N/A'}\n`; | ||
| message += `Element Classes: ${targetClasses && targetClasses.length > 0 ? targetClasses.join(', ') : 'N/A'}\n`; | ||
| message += `Files Dropped: ${files.join(', ')}\n`; | ||
| message += `Coordinates (from Go): X=${dropX.toFixed(2)}, Y=${dropY.toFixed(2)}`; | ||
|
|
||
| outputDiv.textContent = message; | ||
| outputDiv.scrollTop = outputDiv.scrollHeight; // Scroll to bottom | ||
| }); | ||
| document.getElementById("results").innerHTML = resultsHTML; | ||
| }) | ||
| </script> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety check for array access.
The code accesses eventData.data[0] without verifying the array exists and has elements.
wails.Events.On("frontend:FileDropInfo", (eventData) => {
console.log("JS: Received 'frontend:FileDropInfo' event with data:", eventData);
+ if (!eventData.data || eventData.data.length === 0) {
+ console.error("No data in event payload");
+ return;
+ }
const { files, targetID, targetClasses, dropX, dropY } = eventData.data[0]; // Access the first element of the data array📝 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.
| wails.Events.On("frontend:FileDropInfo", (eventData) => { | |
| console.log("JS: Received 'frontend:FileDropInfo' event with data:", eventData); | |
| const { files, targetID, targetClasses, dropX, dropY } = eventData.data[0]; // Access the first element of the data array | |
| let message = `Dropped on Element ID: ${targetID || 'N/A'}\n`; | |
| message += `Element Classes: ${targetClasses && targetClasses.length > 0 ? targetClasses.join(', ') : 'N/A'}\n`; | |
| message += `Files Dropped: ${files.join(', ')}\n`; | |
| message += `Coordinates (from Go): X=${dropX.toFixed(2)}, Y=${dropY.toFixed(2)}`; | |
| outputDiv.textContent = message; | |
| outputDiv.scrollTop = outputDiv.scrollHeight; // Scroll to bottom | |
| }); | |
| document.getElementById("results").innerHTML = resultsHTML; | |
| }) | |
| </script> | |
| wails.Events.On("frontend:FileDropInfo", (eventData) => { | |
| console.log("JS: Received 'frontend:FileDropInfo' event with data:", eventData); | |
| if (!eventData.data || eventData.data.length === 0) { | |
| console.error("No data in event payload"); | |
| return; | |
| } | |
| const { files, targetID, targetClasses, dropX, dropY } = eventData.data[0]; // Access the first element of the data array | |
| let message = `Dropped on Element ID: ${targetID || 'N/A'}\n`; | |
| message += `Element Classes: ${targetClasses && targetClasses.length > 0 ? targetClasses.join(', ') : 'N/A'}\n`; | |
| message += `Files Dropped: ${files.join(', ')}\n`; | |
| message += `Coordinates (from Go): X=${dropX.toFixed(2)}, Y=${dropY.toFixed(2)}`; | |
| outputDiv.textContent = message; | |
| outputDiv.scrollTop = outputDiv.scrollHeight; // Scroll to bottom | |
| }); |
🤖 Prompt for AI Agents
In v3/examples/drag-n-drop/assets/index.html around lines 114 to 126, the code
accesses eventData.data[0] without checking if eventData.data is an array and
contains elements. Add a safety check to verify that eventData.data exists and
has at least one element before accessing eventData.data[0]. If the check fails,
handle the case gracefully, such as logging an error or returning early to avoid
runtime errors.
v3/examples/drag-n-drop/main.go
Outdated
| log.Println("Setting up event listener for 'WindowDropZoneFilesDropped'...") | ||
| win.OnWindowEvent( | ||
| events.Common.WindowDropZoneFilesDropped, | ||
| func(event *application.WindowEvent) { | ||
| droppedFiles := event.Context().DroppedFiles() | ||
| details := event.Context().DropZoneDetails() | ||
| // Call the App method with the extracted data | ||
| if details != nil { | ||
| FilesDroppedOnTarget( | ||
| droppedFiles, | ||
| details.ElementID, | ||
| details.ClassList, | ||
| float64(details.X), | ||
| float64(details.Y), | ||
| details.ElementID != "", // isTargetDropzone based on whether an ID was found | ||
| ) | ||
| } else { | ||
| // This case might occur if DropZoneDetails are nil, meaning the drop was not on a specific registered zone | ||
| // or if the context itself was problematic. | ||
| FilesDroppedOnTarget(droppedFiles, "", nil, 0, 0, false) | ||
| } | ||
|
|
||
| payload := FileDropInfo{ | ||
| Files: droppedFiles, | ||
| TargetID: details.ElementID, | ||
| TargetClasses: details.ClassList, | ||
| DropX: float64(details.X), | ||
| DropY: float64(details.Y), | ||
| } | ||
| application.Get().EmitEvent("frontend:FileDropInfo", payload) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring to eliminate code duplication.
The FilesDroppedOnTarget function and the event handler both create the same FileDropInfo payload. Consider refactoring to have FilesDroppedOnTarget return the payload or emit the event directly.
Would you like me to suggest a refactored version that eliminates this duplication?
🤖 Prompt for AI Agents
In v3/examples/drag-n-drop/main.go around lines 99 to 130, the event handler and
the FilesDroppedOnTarget function both create the same FileDropInfo payload,
causing code duplication. Refactor by modifying FilesDroppedOnTarget to either
return the FileDropInfo payload or emit the event directly, then update the
event handler to use this returned payload or rely on FilesDroppedOnTarget for
emitting the event, thereby eliminating the duplicated payload creation.
| m.Info( | ||
| "[DragDropDebug] processWindowMethod: Entered WindowDropZoneDropped case", | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make debug logging conditional.
The extensive debug logging should be controlled by a debug flag or removed for production builds.
Consider using a debug flag:
- m.Info(
- "[DragDropDebug] processWindowMethod: Entered WindowDropZoneDropped case",
- )
+ if m.logger.Enabled(nil, slog.LevelDebug) {
+ m.logger.Debug("[DragDropDebug] processWindowMethod: Entered WindowDropZoneDropped case")
+ }Apply similar changes to other debug log statements in this case.
Also applies to: 434-434, 442-446, 472-475
🤖 Prompt for AI Agents
In v3/pkg/application/messageprocessor_window.go around lines 424 to 426, the
debug log statement inside the WindowDropZoneDropped case should be made
conditional based on a debug flag to avoid verbose logging in production.
Introduce a debug flag variable and wrap this log call and other debug logs in
this case (lines 434, 442-446, 472-475) with an if statement checking the flag
before logging. This ensures debug logs only appear when debugging is enabled.
| a.Logger.Debug("[DragDropDebug] App.Run: Received message from windowDragAndDropBuffer", "message", fmt.Sprintf("%+v", dragAndDropMessage)) | ||
| go a.handleDragAndDropMessage(dragAndDropMessage) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use structured logging levels for debug messages.
Debug logging should use appropriate log levels rather than always logging.
- a.Logger.Debug("[DragDropDebug] App.Run: Received message from windowDragAndDropBuffer", "message", fmt.Sprintf("%+v", dragAndDropMessage))
+ if a.Logger.Enabled(nil, slog.LevelDebug) {
+ a.Logger.Debug("Received drag and drop message",
+ "windowId", dragAndDropMessage.windowId,
+ "fileCount", len(dragAndDropMessage.filenames),
+ "dropZone", dragAndDropMessage.DropZone)
+ }Also applies to: 817-817, 828-828
🤖 Prompt for AI Agents
In v3/pkg/application/application.go at lines 692-693, 817, and 828, the debug
messages are logged without using structured logging levels. Update these debug
logs to use the appropriate structured logging level methods provided by the
logger, ensuring that debug messages are properly categorized and filtered by
log level. Replace the current debug log calls with the logger's debug level
functions that support structured logging.
| func (w *WebviewWindow) HandleDragAndDropMessage(filenames []string, dropZone *DropZoneDetails) { | ||
| fmt.Printf( | ||
| "[DragDropDebug] HandleDragAndDropMessage called - Files: %v, DropZone: %+v\n", | ||
| filenames, | ||
| dropZone, | ||
| ) | ||
| thisEvent := NewWindowEvent() | ||
| fmt.Printf( | ||
| "[DragDropDebug] HandleDragAndDropMessage: thisEvent created, thisEvent.ctx is initially: %p\n", | ||
| thisEvent.ctx, | ||
| ) | ||
| ctx := newWindowEventContext() | ||
| ctx.setDroppedFiles(filenames) | ||
| if dropZone != nil { // Check if dropZone details are available | ||
| ctx.setDropZoneDetails(dropZone) | ||
| } | ||
| thisEvent.ctx = ctx | ||
| for _, listener := range w.eventListeners[uint(events.Common.WindowFilesDropped)] { | ||
| fmt.Printf( | ||
| "[DragDropDebug] HandleDragAndDropMessage: thisEvent.ctx assigned, thisEvent.ctx is now: %p, ctx is: %p\n", | ||
| thisEvent.ctx, | ||
| ctx, | ||
| ) | ||
| listeners := w.eventListeners[uint(events.Common.WindowDropZoneFilesDropped)] | ||
| fmt.Printf( | ||
| "[DragDropDebug] HandleDragAndDropMessage: Found %d listeners for WindowDropZoneFilesDropped\n", | ||
| len(listeners), | ||
| ) | ||
| fmt.Printf( | ||
| "[DragDropDebug] HandleDragAndDropMessage: Before calling listeners, thisEvent.ctx is: %p\n", | ||
| thisEvent.ctx, | ||
| ) | ||
| for _, listener := range listeners { | ||
| if listener == nil { | ||
| fmt.Println("[DragDropDebug] HandleDragAndDropMessage: Skipping nil listener") | ||
| continue | ||
| } | ||
| listener.callback(thisEvent) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or conditionalize debug print statements.
The method correctly implements drop zone handling, but the extensive debug logging should be removed or made conditional for production use. Additionally, printing memory addresses could be a security concern.
Consider removing all debug print statements or wrapping them in a debug mode check:
func (w *WebviewWindow) HandleDragAndDropMessage(filenames []string, dropZone *DropZoneDetails) {
- fmt.Printf(
- "[DragDropDebug] HandleDragAndDropMessage called - Files: %v, DropZone: %+v\n",
- filenames,
- dropZone,
- )
thisEvent := NewWindowEvent()
- fmt.Printf(
- "[DragDropDebug] HandleDragAndDropMessage: thisEvent created, thisEvent.ctx is initially: %p\n",
- thisEvent.ctx,
- )
ctx := newWindowEventContext()
ctx.setDroppedFiles(filenames)
if dropZone != nil { // Check if dropZone details are available
ctx.setDropZoneDetails(dropZone)
}
thisEvent.ctx = ctx
- fmt.Printf(
- "[DragDropDebug] HandleDragAndDropMessage: thisEvent.ctx assigned, thisEvent.ctx is now: %p, ctx is: %p\n",
- thisEvent.ctx,
- ctx,
- )
listeners := w.eventListeners[uint(events.Common.WindowDropZoneFilesDropped)]
- fmt.Printf(
- "[DragDropDebug] HandleDragAndDropMessage: Found %d listeners for WindowDropZoneFilesDropped\n",
- len(listeners),
- )
- fmt.Printf(
- "[DragDropDebug] HandleDragAndDropMessage: Before calling listeners, thisEvent.ctx is: %p\n",
- thisEvent.ctx,
- )
for _, listener := range listeners {
if listener == nil {
- fmt.Println("[DragDropDebug] HandleDragAndDropMessage: Skipping nil listener")
continue
}
listener.callback(thisEvent)
}
}Alternatively, use the application's debug logging:
if globalApplication.isDebugMode() {
globalApplication.debug("HandleDragAndDropMessage called", "files", filenames, "dropZone", dropZone)
}🤖 Prompt for AI Agents
In v3/pkg/application/webview_window.go from lines 1216 to 1254, remove all
fmt.Printf debug print statements or wrap them in a conditional check that
verifies if the application is in debug mode. Replace direct print calls with
the application's debug logging method, such as calling globalApplication.debug
with appropriate context information, ensuring debug logs are only emitted when
debug mode is enabled to avoid security risks and clutter in production.
| fmt.Printf( | ||
| "[DEBUG] OnWindowEvent: Registering window event listener - eventID: %d, eventType: %s\n", | ||
| eventID, | ||
| eventType, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the application's debug logging system instead of fmt.Printf.
Debug statements should use the application's logging system for consistency and control over log levels.
Replace the debug print with the application's debug method:
- fmt.Printf(
- "[DEBUG] OnWindowEvent: Registering window event listener - eventID: %d, eventType: %s\n",
- eventID,
- eventType,
- )
+ globalApplication.debug("OnWindowEvent: Registering window event listener", "eventID", eventID, "eventType", eventType)📝 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.
| fmt.Printf( | |
| "[DEBUG] OnWindowEvent: Registering window event listener - eventID: %d, eventType: %s\n", | |
| eventID, | |
| eventType, | |
| ) | |
| fmt.Printf( | |
| - "[DEBUG] OnWindowEvent: Registering window event listener - eventID: %d, eventType: %s\n", | |
| - eventID, | |
| - eventType, | |
| - ) | |
| + globalApplication.debug( | |
| + "OnWindowEvent: Registering window event listener", | |
| + "eventID", eventID, | |
| + "eventType", eventType, | |
| + ) |
🤖 Prompt for AI Agents
In v3/pkg/application/webview_window.go around lines 781 to 785, replace the
fmt.Printf debug statement with the application's debug logging method to
maintain consistency and proper log level control. Identify the appropriate
debug logger instance or method used in the application and use it to log the
message instead of fmt.Printf, ensuring the format and variables remain the
same.
| func (w *WebviewWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) { | ||
| fmt.Printf( | ||
| "[DragDropDebug] InitiateFrontendDropProcessing called - X: %d, Y: %d", | ||
| x, | ||
| y, | ||
| ) | ||
| if w.impl == nil || w.isDestroyed() { | ||
| return | ||
| } | ||
|
|
||
| filenamesJSON, err := json.Marshal(filenames) | ||
| if err != nil { | ||
| w.Error("Error marshalling filenames for drop processing: %s", err) | ||
| return | ||
| } | ||
|
|
||
| jsCall := fmt.Sprintf( | ||
| "window.wails.Window.HandlePlatformFileDrop(%s, %d, %d);", | ||
| string(filenamesJSON), | ||
| x, | ||
| y, | ||
| ) | ||
|
|
||
| // Ensure JS is executed after runtime is loaded | ||
| if !w.runtimeLoaded { | ||
| w.pendingJS = append(w.pendingJS, jsCall) | ||
| return | ||
| } | ||
|
|
||
| InvokeSync(func() { | ||
| w.impl.execJS(jsCall) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug statement and consider adding JSDoc comment.
The implementation is solid with proper error handling and runtime state management. However, the debug print should be removed or use the application's logging system.
Apply these improvements:
+// InitiateFrontendDropProcessing invokes JavaScript in the frontend runtime to handle
+// platform-specific file drops with the given coordinates.
func (w *WebviewWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) {
- fmt.Printf(
- "[DragDropDebug] InitiateFrontendDropProcessing called - X: %d, Y: %d",
- x,
- y,
- )
if w.impl == nil || w.isDestroyed() {
return
}
filenamesJSON, err := json.Marshal(filenames)
if err != nil {
w.Error("Error marshalling filenames for drop processing: %s", err)
return
}
jsCall := fmt.Sprintf(
"window.wails.Window.HandlePlatformFileDrop(%s, %d, %d);",
string(filenamesJSON),
x,
y,
)
// Ensure JS is executed after runtime is loaded
if !w.runtimeLoaded {
w.pendingJS = append(w.pendingJS, jsCall)
return
}
InvokeSync(func() {
w.impl.execJS(jsCall)
})
}📝 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.
| func (w *WebviewWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) { | |
| fmt.Printf( | |
| "[DragDropDebug] InitiateFrontendDropProcessing called - X: %d, Y: %d", | |
| x, | |
| y, | |
| ) | |
| if w.impl == nil || w.isDestroyed() { | |
| return | |
| } | |
| filenamesJSON, err := json.Marshal(filenames) | |
| if err != nil { | |
| w.Error("Error marshalling filenames for drop processing: %s", err) | |
| return | |
| } | |
| jsCall := fmt.Sprintf( | |
| "window.wails.Window.HandlePlatformFileDrop(%s, %d, %d);", | |
| string(filenamesJSON), | |
| x, | |
| y, | |
| ) | |
| // Ensure JS is executed after runtime is loaded | |
| if !w.runtimeLoaded { | |
| w.pendingJS = append(w.pendingJS, jsCall) | |
| return | |
| } | |
| InvokeSync(func() { | |
| w.impl.execJS(jsCall) | |
| }) | |
| } | |
| // InitiateFrontendDropProcessing invokes JavaScript in the frontend runtime to handle | |
| // platform-specific file drops with the given coordinates. | |
| func (w *WebviewWindow) InitiateFrontendDropProcessing(filenames []string, x int, y int) { | |
| if w.impl == nil || w.isDestroyed() { | |
| return | |
| } | |
| filenamesJSON, err := json.Marshal(filenames) | |
| if err != nil { | |
| w.Error("Error marshalling filenames for drop processing: %s", err) | |
| return | |
| } | |
| jsCall := fmt.Sprintf( | |
| "window.wails.Window.HandlePlatformFileDrop(%s, %d, %d);", | |
| string(filenamesJSON), | |
| x, | |
| y, | |
| ) | |
| // Ensure JS is executed after runtime is loaded | |
| if !w.runtimeLoaded { | |
| w.pendingJS = append(w.pendingJS, jsCall) | |
| return | |
| } | |
| InvokeSync(func() { | |
| w.impl.execJS(jsCall) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In v3/pkg/application/webview_window.go between lines 1460 and 1492, remove the
fmt.Printf debug statement and replace it with a call to the application's
logging system if logging is needed. Additionally, add a JSDoc-style comment
above the InitiateFrontendDropProcessing function to describe its purpose,
parameters, and behavior for better code documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
v3/examples/drag-n-drop/assets/index.html (1)
114-116: Add safety check for array access.The code accesses
eventData.data[0]without verifying the array exists and has elements.
🧹 Nitpick comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
582-582: Use optional chaining for better null safety.The static analysis correctly identifies opportunities to use optional chaining for cleaner and safer code.
Apply these improvements:
- if (event.dataTransfer && event.dataTransfer.types.includes('Files')) { + if (event.dataTransfer?.types.includes('Files')) {This pattern should be applied at lines 582, 605, and 622 for consistency and improved null safety.
Also applies to: 605-605, 622-622
🧰 Tools
🪛 Biome (1.9.4)
[error] 582-582: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
v3/examples/drag-n-drop/assets/index.html(1 hunks)v3/examples/drag-n-drop/main.go(4 hunks)v3/internal/assetserver/bundledassets/runtime.js(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts(3 hunks)v3/pkg/application/application.go(11 hunks)v3/pkg/application/messageprocessor_window.go(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- v3/examples/drag-n-drop/main.go
- v3/pkg/application/messageprocessor_window.go
- v3/pkg/application/application.go
🧰 Additional context used
🪛 Biome (1.9.4)
v3/internal/assetserver/bundledassets/runtime.js
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
[error] 582-582: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 605-605: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 622-622: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
v3/internal/assetserver/bundledassets/runtime.js (1)
1-2: Skip review of minified/bundled file.This appears to be a minified/bundled JavaScript file. The static analysis warnings are expected artifacts of the minification process and not indicative of actual code issues. The source code for this bundle should be reviewed in its original form rather than this minified version.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
|
Updated Example: v3-dnd-filetree.mov |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
v3/examples/drag-n-drop/assets/index.html (1)
268-271: Add safety check before accessing eventData.data[0]
AccessingeventData.data[0]without verifyingeventData.dataexists or has elements may cause runtime errors. Introduce a guard clause early in the callback:wails.Events.On("frontend:FileDropInfo", (eventData) => { - console.log("File drop detected:", eventData); - - const { files, targetID, targetClasses, dropX, dropY, attributes } = eventData.data[0]; + console.log("File drop detected:", eventData); + + if (!Array.isArray(eventData.data) || eventData.data.length === 0) { + console.error("Received empty or invalid drop data:", eventData.data); + return; + } + const { files, targetID, targetClasses, dropX, dropY, attributes } = eventData.data[0];
🧹 Nitpick comments (2)
v3/examples/drag-n-drop/assets/index.html (2)
81-84: Review CSS class naming and usage for dropzones
The.dropzoneclass is defined in CSS but not applied to any element, and there’s no.wails-dropzonebase style. Consider:
- Renaming
.dropzoneto.wails-dropzoneto match the runtime-injected class.- Defining a base
.wails-dropzonestyle for idle state, complementing the existing hover styles.
This will ensure consistent styling for both idle and active dropzones.
151-156: Enhance accessibility for tree-node elements
The folder.tree-nodedivs act as interactive controls but lack ARIA roles and keyboard focusability. Consider adding:-<div class="tree-node folder folder-dropzone" data-path="..." data-wails-dropzone ...> +<div class="tree-node folder folder-dropzone" role="button" tabindex="0" data-path="..." data-wails-dropzone ...>And handle
keydownevents for Enter/Space to mirror theclickbehavior for keyboard users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/examples/drag-n-drop/assets/index.html(1 hunks)
🔇 Additional comments (1)
v3/examples/drag-n-drop/assets/index.html (1)
1-308: Overall Frontend Implementation Well-Structured
The HTML structure, CSS styling, and integration with the Wails Events API are cleanly implemented and meet the PR objectives for demonstrating drop-zone functionality.
dosubot
bot
added
the
size:XXL
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/src/content/docs/learn/events.mdx (1)
235-243: Align CSS base selector with actual markupThe CSS uses
.dropzoneas the base selector, but the runtime doesn’t inject that class automatically. Either instruct users to addclass="dropzone"in the HTML (as suggested above) or switch the CSS to target the attribute directly:-/* Base style for all dropzones (resting state) */ -.dropzone { +.dropzone, /* if you continue with a class-based approach */ +[data-wails-dropzone] { border: 2px dashed #888; background-color: #303030; /* ... */ }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/content/docs/learn/events.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/content/docs/learn/events.mdx
[uncategorized] ~326-~326: Loose punctuation mark.
Context: ...lement that received the drop. - X int: The X-coordinate of the drop, relative ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~327-~327: Loose punctuation mark.
Context: ... to the window's content area. - Y int: The Y-coordinate of the drop, relative ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~328-~328: Loose punctuation mark.
Context: ...t area. - Attributes map[string]string: A map containing all HTML attributes of...
(UNLIKELY_OPENING_PUNCTUATION)
| switch details.ElementID { | ||
| case "documents": | ||
| log.Printf("Files dropped on Documents folder") | ||
| // Handle document uploads | ||
| case "downloads": | ||
| log.Printf("Files dropped on Downloads folder") | ||
| // Handle download folder drops | ||
| case "trash": |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct element ID usage in Go handler
The Go example switches on details.ElementID, but the advanced HTML example only sets data-folder-id. You can either give the element an id (as suggested) or change the switch to use the attribute:
- switch details.ElementID {
+ folderID := details.Attributes["data-folder-id"]
+ switch folderID {📝 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.
| switch details.ElementID { | |
| case "documents": | |
| log.Printf("Files dropped on Documents folder") | |
| // Handle document uploads | |
| case "downloads": | |
| log.Printf("Files dropped on Downloads folder") | |
| // Handle download folder drops | |
| case "trash": | |
| folderID := details.Attributes["data-folder-id"] | |
| switch folderID { | |
| case "documents": | |
| log.Printf("Files dropped on Documents folder") | |
| // Handle document uploads | |
| case "downloads": | |
| log.Printf("Files dropped on Downloads folder") | |
| // Handle download folder drops | |
| case "trash": |
🤖 Prompt for AI Agents
In docs/src/content/docs/learn/events.mdx around lines 294 to 301, the Go code
switches on details.ElementID, but the HTML example uses data-folder-id
attribute instead of an id. To fix this, either update the HTML elements to
include an id attribute matching the expected values ("documents", "downloads",
"trash") or modify the Go switch statement to read the data-folder-id attribute
value instead of ElementID for correct matching.
| ```html | ||
| <div id="myDropArea" class="my-styles" data-wails-dropzone> | ||
| <p>Drop files here!</p> | ||
| </div> | ||
|
|
||
| <div id="anotherZone" data-wails-dropzone style="width: 300px; height: 100px; border: 1px solid grey;"> | ||
| Another drop target | ||
| </div> | ||
|
|
||
| <!-- Advanced example with custom data attributes --> | ||
| <div class="tree-node folder folder-dropzone" | ||
| data-wails-dropzone | ||
| data-folder-id="documents" | ||
| data-folder-name="Documents" | ||
| data-path="/home/user/Documents"> | ||
| <span>📁 Documents</span> | ||
| </div> | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure HTML examples match styling and event handling
The HTML snippets use data-wails-dropzone but neither include the dropzone class referenced in the CSS examples nor set an id for the advanced example—yet the Go handler switches on ElementID. Update the HTML to:
- Add
class="dropzone"(or whatever base class you choose) so the CSS.dropzonerules apply. - Include
id="documents"on the advanced example to align with theswitch details.ElementIDlogic.
Proposed diff:
-<div id="myDropArea" class="my-styles" data-wails-dropzone>
+<div id="myDropArea" class="dropzone my-styles" data-wails-dropzone>
-<div id="anotherZone" data-wails-dropzone style="width: 300px; height: 100px; border: 1px solid grey;">
+<div id="anotherZone" class="dropzone" data-wails-dropzone style="width: 300px; height: 100px; border: 1px solid grey;">
-<div class="tree-node folder folder-dropzone"
+<div id="documents" class="dropzone tree-node folder folder-dropzone"
data-wails-dropzone
data-folder-id="documents"
data-folder-name="Documents"
data-path="/home/user/Documents">🤖 Prompt for AI Agents
In docs/src/content/docs/learn/events.mdx around lines 212 to 229, the HTML
examples use data-wails-dropzone but lack the dropzone class needed for CSS
styling and the advanced example is missing an id attribute required for Go
handler logic. Add class="dropzone" to all dropzone divs to apply CSS styles and
add id="documents" to the advanced example div to match the ElementID switch in
the Go handler.
Adds support for drop zones in v3
data-wails-dropzonethe runtime will populate listeners and events as neededwails-dropzoneandwails-dropzone-hoverto allow for customizationBasic Usage Example:
WindowEvent.Context().DroppedFiles()-> Returns array of the file paths dropped, if the path is a dir it will not recursively get files but provide the dropped location insteadWindowEvent.Context().DropZoneDetails()-> Returns the data from the element the files were dropped intoX-> X Position based off the top left cornerY-> Y Position based off the top left cornerAttributes-> Map of all data attributes assigned to the elementClassList-> array of the classes currently assigned to the element at that X/Y positionDemo
v3-dnd-dmo.mov
EDIT: Improved demo
v3-dnd-filetree.mov
AI Slop Summary
Enhancements to Drag-and-Drop Functionality:
data-wails-dropzoneattribute, enabling finer control over file drops.wails-dropzone-hoverCSS class to dropzones during drag events, allowing customizable hover effects. [1] [2]FilesDroppedOnTargetfunction to handle file drop events with detailed context, including element ID, classes, and drop coordinates. [1] [2]Documentation Updates:
Example Updates:
v3/examples/drag-n-dropwith new dropzones, CSS styles, and JavaScript logic to demonstrate the feature. [1] [2] [3]Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Style
Refactor