Skip to content
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.

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

Transform.setNodes improvement and bug #3808

Open
BrentFarese opened this issue Jul 31, 2020 · 3 comments
Open

Transform.setNodes improvement and bug #3808

BrentFarese opened this issue Jul 31, 2020 · 3 comments
Assignees
Labels
Milestone

Comments

@BrentFarese
Copy link
Collaborator

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

Both.

What's the current behavior?

Transforms.setNodes currently takes Partial<Node> as its second parameter for props. This means you can only override the props on the nodes you are setting and, for example, you cannot base the new properties on the nodes you are setting off of the already-existing properties on those nodes.

Additionally, Transforms.setNodes has a split option, but the split doesn't occur in a couple edge cases that should likely be considered a bug.

The split bug occurs due to these lines in Transforms.setNodes. Basically, if you look at Transforms.splitNodes, it will NOT split the relevant node if the split point is "at an edge" of the node. This makes sense for single splits, but not for an expanded range. For an expanded range, you always want to split if the start point is not at the start of its relevant node, and if the end point is not at the end of its relevant node. If the start point of the expanded range is at the start of its relevant node, no split is needed since you already capture the full node anyways. Same for if the end point is at the end of its relevant node.

Transforms.setNodes will not split an expanded range if the start/end points are at ANY edge of their respective nodes, which is not correct and results in setting props on nodes that was not intended.

Slate: 0.58.4
Browser: Chrome
OS: Mac

What's the expected behavior?

Transforms.setNodes props parameter should accept a set of props as Partial<Node> OR a function that takes the existing props for the relevant node and returns a new set of props to attach to the node.

Also, the splitting bug should be fixed.

@adisen
Copy link

adisen commented Feb 18, 2021

Hello I would like to take a look at solving this issue, can you assign it to me @BrentFarese @timbuckley

@adisen
Copy link

adisen commented Mar 4, 2021

Is it possible I get a short clip showing the reproduction of this feature?

@iSplasher
Copy link

iSplasher commented Mar 8, 2021

recording

When the last * is detected, the text gets split like this:

Transforms.setNodes(
                  editor,
                  { bold: true },
                  {
                    at: currentRange,
                    match: Text.isText,
                    split: true,
                  }
                );

What should happen is that the characters after and before the bold text should be their own nodes.
Also refer to the example in #4038

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants