fix(block): notify changes from native select elements - #3025
Merged
Merged
Conversation
Delegate select change events to the Block holder during bubbling, keeping Tool state updates ahead of the updated hook. Register synchronously and remove the listener on destruction to avoid deferred rebinding. Adapt the SelectTool fixture and initial regression tests from Sion612's fork-local PR, and cover composite Tools, callback ordering, and early destruction. Tests adapted from: Sion612#1 Fixes codex-team#2975
Sawtone
requested review from
TatianaFomina,
gohabereg,
ilyamore88 and
neSpecc
as code owners
September 15, 2026 08:32
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified.
Pull request overview
Fixes native <select> change detection so Tool updates trigger the existing Block pipeline and onChange callbacks.
Changes:
- Adds delegated select-change handling and lifecycle cleanup.
- Adds regression coverage for ordering, replacement roots, composite Tools, and destruction.
- Adds select-based Cypress fixtures.
File summaries
| File | Description |
|---|---|
test/cypress/tests/onchange.cy.ts |
Adds regression and lifecycle tests. |
test/cypress/fixtures/tools/SelectTool.ts |
Adds a basic select fixture. |
test/cypress/fixtures/tools/CompositeSelectTool.ts |
Adds a multi-select fixture. |
src/components/block/index.ts |
Implements select event handling and cleanup. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
neSpecc
reviewed
Sep 17, 2026
neSpecc
left a comment
Member
There was a problem hiding this comment.
Seems ok for me, please, increment a patch version and add a line in a changelog
Contributor
Author
neSpecc
approved these changes
Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2975.
Native
<select>value changes do not produce DOM mutations, so selecting an option can update a Tool's saved data without triggeringonChange. This PR routes native select changes through the existing Block update pipeline.The listener is delegated to the Block holder, supporting multiple selects and replacement Tool roots. It runs during bubbling so the Tool's own change handler updates its state before
updated()runs. Registration is synchronous and removal happens inBlock.destroy(), preventing deferred initialization from reinstalling this listener after destruction.Validation
onchange.cy.ts: 30/30 passed with Cypress 13.13.3 and Electron 118 against a temporary Vite source fixture with a module-load wait.git diff --checkpassed.The temporary test runner and source fixture changes are not included in this PR. The production build and full cross-browser suite were not run locally.
Credits
The
SelectToolfixture and initial regression tests are adapted from Sion612/editor.js#1. Thanks @Sion612 for that work.Additional coverage checks composite Tools, Tool callback ordering, and destruction before deferred initialization.