-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(theme): strip system-ui from font-family-base #4988
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
base: main
Are you sure you want to change the base?
Conversation
|
||
:root:where(:lang(zh)) { | ||
--vp-font-family-base: | ||
'Punctuation SC', 'Inter', ui-sans-serif, system-ui, 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.
Why was this removed? I think it's still needed because otherwise the punctuation will use Inter instead of Chinese font. 👀
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.
the punctuation will use Inter instead of Chinese font
Do you know which characters/code points are affected? I think it also occurs in Japanese and they should be removed from Inter if possible.
I suppose it is better to respect sans-serif there instead of using a custom font.
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.
Possible candidates:
“
”
‘
’
…
—
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, these -
vitepress/src/client/theme-default/styles/fonts.css
Lines 157 to 159 in 7658ad3
unicode-range: | |
U+201C, U+201D, U+2018, U+2019, U+2E3A, U+2014, U+2013, U+2026, U+00B7, | |
U+007E, U+002F; |
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 found the range of Inter is also defined by us. Let's add a new @font-face
variant for CJK languages.
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.
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.
U+007E, U+002F
Do you know why they were added? I think they should be dedicated to zh(-Hans) if they remain.
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 had copied it from https://github.com/w3c/clreq/blob/f837ffe4a5501e72c17bd02961ce1b440916624f/local.css#L4 😅
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.
Oh, it's the great clreq. It claims that it also covers traditional Chinese. I will follow it in both simplified and traditional Chinese.
I will adopt it in Japanese. Also I will enable |
Description
system-ui has some problems on rendering CJK text especially long paragraphs like contents suitable for Vitepress. sans-serif may be an older font stack especially in Euro-American language, but does not have such problems.
sans-serif
tosystem-ui
in default fonts #4946system-ui
behavior is undesired for some users w3c/csswg-drafts#3658system-ui
in articles, documents, and blogs mdn/content#41244Linked Issues
Fixes #4946
Additional Context
Before
Taken in Japanese Windows
Inappropriate 将/将 glyph & han characters with uneven thickness:
Too condensed and quirky hiragana & katakana:
After
Future work
Additional font kerning settings may be preferred in headings and menus in Japanese (an additional PR).
Tip
The author of this PR can publish a preview release by commenting
/publish
below.