[v3] File drop: hover class "file-drop-target-active" sticks after abandoned drag on Windows#5779
[v3] File drop: hover class "file-drop-target-active" sticks after abandoned drag on Windows#5779Bare7a wants to merge 5 commits into
Conversation
… abandoned drag on Windows
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughWindows-specific ChangesDrag-and-drop hover cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
784-801: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate the duplicate non-Windows comment.
Lines 784-785 and 798-800 explain the same non-Windows behavior. The first comment block becomes stale once the reader sees the Windows branch below. Consider trimming the first to a brief lead-in and keeping the full explanation after the branch.
♻️ Suggested comment consolidation
- // On Linux/WebKitGTK and macOS, dragleave fires immediately with relatedTarget=null when native - // drag handling is involved. Ignore these spurious events - we'll clean up on drop instead. + // When relatedTarget is null, the drag left the window or was cancelled. if (event.relatedTarget === null) { // On Windows the DOM listeners are the only drag tracking, and Chromium fires // dragleave with relatedTarget=null exactly when an external drag leaves the window or // is cancelled - clean up here, otherwise an abandoned drag leaves the hover state stuck. if (IsWindows()) { dragEnterCounter = 0; if (currentDropTarget) { currentDropTarget.classList.remove(DROP_TARGET_ACTIVE_CLASS); currentDropTarget = null; } } - // On Linux/WebKitGTK and macOS, dragleave fires immediately with relatedTarget=null - // while native drag handling is involved. Ignore these spurious events - hover state - // there is driven natively via handleDragEnter/handleDragOver/handleDragLeave. + // On Linux/WebKitGTK and macOS, native handlers drive hover state, so these + // spurious events are ignored - we'll clean up on drop instead. return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/window.ts around lines 784 - 801, Consolidate the duplicate non-Windows explanation in the dragleave handler: shorten the comment before the event.relatedTarget check to a brief lead-in, and retain the full Linux/WebKitGTK and macOS rationale after the IsWindows cleanup branch. Update only the comments around the relatedTarget handling without changing drag behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@v3/internal/runtime/desktop/`@wailsio/runtime/src/window.ts:
- Around line 784-801: Consolidate the duplicate non-Windows explanation in the
dragleave handler: shorten the comment before the event.relatedTarget check to a
brief lead-in, and retain the full Linux/WebKitGTK and macOS rationale after the
IsWindows cleanup branch. Update only the comments around the relatedTarget
handling without changing drag behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37f23258-fa13-4ee9-b20a-d94dac551b74
📒 Files selected for processing (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
dbb6543 to
09bcb01
Compare
Bare7a
changed the title
1 participant
Description
With
EnableFileDrop: trueand adata-file-drop-targetelement, dragging files over the window adds thefile-drop-target-activeclass for hover styling. On Windows, if the user drags the files back out of the window (or cancels with Esc) instead of dropping, the class is never removed - any hover styling (e.g. the full-window overlay from the File Drop docs) stays visible indefinitely.Fixes #5780
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Chores