fix undo and redo scroll

[alissa-tung]

I think the changes are closer to the native CodeMirror 6 undo/redo scroll behavior, which is first do undo/redo, then move view to the line of cursor by the nearest match.

this PR fixes #31

[alissa-tung]

Hello @krassowski and @dmonad , if you had time could you please help me testing this PR? I had test it on the yjs demos repo, and it looks good to me. I am new to web development, I am not familiar of the workflow and code. Any suggestions would be great help!

[dmonad]

Thanks for the PR @alissa-tung !

If I understand correctly, this PR adds functionality to scroll the cursor into view when the user hits undo / redo.

Can you confirm that this is the default CodeMirror behavior?

When the undomanager pops a stack item (i.e. the user performed undo() or redo()), then we already update the selection. This happens here:

y-codemirror.next/src/y-undomanager.js

Line 88 in faffa7b

view.dispatch(view.state.update({ selection }))

If we want to scroll into view, we should perform that action there, without calling dispatch again.

I'd accept an updated PR if you can confirm that this emulates the default CodeMirror behavior. Otherwise, we should provide an API so that you can add this custom behavior.

[krassowski]

If I understand correctly, this PR adds functionality to scroll the cursor into view when the user hits undo / redo.

Can you confirm that this is the default CodeMirror behavior?

The default CodeMirror behaviour is to scroll into view if needed, see recording in #31. To be frank, it is the default behaviour of any other editor I know.

[alissa-tung]

Thanks for the PR @alissa-tung !

If I understand correctly, this PR adds functionality to scroll the cursor into view when the user hits undo / redo.

Can you confirm that this is the default CodeMirror behavior?

When the undomanager pops a stack item (i.e. the user performed undo() or redo()), then we already update the selection. This happens here:

y-codemirror.next/src/y-undomanager.js

Line 88 in faffa7b

view.dispatch(view.state.update({ selection }))

If we want to scroll into view, we should perform that action there, without calling dispatch again.

I'd accept an updated PR if you can confirm that this emulates the default CodeMirror behavior. Otherwise, we should provide an API so that you can add this custom behavior.

Thank you, I am going to do the following:

• test whether if the behavior is native with screen recording
• try to move the effects to where the selection change to be dispatched

[alissa-tung]

The native CodeMirror 6 has the following behavior when undo/redo:

• If the selection is in the current view point, do nothing
• If the selection is above the current view point, make the selection start line to be the first line of view
• If the selection is below the current view point, make the selection line to be the last line of view

I think this is controlled by scrollIntoView using nearest, and this is what we will test against for

[alissa-tung]

2024-06-18.09-04-19.mp4

2024-06-18.08-57-27.mp4

[alissa-tung]

Hello, @dmonad , I had update the code to make dispatch update selection and scroll in view the same transaction. And I have uploaded two videos to test again the behavior. Feel free to request any changes to this.

[dmonad]

Thank you @alissa-tung !

I'm going to make a new release now.