-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
bugfix: On Preact, sometimes DOM & Slate selection are mismatching #4749
Conversation
🦋 Changeset detectedLatest commit: 0fedd28 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a reasonable check that won't slow things down or cause harm. Let me know when it's ready for a real review and remember to add a changeset when you're ready.
Sure! just wanted to check every thing possible |
…n can mismatch which causes React to break
@dylans Hi! Everything is ready, tests are fine... sadly I was not able to figure out how to make stable test that reproduces this behavior. However, all the tests are running fine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was happy to help! |
Sadly this change causes issues for us, checking if the selection is set and in the editor at this stage results in slate-react not updating the selection if it isn't already inside the editor. So focusing and selecting something without having a dom selection (or selection inside the editor) doesn't update/set the dom selection. |
…Selection can mismatch which causes React to break (ianstormtaylor#4749)" This reverts commit a3dfb15.
@dylans could you revert it? There has to be another way to solve this issue, as this pr breaks basic slate functionality. |
Done, will release as soon as the CI finishes. |
Awesome, thanks so much 👍 |
…n can mismatch which causes React to break (ianstormtaylor#4749)
* Revert ianstormtaylor#4749 See note at ianstormtaylor#4749 (comment) * Create olive-bags-talk.md Add changeset
Description
Sadly, I cannot create stand-alone reproduction environment, only happens in real world for me.
When switching between multiple editors quickly, there is around 5% chance of following error (location differs):
which is caused by this line:
https://github.com/ianstormtaylor/slate/blob/main/packages/slate-react/src/components/editable.tsx#L217
No idea why this only happens on real-world, but I was not able to reproduce this behavior on some demo stage.
However, the environment is the following:
Due to fact it is not happening every time, looks like this is race-condition problem and incompatibility between slate and (possibly) preact. However, this logic fully fixes everything.
Checks
yarn test
.yarn lint
. (Fix errors withyarn fix
.)yarn start
.)yarn changeset add
.)