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

Revealing collapsed "advanced editor" cells does not reveal the whole cell #15

Closed
Pfiver opened this issue Dec 8, 2020 · 6 comments
Closed

Comments

@Pfiver
Copy link

Pfiver commented Dec 8, 2020

Would be nice if it worked. My expectation is that the editor is initially expanded vertically to accomodate the whole cell contents even if in "advanced editor" mode.

https://youtu.be/aoQ5DBKBJvk

@gzuidhof
Copy link
Owner

gzuidhof commented Dec 8, 2020

This is indeed an issue, thank you for the detailed report.

Some implementation details & thoughts:

  • Right now Monaco (the advanced editor) listens to resizes and any changes to its model (due to typing / folding).
  • We ask Monaco what size its contents are and resize the editor's element height to that. But apparently it can't estimate it well if the HTML element is hidden.
  • I used to calculate the height the editor should be myself with a rather magic formula (at the time Monaco wasn't able to self-estimate this):
      const lineHeight = editor.getOption(monaco.editor.EditorOption.lineHeight)
      const lineCount = editor.getModel()?.getLineCount() || 1
      const height = editor.getTopForLineNumber(lineCount + 1) + lineHeight
    
    This actually worked pretty well, it just didn't take into account the bottom scroll bar that is visible if your content is wide enough, leading to a small vertical scrollbar too (which isn't the end of the world).
  • There is currently no event that fires for the cell getting collapsed, and there is no obvious DOM event I can listen to for that either.

I think what might be a good fix for now is checking if the Monaco self-calculated height makes sense for the amount of lines, and otherwise revert to the DIY magic calculation above. I'll give that a shot.

@gzuidhof
Copy link
Owner

gzuidhof commented Dec 9, 2020

A small update, it looks like it does actually estimate its size correctly - it just doesn't layout correctly when (previously) invisible, so the fix will have to be different. Maybe it is inevitable an event has to be added for this..

@gzuidhof
Copy link
Owner

gzuidhof commented Dec 9, 2020

I think I managed to fix it, but it's hard to tell (there may be edge cases where this doesn't work). I pushed version 0.7.6 with the hopeful fix.

I'm not sure if you are using this project standalone or on Starboard.gg, if you're using the latter you can add ?runtime=0.7.6 to the URL to force it to use the new version.

EDIT: Looks like sometimes it still takes on a height of only two lines, having a hard time consistently reproducing that locally.

@Pfiver
Copy link
Author

Pfiver commented Dec 9, 2020

Nice. That was quick. But sorry: I forgot to mention that I am using Firefox. This seems to be a bit of burdensome beast. :-/

I am not a fan of Google though and am thus trying to avoid them, including their browser.

So with Firefox 83.0 on a Macbook BigSur 11.0.1, when I surf to https://starboard.gg/?runtime=0.7.6, the problem is still there - right on the first cell. I just need to

  1. enable "advanced editor" and "auto-hide"
  2. unfocus the cell
  3. reveal

The behavior is still as shown in the video.

@Pfiver
Copy link
Author

Pfiver commented Dec 9, 2020

My instinctive first reflex would prompt me to shoot "visibility: hidden" instead of "display: none" at this, but there might be reasons why it wouldn't be a good idea and/or not even work?

@gzuidhof
Copy link
Owner

Been a while for this issue. The underlying behavior wasn't changed and I doubt we can get Monaco to play nicely with resizing and collapsing :(. I think it is inevitable that at some point I will drop support for the Monaco (i.e. advanced editor) entirely.

In the latest version I introduced hiding the top or bottom half independently by pressing the "gutter" line on the left. That has no weird "click-to-reveal" behavior. I think that will do the job for 90% of code you would want to hide like in the video, so I'm closing the issue for now.

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

No branches or pull requests

2 participants