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

Change typeface to Nunito #11932

Merged
merged 12 commits into from
Oct 23, 2018
Merged

Change typeface to Nunito #11932

merged 12 commits into from
Oct 23, 2018

Conversation

jancborchardt
Copy link
Member

Based on discussions we had, most recently in #11873

  • πŸ”€ Change typeface from Open Sans to Nunito β†’ its rounded design works well with our branding, and it also gives a much friendlier feel. 😊
  • πŸ‘€ Move from Light/Semibold to Regular/Bold for better readability
  • πŸŽ›οΈ Change main text color from harsh #000000 to #222222

screenshot from 2018-10-19 12-26-32

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jospoortvliet
Copy link
Member

I absolutely LOVE it. πŸ‘ πŸ₯‡

@skjnldsv
Copy link
Member

I really like it as well!
Could we also rethink our headings at the same time? Since we're changig the font, so we can really have differences between h1,h2,h3 and h4? Pleaaaase @jancborchardt πŸ™ˆ ;)

πŸŽ‰

@MorrisJobke
Copy link
Member

It is far better to read with this on a full HD screen over here.

@karlitschek
Copy link
Member

looks good πŸ‘

@jancborchardt
Copy link
Member Author

@skjnldsv that’s a different discussion – let’s work on things step by step. :)

@MorrisJobke @jospoortvliet @karlitschek could I have your approving review then? ;)

Others from @nextcloud/designers please also check it out!

@jancborchardt
Copy link
Member Author

jancborchardt commented Oct 19, 2018

For the record, once IE11 is phased out or we drop support for it, we can switch to woff2 as font format: https://caniuse.com/#search=woff2

@MorrisJobke
Copy link
Member

For the record, once IE11 is phased out or we drop support for it, we can switch to woff2 as font format: https://caniuse.com/#search=woff2

Not going to happen that soon IMO. :/

@MorrisJobke
Copy link
Member

For the record, once IE11 is phased out or we drop support for it, we can switch to woff2 as font format: https://caniuse.com/#search=woff2

What is wrong with ttf?

@jancborchardt
Copy link
Member Author

jancborchardt commented Oct 19, 2018

What is wrong with ttf?

@MorrisJobke according to caniuse:

WOFF: Compressed TrueType/OpenType font that contains information about the font's source.
WOFF2: TrueType/OpenType font that provides better compression than WOFF 1.0.

So quicker loading times. Nunito Regular+Bold in ttf is 230 kB, whereas in woff2 (with latin extended even) it’s 77 kB.

But alas – different topic for a time after IE11. :)

@skjnldsv
Copy link
Member

Does the browser have a loading preference? If so we should probably keep all of them but in the proper order to make sure the browser try to load the better one? πŸ€”

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt
Copy link
Member Author

jancborchardt commented Oct 19, 2018

I now added the woff2 format, as is apparently best practice. The code style of the fonts.scss is the same as used by Google Fonts, so should be good.

Ref https://www.w3.org/TR/css-fonts-3/#src-desc about @font-face src:

Its value is a prioritized, comma-separated list of external references or locally-installed font face names. When a font is needed the user agent iterates over the set of references listed, using the first one it can successfully activate.

Your reviews please @nextcloud/designers :)

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Awesome!! Nice that the font loading is prioritised now :)

@jancborchardt
Copy link
Member Author

@juliushaertl @karlitschek @MorrisJobke @jospoortvliet so can I have your approving reviews? ;) The +1 does not make the approval process happy. :)

@skjnldsv
Copy link
Member

skjnldsv commented Oct 23, 2018

@jancborchardt tests failing:

There were 6 errors:

1) Test\Preview\BackgroundCleanupJobTest::testCleanupSystemCron
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/lib/private/Preview/GeneratorHelper.php:59
/drone/src/github.com/nextcloud/server/lib/private/Preview/Generator.php:194
/drone/src/github.com/nextcloud/server/lib/private/Preview/Generator.php:118
/drone/src/github.com/nextcloud/server/lib/private/PreviewManager.php:205
/drone/src/github.com/nextcloud/server/tests/lib/Preview/BackgroundCleanupJobTest.php:104
/drone/src/github.com/nextcloud/server/tests/lib/Preview/BackgroundCleanupJobTest.php:112

2) Test\Preview\BackgroundCleanupJobTest::testCleanupAjax
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/lib/private/Preview/GeneratorHelper.php:59
/drone/src/github.com/nextcloud/server/lib/private/Preview/Generator.php:194
/drone/src/github.com/nextcloud/server/lib/private/Preview/Generator.php:118
/drone/src/github.com/nextcloud/server/lib/private/PreviewManager.php:205
/drone/src/github.com/nextcloud/server/tests/lib/Preview/BackgroundCleanupJobTest.php:104
/drone/src/github.com/nextcloud/server/tests/lib/Preview/BackgroundCleanupJobTest.php:134

3) Test\Preview\TXTTest::testGetThumbnail with data set #0 (-72, -43)
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:139
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:103

4) Test\Preview\TXTTest::testGetThumbnail with data set #1 (73, 5)
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:139
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:103

5) Test\Preview\TXTTest::testGetThumbnail with data set #2 (-14, 91)
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:139
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:103

6) Test\Preview\TXTTest::testGetThumbnail with data set #3 (70, -39)
imagettftext(): Could not find/open font

/drone/src/github.com/nextcloud/server/lib/private/Preview/TXT.php:83
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:139
/drone/src/github.com/nextcloud/server/tests/lib/Preview/Provider.php:103

--

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 23, 2018
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
…regular+bold anyway

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@@ -62,15 +62,15 @@ class Avatar implements IAvatar {

/**
* https://github.com/sebdesign/cap-height -- for 500px height
* Open Sans cap-height is 0.72 and we want a 200px caps height size (0.4 letter-to-total-height ratio, 500*0.4=200). 200/0.72 = 278px.
* Nunito cap-height is 0.72 and we want a 200px caps height size (0.4 letter-to-total-height ratio, 500*0.4=200). 200/0.72 = 278px.
Copy link
Member

@skjnldsv skjnldsv Oct 23, 2018

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Really? I just checked with the linked tool and it said 0.7153…

Copy link
Member Author

@jancborchardt jancborchardt Oct 23, 2018

Choose a reason for hiding this comment

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

With your updated pen at https://codepen.io/skjnldsv/full/PydLBK/, it says:

  • Chrome: cap-height of Nunito is 0.7123077184349837
  • Firefox: cap-height of Nunito is 0.7151666666666667

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: Nope, it's 0.713, so it's fine to leave it at 0.72
https://codepen.io/skjnldsv/pen/PydLBK/

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Copy link
Member

@jospoortvliet jospoortvliet left a comment

Choose a reason for hiding this comment

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

Awesome! πŸ‘

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Oct 23, 2018
@skjnldsv
Copy link
Member

Failure unrelated: sh: 1: kill: No such process

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Looks good πŸ‘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants