Slate: Jumpy editor behavior in scrollable container

[stapleup]

Do you want to request a _feature_ or report a _bug_?

_bug_

What's the current behavior?

Editor jumps up and down when it is placed inside a scrollable container. This issue is different from other issues previously reported because they all target older versions of Slate.
GIF: https://share.getcloudapp.com/7KuyN5zo

Slate: 0.57.1
Browser: Chrome
OS: Windows

What's the expected behavior?

Slate should scroll inside its container and not scroll the parent container.

Possible Reason

Slate 0.50+ seems to be using https://github.com/stipsan/scroll-into-view-if-needed in order to bring selected view into focus. Scroll-into-view have a property boundary which allows us to define a parent element inside which it should position the item.. This parameter seems to be missing here https://github.com/ianstormtaylor/slate/blob/e54b07eba8ab23a8c26b275f77e8ec207f8c9b55/packages/slate-react/src/components/editable.tsx#L197 and it causes the parent container to scroll along with editor. This issue can be reproduced on the official site for said library here (https://scroll-into-view-if-needed.netlify.com/) if you disable the boundry check box next to Limit propagation you'll see it scrolls the whole document when positioning the element.

[samosaboy]

I too am experiencing this issue. It is quite apparent when selecting text as well.

[Francesco-Lanciana]

I'm also experiencing this when I format my text, even if the text is completely within the viewport which is wierd since that shouldn't happen with scrollMode: 'if-needed'.

I can't seem to figure why we actually have this scrollIntoView though. My best guess is that it's simply a convenience for users of Slate, so if they programatically update their selection it will also scroll to this new selection?

For my specific use case I just did a bit of digging and it turns out that there is a quirk in browsers where if you set the HTML element height to be 100% then getClientBoundingRect gives a negative value for bottom (which makes no sense). The scroll-into-view-if-needed uses a library called compute-scroll-into-view to check if the element in question is within the viewport (which it does by getting the HTML elements rect and the element in questions rect and doing some checks). If it is and scrollMode is 'if-needed' then it won't scroll. Unfortunately the negative value causes this check to fail and it scrolls anyway. I have raised an issue about this https://github.com/stipsan/compute-scroll-into-view/issues/652

I agree with @stapleup, if we used a boundary then it would fix the issue, however what would you set the boundary too? I think if we made assumptions here we could break peoples editor since we don't know which element of theirs is the scrollable element that should act as the boundary. But I'm also curious, perhaps this could be removed altogether and instead it could be added to Transforms.select method as an option, i.e.
select(editor: Editor, target: Location, scrollIntoView: Boolean)
Will still have the same issue of what to set the boundary too but at least it wouldn't be the default behaviour.

[Francesco-Lanciana]

I think I see why why have it now, actions like moving the cursor with the arrow keys to an element that is off center or typing when the content is out of focus will cause the scroll to center on the selection. So keeping it in it's current location as opposed to in Transforms.select would make sense since the current spot is a good catch all.

Just need to figure out how to choose the boundary I guess

[heymanhn]

This seems to me like behavior that should be customized in userland. Our app superimposes an editor over a full-page-height editor, and this scroll-into-view is causing all kinds of weirdness that we can't control or patch on our side.

[esmooov]

This fix looks nice ^! Very much looking forward to this as I am also struggling with this at the moment.

[mitchellbusby]

Don't want to pile on this thread but the above commit (1652eca) fixes my experience of this bug locally quite nicely.

[zswcat]

Anxiously waiting.I'm also experiencing this

[mpkelly]

I patched this fix and found it still was jumpy in a table cell in a code sandbox demo. I figured the scrolling library used could not handle iframes properly as this was only an issue at Code Sandbox and not in my test system. I didn't look into too much as I used another fix: disable this code completely, and I have not experienced any negative side effects from doing so. As is said in a related issue, this is something that should be handled in user land and not by Slate and I wish Slate would just remove it completely.

[CameronAckermanSEL]

Hey guys, thanks for the feedback. I've been waiting for a PR that does something like: https://github.com/ianstormtaylor/slate/commit/1652ecab52088e64ffbaa130fa4049f60cdce57e. Had a couple of folks get in touch with me via PMs that they've been trying this in forks and it helps alleviate the issue.