Skip to content

Conversation

@kinaluo
Copy link

@kinaluo kinaluo commented Aug 15, 2025

Description
Inserting a void block (such as an image) in the middle of a paragraph would incorrectly insert it before or after the paragraph. This change will cause it to be inserted at the cursor or selection position.

Issue
Fixes: (link to issue)

Example
befor:

befor.mp4

after:

after.mp4

Context
If your change is non-trivial, please include a description of how the new logic works, and why you decided to solve it the way you did. (This is incredibly helpful so that reviewers don't have to guess your intentions based on the code, and without it your pull request will likely not be reviewed as quickly.)

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)
  • You've added a changeset if changing functionality. (Add one with yarn changeset add.)

@changeset-bot
Copy link

changeset-bot bot commented Aug 15, 2025

⚠️ No Changeset found

Latest commit: a10e077

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@12joan
Copy link
Contributor

12joan commented Aug 15, 2025

My concern with this is it might make it harder to modify the behaviour of pasting of void blocks at locations where these blocks are not valid.

List items example
For example, in many list implementations, lists are represented using a UL > LI > LIC > text structure. The LIC (sometimes called list-item-text, or just a paragraph) acts as a block wrapper for text so that the LI can also have a nested list as its second child:

  • UL
    • LI
      • LIC
        • [text]
      • OL
        • ...

Generally, no block elements other than LI, LIC, OL and UL are allowed anywhere in the list structure.

It looks like with this PR, pasting in the middle of the text of a LIC will cause that LIC to be split and the image inserted between the two LICs.

  • LI
    • LIC
    • Image
    • LIC

A normalizer might clean this up by removing the image or relocating it to before or after the list, but this would still leave us with two LICs in the same LI, which isn't valid. Existing normalizers might handle the two-LICs case by splitting the LI.

  • LI
    • LIC
  • LI
    • LIC

But this doesn't make sense since nothing was actually pasted at the point where the LIs were split. Normalizers might instead handle the two-LICs case by re-merging the LICs, but this might require making additional code changes, such as when splitting a LIC intentionally using the enter key or when pasting text containing multiple lines.

Conclusion
I'm not opposed to us making this change, but we should make sure there's still an easy way of handling edge cases like this.

@kinaluo
Copy link
Author

kinaluo commented Aug 18, 2025

My concern with this is it might make it harder to modify the behaviour of pasting of void blocks at locations where these blocks are not valid.

List items example For example, in many list implementations, lists are represented using a UL > LI > LIC > text structure. The LIC (sometimes called list-item-text, or just a paragraph) acts as a block wrapper for text so that the LI can also have a nested list as its second child:

  • UL

    • LI

      • LIC

        • [text]
      • OL

        • ...

Generally, no block elements other than LI, LIC, OL and UL are allowed anywhere in the list structure.

It looks like with this PR, pasting in the middle of the text of a LIC will cause that LIC to be split and the image inserted between the two LICs.

  • LI

    • LIC
    • Image
    • LIC

A normalizer might clean this up by removing the image or relocating it to before or after the list, but this would still leave us with two LICs in the same LI, which isn't valid. Existing normalizers might handle the two-LICs case by splitting the LI.

  • LI

    • LIC
  • LI

    • LIC

But this doesn't make sense since nothing was actually pasted at the point where the LIs were split. Normalizers might instead handle the two-LICs case by re-merging the LICs, but this might require making additional code changes, such as when splitting a LIC intentionally using the enter key or when pasting text containing multiple lines.

Conclusion I'm not opposed to us making this change, but we should make sure there's still an easy way of handling edge cases like this.

Indeed, this behavior might occur, but I understand that in this method, we should focus solely on the behavior of block splitting and connection during content insertion, keeping his role as singular as possible. The normalizer process should be handled by the corresponding plugin implementation, or we could add a parameter in options to let users control whether to split paragraphs instead of accommodating the boundary behavior of different types of tags in this method. Or maybe I've misunderstood insertFragment. If you have other ideas for how to do this, I'd love to hear your suggestions.

@12joan
Copy link
Contributor

12joan commented Aug 18, 2025

I suppose list plugins could override insertFragment and filter the tree of nodes about to be pasted into a list or other structured element. That would probably be the cleanest way of avoiding this problem.

@kinaluo
Copy link
Author

kinaluo commented Aug 18, 2025

I suppose list plugins could override insertFragment and filter the tree of nodes about to be pasted into a list or other structured element. That would probably be the cleanest way of avoiding this problem.

This is indeed a solution, but it requires the user to handle the corresponding plugins themselves. If this change can be merged, all we can do is indicate the impact of this change in the changelog

@12joan
Copy link
Contributor

12joan commented Aug 18, 2025

I don't think there's any way of handling this in the core insertFragment itself. Slate doesn't have any API for determining in advance whether a particular void is valid in a particular location, such as an image inside a list item. Normalization logic can address this after the fact, but as I described above, normalization alone won't be enough in this case due to the split when inserting a void block.

One thing we could do is add options to insertFragment to make overriding it to handle this case easier. For example, it would be useful to have a filter option that controls whether each individual node is eligible for insertion. (Either that or we could introduce a Node.filter method on the Node interface.)

However, for this to be useful, the override code would also need a way of determining which exact block the nodes will be inserted into. The logic for determining this is non-trivial, since it involves getDefaultInsertLocation (not currently exported), as well as performing a deletion and checking the result of a point ref if at is an expanded range.

This is quite a lot of logic for override code to duplicate, so maybe instead the blockMatch could be exposed as an argument on the filter option:

const { insertFragment } = editor

editor.insertFragment = (fragment, options) => {
  const { filter = () => true } = options
  const filteredNodes: Node[] = []

  insertFragment(fragment, {
    ...options,
    filter: (node, filterOptions) => {
      if (!filter(node, filterOptions)) return false

      // Prevent splitting a LIC to insert a void block
      if (
        filterOptions.blockEntry[0].type === 'lic' &&
        Element.isElement(node) &&
        Editor.isBlock(node) &&
        Editor.isVoid(node)
      ) {
        filteredNodes.push(node)
        return false
      }

      return true
    },
  })

  // Insert filteredNodes before or after the list
  // ...
}

I think it's reasonable to expect list plugins to do this, but as you say we should describe this in the changeset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants