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

Prefer adjusting cursors over recreating them #2

Closed
wants to merge 3 commits into from

Conversation

Monkatraz
Copy link
Contributor

@Monkatraz Monkatraz commented Jan 9, 2021

This makes it so that the drawSelection plugin will 'adjust' old cursors in the cursorLayer rather than simply recreating them.

Why? Well, to be honest, it's purely for gratuitous visual flair.

let theme = { '$cursor': { transition: 'left 0.1s ease-out, top 0.1s ease-out' } }

image
(it's hard to see in a gif, but the movement is buttery smooth.)

There is a potential argument about this being more efficient, as most of the time only a single cursor will need to be updated, and when it is updated only its style properties are changed. I doubt that this gives a practical improvement in performance regardless.

I have "tested" this, but I'm pretty unfamiliar with the innards of CodeMirror and the maintainer(s) probably needs to check if this change affects something that I wouldn't know about.

@marijnh
Copy link
Member

marijnh commented Jan 9, 2021

Won't the animation effect go wrong when cursors are added/removed, as the offsets of "corresponding" cursors won't match across the transaction? Say you have a cursor and then add a second one above it. The old cursor will move to the new position, though conceptually it stays in place.

@Monkatraz
Copy link
Contributor Author

This is exactly what happens - and actually it kind of looks interesting. I don't mind it - but it probably shouldn't be an effect. I'll make a commit to prevent that behavior.

@Monkatraz
Copy link
Contributor Author

Monkatraz commented Jan 9, 2021

There - now whenever a cursor is added or removed, the plugin will instead redraw all cursors. That way, there doesn't need to be any memory of what cursors are new or whatever. Cursors will only 'adjust' when they move.

@marijnh
Copy link
Member

marijnh commented Jan 11, 2021

That sounds like a reasonable solution. But now it looks like the else in the 'same cursor count' case is superfluous.

Also, the library targets IE11 which doesn't have Array.from. But that call seems unneccesary anyway (the children object already has a length and can be indexed, so it doesn't need to be converted to an array)

@Monkatraz
Copy link
Contributor Author

Oops, fixed.

marijnh pushed a commit that referenced this pull request Jan 11, 2021
FIX: The `drawSelection` extension will now reuse cursor DOM nodes
when the number of cursors stays the same, allowing some degree of
cursor transition animation.

Issue #2
marijnh added a commit that referenced this pull request Jan 11, 2021
@marijnh
Copy link
Member

marijnh commented Jan 11, 2021

Thanks for the adjustments. Merged as 66c7a28

@marijnh marijnh closed this Jan 11, 2021
x-three added a commit to x-three/codemirror-view that referenced this pull request Jan 17, 2024
x-three added a commit to x-three/codemirror-view that referenced this pull request Jan 17, 2024
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

Successfully merging this pull request may close these issues.

2 participants