taltstidl / AppCompat-Extension-Library

A library that builds on the AppCompat Design Library and provides additional common components such as AccountHeaderView, FloatingActionMenu, CircleImageView, Picker Dialogs, FlexibleToolbarLayout, Delightful Detail Drawables and TypefaceCompat.
Apache License 2.0
304 stars 42 forks source link

Fix forcing use of Roboto for API<21 #19

Closed fountaingeyser closed 8 years ago

fountaingeyser commented 8 years ago

In case the default Sans Serif Font of the device is not Roboto now the custom font of the device (e.g. Choco cooky or Cool Jazz) is used instead of the new Roboto fonts which were introduced with Material Design. Please note that using the custom font only works on API 14 or higher as the Bitmap.sameAs() method requires API 12 or higher. Honeycomb support (or in other words checking for the Droid font) was not added as there are nearly no devices running that platform anymore. Also the README was corrected regarding the incorrect statement that lineSpacing is being set automatically.

fountaingeyser commented 8 years ago

@maniac103 @jahirfiquitiva Maybe this could help you solve part of issue slapperwan/gh4a#283!

maniac103 commented 8 years ago

@fountaingeyser Unless I'm missing something in the description, that's exactly the opposite of what @jahirfiquitiva wanted. He wanted Roboto not to be enforced unless strictly required, so that the CM theme defined font is used instead.

jahirfiquitiva commented 8 years ago

@maniac103

Yeah, forcing roboto is what I DO NOT want. I think apps shouldn't force fonts unless they have something to do with a brand or something (for example in Plaid app, forcing the fonts is ok with me) .... But if you plan to force Roboto, better don't, so themes could override the app font.

fountaingeyser commented 8 years ago

Yes, I know. But this is about not forcing Roboto! In case the default Sans Serif Font of the device is not the Roboto font TypefaceCompat does not return one of the new Roboto fonts but the custom font instead. Propably the term custom font is a bit confusing. With custom font I meant the font the user selected for her or his device!

maniac103 commented 8 years ago

So what's the point of having code to enforce Roboto fonts if that code only enforces Roboto if the system font is already Roboto? :)

fountaingeyser commented 8 years ago

It does make sure the new Roboto fonts are used which were introduced with Material Design!

maniac103 commented 8 years ago

And BTW, that TypefaceUtils.sameAs method looks extremely expensive.

fountaingeyser commented 8 years ago

@jahirfiquitiva Isn't that what you were suggesting in that issue?

fountaingeyser commented 8 years ago

@maniac103 I know that this is more a workaround than a real solution. That's the reason why TypefaceCompat's check for custom fonts is in the static part.

jahirfiquitiva commented 8 years ago

No. I DO NOT want roboto font being forced anywhere in this app. For any reason.

maniac103 commented 8 years ago

...which means it runs every time that particular class is loaded, IOW on app startup, delaying said startup. Not a good idea IMHO.

fountaingeyser commented 8 years ago

@jahirfiquitiva @maniac103 Sorry I missunderstood what that issue was about!

taltstidl commented 8 years ago

I agree, TypefaceCompat shouldn't force the Roboto typeface if the system has a custom font installed (which is the case in many custom roms and even Samsung roms), isn't this what you all suggested and what this code does? @maniac103 Since you're talking about slow startup, any ideas how it could be improved? Thanks all for contributing! :smiley:

maniac103 commented 8 years ago

Not really. What I do in Octodroid (enforce fonts if not all required fonts [medium, condensed, light etc.] are guaranteed to be present, use system fonts if they are) was the best solution I could think of. I don't see a way to get a consistent look and feel of the app e.g. on Samsung KitKat firmware with a custom font applied.

fountaingeyser commented 8 years ago

Still not sure wether I was able to explain what this does! This pull request was about TypefaceCompat trying to mimic Android API 21 (or higher). This means that TypefaceCompat only returns a Roboto font when users want the default font of the device to be used.

fountaingeyser commented 8 years ago

BTW I forgot to check for the Droid Font for API<14.

fountaingeyser commented 8 years ago

@TR4Android Did not add checking for the Droid font as there are nearly no devices running Honeycomb anymore and the Bitmap.sameAs() method cannot be used for platforms before Honeycomb.

taltstidl commented 8 years ago

@fountaingeyser Thanks for integrating support for typeface detection! I've merged your pull request and will make this optional (because of the performance impact it could possibly have) :+1: