search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] CodeMirror calculates incorrectly the min-height when the font size is not the default one #2933

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by TomMalbran Wednesday Mar 13, 2013 at 02:52 GMT Originally opened as https://github.com/adobe/brackets/issues/3115

While doing #3068 i noticed that every line not rendered (not in the dom) was calculated always using the default font size (15px) even when adjusting the font size. You can test this in different ways:

$(".CodeMirror-lines",
        EditorManager.getCurrentFullEditor().getRootElement()).parent().position().top);

And the first rendered line with EditorManager.getCurrentFullEditor()._codeMirror.getViewport().from and then divide the top with the lines.

This is causing some the next visible problems:

core-ai-bot commented 3 years ago

Comment by njx Monday Mar 18, 2013 at 18:22 GMT

@TomMalbran -- could you take a stab at reproducing this in standalone CodeMirror (outside of Brackets)? I think the only thing CM is supposed to require after changing the font size is a refresh(), but we've noticed when looking at other previous issues that various bits of cached line height state don't all seem to get thrown out on a refresh. If you could reproduce this in CM and file it in the main CM repo, that would be very helpful. Thanks!

core-ai-bot commented 3 years ago

Comment by njx Monday Mar 18, 2013 at 18:24 GMT

Also, could you characterize how bad the user-visible aspects of the bug are? If it's just that the cursor doesn't always end up visible after a page down, that's not terrible. But if you can't scroll to see the whole document, for example, that would be pretty bad.

core-ai-bot commented 3 years ago

Comment by TomMalbran Sunday Mar 24, 2013 at 02:06 GMT

I tested in a standalone CodeMirror and understood what is the problem: refresh() doesn't updates the min-height of the container, so things like PageUp from the bottom of the editor, or sometimes scrolling doesn't work as intended and keep modifying the min-height until it can get to the right size and fix everything.

This is what I did:

  1. Went to http://codemirror.net/demo/theme.html.
  2. Copied the whole content of codemirror.js and pasted it in the editor.
  3. Opened the developer tools, selected the .CodeMirror div and checked the font size.
  4. Selected the .CodeMirror-sizer div and checked the min-height.
  5. Right now the min-height / amount of lines is similar to the font size.
  6. Went back to the .CodeMirror div and increased the font size to 25px.
  7. Used the console to refresh the editor with editor.refresh()
  8. Checked in .CodeMirror-sizer div that the min-height didn't changed.
  9. With the cursor at the last line, press PageUp multiple times and check how the min-height changes, until it gets to the start of the document and it has the right height.
core-ai-bot commented 3 years ago

Comment by peterflynn Friday Mar 07, 2014 at 00:48 GMT

It looks like the marijnh/CodeMirror#2157 may have fixed this a couple months back. @TomMalbran are you still seeing this? Comments in #7093 would imply you still are..?

core-ai-bot commented 3 years ago

Comment by TomMalbran Friday Mar 07, 2014 at 00:54 GMT

I haven't checked it in the code, but there is still something wrong. Open a file with over 300 lines and place the scroll at the top, increase the font size a bit and then scroll fast to the end of the file. You should see some issues with the scroll thumb which gets smaller as it reaches the end of the file.

core-ai-bot commented 3 years ago

Comment by TomMalbran Friday Mar 07, 2014 at 01:32 GMT

It does seem to be fixed. I was testing in Sprint 36 before and not in master. I will test a bit more and close if it is fixed.

core-ai-bot commented 3 years ago

Comment by TomMalbran Friday Mar 07, 2014 at 02:54 GMT

Cool, closing as fixed. Will submit a PR soon to remove the hack done to avoid this bug.