-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(caret): trailing spaces handling #2741
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just a minor comment
@@ -34,5 +34,8 @@ | |||
"unknown": true, | |||
"requestAnimationFrame": true, | |||
"navigator": true | |||
}, | |||
"rules": { | |||
"jsdoc/require-returns-type": "off" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/components/modules/caret.ts
Outdated
} | ||
|
||
const { nextInput, currentInput } = currentBlock; | ||
const isAtEnd = currentInput !== undefined ? caretUtils.isAtEndOfInput(currentInput) : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar expression is used several times, maybe worth to extract it to a function. Or allow caret utils to accept undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that utility methods like isAtStartOfInput()
should not handle case when input is undefined
. Module should explicitly handle the cases when block has no inputs or there is no currentBlock
at all since it is a part of business logic.
Problem
If block starts with several spaces, they can't be deleted. Caret jumps to the previous block.
Resolves #2429
Resolves #1382
Resolves #1179
Resolves #2521
Cause
There are two whitespace handling in HTML
Our
Caret.isAtStart
method covers only case №1 and treats all whitespaces like "empty".Solution
I've rewrote
isAtStart()
andisAtEnd()
methods.this.BlockManager
has been removed. Now methods acceptinput
as argument. (so we'll be able to expose them to the api or some package)isAtEndOfInput()
) to the caret position, clone its content to the temporary div and check its textContentisCollapsedWhitespaces()
is used for it.