MoveNodeByPath broken?

[skogsmaskin]

Do you want to request a feature or report a bug?

I was trying to add some tests for the by-path commands, and tried adding a simple test for just switching positions of two blocks. I can't get it to work, it will just replace one of the blocks.

I.e. change.moveNodeByPath([0], [1], 1)

What's the current behavior?

It will replace one of the blocks leaving only one of the two blocks behind.

Relevant fiddle: https://jsfiddle.net/skogsmaskin/t54xc1ap/1/

What's the expected behavior?

As I understand moveNodeByPath it should not replace blocks, but just move it? I want to move the first block below the second one.

Could be I misunderstand something here, some input would be appriciated.

[ianstormtaylor]

@skogsmaskin might it have something to do with the inconsistent behavior mentioned in #2192?

[skogsmaskin]

@ianstormtaylor Yeah, that's probably part of the problem (the path annotation for the newPath param is very confusing). But I consider it a bug that it will actually delete/replace nodes like in the above fiddle? It should maybe throw an error instead until moveNodeByPath is refactored like you propose in #2192.

[jtadmor]

@skogsmaskin @ianstormtaylor I believe that the particular fiddle was not showing an actual break in moveNodeByPath.

I was able to get the operation to work as expected by changing the code to call the underlying applyOperation directly: https://jsfiddle.net/La0bsjm1/3/

I think the reason we were seeing that behavior was because of the CorePlugin rule preventing mixing block and text nodes:

slate/packages/slate/src/plugins/core.js

Line 66 in 7a6de87

// Only allow block nodes or inline and text nodes in blocks.

The jsfiddle is actually moving a paragraph node to be the child of a paragraph that already has an existing text child node, so (I think) the new node was just being removed right after the move_node operation went through.

(See tests in #2555)

[ianstormtaylor]

Is this fixed now?

[nz-chris]

Doesn't seem to be fixed. I ended up using editor.applyOperation with type as move_node instead.

[ianstormtaylor]

As of #3093 (which was just merged), I believe this issue is no longer applicable, because a lot has changed. I'm going through and closing out any potential issues that are not out of date with the overhaul. Thanks for understanding.