[Themes] Default line spacing is larger after Themes landed

[peterflynn]

1. Start Brackets with default settings (no font size or other theme changes)
2. Open a file and take a screenshot
3. Compare to the same file before Themes landed (e.g. ce7e691)

Result: line spacing has gotten looser. Distance between baselines is now 16px; previously was 15px. (On a MBP-sized screen, this translates into 3 fewer lines of code visible).

[MiguelCastillo]

@peterflynn I bumped it to 1.4. There didn't seem to be a value. Is there a value that we should be using?

[peterflynn]

On earlier master, in brackets_themes_default.less the .code-font() rule specifies line-height: 1.25 and font-size: 12px...

[peterflynn]

And actually it's still there on master now: https://github.com/adobe/brackets/blob/master/src/styles/brackets_theme_default.less#L148. So I guess that's getting overridden somewhere else?

[MiguelCastillo]

@peterflynn Yeah it does get overridden. We are actually moving those settings to the new fonts API #8305

So, I will make sure to have 1.25 in there. That value seems pretty low...

If I make another pull request right now to change the lineheight, there might be conflict with #8382

Want me to still make a pull request to change this to 1.25 or want me to wait?

[MiguelCastillo]

@dangoor @larz0 Thoughts on just using line height of 1.25?

[peterflynn]

As long as we land a fix before 0.42 ships, I think you can wait until the conflicting PR lands.

Although it looks like #8305 codifies the "1.4" multiplier in a constant, so maybe we can just change the value directly in that PR, instead of a separate follow-on later?

I assume the "1.4" is also present elsewhere though, since I don't see any "1.4"s being deleted in that PR yet that's already the line-height... so we might also want to clean up any redundant CSS values if #8305 means they'll always be overridden by values coming from prefs.

[MiguelCastillo]

@peterflynn Removing it from the CSS is a good idea. I will make sure to take of that in the fonts PR.

[peterflynn]

I do think we should stick with 1.25 though -- it's what Brackets has used for years.

[MiguelCastillo]

@peterflynn Perfect. I will just make a PR right now. It's real trivial change. I will make sure to remove the CSS cleanup once we transition to the fonts API

[peterflynn]

Great -- thanks!

[MiguelCastillo]

@peterflynn You bet! :)

[MiguelCastillo]

Closing this one... Fixed here #8400

[MiguelCastillo]

Or @peterflynn you can probably close this one :)

[peterflynn]

Confirmed fixed. But note that #8305 will regress this bug if merged as-is -- we need to remember to update the line-height value there too.

[MiguelCastillo]

@peterflynn Yup, that's definitely in my honey todo list :D