Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor our font stack #99429

Merged
merged 5 commits into from
Jun 5, 2020
Merged

Refactor our font stack #99429

merged 5 commits into from
Jun 5, 2020

Conversation

bpasero
Copy link
Member

@bpasero bpasero commented Jun 5, 2020 •
edited
Loading

This PR fixes #99428

Following changes are included:

  • consistently use monaco-monospace-font everywhere and define it for standalone editor too (via 2872e27)
  • define a shared default font family for programmatic access and adopt it (via 05f2cec)
  • define system-ui before Ubuntu but after Segoe (via ff69562 and 61214b5) to only change it for Linux distros unless they ship a Segoe font
  • use the same font rules for different OS and languages in issue reporter and process explorer (via e49f99b)

@bpasero bpasero added this to the May 2020 milestone Jun 5, 2020
@bpasero bpasero self-assigned this Jun 5, 2020
@bpasero bpasero mentioned this pull request Jun 5, 2020
Merged
bpasero
bpasero commented Jun 5, 2020
View reviewed changes
extensions/markdown-language-features/media/markdown.css
@@ -157,7 +157,7 @@ blockquote {
}

code {
font-family: var(--vscode-editor-font-family, Menlo, Monaco, Consolas, "Droid Sans Mono", "Courier New", monospace, "Droid Sans Fallback");
font-family: var(--vscode-editor-font-family, "SF Mono", Monaco, Menlo, Consolas, "Ubuntu Mono", "Liberation Mono", "DejaVu Sans Mono", "Courier New", monospace);
Copy link
Member Author

Choose a reason for hiding this comment

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

@mjbvz please review this change, I tried to adjust this to be our standard monospace font we use in the workbench by combining the rules from our monaco-monospace-font into one

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/code/electron-browser/issue/media/issueReporter.css
html:lang(zh-Hans) {
font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "Noto Sans", "Microsoft YaHei", "PingFang SC", "Hiragino Sans GB", "Source Han Sans SC", "Source Han Sans CN", "Source Han Sans", sans-serif;
}
/* Font Families (with CJK support) */
Copy link
Member Author

Choose a reason for hiding this comment

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

@RMacfarlane please review this change, it copies the CSS rule we define for the workbench to have the exact same font stack.

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/code/electron-browser/processExplorer/media/processExplorer.css
html:lang(zh-Hans) {
font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "Noto Sans", "Microsoft YaHei", "PingFang SC", "Hiragino Sans GB", "Source Han Sans SC", "Source Han Sans CN", "Source Han Sans", sans-serif;
}
/* Font Families (with CJK support) */
Copy link
Member Author

Choose a reason for hiding this comment

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

@RMacfarlane please review this change, it copies the CSS rule we define for the workbench to have the exact same font stack.

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/code/electron-browser/proxy/auth.html
@@ -18,7 +18,7 @@
}

body {
font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "Ubuntu", "Droid Sans", sans-serif;
font-family: -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", system-ui, "Ubuntu", "Droid Sans", sans-serif;
Copy link
Member Author

Choose a reason for hiding this comment

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

@joaomoreno @chrmarti note that auth.html does not seem to have a notion of platform, I think it should follow the same structure as process explorer and issue reporter.

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/editor/standalone/browser/standalone-tokens.css
.monaco-editor {
font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "HelveticaNeue-Light", "Ubuntu", "Droid Sans", sans-serif;
font-family: -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "HelveticaNeue-Light", system-ui, "Ubuntu", "Droid Sans", sans-serif;
--monaco-monospace-font: "SF Mono", Monaco, Menlo, Consolas, "Ubuntu Mono", "Liberation Mono", "DejaVu Sans Mono", "Courier New", monospace;
Copy link
Member Author

Choose a reason for hiding this comment

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

@alexdima fyi this defines the monaco-monospace-font for standalone editor to be a union of the 3 platform specific rules we have in the workbench

Copy link
Member

Choose a reason for hiding this comment

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

👍 Thank you!

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/workbench/contrib/notebook/browser/media/notebook.css
@@ -128,7 +128,7 @@

.monaco-workbench .notebookOverlay .output > div.foreground .output-stream,
.monaco-workbench .notebookOverlay .output > div.foreground .output-plaintext {
font-family: Menlo, Monaco, Consolas, "Droid Sans Mono", "Courier New", monospace, "Droid Sans Fallback";
font-family: var(--monaco-monospace-font);
Copy link
Member Author

Choose a reason for hiding this comment

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

@rebornix fyi this adopts the CSS variable for notebooks

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/workbench/contrib/scm/browser/repositoryPane.ts
@@ -683,7 +684,7 @@ export class ToggleViewModeAction extends Action {
}

export class RepositoryPane extends ViewPane {
private readonly defaultInputFontFamily = 'system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "Ubuntu", "Droid Sans", sans-serif';
private readonly defaultInputFontFamily = DEFAULT_FONT_FAMILY;
Copy link
Member Author

Choose a reason for hiding this comment

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

@joaomoreno fyi this uses the platform specific font rules

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/workbench/contrib/update/browser/releaseNotesEditor.ts
@@ -257,8 +257,9 @@ export class ReleaseNotesManager {
}

code {
font-family: Menlo, Monaco, Consolas, "Droid Sans Mono", "Courier New", monospace, "Droid Sans Fallback";
font-size: 14px;
font-family: var(--vscode-editor-font-family);
Copy link
Member Author

Choose a reason for hiding this comment

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

@mjbvz @joaomoreno fyi this aligns release notes editor with other webviews

bpasero
bpasero commented Jun 5, 2020
View reviewed changes
src/vs/workbench/contrib/webview/browser/themeing.ts
@@ -64,7 +65,7 @@ export class WebviewThemeDataProvider extends Disposable {
}, {} as { [key: string]: string; });

const styles = {
'vscode-font-family': 'system-ui, -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "Ubuntu", "Droid Sans", sans-serif',
'vscode-font-family': DEFAULT_FONT_FAMILY,
Copy link
Member Author

Choose a reason for hiding this comment

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

@mjbvz fyi this uses the platform specific rule that is shared

@bpasero bpasero merged commit 5880971 into master Jun 5, 2020
@bpasero bpasero deleted the ben/fonts branch June 5, 2020 08:56
@RMacfarlane RMacfarlane self-requested a review June 5, 2020 16:29
@mjbvz
Copy link
Collaborator

mjbvz commented Jun 5, 2020

Webview changes look good to me. Thanks @bpasero!

@github-actions github-actions bot locked and limited conversation to collaborators Jul 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexdima alexdima alexdima left review comments

@RMacfarlane RMacfarlane Awaiting requested review from RMacfarlane

Assignees

@bpasero bpasero

Labels
None yet
Projects
None yet
Milestone
May 2020
Development

Successfully merging this pull request may close these issues.

Make our font stack consistent
3 participants
@bpasero @mjbvz @alexdima