chrisjenx / Calligraphy

Custom fonts in Android the easy way...
Apache License 2.0
8.59k stars 1.09k forks source link

Not working in Lollipop and AppCompat library #84

Closed ruqqq closed 9 years ago

ruqqq commented 10 years ago

On Android 4.4 with AppCompat Library, Calligraphy 1.1.+ doesn't work. However, Calligraphy 0.7.+ works fine.

On Android 5.0 with AppCompat Libary, both Calligraphy 1.1.+ and 0.7.+ does not seem to work.

chrisjenx commented 10 years ago

@ruqqq which app compat lib? As I am using support-v4:21.0.0 with no problems.

justinelsberry commented 10 years ago

@chrisjenx not sure its the same issue, but for me the default font set through CalligraphyConfig.initDefault(...) is not applied when using appcompat-v7:21.0.0

chrisjenx commented 10 years ago

@justinelsberry OK. I'll double check tomorrow.

ruqqq commented 10 years ago

@chrisjenx appcompat-v7:21.0.0 library. nothing to do with support library I suppose.

chrisjenx commented 10 years ago

@ruqqq @justinelsberry I've updated the Sample to v21 and everything seems to be working for me. Am I missing something obvious?

chrisjenx commented 10 years ago

@ruqqq @justinelsberry Sorry just seen it, yes you're right

justinelsberry commented 10 years ago

@chrisjenx it does appear to load the font when the layout was inflated manually. e.g. from an adapter.

chrisjenx commented 10 years ago

@justinelsberry yeah I saw that, I ignores the default font very odd.

chrisjenx commented 10 years ago

@justinelsberry @ruqqq Further investigation shows that its just the ActionBar/Toolbar not getting set correctly. I see TextView's inside layouts getting set correctly by the initDefault().

Can you both confirm?

ChrisMCMine commented 10 years ago

After upgrading build tools to 21.0.1, compileSdkVersion to 21 and all support libraries to 21.+ I can confirm that setting the font in the Application class' onCreate using CalligraphyConfig.initDefault() and manually using fontPath="font.ttf" in xml doesn't work.

Btw: Calligraphy version 1.1.+ is added as a dependency

chrisjenx commented 10 years ago

@ChrisMCMine try 1.2.0. I've already fixed this.

n-thilak commented 10 years ago

@chrisjenx Hi Chris, I'm not able to make it work. Have updated to 1.2.0 already.

Have the main theme derived from AppCompat as follows:

  <style name="AppTheme"
        parent="Theme.AppCompat.Light">
        <item name="android:textAppearance">@style/AppTheme.TextAppearance</item>
 </style>

Have my TextAppearance as follows:

    <style name="AppTheme.TextAppearance" parent="android:TextAppearance">
        <item name="android:textStyle">normal</item>
        <!-- Custom Attr-->
        <item name="fontPath">Roboto-Light.ttf</item>
    </style>

    <style name="AppTheme.TextAppearance.Bold" parent="AppTheme.TextAppearance">
        <!-- Custom Attr-->
        <item name="fontPath">Roboto-Medium.ttf</item>
        <item name="android:textStyle">bold</item>
    </style>
chrisjenx commented 10 years ago

Yeah that won't work. Text Appearance applied to the theme is never looked at, you need to either apply the TextAppearance to the View xml or the Style xml, I should probably make that clearer.

n-thilak commented 10 years ago

@chrisjenx This style was working fine ( even in L Preview), support-appcompat is causing this?.

And yes, I'm applying the AppTheme.TextAppearance to each TextView's style.

chrisjenx commented 10 years ago

Oh really, that impl was never tested... mmm OK. I'll check then.

On 20 October 2014 18:02, ngenhit notifications@github.com wrote:

@chrisjenx https://github.com/chrisjenx This style was working fine ( even in L Preview), support-appcompat is causing this?.

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-59798316 .

chrisjenx commented 10 years ago

@ngenhit so this has only stopped it working on 1.2.0?

n-thilak commented 10 years ago

@chrisjenx Just tested the code again. TextAppearance method is working. Everything's working fine except for Views in fragment.

