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

fix: unexpected event triggered when using ReactEditor.focus #5677

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

WindRunnerMax
Copy link
Contributor

Description
When opening the document for the first time, using ReactEditor.focus to focus on the document unexpectedly triggers the onValueChange event, and there will also be an additional onChange event triggered.

Issue
Fixes: #5672

Example
The debug code is shown below, and the autoFocus property of Editable is turned off.

// DEBUG: reproduce the problem env
useMemo(() => {
// @ts-expect-error debug
window.editor = editor
// @ts-expect-error debug
window.ReactEditor = ReactEditor
const { apply } = editor
editor.apply = operation => {
console.log('OnApply', operation)
apply(operation)
}
}, [editor])
return (
<Slate
editor={editor}
initialValue={initialValue}
onChange={(...args) => console.log('OnChange', ...args)}
onValueChange={(...args) => console.log('OnValueChange', ...args)}
onSelectionChange={(...args) => console.log('OnSelectionChange', ...args)}
>

The effect when calling ReactEditor.focus is shown in the image, where you can see the unexpected event triggers.

bad59c70-1387-4948-8fd3-1e28937daeb4

Context

Actually, calling onChange in ReactEditor.focus isn't necessary. If needed, Transforms.select will handle triggering the event.

img_v3_02ci_bd5439c3-9fc0-4e56-9759-36ce9e8ab98g

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

Copy link

changeset-bot bot commented Jul 7, 2024

🦋 Changeset detected

Latest commit: 918b30f

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

@WindRunnerMax

This comment was marked as outdated.

Copy link
Collaborator

@dylans dylans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @WindRunnerMax . I'm going to approve and land this, but it's possible I'm forgetting a reason why that onChange call was in there. If there was a valid reason we may need to revert, we'll see!

@dylans dylans merged commit a9a7040 into ianstormtaylor:main Jul 15, 2024
11 checks passed
@github-actions github-actions bot mentioned this pull request Jul 15, 2024
@WindRunnerMax
Copy link
Contributor Author

@dylans If necessary, I am more than willing to rectify or assist in reverting this issue again and add the corresponding unit tests. 😄

fc0818 This is the commit and unit tests that were introduced at the time. I believe they can serve as a reference.

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.

ReactEditor.focus triggers onValueChange unexpectedly
2 participants