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

Revert "Do NOT use exact match when updating dom selection" #4627

Merged
merged 2 commits into from
Oct 25, 2021

Conversation

jameshfisher
Copy link
Contributor

Description
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:

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

Issue

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

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

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.
@changeset-bot
Copy link

changeset-bot bot commented Oct 25, 2021

🦋 Changeset detected

Latest commit: 9d5a5ab

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

@jameshfisher
Copy link
Contributor Author

Note @dylans we should re-open #4293 until I/we find a proper fix to it

@robin-macpherson
Copy link

Amazing work @jameshfisher ! I'm wondering, do you think this work may relate to #4602 which also involved the cursor jumping to the wrong position, although this appears to be platform-specific to Android?

@jameshfisher
Copy link
Contributor Author

@robin-macpherson thanks although all I did was revert previous work 😅 I can't say much about #4602, although my first question would be: does your Android bug still happen with slate-react@0.68.1 and above? If so, it's probably a separate bug ...

@jameshfisher jameshfisher deleted the revert-exactmatch-false branch November 1, 2021 16:17
@robin-macpherson
Copy link

@jameshfisher ah, apologies for not fully understanding that 🤦🏻‍♂️ I just got the results from our user who does Android testing for us and it appears the bug does indeed still happen. I'll keep digging around, thanks for replying here!

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
3 participants