Should i be injecting into each fragment?.

Edit:

Solved it when i used getActivity().getLayoutInflater() instead of the param1 (inflater) of the onCreateView() in the fragment. Is this the correct approach?

@Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container,
                             Bundle savedInstanceState) {

        return   mInflater.inflate(R.layout.fragment_overview_summary, container, false);
    }
ened commented 10 years ago

Same issue here and still present with 1.2.0. The key issue is the v21 AppCompat library. The issue only occurs in FragmentActivity sub classes.

chrisjenx commented 10 years ago

OK thanks for specifying that.

Question does the sample app work for you guys? On 21 Oct 2014 04:48, "Sebastian Roth" notifications@github.com wrote:

Same issue here and still present with 1.2.0. The key issue is the v21 AppCompat library. The issue only occurs in FragmentActivity sub classes.

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-59875251 .

chrisjenx commented 10 years ago

@ngenhit Ohh really? That is interesting.

But that makes no sense. The sample as you can see for yourself is:

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle state) {
    return inflater.inflate(R.layout.fragment_main, container, false);
}

And that works fine.

It also makes no difference between using the inflater param or using getActivity().getLayoutInflater() works both ways.

Someone is going to need to give me a full example.

mandeep-singh-ck commented 10 years ago

Just chiming in, I am not using the appCompact support lib but I am using FragmentActivity from the supportv4 lib. It does not work for me anymore. Was working before I updated to v21.

chrisjenx commented 10 years ago

@mandeepsingh-ck I'm assuming you mean support-v4:21?

mandeep-singh-ck commented 10 years ago

Yes. I am compiling with Google Apis 21, support-v4:21

LloydBlv commented 10 years ago

@chrisjenx the same problem here and I agree with @ngenhit ,the font is not applied to textviews inside fragments but the ToolBar and actionbar are doing fine

chrisjenx commented 10 years ago

Ok great thanks guys for all the extra info. I'll see if I can get to the bottom of this today. On 22 Oct 2014 09:57, "LloydBlv" notifications@github.com wrote:

@chrisjenx https://github.com/chrisjenx the same problem here,the font is not applied to textviews inside fragments but the ToolBar and actionbar are doing fine

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-60055028 .

LloydBlv commented 10 years ago

@chrisjenx Thanks, the fontPath="fonts/yekan.ttf" does not work in fragments by the way

chrisjenx commented 10 years ago

Yeah which is annoying as it works in the demo. I've been trying to reproduce it but struggling. How do you guys instantiate your fragments? XML or from code? On 22 Oct 2014 10:23, "LloydBlv" notifications@github.com wrote:

@chrisjenx https://github.com/chrisjenx Thanks, the fontPath="fonts/yekan.ttf" does not work in fragments by the way

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-60057886 .

n-thilak commented 10 years ago

@chrisjenx Sorry for the delayed response. Let me just take your sample code and try to make the changes ( which I think is breaking the lib ). Will get back to you in couple of minutes.

LloydBlv commented 10 years ago

@chrisjenx I do it in the code using the lines below: public static MainFragment newInstance(String serverResponse){ MainFragment mainFragment = new MainFragment_(); Bundle bundle = new Bundle(); bundle.putString(SplashActivity.SERVER_PRE_RESPONSE,serverResponse); mainFragment.setArguments(bundle); return mainFragment; }

  compile 'com.android.support:appcompat-v7:21.0.0'
    compile 'uk.co.chrisjenx:calligraphy:1.2.0'

Does androidAnnotations has anything to do with this?

chrisjenx commented 10 years ago

Possibly are you able to try without them I.e. break the example? On 22 Oct 2014 11:12, "LloydBlv" notifications@github.com wrote:

@chrisjenx https://github.com/chrisjenx I do it in the code using the lines below: public static MainFragment newInstance(String serverResponse){ MainFragment mainFragment = new MainFragment_(); Bundle bundle = new Bundle(); bundle.putString(SplashActivity.SERVER_PRE_RESPONSE,serverResponse); mainFragment.setArguments(bundle); return mainFragment; }

