Skip to content

Commit

Permalink
vscode-editor-font-size should have units
Browse files Browse the repository at this point in the history 
Fixes #86722
  • Loading branch information
@mjbvz
mjbvz committed Dec 11, 2019
1 parent 59ee3ce commit 7e2d796
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/webview/common/themeing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class WebviewThemeDataProvider extends Disposable {
'vscode-font-size': '13px',
'vscode-editor-font-family': editorFontFamily,
'vscode-editor-font-weight': editorFontWeight,
'vscode-editor-font-size': editorFontSize,
'vscode-editor-font-size': editorFontSize + 'px',
...exportedColors
};

Expand Down

4 comments on commit 7e2d796

@PEZ
Copy link
Contributor

@PEZ PEZ commented on 7e2d796 Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break some existing extensions, right? I know my extension uses a workaround that is not compatible. I'll go fix it so that it can handle this. But for extensions where the authors are not aware... I do agree it is clean this way, and is mostly interested in learning about the reasoning for these kind of considerations.

@mjbvz
Copy link
Collaborator Author

@mjbvz mjbvz commented on 7e2d796 Dec 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are correct that this does potentially break some extension. I ultimately decided that making this fix is reasonable because:

  • vscode-editor-font-size is rarely used. I checked the top set of extensions and only see a handful of users
  • This was clearly a bug since vscode-font-size does have px units and working around it was not easy
  • The impact of any potential breakage should generally be quite low (just wrong font sizes or odd layouts)

We are also releasing this fix early in the VS Code 1.42 ship cycle so that extensions can report if it does break anything and make fixes

@RandomFractals
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense for consistency and considering your default is 13px :)

@jvgreenaway
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has broken an extension I'm working on. I can fix it but will cause users on an old version to get a broken experience (huge fonts and layout).

This trick was to consume this var as calc(var(--vscode-editor-font-size) * 1px).

Please sign in to comment.