Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various algorithms should scroll the viewport to the selection #3870

Closed
Reinmar opened this issue Oct 28, 2016 · 9 comments · Fixed by ckeditor/ckeditor5-engine#1057
Closed
Assignees
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior. type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 28, 2016

  1. Paste, press enter multiple time, etc.
  2. The caret goes off screen and you stop seeing it.

The viewport should automatically be scrolled to show that position.

@Reinmar Reinmar assigned Reinmar and unassigned Reinmar Oct 28, 2016
@Reinmar
Copy link
Member Author

Reinmar commented Jul 6, 2017

To cover: backspace, enter, pasting, and perhaps block quote and lists too (can change the size of the paragraph by adding margins around).

@oleq
Copy link
Member

oleq commented Jul 25, 2017

It's quite simple to get the viewport-relative position of the selection as new Rect( domRange ) and Range.getDomRangeRects( domRange ) give us that information. Translation of the Rect to the scroll offset which makes sense to window.scrollTo() should not be a problem either because it's more or less rect.top + window.scrollY.

So the real question is: at which level should the editable be scrolled? Engine (something super–generic after editingView#render or similar) or feature-level (like after command execution)?

@Reinmar
Copy link
Member Author

Reinmar commented Jul 25, 2017

Scrolling to the right position is a lot more than that unfortunately. In a simple scenario where only the window has a scrollbar, then this is going to work. But if there are other scrollable regions (e.g. the editable has a fixed height), then you need to scroll all of them. And we'd need to dig in CKE4 to understand how exactly.

So, the question is how soon we'll need this mechanism to be robust. How long can we live with only scrolling the viewport.

It will all depend on how the editor will be used by the first users. We're certainly going to avoid scrolling in our samples. But if we're using some container element with fixed height anywhere (e.g. in the docs) then this is going to be visible quite soon.

Another consideration is that scrolling is performed by the browser if it native action took place. So typing is OK. Deleting, enter, pasting are not. This makes this issue slightly less problematic, but still very easy to hit.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 25, 2017

BTW, out of curiosity – perhaps browsers implement some mechanism such as "scroll to element" or "scroll to range"? We might use it then.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 25, 2017

There are https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoViewIfNeeded and https://developer.mozilla.org/en/docs/Web/API/Element/scrollIntoView :D

The only problem is that we'd need to inject some marker next to the caret temporarily, which is not cool (breaks editing). We'd need to combine it with some other mechanism which will check if the caret is invisible (that we can do) so we could inject those markers as infrequently as possible. However, this would still break the typing in some scenarios ;/

@oleq
Copy link
Member

oleq commented Jul 25, 2017

In v4 we had element scrollIntoView() and element.scrollIntoParent() where the later handles cases where parents have CSS overflow.

There's also range.scrollIntoView() which injects the element, does element.scrollIntoView() and removes it. We can do better than that with v5 Rect utility.

I think it's not a huge deal to import and improve such code. I'd start with the following simple use cases:

  1. Paste, if the caret went off-screen, scroll to it.
  2. Press enter, if the caret went off-screen, scroll to it.

Then we'll find out if it works fine and copy the fix to other places. How about that?

@oleq
Copy link
Member

oleq commented Jul 25, 2017

The only problem is that we'd need to inject some marker next to the caret temporarily, which is not cool (breaks editing). We'd need to combine it with some other mechanism which will check if the caret is invisible (that we can do) so we could inject those markers as infrequently as possible. However, this would still break the typing in some scenarios ;/

I'd rather go with Rect utility which is based on getBoundingClientRects and keep the editing out of it.

@Reinmar
Copy link
Member Author

Reinmar commented Jul 25, 2017

Then we'll find out if it works fine and copy the fix to other places. How about that?

Your question about where this should be handled is still open. TBH, it's not an easy question because we start mixing together view and model at this moment ;|

@oleq oleq self-assigned this Jul 26, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-engine Aug 15, 2017
Feature: Implemented `view.Document#scrollToTheSelection()` method. Closes #660.
Reinmar referenced this issue in ckeditor/ckeditor5-block-quote Aug 15, 2017
Feature: Viewport will be scrolled to the selection upon user input. See ckeditor/ckeditor5-engine#660.
Reinmar referenced this issue in ckeditor/ckeditor5-clipboard Aug 15, 2017
Feature: Viewport will be scrolled to the selection upon user input. See ckeditor/ckeditor5-engine#660.
Reinmar referenced this issue in ckeditor/ckeditor5-enter Aug 15, 2017
Feature: Viewport will be scrolled to the selection upon user input. See ckeditor/ckeditor5-engine#660.
Reinmar referenced this issue in ckeditor/ckeditor5-typing Aug 15, 2017
Feature: Viewport will be scrolled to the selection upon user input. See ckeditor/ckeditor5-engine#660.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. type:improvement This issue reports a possible enhancement of an existing feature. package:engine labels Oct 9, 2019
@APegram
Copy link

APegram commented May 18, 2021

@Reinmar I know this issue has been closed; however, I was curious if there was a way to disable the scrollTo cursor method. I have users that are working with large documents and are referencing higher up in the text editor, but when they type it will auto-scroll to the caret. It appears to do this on more than what I see described; IE - typing anything does this.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants