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

Ensure text remain visible during webfont load with font-display #11814

Closed
wants to merge 1 commit into from

Conversation

bagasme
Copy link
Contributor

@bagasme bagasme commented Jun 9, 2020

The swap argument on font-display instruct browsers to use fallback
font for displaying texts until @font-face have been fully downloaded.
Not specifying font-display cause PageSpeed Insights to complain on
"Opportunities" category.

GIF Example on my LXD container:

gitea-font-display-swap

The `swap` argument on `font-display` instruct browsers to use fallback
font for displaying texts until `@font-face` have been fully downloaded.
Not specifying `font-display` cause PageSpeed Insights to complain on
"Opportunities" category.
@6543
Copy link
Member

6543 commented Jun 9, 2020

@silverwind what do you think?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 9, 2020
@bagasme
Copy link
Contributor Author

bagasme commented Jun 9, 2020

@6543 just make pagespeed happy and for slow internet access

@CirnoT
Copy link
Contributor

CirnoT commented Jun 9, 2020

https://drafts.csswg.org/css-fonts-4/#font-display-desc

This value should only be used when rendering text in a particular font is very important for the page, but rendering in any font will still get a correct message across. It should only be used for small pieces of text.

This is not really a solution, especially for emoji or CJK fonts which will most likely be rendered using font without necessary character set, which is imo worse than not rendering at all.

@CirnoT
Copy link
Contributor

CirnoT commented Jun 9, 2020

Our implementation giving URL to download Roboto font is rather poor solution as it seems the browser tends to wait on remote font download rather than use different one from defined list.

Any specific reason why we are explicitly forcing UA to download remote fonts for generic ones like Roboto? @zeripath you introduced that in #6007

@silverwind
Copy link
Member

silverwind commented Jun 9, 2020

I'd rather like to remove the Roboto webfont and use a system-native font stack. Imho, webfonts are not worth the trouble they bring. We have the option between rendering a invisible font until font has loaded (default browser behaviour) or render system font and accept a flash of wrong font (what this pr does), both seem bad.

@mrsdizzie
Copy link
Member

Would also like to remove Roboto webfont

@silverwind
Copy link
Member

Created #11818 about native font stack.

@silverwind silverwind mentioned this pull request Jul 1, 2020
silverwind added a commit to silverwind/gitea that referenced this pull request Jul 2, 2020
- Use system fonts only for text to avoid FOUT
- Move font-awesome to npm/webpack
- Move NotoColorEmoji to web_src
- Remove presumably unneccesary 'PT Sans Narrow'
- Simplify webpack import exclusions

Fixes: go-gitea#11818
Fixes: go-gitea#11814
lafriks added a commit that referenced this pull request Jul 6, 2020
- Use system fonts only for text to avoid FOUT
- Move font-awesome to npm/webpack
- Move NotoColorEmoji to web_src
- Remove presumably unneccesary 'PT Sans Narrow'
- Simplify webpack import exclusions

Fixes: #11818
Fixes: #11814

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
- Use system fonts only for text to avoid FOUT
- Move font-awesome to npm/webpack
- Move NotoColorEmoji to web_src
- Remove presumably unneccesary 'PT Sans Narrow'
- Simplify webpack import exclusions

Fixes: go-gitea#11818
Fixes: go-gitea#11814

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
@bagasme bagasme deleted the web_src/fontDisplaySwap branch September 21, 2020 08:49
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants