Add a setting allowing the use of system's default font in Web UI by eramdam · Pull Request #4033 · mastodon/mastodon

eramdam · 2017-07-02T07:12:19Z

Disabled by default

This will use the following font stack:

.system-font {
  // system-ui => standard property (Chrome/Android WebView 56+, Opera 43+, Safari 11+)
  font-family: system-ui, 
    // -apple-system => Safari <11 specific
    -apple-system,
    // BlinkMacSystemFont => Chrome <56 on macOS specific
    BlinkMacSystemFont, 
    // Segoe UI => Windows 7/8/10
    "Segoe UI",
    // Oxygen => KDE
    "Oxygen", 
    // Ubuntu => Unity/Ubuntu
    "Ubuntu",
    // Cantarell => GNOME
    "Cantarell",
    // Fira Sans => Firefox OS
    "Fira Sans", 
    // Droid Sans => Older Androids (<4.0)
    "Droid Sans",
    // Helvetica Neue => Older macOS <10.11
    "Helvetica Neue",
    // Roboto => web-font fallback and newer Androids (>=4.0)
    "Roboto", sans-serif;
}

sorin-davidoi · 2017-07-02T11:01:14Z

I've been wanting to look into this for a while, glad someone beat me to it 👍

A couple of questions:

Could we avoid loading the custom fonts if the option is enabled? That would save 374 KB. If I tick the checkbox, I don't expect Roboto to be used - which might very well mean that it will fallback to sans-serif or whatever if all the other fonts are not found.
What about adding system-ui (shipped in Chrome and Safari) as the first font?

eramdam · 2017-07-02T15:50:21Z

@sorin-davidoi

I'd have to check where we try to load Roboto but that sounds doable
I legit didn't know system-ui shipped already!

nightpool · 2017-07-02T15:59:17Z

I'm confused about this implementation. This doesn't work at all if I've changed my system's default font, which seems to kinda defeat the point.

Why not just use font-family: sans-serif; and be done with it? or font-family: none?

nightpool · 2017-07-02T16:01:40Z

app/javascript/mastodon/features/ui/index.js

+    let className = 'ui';
+
+    if (this.props.systemFontUi)
+      className += ' has-system-font';


this would be more cleanly implemented as an array, using classes.join(" ") at the calling site (or maybe react has a better pattern for this?)

sorin-davidoi · 2017-07-02T16:01:53Z

@nightpool I think Chrome's fonts can be changed in its settings (don't know if it reads them from the OS everytime you launch it or just when you start it the first time).

eramdam · 2017-07-02T16:02:57Z

@nightpool You'll notice that we're talking about the system's default font. Quoting the specification for system-ui

This generic font family is intended to let text render with the default user interface font on the platform on which the UA is running.

So if your browser implements system-ui or a similar property, I feel like it should still use the default font. I don't have an OS where I can change my default font so I can't test this unfortunately.

nightpool · 2017-07-02T16:11:10Z

@sorin-davidoi right. But using "Segoe UI" means that, if I was on Windows 10 (or another OS where I happened to have installed Segoe UI for unrelated reasons....), that would override my chrome font choice.

If we want to just use system-ui, then fine. But having the other fonts hard coded seems like a bad decision.

sorin-davidoi · 2017-07-02T16:15:32Z

@nightpool I agree, it is messy. Github seems to use something similar to this (https://css-tricks.com/os-specific-fonts-css/#article-header-id-0). I would personally be happy with system-ui, BlinkMacSystemFont and -apple-system, but not sure how this will look in browsers / OSs that don't support them.

eramdam · 2017-07-02T16:16:34Z

The other fonts are merely fallbacks for all the other browsers that don't support system-ui just yet. The purpose of the setting is to use a font that "feels native to the OS", as per specification. It's messy but we don't have other choices unfortunately (I took inspiration from GitHub's font stack)

The purpose of system-ui is to allow web content to integrate with the look and feel of a native app. On platforms which have a collection of system user interface fonts (e.g. for different languages), user agents may treat system-ui as a virtual font which encompasses all the platform user interface fonts. However, if this is done, the details of the virtual font must not be visible or detectable.

Keep in mind that whatever font that you choose in the setting of your browser is the fallback used for sans-serif, serif and monospace. Which differs in the way that they could probably not be fonts used by your OS.

Also, the setting is disabled by default, and usually people who would want to enable it know what they're getting into.

sorin-davidoi · 2017-07-02T16:37:36Z

app/javascript/styles/basics.scss

  justify-content: center;
 }
