Skip to content

Allow onValueChange and onSelectionChange to trigger on the same frame, fixing a few bugs where one was not being called#5975

Merged
dylans merged 3 commits intoianstormtaylor:mainfrom
nabbydude:onValueChange_fix
Nov 21, 2025
Merged

Allow onValueChange and onSelectionChange to trigger on the same frame, fixing a few bugs where one was not being called#5975
dylans merged 3 commits intoianstormtaylor:mainfrom
nabbydude:onValueChange_fix

Conversation

@nabbydude
Copy link
Contributor

Description
onValueChange and onSelectionChange have been treated as mutually exclusive, which is not the case, both can happen in a single frame. This changes withReact's onContextChange to account for that.

Issue
Fixes #5814, should fix #5910 (i do not have a mac to test with though)
Also an alternative fix for #5955, eliminating the need for the workaround in #5957, so this PR removes it.

Context
Sometimes, either from complex commands or certain browser behavior, multiple operations will happen in one frame before onChange gets called. Sometimes this can be a mix of selection and non-selection operations.
For some reason onChange gets called with an operation parameter, but this is only ever the first operation done in a frame and lacks context of a whole frame. The editor already has an operations member as a list of the operations performed in a frame, so we can just use that.

As written onSelectionChange will ALWAYS run before onValueChange, no matter which is first in the operations list. This can be changed, but this seemed like the simplest way to do it.

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 Nov 20, 2025

🦋 Changeset detected

Latest commit: 084986e

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

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.

The change looks reasonable, but it does appear to have a few failing tests. I've not looked to see if they were already failing, if this change causes them to fail, or if the tests need to be fixed and were passing by coincidence previously.

I believe it is the result of the fix in #5976 so once that lands I'll let the tests re-run.

@dylans dylans merged commit d0d192b into ianstormtaylor:main Nov 21, 2025
11 checks passed
This was referenced Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants