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

Optimize text rendering for legibility #3382

Merged
merged 1 commit into from Feb 11, 2015

Conversation

Projects
None yet
8 participants
@alfredxing
Member

alfredxing commented Jan 30, 2015

Add text-rendering: optimizeLegibility to enable kerning and ligatures. This applies to the site template as well as the jekyllrb.com site.

For the site template, this should produce a more desirable result for the site header when combined with #3379.

Kerning! Ligatures!

@kleinfreund

This comment has been minimized.

Show comment
Hide comment
@kleinfreund

kleinfreund Jan 30, 2015

Contributor

👎

I think we should not add stuff to the basic site template unless there are good reasons for it. I doubt the benefits of text-rendering: optimizeLegibility; when applied to Helvetica/Arial.

Contributor

kleinfreund commented Jan 30, 2015

👎

I think we should not add stuff to the basic site template unless there are good reasons for it. I doubt the benefits of text-rendering: optimizeLegibility; when applied to Helvetica/Arial.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 30, 2015

Member

I think we should not add stuff to the basic site template unless there are good reasons for it.

@kleinfreund I agree with this on principle, which is why the other PR's I received today (yesterday?) were mostly skimmed and deferred. They didn't seem important. The site template as-is is perfectly fine at the moment.

@alfredxing I have no problem with this change.

@jglovier, any great wisdom? 😄

Member

parkr commented Jan 30, 2015

I think we should not add stuff to the basic site template unless there are good reasons for it.

@kleinfreund I agree with this on principle, which is why the other PR's I received today (yesterday?) were mostly skimmed and deferred. They didn't seem important. The site template as-is is perfectly fine at the moment.

@alfredxing I have no problem with this change.

@jglovier, any great wisdom? 😄

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Jan 30, 2015

Member

@kleinfreund Was not aware of the performance issues, thanks for the link!

Do you think font-feature-settings would be better? At the very least I think kerning is needed.

Member

alfredxing commented Jan 30, 2015

@kleinfreund Was not aware of the performance issues, thanks for the link!

Do you think font-feature-settings would be better? At the very least I think kerning is needed.

@jglovier

This comment has been minimized.

Show comment
Hide comment
@jglovier

jglovier Jan 30, 2015

Member

Yeah, as @kleinfreund said the benefits of this property would only be realized with typefaces that we aren't using here by default. For our Helvetica stack, I don't think we'd really see much if any benefit.

Do you think font-feature-settings would be better? At the very least I think kerning is needed.

@alfredxing I'm of the opinion that kerning web-fonts should always be handled manually through the use of the letter-spacing property. There may be a good argument against that of which I'm unaware, so feel free to push back if you think that's not a good approach.

Member

jglovier commented Jan 30, 2015

Yeah, as @kleinfreund said the benefits of this property would only be realized with typefaces that we aren't using here by default. For our Helvetica stack, I don't think we'd really see much if any benefit.

Do you think font-feature-settings would be better? At the very least I think kerning is needed.

@alfredxing I'm of the opinion that kerning web-fonts should always be handled manually through the use of the letter-spacing property. There may be a good argument against that of which I'm unaware, so feel free to push back if you think that's not a good approach.

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Jan 30, 2015

Member

@jglovier Almost all fonts have kerning properties. Kerning is defined as letter spacing adjustments between certain pairs of letters, so using letter-spacing in general won't bring the same effect (unless you manually adjust letter-spacing for each pair of letters).

Here's a comparison of kerning vs no kerning (notice the Yo pair):

Comparison

Member

alfredxing commented Jan 30, 2015

@jglovier Almost all fonts have kerning properties. Kerning is defined as letter spacing adjustments between certain pairs of letters, so using letter-spacing in general won't bring the same effect (unless you manually adjust letter-spacing for each pair of letters).

Here's a comparison of kerning vs no kerning (notice the Yo pair):

Comparison

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jan 31, 2015

Member

I'm really indifferent as long as it doesn't raise the barrier to entry. @bshackelford, ever used this property before? What do you think are its virtues?

Member

parkr commented Jan 31, 2015

I'm really indifferent as long as it doesn't raise the barrier to entry. @bshackelford, ever used this property before? What do you think are its virtues?

@bshackelford

This comment has been minimized.

Show comment
Hide comment
@bshackelford

bshackelford Feb 1, 2015

@parkr I'd recommend using

font-feature-settings: "kern" 1;
font-kerning: normal;

instead of text-rendering: optimizeLegibility since it's not an official css property and could have performance implications. Here's a great article from Elliot Jay Stocks on the subject: http://www.elliotjaystocks.com/blog/kerning/

bshackelford commented Feb 1, 2015

@parkr I'd recommend using

font-feature-settings: "kern" 1;
font-kerning: normal;

instead of text-rendering: optimizeLegibility since it's not an official css property and could have performance implications. Here's a great article from Elliot Jay Stocks on the subject: http://www.elliotjaystocks.com/blog/kerning/

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 1, 2015

Member

Excellent, thank you!!

What do you think, @alfredxing?

Member

parkr commented Feb 1, 2015

Excellent, thank you!!

What do you think, @alfredxing?

@nternetinspired

This comment has been minimized.

Show comment
Hide comment
@nternetinspired

nternetinspired Feb 2, 2015

Contributor

I'd agree that font-feature is preferable, though this property always needs prefixing: http://caniuse.com/#feat=font-feature

-webkit-font-feature-settings: "kern" 1, "liga" 1;
-moz-font-feature-settings: "kern" 1, "liga" 1;
font-feature-settings: "kern" 1, "liga" 1;

Contributor

nternetinspired commented Feb 2, 2015

I'd agree that font-feature is preferable, though this property always needs prefixing: http://caniuse.com/#feat=font-feature

-webkit-font-feature-settings: "kern" 1, "liga" 1;
-moz-font-feature-settings: "kern" 1, "liga" 1;
font-feature-settings: "kern" 1, "liga" 1;

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Feb 2, 2015

Member

@parkr That looks great, I'll fix it up!

I don't think font-kerning is required, though, since MDN lists compatibility as Firefox only, and Firefox also supports font-feature-settings.

Member

alfredxing commented Feb 2, 2015

@parkr That looks great, I'll fix it up!

I don't think font-kerning is required, though, since MDN lists compatibility as Firefox only, and Firefox also supports font-feature-settings.

@jglovier

This comment has been minimized.

Show comment
Hide comment
@jglovier

jglovier Feb 4, 2015

Member

+💯 for font-feature-settings with prefixes.

Member

jglovier commented Feb 4, 2015

+💯 for font-feature-settings with prefixes.

@tkrotoff

This comment has been minimized.

Show comment
Hide comment
@tkrotoff

tkrotoff Feb 11, 2015

Contributor

@alfredxing In your commit you define font-feature-settings with -webkit, -moz and -o prefixes.
The rest of the code inside site_template does not use -moz and -o (so just -webkit for old Android and iOS devices I guess).

  • -o is useless: recent versions of Opera (>= 15) uses -webkit, -o is for old versions (<= 12). Opera 27 (latest) uses the -webkit prefix for font-feature-settings and v12 does not even support font-feature-settings.
  • -moz is useless: recent versions of Firefox (at least since v16) enable font-feature-settings by default (same for Apple browsers).
  • -webkit is for Chrome/Opera even with the latest versions
  • no prefix is for IE >= 10

I've created a demo: http://plnkr.co/edit/CT1pyW?p=preview (http://run.plnkr.co/plunks/CT1pyW/), check the results on BrowserStack: http://www.browserstack.com/screenshots/f0f9268efda5b4b9a58d802fc9061a08035fa324

So the proper code with clean indentation is:

-webkit-font-feature-settings: "kern" 1;
        font-feature-settings: "kern" 1;

Other than that 👍 specially if it means removing letter-spacing: -1px :)

Contributor

tkrotoff commented Feb 11, 2015

@alfredxing In your commit you define font-feature-settings with -webkit, -moz and -o prefixes.
The rest of the code inside site_template does not use -moz and -o (so just -webkit for old Android and iOS devices I guess).

  • -o is useless: recent versions of Opera (>= 15) uses -webkit, -o is for old versions (<= 12). Opera 27 (latest) uses the -webkit prefix for font-feature-settings and v12 does not even support font-feature-settings.
  • -moz is useless: recent versions of Firefox (at least since v16) enable font-feature-settings by default (same for Apple browsers).
  • -webkit is for Chrome/Opera even with the latest versions
  • no prefix is for IE >= 10

I've created a demo: http://plnkr.co/edit/CT1pyW?p=preview (http://run.plnkr.co/plunks/CT1pyW/), check the results on BrowserStack: http://www.browserstack.com/screenshots/f0f9268efda5b4b9a58d802fc9061a08035fa324

So the proper code with clean indentation is:

-webkit-font-feature-settings: "kern" 1;
        font-feature-settings: "kern" 1;

Other than that 👍 specially if it means removing letter-spacing: -1px :)

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Feb 11, 2015

Member

@tkrotoff I usually look to MDN for this: https://developer.mozilla.org/en/docs/Web/CSS/font-feature-settings#Browser_compatibility. Can I Use lists -webkit and -moz as well, though it doesn't list -o. Do you know of any other resources for finding compatibility tables?

