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 Noto Sans from Fontsource #2234

Merged
merged 2 commits into from
Dec 16, 2020
Merged

Conversation

thornbill
Copy link
Member

Changes

  • Removes jellyfin-noto
  • Uses noto-sans font packages from fontsource
  • Uses custom stylesheets to only include woff2 files for fonts

Issues
Fixes the vertical alignment issues of jellyfin-noto (particularly noticeable on buttons and badges)

@thornbill thornbill added the stable backport Backport into the next stable release label Dec 14, 2020
@thornbill thornbill added this to Active PRs in Release 10.7.0 via automation Dec 14, 2020
@sonarcloud
Copy link

sonarcloud bot commented Dec 14, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.4% 0.4% Duplication

@mixin font($weight: null, $size: null) {
font-family: "Noto Sans", sans-serif;
font-family: "Noto Sans", "Noto Sans HK", "Noto Sans JP", "Noto Sans KR", "Noto Sans SC", sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning that some of these have conflicting characters (JP and SC, from memory).

Some of the kanji are different between the two scripts, but share the same Unicode number.

Copy link
Member Author

@thornbill thornbill Dec 14, 2020

Choose a reason for hiding this comment

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

Yeah I found this issue #913 and read through the notes there. I guess we should add a setting to set a hierarchy or preferred character set somewhere. I consider that a 10.8 change though... this should match the current behavior of jellyfin-noto for now.

Copy link
Member

Choose a reason for hiding this comment

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

KR also has all the Hanja (full characters) in it, so this needs to be set on a per language basis.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking we probably shouldn't just rely on the display language since some people may have a larger amount of content in a specific language that is different than their display language. Then they would probably have a preference for which language should be preferred.

This could be done with custom css until a proper preference is added.

@nyanmisaka
Copy link
Member

nyanmisaka commented Dec 15, 2020

There are still some vertical alignment issues in TextButton. Especially in mobile safari.

@thornbill
Copy link
Member Author

@nyanmisaka that seems to not be caused by the font itself, but the styles that are being applied.

@thornbill thornbill merged commit d66d26b into jellyfin:master Dec 16, 2020
Release 10.7.0 automation moved this from Active PRs to Completed PRs Dec 16, 2020
@thornbill thornbill deleted the no-no-noto branch December 16, 2020 19:46
joshuaboniface pushed a commit that referenced this pull request Dec 31, 2020
Use Noto Sans from Fontsource

(cherry picked from commit d66d26b)
Signed-off-by: Joshua M. Boniface <joshua@boniface.me>
@joshuaboniface joshuaboniface removed the stable backport Backport into the next stable release label Dec 31, 2020
@joshuaboniface joshuaboniface moved this from Completed PRs to Jellyfinished in Release 10.7.0 Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Release 10.7.0
  
Jellyfinished
Development

Successfully merging this pull request may close these issues.

None yet

6 participants