Fix bugs when ctrl+backspace (mac) throws an error at the block start

[zhujinxuan]

Is this adding or improving a feature or fixing a bug?

bugs;

How does this change work?

Because ctrl+backspace is un-handled by the onKeyDown; it will create crazy onInput event, this PR aims to provides default behavior for un-handled backspace problems.

Have you checked that...?

• The new code matches the existing patterns and styles.
• The tests pass with yarn test.
• The linter passes with yarn lint. (Fix errors with yarn prettier.)
• The relevant examples still work. (Run examples with yarn watch.)

Does this fix any issues or need any specific reviewers?

Fixes: #1887
Reviewers: @

[ianstormtaylor]

@zhujinxuan I don't understand this one. What is ctrl-backspace supposed to do on Mac? If it's a Mac-specific change why is there not an environment check? Why also for delete?

[zhujinxuan]

@ianstormtaylor Sorry I forgot to add gif;

When ctrl-backspace is clicked, it will fire a keyboard event. Because this keyboard event is not captured in slateJS onKeyDown function, the event will be handled by browser. The browser will remove a char as in contenteditable, and the browser's removal will fire an onInput event and an onChange event, and these two events can cause slate-react collapse.

I use the deleteCharBackward because it most alike the chromium's default for a generic modifier+backspace

I did not do the environment check because perhaps something similar can happen in other platforms. if a modifier+backspace is not handled, then the browser will fire un-expected onInput and onChange events. So we want to prevents these events and make slate-react handle these things.

[ianstormtaylor]

I see what you mean. I think we should just be changing the behavior for Hotkeys.deleteBackward to allow for ctrl+backspace on the Mac. (And the same for deleteForward for delete.)

I appreciate wanting to account for other platforms, but until we discover those I think it's better to keep it in the same system.

[zhujinxuan]

@ianstormtaylor Great. I think it is much nicer to put it in slate-hotkeys rather than slate-react. I will fix that tomorrow.

[zhujinxuan]

Fixed in the slate-hotkeys. To resolve all modifier+backspace/delete regardless of the platforms; here I use event.key and escape all other delete events.

[zhujinxuan]

Reopen to generate deploy preview.
lol, the deploy site cannot be generated by reopen.

[ianstormtaylor]

I don't understand what's happening here, why is this needed? This method doesn't read like the others.

[zhujinxuan]

Hi, this method means:

if a keyevent includes backspace or delete, and is not captured by other handlers. Then it will be captured by deleteCharBackward. It ensures that all backspace/delete events are catched. Therefore, it will prevents issues like #1966 and #1983

Because when backspace events are not catched, the browser will catch the key and remove content without notifying the React.

[ianstormtaylor]

Okay... but why would that be inside a function called isDeleteCharBackward then? It doesn't make sense. If anything, you'd make a new function called isDelete and use that.

But further, there's already one called isContentEditable that is used in the Before plugin that results in e.preventDefault() being called. So it seems redundant?

So maybe there's some other reason?

[zhujinxuan]

Oh, I see what you mean. I will fix it with isDelete and isContentEditable

[zhujinxuan]

Fixed~

[zhujinxuan]

Hi @ianstormtaylor . Anything else I shall do about this PR?

[ianstormtaylor]

Instead of adding isOtherDelete here, can we just re-use the existing isBackspace or isDelete from above? That seems like it would solve it in a way that's more clear and with less code.

[zhujinxuan]

@ianstormtaylor Yes, we can. I put e.key === here. Fixed in a53e991

[codecov]

Codecov Report

Merging #1888 into master will decrease coverage by 0.06%.
The diff coverage is 40%.

@@            Coverage Diff            @@
##           master   #1888      +/-   ##
=========================================
- Coverage   67.06%     67%   -0.07%     
=========================================
  Files          68      68              
  Lines        5660    5670      +10     
=========================================
+ Hits         3796    3799       +3     
- Misses       1864    1871       +7

Impacted Files	Coverage Δ
packages/slate-react/src/plugins/after.js	5.98% <0%> (ø)	⬆️
packages/slate-hotkeys/src/index.js	76.11% <42.85%> (-8.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3eabdea...a53e991. Read the comment docs.

[ianstormtaylor]

@zhujinxuan sorry for being unclear, I was trying to get at the fact that it was really a problem in the detection of the individual delete* utils, and not something that needed extra utils to be added. But this was hard to convey because the original slate-hotkeys code was kind of crazy. I've refactored it in #2048 and fixed this issue there. I hope that makes sense. Thank you!