codex-team
Added handling for delete key
Pressing delete at the end of a block merges blocks (fixes #821) Pressing delete after a rectangular block selection removes the blocks (fixes #1315)
Thanks for the review. Should I rebase on the next branch to integrate the new commits ?
For some reason I can't merge anything except two paragraphs using delete key and also have noticed few more uncovered cases. Will investigate it on the weekend and get back to you
You are right, they shouldn't, but there are some of my discovers:
| Pre-requirements | Steps | Actual | Expected |
|---|---|---|---|
| Two not-empty different blocks one by one | Set cursor to the end of the first block and press delete | Nothing is happen | If the same happens for backspace, caret is set to the previous block. So for delete I'd expect caret to set to the next block |
| Not-empty block and then an empty block | - "" - | - "" - | I'd expect empty Block to be deleted |
| - "" - | Set cursor to the empty block and press delete | Block is deleted, caret is set to previous block | I'd expect caret to be set to the next block |
| Not-empty block and then block without inputs (eg Delimiter) | Set cursor to the end of the first block and press delete | Nothing is happen | I'd expect Delimiter to be deleted |
Two not-empty different blocks one by one
I'm not sure, but I'd consider it a bug of the backspace key. I'm not sure that's something I would expect to be normal : If I pressed backspace instead of the left arrow key, that's because I wanted to destroy something, which obviously didn't work but had a "side effect" of moving the caret.
EDIT : This seems to be explicitely wanted : https://github.com/codex-team/editor.js/blob/32ac52a62fb1e36dddaad1f9fe224e46a931f056/src/components/modules/blockEvents.ts#L357 . I'm still not sure that's a desired behavior, but I can match that the other way around
Other points
Seems like the mergeBlocks methods assume that targetBlock is just before blockToMerge, but it doesn't handle well the cases you mentionned. I'm digging further and I'll get back to you
So I fixed the 3 last points (in 3 different commits to make it easier to review). Thanks for testing and pointing that out !
As for the first point, I'm not sure how this should be handled:
https://github.com/codex-team/editor.js/blob/32ac52a62fb1e36dddaad1f9fe224e46a931f056/src/components/modules/blockEvents.ts#L370-L372
In this block, I dont know in which direction the merge occurred. Sure, I could detect it by the position of the caret, or by passing an additional parameter. Is that really something useful though ?
For example, I'm on the image caption control :
I made an error, and I press backspace to the beginning of the input, as I am not patient, I'm holding down the backspace key, until everything is removed. If I don't release the key on time, the caret is moved to the previous block, and starts removing code above.
@jeremyVignelles agree the first point is arguable, I'm ok with the current behaviour, but it conflicts with backspace behaviour. Let's listen to @neSpecc and @khaydarov opinion.
I've added some test cases, hope you don't mind
Great, thanks for the test case, I never used cypress, I'll see how it works then 🙂
Just merged the latest "next" and tried to resolve the conflicts that I had. Ready again for review
Resolved by #2402 Available starting 2.28.0-rc.1 2.28 will be released this week.
