Slate: There was behavior change with selection after inserting inline node. How to restore old behavior?

Created on 27 Dec 2017  Â·  7Comments  Â·  Source: ianstormtaylor/slate

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

I suppose it's a bug since it's a breaking change that was unwanted because it broke my particular code, but it is beta and perhaps the change was intended and the old behavior was actually the bug so I'm not sure.

What's the current behavior?

With latest (slate 0.31.7 slate-react 0.10.22) the text is added to the text node after the inline node

What's the expected behavior?

With slate 0.31.3 slate-react 0.10.11 when you inserted an inline node and then continued typing, the text would be added to the text node within the inline node you inserted

http://recordit.co/FtoTMFdq1E

Working (Old slate): https://jsfiddle.net/mattmazzola/drzuL564/4/
(It seems jsfiddle doesn't save the change to external resources so you might have to manally update them to specific package versions 0.31.3 and 0.10.11)

Broken (Latest slate): https://jsfiddle.net/mattmazzola/d2geoc8h/3/

Two main questions:

  1. Is this breaking change intended? (I assume this is still beta so semver isn't really in affect yet and breaking changes could be expected)
  2. If no, then I assume there will be anothe PR to revert this or restore old behavior.
    If yes, how do I achieve the old behavior with latest slate?

My analysis is that the editor starts like:
text

  1. You insert inline node with text node. I'm using parentheses to designate nested node:
    text, inline ( text ), text <- notice slate alway ensures there is ending text node even if it's empty
  2. User continues typing

Here is there behavior diverges ( in old code it would be continuing on the inserted text node
On the new code I suspected it was continuing on the ending text node

However, the confusion for me is that I tried using collapseToEndOfPreviousText() to restore old behavior but that doesn't work either.

bug ♥ help

Most helpful comment

@ianstormtaylor it would be good to have possibility to change default behaviors in After plugin, which is specified in Slate. Because, as we see, in some situations, we need different behavior, but Slate doesn't allow it. What do you think about that and how it could be possible?

All 7 comments

I originally noticed this issue as I was trying to diagnose and error I was seeing when pressing delete after inserting the inline node.

It can be reproduced on https://jsfiddle.net/mattmazzola/drzuL564/4/

  1. Type some text
  2. Press '[' to insert inline node
  3. Press delete

See: http://recordit.co/udqGd3BI03

This doesn't occur in the new slate because of the behavior change above, but thought it might give some more insights for someone who knows more about slate internals and how to fix it

Hey @mattmazzola, sorry about that. I didn't realize this happened. This is a tough problem and I'm not sure what the ideal solution is. There are browser bugs that prevent us from putting the cursor in certain situations, and if I remember correctly I think the end of an inline is one, but I'm not sure. Right now the behavior is consistent with what I would have expected Slate permitted (even if not ideal).

I'm open to solutions to this, but they would need to be well cross-browser researched for inlines and marks, because I think quick attempts to solve it will add other breakages.

Hmm, I'm a little confused by this part:

There are browser bugs that prevent us from putting the cursor in certain situations, and if I remember correctly I think the end of an inline is one, but I'm not sure.

If the browser was the limiting factor and I assume this browser bug existed before the changes between 0.31.3 and 0.31.7 it implies slate does have control over the cursor in this situation since it was due to code change. Perhaps it was just an oversight during one of the commits.

Also you mentioned the current behavior is what you would expect slate to do. Could you elaborate on this. Was my conceptual model of the text nodes correct or is there something deeper I'm missing? I'm not familiar enough with how these browser bugs could affect something that seems higher level

Old: text, inline (text [cursor]), text
New: text, inline (text), text [cursor]

I added some code to print out the nodes with their keys and the current selections anchorKey and it seems this is correct:

I updated the JSBins:
https://jsfiddle.net/drzuL564/5/
https://jsfiddle.net/d2geoc8h/4/

Old:

Node 0: block 2
Node 0: text 3
Node 1: inline 4
Node 0: text 5
Node 2: text 6
Current Key:  5

Notice that the current key is the text node inside the inline node

New:

Node 0: block 2
Node 0: text 3
Node 1: inline 4
Node 0: text 5
Node 2: text 6
Current Key:  6

Notice that the current key is the text node at the end

I'm not familiar enough with slate internals to know how to go about proposing a solution, but I assume there should be away for me to manually set the cursor to the desired position/text node even if slate can't do it by default due to these browser bugs. This is why I was trying to use things like collapseToEndOfPreviousText but that didn't work even though it should if the above is correct.

If the browser was the limiting factor and I assume this browser bug existed before the changes between 0.31.3 and 0.31.7 it implies slate does have control over the cursor in this situation since it was due to code change. Perhaps it was just an oversight during one of the commits.

Also you mentioned the current behavior is what you would expect slate to do. Could you elaborate on this. Was my conceptual model of the text nodes correct or is there something deeper I'm missing? I'm not familiar enough with how these browser bugs could affect something that seems higher level

Very possible it was an oversight!

Although, the alternative is that it could have been a change to make cross-browser behavior more consistent. If I recall correctly, there are situations where Chrome/Safari disallow a cursor from being rendered at the end of an inline DOM element, and will automatically render it at the start of an adjacent text node instead. To keep behaviors from diverging across browsers, I think we've taken the approach of forcing the selections to only ever be selections that all the popular browsers are capable of rendering. There's a chance that the behavior was tweaked/added to account for this recently, or maybe it had been added before but was added incorrectly and is now correct.

The reason I say "expected" is because of that—my latest recollection from the last time I dove deeper into the cross-browser inconsistencies.

There's a chance though, that with the more recent change in Slate to always re-render nodes, and disallow any browser-level text insertion, that we could remove these restrictions now. It would require doing cross-browser testing to ensure that removing them still results in consistent behavior. I think there's a decent chance this is possible.

Actually though, looking at these lines of code it seems like it might not be cross-browser, but due to the need to standardize where a selection is placed when someone puts the cursor there. So it may be a different problem to resolve. Although, ideally the click-to-place-cursor situation would be deterministic without having to prevent end-of-inline selections entirely, like you said.

Hope that helps, feel free to ask followups if anything is still not clear.

@ianstormtaylor it would be good to have possibility to change default behaviors in After plugin, which is specified in Slate. Because, as we see, in some situations, we need different behavior, but Slate doesn't allow it. What do you think about that and how it could be possible?

Yea, I still don't have full understanding of the nuances around cursor placement / browsers limitations but if it's reasonable to add an option props to the editor that configures its behavior or even some global feature flags you setup once I think it would a good intermediate solution for this issue, and also allow other experimentation on other new possibly breaking stuff.

I'm currently deciding it's better to staying on the older version of slate which have this ability to write within the inline node until I find a solution that can mimic behavior with the later version of slate.

Ideally I would be able to update to the latest slate to get all the other bug fixes and stability and then enable this flag to open up ability to have cursor at the end of inline node.

I believe that this may be fixed by https://github.com/ianstormtaylor/slate/pull/3093, which has changed a lot of the logic in Slate and slate-react especially. I'm going to close this out, but as always, feel free to open a new issue if it persists for you. Thanks for understanding.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

yalu picture yalu  Â·  3Comments

vdms picture vdms  Â·  3Comments

ezakto picture ezakto  Â·  3Comments

bunterWolf picture bunterWolf  Â·  3Comments

Slapbox picture Slapbox  Â·  3Comments