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

Use tabular-nums font variant instead of switching to Tahoma for figures #11567

Merged
merged 1 commit into from Aug 4, 2018
Merged

Conversation

tibdex
Copy link
Contributor

@tibdex tibdex commented Aug 1, 2018

Using a completely different font for figures leads to inconsistencies: #9422.
Most modern font systems default to tabular figures.
One exception to this is Apple's San Francisco which defaults to proportional figures.
There is, however, an official and clean way to switch to tabular figures. It's the font-variant CSS property that can be set to tabular-nums.

This commit removes the "Monospaced Number" font altogether and use font-variant: tabular-nums instead where it was used.
It also moves closer to the sans-serif and monospace system font stacks used by GitHub.


Here is the before/after on Windows where Segoe UI is now used for figures too. The difference is subtle but you can see it. Especially for the figure 1.

Before

before

After

after

Using a completely different font for figures leads to inconsistencies: #9422.
Most modern font systems defaults to [tabular figures](https://www.fonts.com/content/learning/fontology/level-3/numbers/proportional-vs-tabular-figures).
One exception to this is Apple's San Francisco which defaults to [proportional figures](http://martiancraft.com/blog/2015/10/san-francisco-part-2/#special-features-numerals).
There is, however, an official and clean way to switch to tabular figures. It's the [`font-variant` CSS property](https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-numeric) that can be set to `tabular-nums`.

This commit removes the "Monospaced Number" font altogether and use `font-variant: tabular-nums` instead where it was used.
It also moves closer to the sans-serif and monospace [system font stacks used by GitHub](http://markdotto.com/2018/02/07/github-system-fonts/).
@ant-design-bot
Copy link
Contributor

Deploy preview for ant-design ready!

Built with commit 7b8b41d

https://deploy-preview-11567--ant-design.netlify.com

font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto,
"Helvetica Neue", Helvetica, "PingFang SC", "Hiragino Sans GB", "Microsoft YaHei",
SimSun, sans-serif;
font-family: "Chinese Quote", -apple-system, BlinkMacSystemFont, "Segoe UI", "PingFang SC", "Hiragino Sans GB", "Microsoft YaHei", "Helvetica Neue", Helvetica, Arial, sans-serif,
Copy link
Contributor Author

@tibdex tibdex Aug 1, 2018

Choose a reason for hiding this comment

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

I added "Chinese Quote" and removed SimSun because that's what was actually used.

I didn't change the font.zh-CN.md file, but the PR is configure to "Allow edits from maintainers.". Feel free to edit the Chinese page and to tweak the line breaking if you want shorter lines in the docs.

Copy link

Choose a reason for hiding this comment

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

Hi @tibdex the adding of "Chinese Quote" replaced Segoe UI everywhere in IE11 :(
Had to fix it manually

2018-08-06_113446

(chrome looks ok)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Gyokur0, as you can see in my comment above, I only added "Chinese Quote" in the docs to keep them in sync with the actual font stack used in Less files.

This is the font stack before my commit:

@font-family-no-number : "Chinese Quote", -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "PingFang SC", "Hiragino Sans GB", "Microsoft YaHei", "Helvetica Neue", Helvetica, Arial, sans-serif;
@font-family : "Monospaced Number", @font-family-no-number;
@code-family : Consolas, Menlo, Courier, monospace;

Note the presence of "Chinese Quote".

And after:

@font-family : "Chinese Quote", -apple-system, BlinkMacSystemFont, "Segoe UI", "PingFang SC", "Hiragino Sans GB", "Microsoft YaHei", "Helvetica Neue", Helvetica, Arial, sans-serif,
"Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol";
@code-family : "SFMono-Regular", Consolas, "Liberation Mono", Menlo, Courier, monospace;

Still starts with "Chinese Quote", -apple-system, BlinkMacSystemFont, "Segoe UI".

I don't see how what you described would have happened.

Besides, I just visited https://ant.design/components/table/, which ships with this commit, on IE 11 and it displays with Segoe UI as expected.

@codecov
Copy link

codecov bot commented Aug 4, 2018

Codecov Report

Merging #11567 into feature-3.8.0 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           feature-3.8.0   #11567   +/-   ##
==============================================
  Coverage          91.94%   91.94%           
==============================================
  Files                199      199           
  Lines               5089     5089           
  Branches            1426     1426           
==============================================
  Hits                4679     4679           
  Misses               406      406           
  Partials               4        4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f81349...7b8b41d. Read the comment docs.

@afc163 afc163 merged commit 43abea3 into ant-design:feature-3.8.0 Aug 4, 2018
@afc163
Copy link
Member

afc163 commented Sep 8, 2018

#11456
#11843

font-variant: tabular-nums; will reduce render perfermance of long word in chome. I fix this by resetting this in Select in 1a30e5d for now. Looking forword to further solution or wait for chrome fix.

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

4 participants