slate: Scroll-to-cursor is broken inside scrollable containers
Do you want to request a feature or report a bug?
bug
What’s the current behavior?
Set the editor to a fixed height and overflow-y: auto (standard procedure for scrolling content). Press enter several times to exceed this height and the and cursor disappears; a scrollbar appears but the editor does not scroll down. However, if you type a few characters (instead of just creating new lines with 0-1 characters each), the editor suddenly snaps down as expected.
Demo (direct from example page):
Slate JSFiddle with same issue: https://jsfiddle.net/f4150bbq/
This is not reproduceable in a simple JSFiddle with a contenteditable div: https://jsfiddle.net/gz2wrkxz/1/
What’s the expected behavior?
When content exceeds available height, scroll editor down to cursor (i.e., the behavior that happens when you type several characters in the demo above).
I’ve tried my best to figure out the cause of this, but I’m coming up short- could there be a problem with scrollToSelection? Seeing the expected behavior when typing several characters is also interesting; are we accidentally overriding some default browser behavior?
Mac OS 10.12.3, Chrome 60.0.3112.101, Slate current
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Comments: 18 (9 by maintainers)
Commits related to this issue
- Scoll to selection tweaks, possible fix for #1032 — committed to skogsmaskin/slate by skogsmaskin 7 years ago
- [WIP] Scoll to selection tweaks, possible fix for #1032 (#1165) * Scoll to selection tweaks, possible fix for #1032 * Update scroll-to-selection.js * Update scroll-to-selection.js — committed to ianstormtaylor/slate by skogsmaskin 7 years ago
- corrects overscroll issue inside scrollable containers; another fix for #1032 — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- corrects overscroll issue inside scrollable containers; another fix for #1032 — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- fix(#1032) - set scrollers offset if within scrollable container — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- fix(#1032) - set scrollers offset if within scrollable container — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- fix(#1032) - set scrollers offset if within scrollable container — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- fix(#1032) - set scrollers offset if within scrollable container — committed to RobertDaleSmith/slate by RobertDaleSmith 7 years ago
- corrects overscroll issue inside scrollable containers (#1032) (#1383) * corrects overscroll issue inside scrollable containers; another fix for #1032 * fix(#1032) - set scrollers offset if within... — committed to ianstormtaylor/slate by RobertDaleSmith 7 years ago
- Revert "corrects overscroll issue inside scrollable containers (#1032) (#1383)" This reverts commit b973580c54be4cfddd3e7b61e6c7f4222d6ee5b2 due to regression of scroll behavior outlined in #1416. — committed to erquhart/slate by erquhart 7 years ago
Either way, this is still something that would be nice to fix in core. Since the current behavior seems obviously wrong. If someone wants to make a PR I’d be happy to have this fixed. Thanks!
I’d like to add my 2¢ by saying that scrolling logic should probably be a separate plugin.
Not sure if this is possible to implement with the way Slate operates now, however, scrolling behaviour is not universal for all types of applications. For example, focusing on void blocks (i.e. an image) may make sense in some cases to have the view scroll to the top edge of the block, and in others, to the bottom edge; and in some cases no scrolling is desired at all. Another case is a textfield with a limited input that requires no scrolling, which could make Slate component a lot lighter if it isn’t requiring additional logic. Finally, there may be apps that prefer smooth scrolling or other clever methods.
@ianstormtaylor @jennaplusplus @conorcussell Looking at this more, I think I see some potential issues.
No matter what ends up happening here, we do a
window.scrollTo, everything in this function is assuming that the scrolling container is the window, i’d wager a guess that for most people experiencing this issue, the window isn’t there scrolling container. I know that even for myself I use a custom scrolling container, because my editor can be positioned around the screen using positionfixed.So now the interesting part, what do we do about this? Should we try and figure out every time we run this function what the true scrolling container is?
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-reactespecially. 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.Just thought I’d add my experience, hopefully it’ll help someone else.
I ran into this issue, where the editor wouldn’t scroll correctly on pressing enter in a scrollable container. However, after setting some breakpoints around the relevant logic, the issue I had was that findScrollContainer() was finding the “wrong” container. I had another container with auto overflow around the editor, which, however, wasn’t doing the scrolling as its parent also had auto scroll.
findScrollContainer()was finding the inner one.I fixed it/worked around it by just setting
overflow: visibleon the container that was incorrectly identified by slate as thescrollerbyfindScrollContainer().