ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.72k stars 3.24k forks source link

cmd+a + cmd+x doesn't work in rich text example #2309

Closed geakstr closed 5 years ago

geakstr commented 5 years ago

Do you want to request a feature or report a bug?

Bug

What's the current behavior?

cmd+x doesn't work in rich text example

What's the expected behavior?

cmx+x works

ezgif-4-99749bb270d5

P.S. I noticed other similar troubles with corrupted documents in my project too, but not investigated yet.

Nantris commented 5 years ago

Duplicate of #2279

zhujinxuan commented 5 years ago

I find the reason. When delete all, the paragraph will include no leaves. Then moveToStart have no place to move. (Not this reason)

screen shot 2018-10-24 at 11 14 55 am
zhujinxuan commented 5 years ago

My fault. I forgot to think about delete all situation in removeText. I will submit a PR to fix.

https://github.com/ianstormtaylor/slate/blob/master/packages/slate/src/models/text.js#L509-L511

Not this problem.... I will investigate into the deleteAtRange to check what the problem is.

Nantris commented 5 years ago

@zhujinxuan - I believe this rule should prevent empty paragraphs. I suspect it is not being applied. https://github.com/ianstormtaylor/slate/blob/master/packages/slate/src/plugins/core.js#L97-L107

I suspect the majority of all issues from the past week are caused by the schema rules not being properly applied. With that in mind, this issue too is probably related to https://github.com/ianstormtaylor/slate/issues/2264

zhujinxuan commented 5 years ago

@Slapbox I am unsure how the Ctrl-X and backspace is handled differently. It seems backspace works after C-a, but C-x does not work after C-a

Nantris commented 5 years ago

Great point.

zhujinxuan commented 5 years ago

Hi @Slapbox @ianstormtaylor Is cmd+x is not defined in slate-hotkeys? I cannot see the binding.

Nantris commented 5 years ago

I guess it's not. Looking through the history I guess it's never been. If it was there it would be in as mod+x.

Nantris commented 5 years ago

This is the line that crashes things: https://github.com/ianstormtaylor/slate/blob/master/packages/slate-react/src/utils/clone-fragment.js#L133

The description of this block of code is: // Revert to the previous selection right after copying. - Obviously after cutting the selection is not there anymore.

@zhujinxuan here's the code where the copy and cut code starts doing its thing. We need to avoid setting the original range again on cut operations.

https://github.com/ianstormtaylor/slate/blob/master/packages/slate-react/src/plugins/after.js#L201-L241

zhujinxuan commented 5 years ago

@Slapbox Oh, I see. It is fixed in https://github.com/ianstormtaylor/slate/pull/2298

Nantris commented 5 years ago

This is greatly improved in the latest releases, which include #2298, but it's not perfect.

@kalley

New Behavior:

Cutting the entire contents does not crash the editor, and does copy the contents to clipboard, but it does not clear out the editor and it throws the following error:

Uncaught Error:Node.assertNodecould not find node with path or key: List [ 1 ]

Additionally trying to paste over it all with the copied contents gives:

Uncaught Error:Node.assertNodecould not find node with path or key: List [ 4 ] - This one probably belongs in a new issue

skogsmaskin commented 5 years ago

Uncaught Error:Node.assertNodecould not find node with path or key: List [ 1 ]

This is probably https://github.com/ianstormtaylor/slate/issues/2291#issuecomment-432871992

Nantris commented 5 years ago

@skogsmaskin I agree. My mistake.

steobrien commented 5 years ago

I think this has been fixed – can no longer reproduce at https://www.slatejs.org/#/rich-text.

Perhaps this had the same root cause as https://github.com/ianstormtaylor/slate/issues/2291?

Nantris commented 5 years ago

It would appear so!