[FIX] Remove fixed height for editor #4915
Conversation
We used to set fixed pixel heights for the (CodeMirror) editor; one of the things we would do is measure the height of the container during editor initialization, and then force the editor to have that height. On the slides page, where we would load a lot of pages in `<iframe>`s that would start out hidden, the calculated height would sometimes be measured as `0px`, which would then be copied onto the editor. The end result would be that the editor would appear empty on some pages. Instead, in this PR: remove all height calculations. We will set a height on the entire editor grid (containing the editor and output box, and navigation buttons), and add the CSS declarations necessary to make the editor automatically resize to take up available remaining space, after the fixed elements are taken away (like the buttons at the bottom, the error and success popups, the "handed in" warning box, etc). This makes it no longer necessary to recalculate the height of the editor every time one of those popups is shown or hidden.
There was a problem hiding this comment.
This works great! When I am in the /hedy page, the editor get resized properly. Also, now the embedded editor adjusts nicely to the iframe settings. There are some things that I wonder if we might do a bit better.
In the slides when the code overflows the width, 2 scroll-bars are shown.
Now the gutter looks a bit different than the background, I wonder if maybe we can set it to be the same color as the background so the editor doesn't look incomplete?
Sort of like Vs Code does
|
Ah good call. Let me fiddle some more then. |
There was a problem hiding this comment.
Switching to dev mode works.
I also noticed the issue that Jesus pointed out. In addition, we have the following:
The iframe is not taking the full height. It's more noticable here:
and
I think both can be solved in a separate PR? Shall we create an issue for this?
|
Yeah, the layout of the slides itself is out of scope for this PR. Good attention to detail! We can ask Felienne how she wants it to look. |
|
@jpelay @hasan-sh I've changed the method to limit the height of the editor; instead of having a growing editor inside a container that we limit using Tailwind, I've added some styles on the Added a new container tag so this readonly editors used for sample code highlighting (#4924) can still grow with the content. I don't have an easy way to check how this looks on Windows. Can one of you check if this fixes the scrollbar issue? |
Great!
I use and it works on linux so I don't have Windows either. Hopefully Jesus does! Otherwise will figure something out! |
|
Linux is also fine. Just another platform than Mac -- Mac really de-emphasizes the scroll bars by default which makes it hard to see when they're wrong. |
|
Great! If all is well you may approve this @hasan-sh! |
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
|
Works great!!! Just tested this on windows and it works properly there too on Chrome and Firefox |
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Fixes #4895
We used to set fixed pixel heights for the (CodeMirror) editor; one of the things we would do is measure the height of the container during editor initialization, and then force the editor to have that height.
On the slides page, where we would load a lot of pages in
<iframe>s that would start out hidden, the calculated height would sometimes be measured as0px, which would then be copied onto the editor. The end result would be that the editor would appear empty on some pages.Instead, in this PR: remove all height calculations. We will set a height on the grid cells that contain the editor and output box, and add the CSS declarations necessary to make the actual editor automatically resize to take up available remaining space, after any fixed elements in the same cell are taken away (like the error and success popups, the "handed in" warning box, etc).
This makes it no longer necessary to recalculate the height of the editor every time one of those popups is shown or hidden.
For the "developer mode" toggle, we introduce a new attribute:
data-devmodeheight="22rem,36rem", and the toggle will switch between the two sizes in the attribute. This avoids having to duplicate element sizes between HTML and JavaScript.How to test
Verify this solves #4895 by examining the slides of level 1 and seeing that all code blocks are always shown.