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

Selecting all and then checking whether the value isEmpty throws #2004

Closed kalley closed 6 years ago

kalley commented 6 years ago

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

bug

What's the current behavior?

Go to: https://www.slatejs.org/#/hovering-menu

Click inside and command+a (or ctrl+a) to select all. You'll see an error.

This is coming from value.isEmpty being called in the check. I'm not sure what should change though.

What's the expected behavior?

Expected behavior would be that it selects all and does not error out.

linonetwo commented 6 years ago

Try https://deploy-preview-1985--slatejs.netlify.com/#/hovering-menu Seems to be fixed in https://github.com/ianstormtaylor/slate/pull/1985 by @zhujinxuan

steida commented 6 years ago

@linonetwo For some reason, that fix does not work for me.

steida commented 6 years ago

It looks like number passed where a string is expected. Btw, what is the difference between isCollapsed (which works) and isEmpty (which fails for now)?

zhujinxuan commented 6 years ago

I think it is perhaps related to https://github.com/ianstormtaylor/slate/pull/1997.

kalley commented 6 years ago

I think I've narrowed it down to getFragmentAtRange not passing a path to splitNode. I'm still getting up to speed with paths, so wanted to put this here in case someone sees a quick fix.

zhujinxuan commented 6 years ago

@kalley splitNode is not a change method. (It is a method under the same model/node.js....) . I think the error is perhaps from somewhere else.

I think some proxy method in https://github.com/ianstormtaylor/slate/blob/master/packages/slate/src/changes/on-selection.js is not right.

kalley commented 6 years ago

This is not about a change method. This is about value.isEmpty being called after a change action in an component update call. So you have to follow the call stack from value.isEmpty, which is how I figured that the while loop is passing a number to splitNode. I just don't know enough about paths yet to figure out what should be happening.

steida commented 6 years ago

isCollapsed is quick workaround, working isEmpty would be better ofc.

ianstormtaylor commented 6 years ago

Fixed by #2012, thanks for reporting @kalley.