Adding interfaces to change font size, font family, and line height. #8305
Conversation
…. The float value is treated as ems internally still
…closing documents and changing font settings
|
@dangoor @TomMalbran Hey guys, I wanted to follow up on this PR. So, I have been using line height and I really like that it can be adjusted. I myself like it at 1.4... However, it seems like I am chasing after small adjustments when I start setting spans to inline-block. For example, I am using interactive linter and I set the border-bottom to underline problems in the code. That's causing visual artifacts when moving the cursor through some of the content that's been underlined because the border-bottom is now adding to the height of items like curly braces. So, happy path works well. But we might be causing more trouble by adding the ability to adjust line height as it seems a bit fragile to me. Maybe removing it for now might not be a bad idea. Thoughts? |
|
@MiguelCastillo Yes, I noticed the bottom border issue too by having the highlight selection active. Maybe adding |
|
@TomMalbran I had played with box sizing but it didn't help much. I could try box-shadow, that's not a bad idea. But that's basically my point. We can make it work, but it seems like a we might cause extension authors some headaches. How about we leave it in for now, and use it a bit... And let's adjust as needed. |
…licts where line height should not be applied.
|
@TomMalbran So, what I ended up doing with the border-bottom that interactive linter adds is that I added a negative margin-bottom... This fixed the issue where the whole line was pushing down the rest of the document. Is not a bad way to work with this particular scenario... So, maybe the answer is to just write about this issue so that people are not left to resolving the same issue over and over. |
|
@TomMalbran Just to make sure we know the exact use case. The issue is when matching braces get a border-bottom. In this case, interactive linter was adding the border-bottom. |
| .CodeMirror-matchingtag, | ||
| .CodeMirror-matchingbracket { | ||
| display: inline-block; | ||
| } |
There was a problem hiding this comment.
Why is this required now when it wasn't before? I'm unsure what about making font size configurable would change something about this...
There was a problem hiding this comment.
So, when line height is changed some items like those don't properly scale because they are spans. To quickly test this behavior, try increasing the line height in the current themes to something like 1.8 and place the cursor on a matching bracket. Or select a word and press CMD+F to find matching search.
Resolved conflict in code mirror overrides and in ViewCommandHandler
|
So, there are a couple of changes that I am waiting to be merged in for fixes to themes... To reduce the amount of friction merging themes and this PR, I am just going to wait for the lingering issues with themes to be cleaned up, and then I will merge and update this PR. |
| .code-cursor(); | ||
| } | ||
|
|
||
| <<<<<<< HEAD |
There was a problem hiding this comment.
A merge conflict made it into this patch
|
I can try to work on this one this afternoon. We need to get this in so that we can fix two other bugs that block the release. |
|
@dangoor, I was waiting for the PRs you just merged to get this one finished up. I will clean up this PR in the evening so that we can get this one in. |
|
@MiguelCastillo OK, I'll likely focus on the dark core UI first. I'll let you know if I do end up hacking on this at all during the afternoon. |
|
@dangoor I am going to start another branch/PR. It will be much easier/cleaner for me... |
|
@MiguelCastillo Whatever works for you! |
|
Closing this one in favor of #8492 |
#7800