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

Do NOT use exact match when updating dom selection #4304

Merged
merged 2 commits into from
Sep 27, 2021
Merged

Do NOT use exact match when updating dom selection #4304

merged 2 commits into from
Sep 27, 2021

Conversation

davidruisinger
Copy link
Contributor

@davidruisinger davidruisinger commented May 31, 2021

Description
When there is NO text, yet within a nested, editable void Slate Editor (see editable voids example, starting to type some text leads to backwards typing.

Issue
Fixes: #4293

Example
Before:
khkYcJF2rp

After:
ayoBo1dHIj

Context
The bug seems to have been introduced with #4157 so it would be great if @githoniel could have another look at this

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 May 31, 2021

🦋 Changeset detected

Latest commit: 528d3c0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate-react Patch

Not sure what this means? Click here to learn what changesets are.

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

@githoniel
Copy link
Contributor

githoniel commented Jun 1, 2021

thanks for metion

I think this is ReactEditor.toSlateRange problem.

after typing when text in empty, DOM Selection is

image

ReactEditor.toSlateRange convert it to offset: 1 while it should be offset: 0, so Slate point to start after selection sync. I think We should fix it.

@davidruisinger
Copy link
Contributor Author

thanks for metion

I think this is ReactEditor.toSlateRange problem.

after typing when text in empty, DOM Selection is

image

ReactEditor.toSlateRange convert it to offset: 1 while it should be offset: 0, so Slate point to start after selection sync. I think We should fix it.

I tried to debug the toSlateRange method and I the problem seems to be within toSlatePoint. There is a difference between "normal" nodes and void nodes when setting the offset.
In "normal" nodes, the offset is calculated by the length of the text of a DocumentFragment that is received through a created range (see here).
In void nodes, the length is also taken from a text length, but this time the domNode itself is used (see here).

Unfortunately I wasn't able to find a fix, yet. I'd really appreciate if someone else could have a look at this as well.

@hueyhe
Copy link
Contributor

hueyhe commented Jun 26, 2021

I also ran into this issue. After debugging the toSlatePoint, I think the problem is caused by the unexpected element type of the anchor/focus node of the DOM selection.

Here in toSlatePoint, the nearestNode, which refers to the domPoint if exact match is used, is expected to be an html text node. However, the anchor/focus node of the DOM selection we got from here isn't always a text node.

In the case below, the anchor node of the DOM selection is the editable DOM element rather than the html text node. So toSlatePoint can't find the corresponding slate point of the anchor node.

Screen.Recording.2021-06-26.at.2.47.43.PM.mov

The DOM selection after deletion:

Screen Shot 2021-06-26 at 2 41 06 PM

Screen Shot 2021-06-26 at 2 46 06 PM

Things work fine outside the editable void because the leaf node will always be undefined and won't be corrected by the void node. So the DOM selection is always updated by the slate selection since it's out of sync.

@githoniel
Copy link
Contributor

I will look this later. I think we should fix toSlateRange instead

@zbeyens
Copy link
Contributor

zbeyens commented Sep 26, 2021

This cursor bug makes the nested editors unusable in production, I would be in favor to merge this until we find a fix inside toSlateRange.

@dylans
Copy link
Collaborator

dylans commented Sep 27, 2021

Accepting and landing this for now, until we can come up with something better.

@dylans dylans merged commit 7ba486a into ianstormtaylor:main Sep 27, 2021
@github-actions github-actions bot mentioned this pull request Sep 27, 2021
jameshfisher added a commit to jameshfisher/slate that referenced this pull request Oct 25, 2021
The change to `exactMatch: false` in ianstormtaylor#4304
was intended to fix ianstormtaylor#4293,
a bug where "backwards typing" happened in nested editors.

But this change has introduced at least two new bugs:

- ianstormtaylor#4601
- ianstormtaylor#4626

These are (IMO) worse than the original "backwards typing" bug.

From discussion in ianstormtaylor#4304,
the true underlying bug is in ReactEditor.toSlateRange. I'll attempt to
fix this underlying bug instead.
dylans pushed a commit that referenced this pull request Oct 25, 2021
* Revert "Do NOT use exact match when updating dom selection"

The change to `exactMatch: false` in #4304
was intended to fix #4293,
a bug where "backwards typing" happened in nested editors.

But this change has introduced at least two new bugs:

- #4601
- #4626

These are (IMO) worse than the original "backwards typing" bug.

From discussion in #4304,
the true underlying bug is in ReactEditor.toSlateRange. I'll attempt to
fix this underlying bug instead.

* 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.

Backwards text typing on empty, nested, editable void Slate Editor
5 participants