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.

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

Fix font stack output #2947

Merged
merged 4 commits into from Mar 12, 2019
Merged

Fix font stack output #2947

merged 4 commits into from Mar 12, 2019

Conversation

thisisdano
Copy link
Member

@thisisdano thisisdano commented Mar 12, 2019

This fixes a problem where we might be outputting font stacks with a leading comma or using a redundant display name at the start of the stack.


We have four internal font stacks that we can assign to to typefaces, composed of common fallbacks installed on most systems:

$font-stack-system: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol';
$font-stack-georgia: 'Georgia', 'Cambria', 'Times New Roman', 'Times', serif;
$font-stack-helvetica: 'Helvetica Neue', 'Helvetica', 'Roboto', 'Arial', sans-serif;
$font-stack-monospace: 'Bitstream Vera Sans Mono', 'Consolas', 'Courier', monospace;

Plus, users can define custom stacks for font types in settings:

$theme-font-cond-custom-stack:  false !default;
$theme-font-icon-custom-stack:  false !default;
$theme-font-lang-custom-stack:  false !default;
$theme-font-mono-custom-stack:  false !default;
$theme-font-sans-custom-stack:  false !default;
$theme-font-serif-custom-stack: false !default;

The output font stack is the display name of the typeface plus the value of either the system-assigned stack or the user's custom stack.

For example, Source Sans Pro gets the $font-stack-helvetica stack by default and is output as

font-family: #{display-name}, #{font-stack};

But the builder should not put a display name at the top of the stack in a couple of scenarios:

  • If the user is using system fonts
  • If the typeface is the same as the stack (i.e. helvetica or georgia) and no custom stack is defined

Now the stack builder function is smart enough to do that. It will withhold the leading display name if the display name is null (in the case of 'system') or if the assigned typeface is one of the common system fonts we're already using as a stack (in the case of 'georgia' or 'helvetica').


Fixes #2944

- Output only is font has a display name
- If font is a system font, output only if a custom stack is defined
@thisisdano thisisdano requested a review from a team March 12, 2019 13:43
@maya maya merged commit dc4184a into release-2.0 Mar 12, 2019
@maya maya deleted the dw-fix-font-stacks branch March 12, 2019 18:06
@maya maya mentioned this pull request Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants