-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
UI: update font family #1560
UI: update font family #1560
Conversation
-apple-system, ".SFNSText-Regular", "San Francisco", "Roboto", | ||
"Segoe UI", "Helvetica Neue", "Lucida Grande", sans-serif | ||
-apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", | ||
"Cantarell", "Fira Sans", "Droid Sans", "Arial", sans-serif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Helvetica Neue
is still necessary for OS X 10.11 and below when not in Chrome:
https://bitsofco.de/the-new-system-font-stack/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't Arial
be available there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. I think it's included to conform to the system's original styles and Arial is an unfamiliar and clunky looking font to Mac users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well if the alternative is to have a font that jumps when going from normal to bold (and we have a menu in which it happens all the time), the unfamiliarity is a lesser evil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm fair enough.
Codecov Report
@@ Coverage Diff @@
## master #1560 +/- ##
==========================================
+ Coverage 20.43% 20.46% +0.02%
==========================================
Files 241 241
Lines 5250 5243 -7
Branches 649 647 -2
==========================================
Hits 1073 1073
Misses 3682 3682
+ Partials 495 488 -7
Continue to review full report at Codecov.
|
When I mentioned changing the fonts is inconstant in the prev PR, I meant that IMO it's wrong to change this only in one place (lib/ui). This kind of theme is used in a few other places around this repo (for example addons), and styleguide-wise I think all the fonts should be the same (at least in the official addons / UI). But maybe I 'm too picky =) Anyway if it solves this jumping glitch - 👍 👍 |
Well, in fact there isn't any consistensy of font-family across the codebase at the moment: https://github.com/storybooks/storybook/search?utf8=%E2%9C%93&q=fontFamily&type= And if we want to make it more consistent, it would be better to use a shared theme instead of repeating ourselves. This can be a part of the major task of moving UI components to |
Based on Slack discussion