Setting editor font size should set Themes font size

[lkcampbell]

OS: Mac OSX Mavericks

Brackets Version: sprint 36 experimental build 0.36.0-11506 (release 942505c3a)

Extension Version: 0.6.0

Repro Steps:

1. Launch Brackets and select Increase Font Size a few times.
2. Close and reopen browser.

Expected Results: Font size should stay the same.

Observed Results: Font size changes to match the settings in Themes setting.

[lkcampbell]

There are other font sizing bugs caused by this as well. They all come down to the same problem. The Themes font size gets out of sync with the editor font size. They both need to be the same value regardless of which one changes.

[MiguelCastillo]

Yeah I'm just realizing that brackets is doing font sizing... I will change thing around so that brackets font sizing is in sync with themes. Thanks for the heads up dude

[lkcampbell]

I think the editor font size is a relative adjustment. For example: 0 or +3 or -7. It takes the default font size (is it 12px? not sure) and adds the size adjustment to get the final font size. Theme preference font size, I assume changes the default font size? Then the combination of those two values make the final value. It's very confusing.

Question: Why is the font size one of the preferences for Themes?

I can understand people wanting to change the font face but why do they need to change the font size when they can do that in editor?

Maybe the easiest solution is to remove font size preferences for Themes.

[MiguelCastillo]

Is there a way to specify a particular font size? Or just the increase/decrease functionality?

[lkcampbell]

Just the increase/decrease functionality.

All the code is in /src/view/ViewCommandHandlers.js. It uses a preference called fontSizeAdjustment, which is the adjustment of the default font size.

Again, though, I have to ask, why is font size a Themes preference when functionality for adjusting font size is already available in the editor? This question needs to be answered first before you spend time adding redundant functionality.

[lkcampbell]

Also, you might want to look at this pull request:

https://github.com/adobe/brackets/pull/7185/files

It looks like they will be specifying a particular font size soon.

[MiguelCastillo]

Why specify a font size in Themes? Because increase/decrease doesn't allow you to.

I was just chatting with @TomMalbran and they currently don't have the ability to set a specific font size. He suggested I do a pull request. This way I can have Themes go through Brackets settings to set the font size and keep brackets and Theme in sync.

[lkcampbell]

Oh yeah, heh, you are already in the comments. I failed to scroll down and see you before I sent that URL :). Sounds like you are on top of it.

[bradgearon]

From what I found in the brackets code - view/viewCommandsHandler stores a fontSizeAdjustment (relative integral value) in PreferencesManager viewState (user) property "fontSizeAdjustment". Then it sticks a #codemirror-dynamic-fonts css element in the head, updates the font size via that element. It does seem like https://github.com/MiguelCastillo/Brackets-Themes/pull/46 will fix this, but I remember reading something about preference storage being silly on OSX. I'm going to check (its just a json file) regardless.

[TomMalbran]

@bradgearon That is partially wrong in the current master, since now we store the font size css value, which includes both the number and the unit.

@MiguelCastillo That PR landed in master if you want to create a PR to either allow setting any possible font-size or expose _setSizeAndRestoreScroll as a public API. In both cases it might be good to move the view state set to _setSizeAndRestoreScroll.

[bradgearon]

In current master, yes I see the fontSizeStyle being set with px. Sorry, was actually looking at that earlier today - but was only talking about the fontSizeAdjustment that was previously used (as an offset). Thank you for pointing that out though, cause what I'm doing I'll add something for that for when master == sprint 38 or whatnot.

[MiguelCastillo]

Fixed by @bradgearon with https://github.com/MiguelCastillo/Brackets-Themes/pull/46

[lkcampbell]

This isn't fixed for me at all. Reopening.

I start out with font size 12px. I open the Themes settings and change it to 24px. I save the settings. The dialog box closes and the font size in my editor does not change. It still looks like it is 12px.

Then I close Brackets and start it up again and now the font is huge and the Themes settings is at 36px.

Can you guys repro these problems?

[MiguelCastillo]

@lkcampbell cant reproduce... I have tested this one quite a bit and it works pretty well. Really weird. I assume you are running latest.

[lkcampbell]

Yeah, I am running version 0.7.4. Brackets Sprint 37.

[lkcampbell]

Maybe it's the changes in the preference system in 36 and 37. What version of Brackets are you running @MiguelCastillo?

[MiguelCastillo]

@lkcampbell I am running 37 and 38 (master) with no problems

[lkcampbell]

No console errors either. It simply does not work. Let me know what I can do to help you guys troubleshoot the problem. And if you haven't tested it on Sprint 37, you might want to try it.

[MiguelCastillo]

@lkcampbell are you running from latest Themes in github?

[lkcampbell]

Yeah, I tried both. Installed from Extension Manager and installed from github url. They both act the same.

[MiguelCastillo]

@lkcampbell I reproduced it! I am viewing an HTML file when I randomly get this issue... Let me do something about this issue.

[lkcampbell]

@MiguelCastillo, it is strange that you can only randomly recreate the problem. I have it constantly. But, yeah, at any rate, I will be interested to see what you come up with. Let me know if there is anything else I can do as well to help you.

[MiguelCastillo]

@lkcampbell Can you give this https://github.com/MiguelCastillo/Brackets-Themes/archive/sprint38.zip a quick try? Install it via the extension manager as a URL. Let me know if that helps at all.

[MiguelCastillo]

@lkcampbell Nevermind, I tagged the package.json to require 38... So, this wont work for ya.

[lkcampbell]

@MiguelCastillo, I think this has something to with recent preference and font size changes. I've got old local storage preferences mixing with new state.json preferences. I've got Sprint 37 and Sprint 38 dev running, probably mixing the old and new font size formats. If it is working for you guys, that's fine. I think I will close this for now. If the problems crop up again after you have converted over to the new preferences system, I will consider reopening the issue. There are just too many variables on my system right now.

[MiguelCastillo]

Lance, I had taken a more thorough approach to test this issue. It just seems so strange that themes has had some many issue lately and I am unable to reproduce them :/ So wiped my brackets installation and with a fresh copy installed I could reproduce this issue very easily. So, I have been able to pin point the problem and fixed it. https://github.com/MiguelCastillo/Brackets-Themes/commit/aa8a16dfd7559009ca760bc09fab049379b21304

Do you think you can test this from github master branch? Thanks buddy.

[lkcampbell]

@MiguelCastillo, yeah, that appears to have fixed it, thanks!

I think there has just been some code churn in the core around two critical pieces the affect this functionality: font size and preferences. That's probably why it's been hard to reproduce these problems.