MouseEventManager / LineSelectionManager -> InteractionManager Refactor#362
MouseEventManager / LineSelectionManager -> InteractionManager Refactor#362
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
* combines MouseEventManager & InteractionManager * Mostly because there was a lot of shared logic and it keeps a lot of things better combines. * This was mostly an AI refactor, but I think it should be in a good place
amadeus
force-pushed
the
amadeus/interaction-manager
branch
from
1567eb2 to
377ddde
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // onLineSelectionEnd(props) { | ||
| // console.log('onLineSelectionEnd', props); | ||
| // }, | ||
| // Super noisy, but for debuggin |
There was a problem hiding this comment.
Typo in comment: "debuggin" should be "debugging".
| // Super noisy, but for debuggin | |
| // Super noisy, but for debugging |
| const len = content.children.length; | ||
| if (len !== gutter.children.length) { | ||
| throw new Error( | ||
| 'InteractionManager.renderSelection: gutter and content children dont match, something is wrong' |
There was a problem hiding this comment.
Typo in error message: "dont" should be "don't".
| 'InteractionManager.renderSelection: gutter and content children dont match, something is wrong' | |
| "InteractionManager.renderSelection: gutter and content children don't match, something is wrong" |
| if (this.queuedSelectionRender != null) { | ||
| cancelAnimationFrame(this.queuedSelectionRender); | ||
| this.queuedSelectionRender = undefined; | ||
| } |
There was a problem hiding this comment.
The attribute 'data-interactive-line-numbers' is removed twice in the cleanUp method - once at line 192 and again at line 204. This is redundant and should be removed.
| // console.log('onLineLeave', props.annotationSide, props.lineNumber); | ||
| // console.log('onLineLeave', props); | ||
| // }, | ||
| // __debugMouseEvents: 'click', |
There was a problem hiding this comment.
The option name has changed from __debugMouseEvents to __debugPointerEvents in the InteractionManager, but this commented-out code still references the old name. This should be updated to __debugPointerEvents for consistency.
| // __debugMouseEvents: 'click', | |
| // __debugPointerEvents: 'click', |
| // onLineSelectionEnd(props) { | ||
| // console.log('onLineSelectionEnd', props); | ||
| // }, | ||
| // Super noisy, but for debuggin |
There was a problem hiding this comment.
Typo in comment: "debuggin" should be "debugging".
| // Super noisy, but for debuggin | |
| // Super noisy, but for debugging |
Basically LineSelectionManager and MouseEventManager were doing very similar but different things, and were starting to need similar types of state to react to, so I decided to merge them into a singular manager.
This was mostly done with AI and a lot of back and forth. I think it's definitely an improvement, and will fix up some of the functionality with some of the existing features (like allowing drag select from the gutter utility)