feat: implement onEditCommit host callback (spec §6.2) - #207
Closed
timurbazhirov wants to merge 1 commit into
Closed
feat: implement onEditCommit host callback (spec §6.2)#207timurbazhirov wants to merge 1 commit into
timurbazhirov wants to merge 1 commit into
Conversation
Completes the host-API gap the TB-persona review flagged: onEditCommit was
listed in spec §6.2 (P1 scope, alongside onSelectionChanged/
onEditModeChanged) but never implemented - only onUpdate's every-edit,
source-agnostic channel existed, which is exactly the flood that broke
materials-designer's redux history integration in the old modal editor.
Threads a `source` string through every commit path down to
onStructureModified(material, source): the mixin's commitMovedAtom_/
commitMovedAtoms_ now tag "drag" (direct body-drag, endAtomDrag_) vs
"gizmo" (TransformControls "mouseUp" listener, translate or rotate alike);
addAtom/removeSelectedAtom(s)/cloneSelectedAtoms tag "add"/"remove"/"clone".
ThreeDEditor.jsx forwards this to a new onEditCommit(material, {source})
prop via _applyMaterialToViewer, and adds "coordinate-input"/"element-input"
for its own typed-panel edits and "undo"/"redo" for handleUndo/handleRedo
(which bypass handleStructureModified entirely). Extends spec §6.2's
original 7-value source enum with "element-input" and "clone" for this
round's type-to-change and clone-atom features, which postdate the enum.
Also fixes tests/helpers/editor.js's simulateAtomDrag, whose
onStructureModified interceptor only forwarded the first (material)
argument - silently dropping source and would have masked this feature
working correctly for the direct-drag path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
This was referenced Aug 12, 2026
Member
Author
|
Closing — consolidated into #216 (then #214 on top). Verified contained: this PR's head The branch Generated by Claude Code |
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
1 participant
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.
Summary
Completes a host-API gap the TB-persona review of #204 flagged:
onEditCommitwas listed indocs/design/interactive-editor-spec.md§6.2 (P1 scope, alongsideonSelectionChanged/onEditModeChanged) but never actually implemented — onlyonUpdate's every-edit, source-agnostic channel existed. That's exactly the flood that broke materials-designer's redux history integration with the old modal editor in the first place.sourcestring through every commit path down toonStructureModified(material, source)in the mixin:"drag"(direct body-drag) vs"gizmo"(TransformControls-driven, translate or rotate alike),"add","remove","clone".ThreeDEditor.jsxforwards this to a newonEditCommit(material, {source})prop, and adds"coordinate-input"/"element-input"for its own typed-panel edits and"undo"/"redo"forhandleUndo/handleRedo(which bypass the shared commit path entirely).sourceenum with"element-input"and"clone", since those two features postdate when §6.2 was written.tests/helpers/editor.js'ssimulateAtomDrag: itsonStructureModifiedinterceptor only forwarded the first (material) argument, silently droppingsource— would have masked this working correctly for the direct-drag path specifically.Test plan
npx tsc --noEmitcleannpx eslintclean (pre-existinganywarnings only)sourcevalue at both the mixin level (onStructureModified's second argument) and the component level (onEditCommit's{source}shape), plus one confirming the prop is optionalnpm run buildclean🤖 Generated with Claude Code