Use correct parent depth when dropping context #25

Closed

ocavue wants to merge 2 commits from ocavue-fix-replace-with2 into master

ocavue commented

2023-08-17 12:22:03 +02:00

(Migrated from github.com)

This pull request addresses two issues in replaceRange related to the context being dropped.

The first issue arises when ProseMirror fails to drop the context if the paste target is sufficiently deep. This bug is shown in the following video:

https://github.com/ProseMirror/prosemirror-transform/assets/24715727/a577355b-8b22-4fdb-98e9-65d7342a5cb0

When pasting <blockquote>one</blockquote> into an empty blockquote that inserts a list item (i.e., Case 2 and Case 3 in the video). The defining context is not dropped as expected, leading to an unexpected nested blockquote structure.

In this line, the defining context is derived from $from.node(preferredTargetIndex). However, it appears that preferredTargetIndex is merely an index of various depths, and it doesn't necessarily represent a valid depth. I replaced preferredTargetIndex with Math.abs(preferredTarget) - 1, following a similar logic here.

The second issue is shown in the video below:

https://github.com/ProseMirror/prosemirror-transform/assets/24715727/bc165ecb-06fe-4d2a-851e-8f4e8349ffc7

The source code for this demo can be found here. In this demo, a blockquote node has an attribute to mark its border color.

When copying red lightblue and pasting them into a gray blockquote, I expected the gray color to be dropped and replaced by the red color. However, the behavior is inconsistent. If the gray blockquote is inside a nested blockquote (i.e., Case 2 and Case 3), it doesn't get dropped.

I resolved this issue by comparing not only the node types of the incoming node and the existing context, but also their attributes. I replaced (...).type != type with !type.sameMarkup(...) in this line.

I have included the demos from the two videos as test code in this PR. I appreciate your time in reviewing this PR. I acknowledge that I still have a lot to learn and I warmly welcome any corrections. Thank you!

This pull request addresses two issues in `replaceRange` related to the context being dropped. The first issue arises when ProseMirror fails to drop the context if the paste target is sufficiently deep. This bug is shown in the following video: https://github.com/ProseMirror/prosemirror-transform/assets/24715727/a577355b-8b22-4fdb-98e9-65d7342a5cb0 When pasting `<blockquote>one</blockquote>` into an empty `blockquote` that inserts a list item (i.e., Case 2 and Case 3 in the video). The defining context is not dropped as expected, leading to an unexpected nested blockquote structure. In [this line](https://github.com/ProseMirror/prosemirror-transform/blob/75123e4246857fcdbdc7a1ffba3ce14f3b2dfb6e/src/replace.ts#L374), the defining context is derived from `$from.node(preferredTargetIndex)`. However, it appears that `preferredTargetIndex` is merely an index of various depths, and it doesn't necessarily represent a valid depth. I replaced `preferredTargetIndex` with `Math.abs(preferredTarget) - 1`, following a similar logic [here](https://github.com/ProseMirror/prosemirror-transform/blob/75123e4246857fcdbdc7a1ffba3ce14f3b2dfb6e/src/replace.ts#L386-L387). --- The second issue is shown in the video below: https://github.com/ProseMirror/prosemirror-transform/assets/24715727/bc165ecb-06fe-4d2a-851e-8f4e8349ffc7 The source code for this demo can be found [here](https://stackblitz.com/github/issueset/pm-color-blockquote). In this demo, a blockquote node has an attribute to mark its border color. When copying `red lightblue` and pasting them into a gray blockquote, I expected the gray color to be dropped and replaced by the red color. However, the behavior is inconsistent. If the gray blockquote is inside a nested blockquote (i.e., Case 2 and Case 3), it doesn't get dropped. I resolved this issue by comparing not only the node types of the incoming node and the existing context, but also their attributes. I replaced `(...).type != type` with `!type.sameMarkup(...)` in [this line](https://github.com/ProseMirror/prosemirror-transform/blob/75123e4246857fcdbdc7a1ffba3ce14f3b2dfb6e/src/replace.ts#L374). --- I have included the demos from the two videos as test code in this PR. I appreciate your time in reviewing this PR. I acknowledge that I still have a lot to learn and I warmly welcome any corrections. Thank you!

👍 1

marijnh commented

2023-08-18 12:31:23 +02:00

(Migrated from github.com)

Thanks! Merged as fae874cf09

Thanks! Merged as fae874cf09c11ed4ce905e1400e941d49b4e1308

❤️ 1

ocavue commented

2023-08-21 23:53:56 +02:00

(Migrated from github.com)

@marijnh Thanks for merging my PR. Could we consider publishing a new version to include these changes?

marijnh commented