Member

alfredxing commented Feb 11, 2015

@tkrotoff I usually look to MDN for this: https://developer.mozilla.org/en/docs/Web/CSS/font-feature-settings#Browser_compatibility. Can I Use lists -webkit and -moz as well, though it doesn't list -o. Do you know of any other resources for finding compatibility tables?

@kleinfreund

This comment has been minimized.

Show comment
Hide comment
@kleinfreund

kleinfreund Feb 11, 2015

Contributor

What we really need for these kinds of questions is which browser we want to target. /cc @parkr

Firefox has the unprefixed version since 34, so we can drop the -moz- prefix as soon as Firefox for Android has it too. Webkit-based browsers need it as well. Also we don't need the prefix for Opera since their latest version is webkit-based as well.

@alfredxing http://caniuse.com is widely regarded the go-to resource I believe. MDN is good as well.

Contributor

kleinfreund commented Feb 11, 2015

What we really need for these kinds of questions is which browser we want to target. /cc @parkr

Firefox has the unprefixed version since 34, so we can drop the -moz- prefix as soon as Firefox for Android has it too. Webkit-based browsers need it as well. Also we don't need the prefix for Opera since their latest version is webkit-based as well.

@alfredxing http://caniuse.com is widely regarded the go-to resource I believe. MDN is good as well.

@tkrotoff

This comment has been minimized.

Show comment
Hide comment
@tkrotoff

tkrotoff Feb 11, 2015

Contributor

@alfredxing

Can I Use lists -webkit and -moz as well

@kleinfreund

Firefox has the unprefixed version since 34

  1. MDN and CanIUse are nice but not always accurate
  2. Experimentation shows that Firefox has font-feature-settings: "kern" 1 enabled by default since at least v16 on Windows and OS X (edit: and Android - tested by myself)
  3. Link given earlier by bshackelford: "it’s worth noting that kerning is turned on by default in Firefox" (2014-04)

=> so what the point to discuss about -moz again? You have already the answers in my comment above

Contributor

tkrotoff commented Feb 11, 2015

@alfredxing

Can I Use lists -webkit and -moz as well

@kleinfreund

Firefox has the unprefixed version since 34

  1. MDN and CanIUse are nice but not always accurate
  2. Experimentation shows that Firefox has font-feature-settings: "kern" 1 enabled by default since at least v16 on Windows and OS X (edit: and Android - tested by myself)
  3. Link given earlier by bshackelford: "it’s worth noting that kerning is turned on by default in Firefox" (2014-04)

=> so what the point to discuss about -moz again? You have already the answers in my comment above

@nternetinspired

This comment has been minimized.

Show comment
Hide comment
@nternetinspired

nternetinspired Feb 11, 2015

Contributor

What we really need for these kinds of questions is which browser we want to target.

💯 This would be really helpful for the project I think, a clear statement of what browsers the default template has been designed to be compatible with. In general I think that extending backward compatibility to older browsers should be something that is left up to the site builder, but there are certain key components like html5 tags and media-queries that we need to be clear about.

It only needs to be a simple statement like: “Latest versions of major browsers and IE9/10.”

Contributor

nternetinspired commented Feb 11, 2015

What we really need for these kinds of questions is which browser we want to target.

💯 This would be really helpful for the project I think, a clear statement of what browsers the default template has been designed to be compatible with. In general I think that extending backward compatibility to older browsers should be something that is left up to the site builder, but there are certain key components like html5 tags and media-queries that we need to be clear about.

It only needs to be a simple statement like: “Latest versions of major browsers and IE9/10.”

@jglovier

This comment has been minimized.

Show comment
Hide comment
@jglovier

jglovier Feb 11, 2015

Member

💯 This would be really helpful for the project I think, a clear statement of what browsers the default template has been designed to be compatible with. In general I think that extending backward compatibility to older browsers should be something that is left up to the site builder, but there are certain key components like html5 tags and media-queries that we need to be clear about.

👍

  1. MDN and CanIUse are nice but not always accurate

@tkrotoff curious if you have any examples of such inaccuracy? They've always been reliable in my experience.


Personally, I'm of the opinion that a project like this should simply support modern browsers. In the case where we want to do things that even modern browsers require prefixes for, then only should prefixes be used.

Member

jglovier commented Feb 11, 2015

💯 This would be really helpful for the project I think, a clear statement of what browsers the default template has been designed to be compatible with. In general I think that extending backward compatibility to older browsers should be something that is left up to the site builder, but there are certain key components like html5 tags and media-queries that we need to be clear about.

👍

  1. MDN and CanIUse are nice but not always accurate

@tkrotoff curious if you have any examples of such inaccuracy? They've always been reliable in my experience.


