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

scroll-change event and scrollTo method #183

Closed
benbro opened this issue Jul 31, 2014 · 9 comments
Closed

scroll-change event and scrollTo method #183

benbro opened this issue Jul 31, 2014 · 9 comments

Comments

@benbro
Copy link
Contributor

benbro commented Jul 31, 2014

Is it possible to add 'scroll-change' event that fire when a user is scrolling (or finished scrolling) with scrollTop?
I couldn't find this event in the list:
https://github.com/quilljs/quill/blob/develop/src/quill.coffee#L48

It will also be useful to have quill.editor.scrollTo method that will let us scroll the editor.

Thanks

@rafales
Copy link

rafales commented Aug 1, 2014

Wouldn't this decrease scrolling performance? It still could be implemented as a separate module.

@benbro
Copy link
Contributor Author

benbro commented Aug 1, 2014

I think browsers are smart enough to not let it effect scrolling performance.
Usually, you have some threshold and you only fire the event after it
or you fire the event only after the user finished scrolling.

@benbro
Copy link
Contributor Author

benbro commented Aug 9, 2014

I think that the event and method should be in core.
It could be nice to have a module that automatically sync the scroll position between participants.
In case the editor height is different, it could make sure the top row is the same.

@jhchen
Copy link
Member

jhchen commented Aug 11, 2014

Scroll events are not implemented in Quill but might be reasonable to add. What would a desirable API look like?

I believe natively, browsers will fire the scroll event even when scrolled 1px though there may be some maximum fire rate. This probably is not a big issue unless users attach a listener that does a lot of computation so Quill might want to limit this rate even more. Alternatively this event could be asynchronous and would not re-fire until all listeners callbacks have returned.

Also what information should the scroll event provide?

@benbro
Copy link
Contributor Author

benbro commented Aug 12, 2014

How about this API?

// event
editor.on('scroll', function(scrollLeft, scrollTop) {
});

//setters
editor.scrollTo(x, y);
editor.scrollTop(y);
editor.scrollLeft(x);

//getters
editor.scrollTop();
editor.scrollLeft();

Simplest option is to just fire the event (like CodeMirror does) and leave throttling to the user.
https://github.com/marijnh/CodeMirror/blob/master/lib/codemirror.js#L2541

It's possible to throttle the scroll event. When the browser fires the event, we could start a timeout. If another scroll event is fired, cancel the timeout and create a new timeout. When the timeout fires, dispatch the editor's scroll event.
We could also have two events.
scrolldone - fires only after the user done scrolling (or paused long enough).
scroll - fires every X*timeout.

@benbro
Copy link
Contributor Author

benbro commented Aug 13, 2014

A nice idea to debounce the scroll event with requestAnimationFrame
http://www.html5rocks.com/en/tutorials/speed/animations/

@jhchen
Copy link
Member

jhchen commented Aug 14, 2014

Wish there was a way in JS to return multiple values to avoid two APIs for top and left. Is there a feature you are trying to build that would use these APIs?

@benbro
Copy link
Contributor Author

benbro commented Aug 14, 2014

I actually just need scrollTop.
I'm trying to sync the scroll position for two users.

@jhchen
Copy link
Member

jhchen commented Nov 7, 2014

With v0.19 Quill no longer hides the editor in an iframe so using DOM events and apis directly should be sufficient for this.

@jhchen jhchen closed this as completed Nov 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants