search

brownplt / code.pyret.org

Website for serving Pyret to folks.
Other
24 stars 45 forks source link

Fix bug with editor/repl height #386

Closed asolove closed 3 years ago

asolove commented 3 years ago

This addresses the issue raised in #384.

The minimal change here was to use offsetHeight instead of scrollHeight so we no longer count the height of the absolutely-positioned menu.

A major question I have is in what case this code is needed at all. Seems like the same goal could likely be met in CSS and I don't understand in what case the toolbar is larger than the CSS size and requires running this code dynamically on all scroll/resize events. If someone can explain that, I'd be happy to take a crack at a more-complete solution.

schanzer commented 3 years ago

Oh dear god yes!!

jpolitz commented 3 years ago

@ds26gte do you have any more context/memory of the issue or how to reproduce why we needed this?

https://github.com/brownplt/code.pyret.org/commit/a365632a56f1659d1e7bb88624f08c67647fb2e8

blerner commented 3 years ago

I vaguely recall that CodeMirror tends to not to like to extend to the full vertical height of its container, and occasionally Shenanigans must be employed to convince it to stay the height of the window -- I don't think a pure-CSS solution sufficed, and the CM state would become stale. If I'm recalling that correctly, then this code makes sense and then flipping scrollHeight to offsetHeight makes sense as well...

jpolitz commented 3 years ago

Merging because it's obviously a fix, so no reason to wait.