[#12] Support for webfonts. #303

Merged
merged 4 commits into from Oct 3, 2012

Conversation

Projects
None yet
3 participants
Member

giorgian commented Oct 2, 2012

Load webfonts and explicitly set which ones have been successfully loaded and which have not, with a timeout, thus allowing treesaver to know exactly what the columns will look like.

giorgian added some commits Sep 13, 2012

@giorgian giorgian [#12] Support for webfonts.
The purpose is to have a reliable way to detect which fonts are ready
when treesaver starts, and to prevent fonts loaded afterward from
messing the measurements up.

The actual font loading is delegated to an adapter; currently, the
only provided one is googleadapter, which uses WebFont Loader.
be12827
@giorgian giorgian [#12] Example for treesaver.fonts (copied from simple). 7458521
@giorgian giorgian [#12] Updated deps. cc99cf0
@giorgian giorgian [#12] Set a shorter timeout than the one used internally by WebFont l…
…oader to

prevent treesaver from unbooting if font loading takes too much time.
7b7456c
Member

bramstein commented Oct 2, 2012

I don't want to stall this pull request, but the reason we never integrated the Webfontloader is that its events are not reliable. There are many cases where the active event fires before the font is actually applied resulting in an incorrect layout. I've reported this: typekit/webfontloader#54 and implemented a proof of concept font loader here: https://github.com/bramstein/fonzie

Member

andreacampi commented Oct 2, 2012

Yes, but even in that case we would not be worse off than without it, would we? :)

Note that this is exactly why we implemented this using a separate adapter class: it would be possible to swap the Google fontloader for a better one. We'll have a look at yours.

So I take it you're ok with me merging this?
With this and the Chrome fix, I'm going to go ahead with a 0.9.4 release.

Member

bramstein commented Oct 2, 2012

Yes, but even in that case we would not be worse off than without it, would we? :)

I don't know. I think having an unreliable feature is a problem. If the active event fires before the font is applied and treesaver will use incorrect column heights and possible have text cut off at wrong positions.

I agree that it is a good idea to use an adapter class.

I'll leave it up to you to decide whether or not to merge this in.

Member

andreacampi commented Oct 2, 2012

Yes, but even in that case we would not be worse off than without it, would we? :)

I don't know. I think having an unreliable feature is a problem. If the active event fires before the font is applied and treesaver will use incorrect column heights and possible have text cut off at wrong positions.

I agree on that, but my point is: people are already using Webfonts, and without these changes it is quite unreliable indeed. We have observed exactly what you mention.

So the main change here is not so much adding a feature, reliable or not; it's really only adding some extra control.
With these changes applied we have had zero reports of failures. It may not be the perfect fix, but I do think it's a definite improvement.

Member

bramstein commented Oct 2, 2012

Ok, good point. People probably are hacking in webfont support themselves already, so this pull request might actually make it a bit easier for them.

I'm fine with merging it in.

andreacampi merged commit 99e3549 into Treesaver:master Oct 3, 2012

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