Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Set _fontSizePrefsLoaded to true before calling _adjustFontSize. #7165

Closed
wants to merge 1 commit into from

Conversation

RaymondLim
Copy link
Contributor

This change fixes the issue #7093 since the flag has to be true in order to set the scroll position to the right location.

@TomMalbran Can you review this since you're the one who works on this area a lot?

Note that this specific issue is also introduced by recent CodeMirror update. If I use the CodeMirrror version from sprint-36, then I don't see the issue even if I'm running in the master b64a2a1 with the new view state settings implementation.

…t the scroll position to the right location.
@TomMalbran
Copy link
Contributor

The idea behind that flag is that the last time you save the scroll position it is saved using the new font size. When the font size is 15px, line 100 is roughly around 165px, but is at 115px when the font size is 10px. So I change the font size to 15px, and then scroll to line 100 and restart Brackets. The scroll was saved at 165px, so when I restore the scroll position, I know I need to use the saved scroll and not change it. If instead I readjusted the scroll, it would be way off.

I tried your changes without the zoom and the scroll is way off after restarting Brackets.

@redmunds
Copy link
Contributor

This is much better for recipe #7093 . Using ProjectManager.js (2255 lines):

  • in master branch, shutting down and restarting Brackets re-opens file with line 1880 at bottom
  • in this branch, shutting down and restarting Brackets re-opens file with line 2246 at bottom

So, this seems to fix the second problem in this comment, but not the first problem.

@RaymondLim
Copy link
Contributor Author

@redmunds The first problem in your comment has to do with Word Wrap and I've already logged an issue #7168 for that.

@redmunds
Copy link
Contributor

@RaymondLim I have Word Wrap off, so I don't think that issue is related to what I am seeing.

@TomMalbran
Copy link
Contributor

The real issue here is that on the initial load the scroll position is restored before restoring the font size. So if the scroll was at the end of a file before restarting, when restarting, on the original font-size, the saved scroll is higher than the length of the document, so it doesn't scroll to the saved one, and when later the font is increased, the scroll is in the wrong position.

The same is what happens with issue #7168. But there is another issue with wordwrap on, when you are just trying to increase the font size. Once a line lower than the visible one wraps into 2 lines, the restored scroll will be off by 1 line.

@RaymondLim
Copy link
Contributor Author

Closing as this is not the correct fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants