Scroll position changes when other clients are editing

[tommoor]

To Reproduce

• Create a doc long enough to scroll
• In one client place your cursor at the bottom of the screen and scroll to the top (common behavior when reading)
• In another client type at the top of the doc
• The scroll position in the first client will jump to the bottom

Screencast

From the y-prosemirror demo site:

I tracked it down to this line – could you enlighten in which circumstances this is supposed to trigger? It feels like it would be fine if remote edits never caused the local scroll state to change.

y-prosemirror/src/plugins/sync-plugin.js

Lines 344 to 346 in 52116dd

	if (this.beforeTransactionSelection !== null && this._isLocalCursorInView()) {
	tr.scrollIntoView()
	}

[dmonad]

I can't reproduce this issue in Chrome Version 87.0.4280.66 (Official Build) (64-bit).

The code you mentioned scrolls the window to the current editing position when a remote change happens IF the cursor is currently in view. This prevents weird scrolling behavior when a user is adding/removing a lot of content at the beginning of the document.

[tommoor]

In which case the "if cursor is in view" logic must not be correct 🤔

[dmonad]

Apparantly. But your comment doesn't help me to fix the problem. As said, I can't reproduce the issue using the description you gave me.

[tommoor]

I think the key is to scroll up without clicking anywhere outside of the editor in the first window. Probably easiest to reproduce with two machines as this is how it was first found in testing.

Some more debugging, when it reachesisDomSelectionInView the documentElement is actually just the html tag, so seems like it would be safe to guard that this isn't the case? It explains why the calculation fails and causes scroll to jump to the top.

[dmonad]

That is intended. Maybe the check fails because documentElement.clientHeight is the full height of the website? That is why it is important to specify which browser / which OS you are using...

In your environment, is that different from window.innerHeight? innerHeight might be more appropriate to use here.

[tommoor]

That is why it is important to specify which browser / which OS you are using...

Totally agreed, that information wasn't requested in the template previously though and I thought it would be relatively clear in the gif that it's Chrome on macOS.

[dmonad]

This is fixed in y-prosemirror@1.0.5