+
+.has-system-font {


Minor nitpick, maybe something like .system-font would be better? has for me implies that we do some feature detection (e.g. has-flex, has-grid).

I would name it '.use-system-font'

eramdam · 2017-07-02T16:40:55Z

@sorin-davidoi I tested and Roboto is not loaded (except for the regular family because of the style on body) if it's at the "end" of the stack. Also I can't remove it entirely since Roboto is the default font on newer Android systems.

sorin-davidoi

Off by default, saves ~300 KB if enabled 👍

unarist · 2017-07-03T11:53:36Z

app/javascript/mastodon/features/ui/index.js

+
    return (
-      <div className='ui' ref={this.setRef}>
+      <div className={classNames.join(' ')} ref={this.setRef}>


How about using classnames? It's used in some components already.

@unarist I didn't know about that! I'll update the code to use it for consistency.

@unarist I just pushed a commit to use classnames! 👍

Gargron · 2017-07-06T19:28:16Z

app/javascript/styles/basics.scss

+  // Droid Sans => Older Androids (<4.0)
+  // Helvetica Neue => Older macOS <10.11
+  // Roboto => web-font fallback and newer Androids (>=4.0)
+  font-family: system-ui, -apple-system,BlinkMacSystemFont, "Segoe UI","Oxygen", "Ubuntu","Cantarell","Fira Sans","Droid Sans","Helvetica Neue","Roboto", sans-serif;


hey, the webfont for Roboto is actually called "mastodon-font-sans-serif" in our CSS

@Gargron I just fixed this. Sorry for the overlooking 😅

* Add Japanese translations for #3879 * Add Japanese translations for #4033 * Add Japanese translations for #4136

eramdam added 7 commits June 28, 2017 22:34

add a system_font_ui setting on the server

d6a63d1

Plug the system_font_ui on the front-end

6df87e7

add EN/FR locales for the new setting

4bc31ed

put Roboto after all other fonts

2bd2c07

remove trailing whitespace so CodeClimate is happy

6169f0e

fix user_spec.rb

aa9101e

correctly write user_spect this time

f29f5c4

nightpool reviewed Jul 2, 2017

View reviewed changes

slightly better way of adding the classes

57e7da9

sorin-davidoi reviewed Jul 2, 2017

View reviewed changes

eramdam added 4 commits July 2, 2017 09:45

add comments to the system-font stack for clarification

4ec9604

use .system-font for the class instead

327d937

don't use multiple lines for comments

6f21987

remove trailing whitespace

4a36138

sorin-davidoi approved these changes Jul 2, 2017

View reviewed changes

akihikodaki added enhancement ui Front-end, design labels Jul 3, 2017

unarist reviewed Jul 3, 2017

View reviewed changes

use the classnames module for consistency

98eb9d9

Gargron requested changes Jul 6, 2017

View reviewed changes

use mastodon-font-sans-serif instead of Roboto directly

9eb0ba3

Gargron approved these changes Jul 6, 2017

View reviewed changes

Gargron merged commit 18d3fa9 into mastodon:master Jul 6, 2017

ykzts mentioned this pull request Jul 10, 2017

Add Japanese translations #4140

Merged

Gargron pushed a commit that referenced this pull request Jul 10, 2017

Add Japanese translations (#4140)

31490e0

* Add Japanese translations for #3879 * Add Japanese translations for #4033 * Add Japanese translations for #4136

ykzts mentioned this pull request Aug 8, 2017

Use system's default font on non web UI pages #4553

Merged

snyk-bot mentioned this pull request Jan 21, 2022

[Snyk] Fix for 25 vulnerabilities ericblade/mastodon#3

Open

Uh oh!

Conversation

eramdam commented Jul 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sorin-davidoi commented Jul 2, 2017

Uh oh!

eramdam commented Jul 2, 2017

Uh oh!

nightpool commented Jul 2, 2017

Uh oh!

nightpool Jul 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sorin-davidoi commented Jul 2, 2017

Uh oh!

eramdam commented Jul 2, 2017

Uh oh!

nightpool commented Jul 2, 2017

Uh oh!

sorin-davidoi commented Jul 2, 2017

Uh oh!

eramdam commented Jul 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sorin-davidoi Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

nightpool Jul 2, 2017

Choose a reason for hiding this comment

Uh oh!

eramdam commented Jul 2, 2017

Uh oh!

sorin-davidoi left a comment

Choose a reason for hiding this comment

Uh oh!

unarist Jul 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramdam Jul 3, 2017

Choose a reason for hiding this comment

Uh oh!

eramdam Jul 4, 2017

Choose a reason for hiding this comment

Uh oh!

Gargron Jul 6, 2017

Choose a reason for hiding this comment

Uh oh!

eramdam Jul 6, 2017

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eramdam commented Jul 2, 2017 •

edited

Loading

nightpool Jul 2, 2017 •

edited

Loading

eramdam commented Jul 2, 2017 •

edited

Loading

unarist Jul 3, 2017 •

edited

Loading