basecamp / basecamp/lexxy

Dragging a PDF onto another PDF deletes it

Open Beginner friendly
#1,235 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
1.2k
Forks
119
Avg merge
4d 8h
Merged PRs (30d)
15

Description

Dragging a PDF onto another PDF deletes the dragged one. It is not added to a gallery and not returned to where it came from, so the attachment is simply lost. The usual gallery drop markers appear beforehand, so it looks like a supported drop right up until the content disappears.

This affects any previewable non-image attachment, so videos too.

## Steps to reproduce

1. Attach a PDF, press Enter, attach a second PDF, so there are two standalone attachments.
2. Wait for both first-page thumbnails to render. This matters: the attachments have to be previewable for the drop target to match.
3. Drag the second PDF onto the middle of the first.

Expected: either they form a gallery, or nothing happens.
Actual: the dragged PDF is deleted.

## Cause

The drop target is resolved in the DOM, where the selector matches any previewable attachment:

```js
// src/editor/attachments/drag_and_drop.js
const targetFigure = element.closest("figure.attachment--preview[data-lexical-node-key]")
```

Gallery membership, though, is decided on content type, which excludes PDFs and videos:

```js
// src/nodes/image_gallery_node.js
static isValidChild(node) {
return $isActionTextAttachmentNode(node) && node.isPreviewableImage
}
```

`#dropOntoImage` removes the dragged node before consulting the gallery, and re-inserts it only when `$findOrCreateGalleryForImage` returns one. When those two checks disagree, the node is removed and never put back:

```js
#dropOntoImage(draggedNode, targetKey, position) {
const targetNode = $getNodeByKey(targetKey)
if (!targetNode || !$isActionTextAttachmentNode(targetNode)) return
if (draggedNode.is(targetNode)) return

draggedNode.remove()

const gallery = $findOrCreateGalleryForImage(targetNode)
if (gallery) {
if (position === "before") {
targetNode.insertBefore(draggedNode)
} else {
targetNode.insertAfter(draggedNode)
}
}
}
```

`#reorderInGallery` has the same remove-then-insert shape but always inserts, so it is unaffected.

## Suggested fix

Resolve the gallery first and bail out before mutating anything, so an unsupported drop is a no-op rather than a deletion.

Separately, it would be worth not showing the gallery drop markers for a target that cannot accept the drop, since the DOM-level `attachment--preview` check is broader than `isValidChild`.

## Version

lexxy 0.9.29, Rails 8.2.0.alpha, Active Storage with the Poppler previewer. Reproduced in Chromium via the browser test suite.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/editor/attachments/drag_and_drop.js at #dropOntoImage and compare its behavior with #reorderInGallery and the gallery checks in src/nodes/image_gallery_node.js. Run the browser test suite with the PDF or video reproduction. Done means dropping an unsupported previewable attachment is a no-op rather than deleting it, with gallery markers considered separately.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.