Personally, I'm of the opinion that a project like this should simply support modern browsers. In the case where we want to do things that even modern browsers require prefixes for, then only should prefixes be used.

@tkrotoff

This comment has been minimized.

Show comment
Hide comment
@tkrotoff

tkrotoff Feb 11, 2015

Contributor

any examples of such inaccuracy?

See my comments above (*). MDN and CanIUse don't mention that Firefox and Safari have kerning (font-feature-settings: "kern" 1) enabled by default (for quite some time now): there is a lack of precision here.

MDN mentions that Opera >= 18 needs -o prefix for font-feature-settings and CanIUse mentions the -webkit prefix for Opera >= 15.

(It does not mean they are bad resources, just that you need to cross-check sometimes - with real tests for example).

a clear statement of what browsers the default template has been designed to be compatible with

Please open a new issue in order to not pollute this one.

(*) Do people read them or are they unclear??

Contributor

tkrotoff commented Feb 11, 2015

any examples of such inaccuracy?

See my comments above (*). MDN and CanIUse don't mention that Firefox and Safari have kerning (font-feature-settings: "kern" 1) enabled by default (for quite some time now): there is a lack of precision here.

MDN mentions that Opera >= 18 needs -o prefix for font-feature-settings and CanIUse mentions the -webkit prefix for Opera >= 15.

(It does not mean they are bad resources, just that you need to cross-check sometimes - with real tests for example).

a clear statement of what browsers the default template has been designed to be compatible with

Please open a new issue in order to not pollute this one.

(*) Do people read them or are they unclear??

@kleinfreund

This comment has been minimized.

Show comment
Hide comment
@kleinfreund

kleinfreund Feb 11, 2015

Contributor

See my comments above (*). MDN and CanIUse don't mention that Firefox and Safari have kerning (font-feature-settings: "kern" 1) enabled by default (for quite some time now): there is a lack of precision here.

That is not true. caniuse shows the browser support for certain features, not what user agent styles might yield as a value in that browser. Firefox supports font feature settings from version 15 with prefix, and without a prefix up from version 34.

You put a lot of efforts in to your issues and I believe people are appreciating that, but in this case, I feel a bit of an aggression.

Contributor

kleinfreund commented Feb 11, 2015

See my comments above (*). MDN and CanIUse don't mention that Firefox and Safari have kerning (font-feature-settings: "kern" 1) enabled by default (for quite some time now): there is a lack of precision here.

That is not true. caniuse shows the browser support for certain features, not what user agent styles might yield as a value in that browser. Firefox supports font feature settings from version 15 with prefix, and without a prefix up from version 34.

You put a lot of efforts in to your issues and I believe people are appreciating that, but in this case, I feel a bit of an aggression.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 11, 2015

Member

Modern browsers. IE10, Safari, Chrome, Opera, Firefox. That said, this is a sample/example site. It's a starter, not meant to be the end product. So let's not get too crazy here.

Member

parkr commented Feb 11, 2015

Modern browsers. IE10, Safari, Chrome, Opera, Firefox. That said, this is a sample/example site. It's a starter, not meant to be the end product. So let's not get too crazy here.

@nternetinspired

This comment has been minimized.

Show comment
Hide comment
@nternetinspired

nternetinspired Feb 11, 2015

Contributor

Modern browsers. IE10, Safari, Chrome, Opera, Firefox.

👍

Contributor

nternetinspired commented Feb 11, 2015

Modern browsers. IE10, Safari, Chrome, Opera, Firefox.

👍

@alfredxing

This comment has been minimized.

Show comment
Hide comment
@alfredxing

alfredxing Feb 11, 2015

Member

I think, because of the inconsistencies in prefixes, that I'll just keep all 3 prefixes. It's not going to hurt at all.

@parkr Any other suggestions for this PR?

Member

alfredxing commented Feb 11, 2015

I think, because of the inconsistencies in prefixes, that I'll just keep all 3 prefixes. It's not going to hurt at all.

@parkr Any other suggestions for this PR?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 11, 2015

Member

If you could please add font-kerning: normal; to the bottom of each and call it a day. :shipit:

Member

parkr commented Feb 11, 2015

If you could please add font-kerning: normal; to the bottom of each and call it a day. :shipit:

@jekyll jekyll locked and limited conversation to collaborators Feb 11, 2015

@alfredxing alfredxing merged commit 0d60201 into jekyll:master Feb 11, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@jglovier

This comment has been minimized.

Show comment
Hide comment
@jglovier
Member

jglovier commented Feb 12, 2015

@alfredxing 🤘

tkrotoff added a commit to tkrotoff/osteo15.com that referenced this pull request Feb 16, 2015

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.