fix(caret): disable smooth animation on line transitions (@eikomaniac) #7386
Conversation
github-actions
bot
added
the
waiting for review
There was a problem hiding this comment.
Pull request overview
This PR disables smooth caret animation when transitioning between lines to prevent the caret from visibly "flying" across the screen during line changes.
Changes:
- Added line change detection using
isLineChangeflag andpreviousWordToptracking - Modified animation logic to skip smooth animations during line transitions
- Updated position calculation to return
wordTopfor line change detection
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.readyToResetMarginTop = false; | ||
| this.readyToResetMarginLeft = false; | ||
| this.cumulativeTapeMarginCorrection = 0; | ||
| this.previousWordTop = -1; |
There was a problem hiding this comment.
The previousWordTop reset is missing initialization in the constructor. While it's reset here in what appears to be a reset method, the field should also be initialized in the constructor at line 46 for consistency with other private fields.
| if (this.previousWordTop !== -1 && wordTop !== this.previousWordTop) { | ||
| this.isLineChange = true; | ||
| } |
There was a problem hiding this comment.
The isLineChange flag is set to true but never reset to false before the next position update in some code paths. If options.animate is false at line 409, the flag is reset at line 414, but if the condition at line 397 triggers multiple times before reaching line 414, the flag state could be incorrect for subsequent updates within the same animation frame.
| // affect the position animations | ||
|
|
||
| // skip the next smooth caret animation to prevent the caret from | ||
| // flying across the screen when transitioning to a new line |
There was a problem hiding this comment.
Setting isLineChange = true in handleLineJump without any corresponding reset within this method creates an implicit dependency on the reset happening elsewhere (line 414). Consider adding a comment explaining that this flag is reset in the updatePosition method, or refactor to make the lifecycle of this flag more explicit and easier to maintain.
| // flying across the screen when transitioning to a new line | |
| // flying across the screen when transitioning to a new line | |
| // note: this flag is reset in updatePosition to avoid affecting subsequent animations |
Description
skips smooth caret animation when moving to a new line