Skip to content

fix(nodeUtil): moved blocks now included in the editor changes #1931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
+28 −2
Closed

fix(nodeUtil): moved blocks now included in the editor changes #1931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f96697e
fix(nodeUtil): moved blocks now included in the editor changes
rmolinamir Aug 9, 2025
80ce2a5
fix(nodeUtil): only firing event for dragged block
rmolinamir Aug 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/core/src/api/nodeUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,22 +184,25 @@ function collectAllBlocks<
{
block: Block<BSchema, ISchema, SSchema>;
parentId: string | undefined;
index: number;
}
> {
const blocks: Record<
string,
{
block: Block<BSchema, ISchema, SSchema>;
parentId: string | undefined;
index: number;
}
> = {};
const pmSchema = getPmSchema(doc);
doc.descendants((node, pos) => {
doc.descendants((node, pos, _, index) => {
if (isNodeBlock(node)) {
const parentId = getParentBlockId(doc, pos);
blocks[node.attrs.id] = {
block: nodeToBlock(node, pmSchema),
parentId,
index,
};
}
return true;
Expand Down Expand Up @@ -257,15 +260,18 @@ export function getBlocksChangedByTransaction<
});
});

const draggedBlockId = transaction.getMeta("draggedBlockId");

// Handle updated, moved, indented, outdented blocks
Object.keys(nextBlocks)
.filter((id) => id in prevBlocks)
.forEach((id) => {
const prev = prevBlocks[id];
const next = nextBlocks[id];
const isParentDifferent = prev.parentId !== next.parentId;
const isDraggedBlock = prev.index !== next.index && draggedBlockId === id;

if (isParentDifferent) {
if (isParentDifferent || isDraggedBlock) {
changes.push({
type: "move",
block: next.block,
Expand Down
20 changes: 20 additions & 0 deletions packages/core/src/extensions/SideMenu/SideMenuPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { initializeESMDependencies } from "../../util/esmDependencies.js";
import { getDraggableBlockFromElement } from "../getDraggableBlockFromElement.js";
import { dragStart, unsetDragImage } from "./dragging.js";
import { nodeToBlock } from "../../api/nodeConversions/nodeToBlock.js";

export type SideMenuState<
BSchema extends BlockSchema,
Expand Down Expand Up @@ -695,6 +696,8 @@ export class SideMenuProsemirrorPlugin<

public view: SideMenuView<BSchema, I, S> | undefined;

private draggedBlockId: string | undefined;

constructor(private readonly editor: BlockNoteEditor<BSchema, I, S>) {
super();
this.addProsemirrorPlugin(
Expand All @@ -706,6 +709,23 @@ export class SideMenuProsemirrorPlugin<
});
return this.view;
},
props: {
handleDrop: (_view, _event, slice) => {
const draggedBlock = nodeToBlock(
slice.content.child(0),
editor.pmSchema,
);
this.draggedBlockId = draggedBlock.id;
return false;
},
},
filterTransaction: (tr) => {
if (tr.getMeta("uiEvent") === "drop" && this.draggedBlockId) {
tr.setMeta("draggedBlockId", this.draggedBlockId);
this.draggedBlockId = undefined;
}
return true;
},
Comment on lines +713 to +728
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up attaching the draggedBlockId to the transaction, I'm unsure if this is the ideal approach though 😅

Screen.Recording.2025-08-10.at.12.50.38.AM.mp4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think this is the right approach. What should happen here is using the order of the blocks only, derive which block has actually moved

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I meant here. It's not possible with just indexes.

Say you have an editor with these values: [1,2,3]. If you drag 2 to the first slot, you end up with [2,1,3], but the user could also have dragged 1 into the second slot ending up with [2,1,3], this is why it's impossible to do it with indexes alone. Knowing which block was dragged is necessary if you want to filter the rest of the blocks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yea. This is a good argument for over reporting on all of the indexes that have changed.

I don't want for dragged blocks to be handled specially in some sort of way, since that will mean having to maintain every possible way the editor may change (to get the most "fine-grained" update), when this should be a general solution.

Sorry to make you do more work here. I didn't see your comment since it was marked as resolved.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries at all, that's fair. In that case, would you rather go with the original approach (i.e., only check for blocks with different indexes)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmolinamir yea, that might be the best way forward here.

Sorry, I've been on holiday & didn't answer this. Did you mean to close this PR?

}),
);
}
Expand Down