harc / moonchild

A toolkit for experimenting with new kinds of programming interfaces.
190 stars 15 forks source link

Markdown rendering plugin makes page scroll to top on rerender #9

Closed Azeirah closed 9 years ago

Azeirah commented 9 years ago

If you have some rendered markdown, and you're scrolled down, typing a character scrolls the page to the top. Not good!

pdubroy commented 9 years ago

Hmmm, not sure what's going on there.

Azeirah commented 9 years ago

I've found what line triggers this behavior var widget = codeMirror.addLineWidget(loc.line, el, { above: true }); in collectTests.js

I was wrong, it wasn't a markdown issue but a collectTests plugin issue.

addLineWidget makes codemirror scroll to the newly inserted widget for some reason, I'm not really sure why. It's not documented addLineWidget behavior, nor is it a togglable option.

Edit: Even tried updating codemirror to 5.4.0 (most recent version), and it still persists, probably not a bug, but just a hidden feature or something :(

Edit2: Inserting a line widget in the widget.html demo

var d = document.createElement("div");
d.innerHTML = "hello there";
editor.addLineWidget(25, d, {coverGutter: false});

Alright, I found out what function in codemirror is responsible, and it seems it's being called far too many times.

When a widget gets cleared in CodeMirror, a function named addToScrollPos(...) gets called, which pretty much does editor.scroll.height -= removedWidget.height, which makes sure the viewport of the editor stays the same after the removal of a widget.

However, it seems that it gets called far too often when a widget is not visible in the current viewport, this happens in Moonchild only, not in CodeMirror.

Adding a console.log call to LineWidget.prototype.clear gets me this: first the widget gets cleared once, then twice, three times, four times...

This is likely a case of not cleaning up widgets properly. Fixing this bug will result in preeettyy damn big performance improvements for editing a file for any longer than 2 minutes.