compile 'com.android.support:appcompat-v7:21.0.0' compile 'uk.co.chrisjenx:calligraphy:1.2.0'

Does androidAnnotations has anything to do with this?

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-60063310 .

n-thilak commented 10 years ago

@chrisjenx I just ran the sample application. lib is failing to apply fonts when the fragment is instantiated dynamically through:

 FragmentManager fragmentManager = getSupportFragmentManager();
        fragmentManager.beginTransaction()
                .replace(R.id.container, PlaceholderFragment.newInstance())
                .commit();

However as a workaround, if I apply the layout inflater from getActivity() in the fragment, it works fine. This works out

 @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle state) {
        return getActivity().getLayoutInflater().inflate(R.layout.fragment_main, container, false);
    }

This does not:

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle state) {
        return inflater.inflate(R.layout.fragment_main, container, false);
    }
chrisjenx commented 10 years ago

OK managed to replicate. Investigating.

chrisjenx commented 10 years ago

Well utter failure in the support lib, there is no source yet (Not until Lollipop reaches ASOP). So I have no code for reference.

It seems to clone the LayoutInflater in the FragmentManager for the Fragments (But I can't confirm that). Thusly it sets a different context (and factory) over CalligraphyFactory.

Looks like getActivity().getLayoutInflater() is the way to fix it for now. FYI. thats the same object that onCreateView gives you anyway.

For whatever reason they have changed this (I think for Transitions).

I can only suggest the work around for now. Apologies about that.

ChrisMCMine commented 10 years ago

So we can't expect a fix until the release of the Lollipop source code? 😕

chrisjenx commented 10 years ago

I've been talking to @chrisbanes about it. Unless he has a suggestion before then, its very difficult as am guessing from errors what is going on.

ened commented 10 years ago

The source code for the support lib could be found in $ANDROID_HOME/extras/android/m2repository/com/android/support/support-v4/21.0.0/support-v4-21.0.0-sources.jar

FragmentManager at line 920 & line 947 will create the Fragment view like this:

f.mView = f.performCreateView(f.getLayoutInflater(
    f.mSavedFragmentState), null, f.mSavedFragmentState);

When you then check out the default implementation of Fragment#getLayoutInflater(Bundle savedInstanceState), you see this:

    /**
     * @hide Hack so that DialogFragment can make its Dialog before creating
     * its views, and the view construction can use the dialog's context for
     * inflation.  Maybe this should become a public API. Note sure.
     */
    public LayoutInflater getLayoutInflater(Bundle savedInstanceState) {
        LayoutInflater result = mActivity.getLayoutInflater().cloneInContext(mActivity);
        getChildFragmentManager(); // Init if needed; use raw implementation below.
        result.setFactory(mChildFragmentManager.getLayoutInflaterFactory());
        return result;
    }

The "cloneInContext" method perhaps doesn't honour the ContextWrapper as it's used by Calligraphy?

Anyhow, once I overwrite the getLayoutInflater method in my Fragment like this:

    @Override
    public LayoutInflater getLayoutInflater(Bundle savedInstanceState) {
        return getActivity().getLayoutInflater();
    }

Then it can work, Calligraphy is back on.

Maybe it helps us to find a clean solution. I'm still testing for side effects.

chrisjenx commented 10 years ago

Ahh nice one.

I was trying to override setFactory but of course if they are cloning then it won't have our factory set on it primarily.

I might be able to do something to work around this now.

Not sure the reason they decided to do this change now. I'll grab one of the google guys at Droidcon see if they know more about it. On 29 Oct 2014 03:06, "Sebastian Roth" notifications@github.com wrote:

The source code for the support lib could be found in $ANDROID_HOME/extras/android/m2repository/com/android/support/support-v4/21.0.0/support-v4-21.0.0-sources.jar

FragmentManager at line 920 & line 947 will create the Fragment view like this:

f.mView = f.performCreateView(f.getLayoutInflater( f.mSavedFragmentState), null, f.mSavedFragmentState);

When you then check out the default implementation of Fragment#getLayoutInflater(Bundle savedInstanceState), you see this:

/**
 * @hide Hack so that DialogFragment can make its Dialog before creating
 * its views, and the view construction can use the dialog's context for
 * inflation.  Maybe this should become a public API. Note sure.
 */
public LayoutInflater getLayoutInflater(Bundle savedInstanceState) {
    LayoutInflater result = mActivity.getLayoutInflater().cloneInContext(mActivity);
    getChildFragmentManager(); // Init if needed; use raw implementation below.
    result.setFactory(mChildFragmentManager.getLayoutInflaterFactory());
    return result;
}

The "cloneInContext" method perhaps doesn't honour the ContextWrapper as it's used by Calligraphy?

Anyhow, once I overwrite the getLayoutInflater method in my Fragment like this:

@Override
public LayoutInflater getLayoutInflater(Bundle savedInstanceState) {
    return getActivity().getLayoutInflater();
}

Then it can work, Calligraphy is back on.

Maybe it helps us to find a clean solution. I'm still testing for side effects.

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-60867464 .

chrisjenx commented 10 years ago

Boom fixed it!

Hat tip to @ened for the nudge in the right direction.

1.2.1-SNAPSHOT just been pushed.

justinelsberry commented 10 years ago

@chrisjenx Testing 1.2.1-SNAPSHOT. It now appears to work with fragment transactions but layouts inflated in my adapters that were working previously are not. :(. Using something like: LayoutInflater.from(getContext()).inflate(...) in an ArrayAdapter.

chrisjenx commented 10 years ago

@justinelsberry pushed another snapshot, try that and let me know.

justinelsberry commented 10 years ago

@chrisjenx Still not working for me. This is the latest snapshot i see: calligraphy-1.2.1-20141029.163410-1

chrisjenx commented 10 years ago

Refresh your dependencies --refresh-dependencies for gradle it should be -2 at the end, [https://oss.sonatype.org/content/repositories/snapshots/uk/co/chrisjenx/calligraphy/1.2.1-SNAPSHOT/]() /cc @justinelsberry

justinelsberry commented 10 years ago

@chrisjenx Sorry about that I did refresh but I am proxying through another sonatype server and it didn't update the index. Rebuilt the index and now its working great! Thanks for all your help.

chrisjenx commented 10 years ago

@justinelsberry no worries! Good to hear. If some more people could have a look through and give it a go before I merge and release that would be great! All in time for Droidcon tomorrow :+1:

lvialle commented 10 years ago

Any news about this? I'd like to try the snapshot version but I have no idea on how to set it to snapshot in my gradle build

n-thilak commented 10 years ago

You can try changing the version number to Snapshot build code 1.2.1-SNAPSHOT instead of the 1.2.1+ you're having now. I haven't updated to Snapshot yet. But this should work. On Nov 8, 2014 5:16 PM, "Ludovic Vialle" notifications@github.com wrote:

Any news about this? I'd like to try the snapshot version but I have no idea on how to set it to snapshot in my gradle build

— Reply to this email directly or view it on GitHub https://github.com/chrisjenx/Calligraphy/issues/84#issuecomment-62255041 .

lvialle commented 10 years ago

unfortunately that is not enough: Error:Failed to find: uk.co.chrisjenx:calligraphy:1.2.1-SNAPSHOT

Edit: I downloaded the .jar (SNAPSHOT-2) and for me Calligraphy is still not working

ruqqq commented 10 years ago

I can't find uk.co.chrisjenx:calligraphy:1.2.1-SNAPSHOT either. Forgive my noobness, where to I get the jar?

chrisjenx commented 10 years ago

The snapshot repos is : [https://oss.sonatype.org/content/repositories/snapshots/]().

Add it like:

maven { url "https://oss.sonatype.org/content/repositories/snapshots/" }

into you dependencies after jcenter or mavenCentral

ruqqq commented 10 years ago

Thanks. Tested with Android Annotations + App Compat + Android 4.4/5.0. Looks like things are working fine now! Great work.

Just a side note, perhaps because I'm using AA, I didn't need the support-v4:21.0.0 fix:

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle state) {
    return getActivity().getLayoutInflater().inflate(R.layout.fragment_main, container, false);
}