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

Bold/italic/underline can be applied to more than selected text #4038

Closed
muster-mark opened this issue Jan 5, 2021 · 2 comments · Fixed by #4168
Closed

Bold/italic/underline can be applied to more than selected text #4038

muster-mark opened this issue Jan 5, 2021 · 2 comments · Fixed by #4168

Comments

@muster-mark
Copy link

muster-mark commented Jan 5, 2021

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

Report a bug

What's the current behavior?

bug1

Note that the selection was started by double clicking on the first word.
Not providing a sandbox as the issue is appearing on slate.js website - above gif was recorded on https://www.slatejs.org/examples/hovering-toolbar
Slate version: whatever version is currently used for examples on www.slatejs.org, presumable the latest
On Firefox 84, Mac OS 11.1. Does not happen in Chrome.

What's the expected behavior?

The bold/italic/underline should only be applied the the text that is selected.

@BrentFarese
Copy link
Collaborator

I think this bug occurs because the splitting logic in setNodes has a bug. Specifically, in splitNodes at these lines doesn't split a node if the selection is at an edge of the node, even if split: true. You can see in your example that your selection is at the edge of the italic word "shows", then you expand the selection so the anchor is either at the start of "shows" or more likely the end of "This example ". Since the anchor is at the end of "This example ", Slate will then not split the text between "This example " and the italic "shows".

Can you try your bug if you set always: true on setNodes and see if the bug goes away? It would be great if you could submit a PR that improves the splitting logic in setNodes!

In our local branch, we always split if the end point of the selection is not at the end of the related node, and always if the start point of the selection is not at the start of the related node. It looks like this:

const atRange = Editor.range(editor, at);
    const rangeCollapsed = Range.isCollapsed(atRange);
    const rangeRef = Editor.rangeRef(editor, atRange, {
      affinity: rangeCollapsed ? 'forward' : 'inward'
    });

    Editor.withoutNormalizing(editor, () => {
      if (rangeCollapsed) {
        Transforms.splitNodes(editor, { at: atRange.anchor, match: Text.isText, voids });

        return;
      }

      const [startPoint, endPoint] = Range.edges(atRange);
      const startPointAtStartOfNode = Editor.isStart(editor, startPoint, startPoint.path);
      const endPointAtEndOfNode = Editor.isEnd(editor, endPoint, endPoint.path);

      /**
       * IMPORTANT: Split the end point first so we don't affect the path of the start point by splitting the end.
       * Also, we don't split if the start point is at the start of its text node or the end point is at the end of its text node since a split isn't needed.
       */
      Transforms.splitNodes(editor, {
        always: !endPointAtEndOfNode,
        at: endPoint,
        match: Text.isText,
        voids
      });

      Transforms.splitNodes(editor, {
        always: !startPointAtStartOfNode,
        at: startPoint,
        match: Text.isText,
        voids
      });
    });

@BrentFarese
Copy link
Collaborator

Related to/duplicate of #3808.

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