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.

Sign up for GitHub

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

Avoid unnecessary system-ui expansion #12522

Merged
merged 4 commits into from
Aug 28, 2020
Merged

Avoid unnecessary system-ui expansion #12522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b721822
Avoid unnecessary system-ui expansion (fix #12325)
CL-Jeremy Aug 18, 2020
0627cab
extract config to static object
silverwind Aug 26, 2020
8688513
Merge branch 'master' of https://github.com/go-gitea/gitea into maste…
CL-Jeremy Aug 28, 2020
0f9a113
Merge branch 'master' into master
techknowlogick Aug 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions web_src/less/_base.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
url('../fonts/noto-color-emoji/NotoColorEmoji.ttf') format('truetype');
}

@default-fonts: -apple-system, BlinkMacSystemFont, system-ui, 'Segoe UI', Roboto, Helvetica, Arial, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji" sans-serif;
@default-fonts: -apple-system, BlinkMacSystemFont, system-ui, 'Segoe UI', Roboto, Helvetica, Arial, "Apple Color Emoji", "Segoe UI Emoji", "Noto Color Emoji", "Twemoji Mozilla";
Copy link
Member

Choose a reason for hiding this comment

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

Just noticed it: Is the removal of Segoe UI Symbol intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I take it as something in the same league as JIS fonts and DejaVu Sans. As I said earlier, it's not a major goal to cater users with need of these black-and-white icons (except for code, but maybe even there as well). I'm removing it to align the UX of legacy Windows to legacy Linux, where DejaVu Sans is only matched by sans-serif. Testing it on Windows 7 shows the same effect with Segoe UI Symbol being matched by sans-serif, so at least there won't be tofus.

There have been discussions regarding this in the past when browsers switched to prioritize emojis, a behavior originated on mobile platforms, cf. https://bugzilla.mozilla.org/show_bug.cgi?id=1054780.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another (positive?) side effect of this would be that Japanese users would see more native glyphs of dingbat symbols if they have compatible fonts in the :lang(ja) stack installed. emoticons, people, objects and stuff will be unaffected. If I recall correctly, many JIS symbol designs from non-Japanese fonts (e. g. emoji fonts) look poor and broken. I haven't investigated the Segoe ones though.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Agree that plain color glyphs should not be used if better versions are available.

@monospaced-fonts: 'SF Mono', Consolas, Menlo, 'Liberation Mono', Monaco, 'Lucida Console';

.override-fonts(@fonts) {
Expand Down Expand Up @@ -79,7 +79,7 @@
}
}

.override-fonts(@default-fonts);
.override-fonts(@default-fonts, sans-serif;);

body {
background-color: #ffffff;
Expand All @@ -90,27 +90,27 @@ body {

@ja-fonts: 'Hiragino Kaku Gothic ProN', 'Yu Gothic', 'Source Han Sans JP', 'Noto Sans CJK JP', 'Droid Sans Japanese', 'Meiryo', 'MS PGothic';
:lang(ja) {
.override-fonts(@default-fonts, @ja-fonts;);
.override-fonts(@default-fonts, @ja-fonts, sans-serif;);
}

@zh-CN-fonts: 'PingFang SC', 'Hiragino Sans GB', 'Source Han Sans CN', 'Source Han Sans SC', 'Noto Sans CJK SC', 'Microsoft YaHei', 'Heiti SC', SimHei;
:lang(zh-CN) {
.override-fonts(@default-fonts, @zh-CN-fonts;);
.override-fonts(@default-fonts, @zh-CN-fonts, sans-serif;);
}

@zh-TW-fonts: 'PingFang TC', 'Hiragino Sans TC', 'Source Han Sans TW', 'Source Han Sans TC', 'Noto Sans CJK TC', 'Microsoft JhengHei', 'Heiti TC', PMingLiU;
:lang(zh-TW) {
.override-fonts(@default-fonts, @zh-TW-fonts;);
.override-fonts(@default-fonts, @zh-TW-fonts, sans-serif;);
}

@zh-HK-fonts: 'PingFang HK', 'Hiragino Sans TC', 'Source Han Sans HK', 'Source Han Sans TC', 'Noto Sans CJK TC', 'Microsoft JhengHei', 'Heiti TC', PMingLiU_HKSCS, PMingLiU;
:lang(zh-HK) {
.override-fonts(@default-fonts, @zh-HK-fonts;);
.override-fonts(@default-fonts, @zh-HK-fonts, sans-serif;);
}

@ko-fonts: 'Apple SD Gothic Neo', 'NanumBarunGothic', 'Malgun Gothic', 'Gulim', 'Dotum', 'Nanum Gothic', 'Source Han Sans KR', 'Noto Sans CJK KR';
:lang(ko) {
.override-fonts(@default-fonts, @ko-fonts;);
.override-fonts(@default-fonts, @ko-fonts, sans-serif;);
}

img {
Expand Down Expand Up @@ -1072,7 +1072,7 @@ i.icon.centerlock {

.blame-data {
display: flex;
font-family: @default-fonts;
font-family: @default-fonts, sans-serif;

.blame-message {
flex-grow: 2;
Expand Down
10 changes: 8 additions & 2 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ const {resolve, parse} = require('path');
const {LicenseWebpackPlugin} = require('license-webpack-plugin');
const {SourceMapDevToolPlugin} = require('webpack');

const postCssPresetEnvConfig = {
features: {
'system-ui-font-family': false,
}
};

const glob = (pattern) => fastGlob.sync(pattern, {cwd: __dirname, absolute: true});

const themes = {};
Expand Down Expand Up @@ -178,7 +184,7 @@ module.exports = {
loader: 'postcss-loader',
options: {
plugins: () => [
PostCSSPresetEnv(),
PostCSSPresetEnv(postCssPresetEnvConfig),
],
sourceMap: true,
},
Expand All @@ -204,7 +210,7 @@ module.exports = {
loader: 'postcss-loader',
options: {
plugins: () => [
PostCSSPresetEnv(),
PostCSSPresetEnv(postCssPresetEnvConfig),
],
sourceMap: true,
},
Expand Down