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

Clicking on a zero-width node and dragging does not select text #1969

Closed
thallada opened this issue Jul 13, 2018 · 12 comments · Fixed by #3594
Closed

Clicking on a zero-width node and dragging does not select text #1969

thallada opened this issue Jul 13, 2018 · 12 comments · Fixed by #3594

Comments

@thallada
Copy link
Contributor

What's the current behavior?

Clicking on an empty paragraph or next to a inline link (both have data-slate-zero-width nodes) and then dragging to select text only moves the cursor and does not select text.

slate_bug

What's the expected behavior?

I should be able to click and drag anywhere on the document to select text.

@willbryk720
Copy link

I have the same problem with the cursor. Sometimes it just doesn't highlight. It happens randomly about every 1 in 4 attempts to highlight. For me, it's not just inline nodes, it happens anywhere. It more often happens when I start highlighting from within an already highlighted section. So if I place my cursor inside a highlighted portion and try to highlight, it will not highlight.

strangecursor

@zhujinxuan
Copy link
Contributor

@willbryk720 Can you provide an example to reproduce? I can only reproduce with zero-width blocks.

@willbryk720
Copy link

willbryk720 commented Aug 24, 2018

Interestingly, after investigating the problem more deeply, it seems the cause of my problem was completely different from @thallada's.

I wrap my editor so that I can access its value from a parent component. The onChange function passed to the editor called a function that passed the editor value to the parent component. Every time the editor changed, it passed it to the parent component. I tested out the same editor without any passing to parent function, and the editor stopped having this problem. This bad cursor behavior then must be due to the parent component re-rendering so often.

For anyone who has a similar problem, instead of passing the value to the parent on every onChange, you can just pass the value to the parent only when you need it.

@ianstormtaylor
Copy link
Owner

@willbryk720 thank you! That is actually super helpful to figuring out the original issue too.

I think it might be the case that the original issue is caused by re-rendering too, where clicking a zero-width space happens to result in updateSelection being triggered, cancelling the draggability. Which means we might be able to solve it without needing to listen to mousemove events? cc @zhujinxuan

@zhujinxuan
Copy link
Contributor

zhujinxuan commented Aug 24, 2018

@ianstormtaylor I think the re-rendering is perhaps the reason. We shall not have the re-rendering just after selection change.

I will have a try whether udpateSelection in onChange and change shouldComponentUpdate works

@zhujinxuan
Copy link
Contributor

@ianstormtaylor Hi, there is a problem in preventing re-rendering. We always re-render the selected node because in component/node.js calculates isSelected:

  /**
   * Render a `child` node.
   *
   * @param {Node} child
   * @param {Boolean} isSelected
   * @param {Array<Decoration>} decorations
   * @return {Element}
   */

  renderNode = (child, isSelected, decorations) => {
    const { block, editor, node, readOnly, isFocused } = this.props
    const Component = child.object == 'text' ? Text : Node

    return (
      <Component
        block={node.object == 'block' ? node : block}
        decorations={decorations}
        editor={editor}
        isSelected={isSelected}
        isFocused={isFocused && isSelected}
        key={child.key}
        node={child}
        parent={node}
        readOnly={readOnly}
      />
    )
  }

Can I know why isSelected is provided?

@zhujinxuan
Copy link
Contributor

zhujinxuan commented Aug 24, 2018

I am not sure about this part: https://github.com/ianstormtaylor/slate/blob/master/packages/slate-react/src/components/node.js#L105

Why the node is always re-rendered if isSelected is true.

@ianstormtaylor
Copy link
Owner

@zhujinxuan it might not be specifically the re-rendering, but actually the call to updateSelection which changes the native DOM selection that causes the drag to fail. So I don't think we need to prevent re-rendering, but maybe use updateSelection less when we don't need to? Not sure.

@zhujinxuan
Copy link
Contributor

@ianstormtaylor I think you are right. I commented out the updateSelection and then the problem does not happen.

I will have a look at updateSelection

@steobrien
Copy link
Contributor

I can no longer reproduce this at https://www.slatejs.org/#/links, @thallada, and think it might be fixed. Would you mind confirming?

@zhujinxuan
Copy link
Contributor

zhujinxuan commented Oct 17, 2018

@steobrien It is still producible. You need to have a blank paragraph and move cursor around beginnings of paragraphs....

The strange thing is that it is somehow reproducible by chance; I have reproduce it by a chance about 1/3. I am still unknown of why it happens.

@czechdave czechdave mentioned this issue Apr 8, 2020
4 tasks
@knubie
Copy link
Contributor

knubie commented Apr 18, 2020

Seems like this was fixed at some point, but the bug is back in 5.x and reproducible on all of the demos.

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