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.

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

Already on GitHub? Sign in to your account

Replace font stack with Chromium 56's "system-ui" generic font family #15080

Merged
merged 1 commit into from Feb 14, 2018

Conversation

Projects
None yet
6 participants
@p-e-w
Contributor

p-e-w commented Jul 23, 2017

Description of the Change

Atom currently does not use the system font, but instead hardcodes a bunch of fonts in a "font stack" (priority list), hoping that the actual system font is present in that list and in the right position.

This doesn't always work and gives particularly poor results on Linux, where changing the system font is easy and Atom stands out as the one application that doesn't respect the change. The reason this long-standing issue is still open is that until recently this wasn't possible to fix from CSS (except on macOS with the BlinkMacSystemFont placeholder family) and would have required platform-specific code that resolves the correct family.

Enter Chromium 56, shipping with Atom 1.19! It introduces the system-ui generic font family that "allows authors to style contents so it fits within the system UI". It works on all platforms! 馃帀 馃巻 馃嵕

Closes #8656. The same change should probably be made also in Atom's default UI themes.

Alternate Designs

The alternatives are keeping the font stack, or resolving the system font family programmatically. Both are clearly worse.

Why Should This Be In Core?

Because the font family is defined in core.

Benefits

Atom looks a tiny bit more like a native application.

Possible Drawbacks

Can't think of any.

Applicable Issues

#8656

@p-e-w

This comment has been minimized.

Contributor

p-e-w commented Feb 3, 2018

@atom/maintainers It's been a while... any chance this might get merged soon? It's just a one-line change which should be trivial to test by simply starting Atom on a few different platforms, but it fixes a long-standing UI issue on Linux.

@ungb

This comment has been minimized.

Contributor

ungb commented Feb 7, 2018

Hey @simurai, is this something you would be able to take a look at?

@simurai

This comment has been minimized.

Member

simurai commented Feb 8, 2018

Tested system-ui in styles.less with:

.workspace {
  font-family: system-ui;
}

Works on:

  • MacOS 馃憠 San Francisco
  • Windows 馃憠 Segoe UI

@ungb Do you mind trying it on Linux? No need to build Atom, you can paste that snippet above in your styles.less and see if the font looks close to the rest of the OS.

In the meantime, I'll make some PRs for the bundled themes.

@ungb

This comment has been minimized.

Contributor

ungb commented Feb 14, 2018

After further testing with @simurai, everything looks good!

@ungb ungb merged commit 22f069a into atom:master Feb 14, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@simurai

This comment has been minimized.

Member

simurai commented Feb 14, 2018

Here @ungb screenshot with the change:

image

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Feb 14, 2018

Great fix! Thanks @p-e-w.

@p-e-w p-e-w deleted the p-e-w:font-family-system-ui branch Feb 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment