Repository navigation
fix(upload): answer a refused file drop, and leave an open Add modal alone - #3347
Merged
Merged
Conversation
Two gaps from review, both in what the page-wide drop does once it has claimed the drag. Every dragover on the page is cancelled — it has to be, since an uncancelled one lets the browser navigate the tab to the dropped file. So the cursor spends the whole drag telling the operator this is a drop target. When an edit form, a preview or a delete prompt owns the screen, acceptsPageDrop() then refused the release in silence: the file appeared to evaporate. It now says which way out of it, the same way the mid-batch overlay already did rather than promising an upload it would not start. The other half is the Add modal. Routing every drop through openAdd() gave it the same reset as every other way in, which is right when the drop is what opens it — the last asset's URL has no business still being in the box. It is wrong when the modal is already up, because that box is where the operator is mid-sentence. Reset only on the opens that are actually opens; the drop still moves them to the upload pane, and what they typed is there when they come back. That pane move needed feedback it did not have. The overlay stood down for any open modal, so a drag over the "From URL" form — which does take the file — showed nothing at all, the dashed dropzone being on a pane that is not on screen. It now shows there in a --layered form: the card without a second veil, since the modal already dimmed the page. It still stands down on the upload pane, where the dropzone is lit and a second signal would only compete with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
|
vpetersson
approved these changes
Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two review findings against the page-wide file drop landed in PR 3334, both in what the page does once it has already claimed the drag. That PR squash-merged before the findings were written up, so they come as a follow-up.
The refused drop was silent
Every
dragoveron the page is cancelled — it has to be, since an uncancelled one lets the browser navigate the tab to the dropped file and take the page with it. So the cursor spends the whole drag telling the operator this is a drop target.When an edit form, a preview or a delete prompt owns the screen,
acceptsPageDrop()then refused the release without a word: the file appeared to evaporate. It now surfaces an info toast naming the way out ("Close this dialog first, then drop your file to upload it."), the same way the mid-drag overlay already declined to promise an upload it would not start.A drop reset the Add modal under it
Routing every drop through
openAdd()gave it the same reset every other way in gets, which is right when the drop is what opens the modal — the last asset's URL has no business still being in the box. It is wrong when the modal is already up, because that box is where the operator is mid-sentence: a dropped file wiped what they had typed, and reset the Apps pane's config form with it.openAdd()now runs only on the opens that are actually opens. The drop still moves them to the upload pane, and what they typed is intact when they switch back.That pane move needed feedback it did not have
The drop overlay stood down for any open modal, so a drag over the "From URL" form — which does take the file — showed nothing at all: the dashed dropzone lives on the upload pane, which is not on screen there. It now shows in a
--layeredform, the card without a second veil, since the modal already dimmed the page. It still stands down on the upload pane itself, where the dropzone is lit and a second signal would only compete with it.Testing
Everything below ran locally, integration suite included — the Docker test stack that was unavailable during the original PR builds fine now.
bun test— 164 pass. Four new cases: the refusal toast on each of the five blocking overlays, a drop into an open Add modal preservingaddUri, and the other half of that rule (a drop that opens the modal still resets the pane).pytest -n auto -m "not integration"— 2208 pass.pytest -m integration --reuse-db— 68 pass, 2 skipped. Two new Playwright cases: a file dropped while the delete prompt is open raises the toast and starts no upload, and a drag over the Add modal's URL pane shows the layered overlay with its own veil suppressed.ruff check .,ruff format --check .,mypy .clean.Still open, unchanged
The preview iframe's residual drop gap, recorded in
.preview-media--framein_styles.scss. A fast enough flick lands its first sampled position inside the frame and skips the parent document; the frame is sandboxed withoutallow-top-navigation, so the page behind is untouched and closing the preview restores it. Closing that gap costs either preview interactivity or a click-to-interact shim, both of which cost more than the glitch does.🤖 Generated with Claude Code