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

Jump to bottom

Change font to Roboto to support more charsets #9803

Merged
merged 4 commits into from
Jan 25, 2020
Merged

Change font to Roboto to support more charsets #9803

merged 4 commits into from
Jan 25, 2020

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Jan 16, 2020

I can't really reproduce issue, could be my local setup maybe.

Anyway Roboto font supports way more charsets than Lato does

Fixes #9801

Also drops old browser support that we don't support anyway

Before

image

After

image

@lafriks lafriks added topic/ui Change the appearance of the Gitea UI type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jan 16, 2020
@lafriks lafriks added this to the 1.12.0 milestone Jan 16, 2020
@6543
Copy link
Member

6543 commented Jan 16, 2020

I get

GET /vendor/assets/lato-fonts/lato-v16-latin-regular.woff2 404 Not Found
GET /vendor/assets/lato-fonts/lato-v16-latin-700.woff2 404 Not Found

in the logs

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 16, 2020
sapk
sapk suggested changes Jan 16, 2020
View reviewed changes
Copy link
Member

@sapk sapk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This PR need to update librejs and serviceworker cache definition:

gitea/public/vendor/librejs.html

Line 124 in 590d56d

<td><a href="./assets/lato-fonts/">lato-fonts</a></td>

gitea/templates/pwa/serviceworker_js.tmpl

Line 50 in f00961a

'{{StaticUrlPrefix}}/vendor/assets/lato-fonts/lato-v16-latin-regular.woff2',

@lunny
Copy link
Member

lunny commented Jan 16, 2020

@lafriks Fomantic-ui still depend on lato fonts...

@sapk
Copy link
Member

sapk commented Jan 16, 2020

If I remember well we build fomantic-ui now so we could override some variable like :
https://github.com/Semantic-Org/example-github/blob/master/semantic/src/themes/github/globals/site.variables#L10

@lunny
Copy link
Member

lunny commented Jan 16, 2020

@sapk yes. so we need #9561

@codecov-io
Copy link

codecov-io commented Jan 16, 2020 •
edited
Loading

Codecov Report

Merging #9803 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9803      +/-   ##
==========================================
+ Coverage   42.25%   42.26%   +<.01%     
==========================================
  Files         610      610              
  Lines       80370    80370              
==========================================
+ Hits        33963    33969       +6     
+ Misses      42229    42222       -7     
- Partials     4178     4179       +1
Impacted Files Coverage Δ
routers/repo/view.go 39.47% <0%> (+0.87%) ⬆️
models/unit.go 39.5% <0%> (+2.46%) ⬆️

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 32427ed...01cd326. Read the comment docs.

@silverwind
Copy link
Member

#9561 has landed now

@silverwind
Copy link
Member

Not sure if worth it but the font could be sourced from https://www.npmjs.com/package/typeface-roboto. It does seem to update sometimes, but probably lags behind the google fonts version https://github.com/KyleAMathews/typefaces/blob/master/packages/roboto/files-hash.json.

@lafriks lafriks force-pushed the feat/roboto_font branch 2 times, most recently from 74d9a4d to 08a79e1 Compare January 23, 2020 19:16
@lafriks
Copy link
Member Author

lafriks commented Jan 23, 2020

@silverwind @sapk @lunny rebased and updated to set roboto font also for fomantic build

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 23, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 25, 2020
@zeripath
Copy link
Contributor

@lafriks I can't update your branch...

@lafriks
Copy link
Member Author

lafriks commented Jan 25, 2020

Updated, don't know why you can't uodate

@lafriks lafriks merged commit 59510d6 into go-gitea:master Jan 25, 2020
@lafriks lafriks deleted the feat/roboto_font branch January 25, 2020 21:47
@lafriks lafriks removed the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jan 25, 2020
@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.
Reviewers

@silverwind silverwind silverwind approved these changes

@sapk sapk sapk approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

The font Lato does not support all characters
8 participants
@lafriks @6543 @lunny @sapk @codecov-io @silverwind @zeripath @GiteaBot