search

MvvmCross / MvvmCross-AndroidSupport

Android support library packages for MvvmCross: The .NET MVVM framework for cross-platform solutions.
http://mvvmcross.com
15 stars 0 forks source link

AppCompat view switching seems to be reversed #291

Open Ryan-Palmer opened 8 years ago

Ryan-Palmer commented 8 years ago

I understand that Mvx switches out standard views for app compat versions behind-the-scenes.

I just found a weird bug where one of my team were attempting to cast a radio group to the app compat version. I only noticed as when I was running it on Gingerbread to test it I got an invalid cast exception, saying that it couldn't cast the standard one to app compat (which rang alarm bells straight away).

I then changed the cast to a standard MvxRadioGroup and it worked fine, but when running on Lollipop it now complains that it can't cast the AppCompat version to the standard version.

This looks like the views are being switched, but the wrong way round, as I get the compat one on Lollipop and the standard on Gingerbread. Gingerbread does seem to work ok with that standard one though.

Just to be clear, the layout file only uses the standard MvxRadioGroup. I guess the other dev had put in the app compat cast to get around the bug, but it just pushed it elsewhere.

Our setup file inherits from MvxAppCompatSetup as I believe that does a lot of the wiring under the hood now. Could it be related to this? It's pretty slim otherwise, just a few custom bindings and view assemblies, plus the service class registration. I don't want to bombard you with useless code so if there is anything that might be useful just say.

Ryan-Palmer commented 8 years ago

Anything to do with this change? Looking through it myself to try and spot anything. https://github.com/MvvmCross/MvvmCross-AndroidSupport/pull/159/files

Ryan-Palmer commented 8 years ago

It seems to be this which does the swapping: https://github.com/MvvmCross/MvvmCross-AndroidSupport/blob/18781217b9abba83db2d3d73008711b08d27a0e9/MvvmCross.Droid.Support.V7.AppCompat/MvxAppCompatActivityHelper.cs

This is only used by Mvx when creating MvxAppCompatActivity or MvxCachingFragmentCompatActivity views. Should we be using it in the fragment's OnCreateView override as well? I am really trying to follow it through but the fragment handling just baffles me if I am honest.

kjeremy commented 8 years ago

Are you using a custom fragment?

Ryan-Palmer commented 8 years ago

All the fragments inherit from MvxFragment if that's what you mean.

kjeremy commented 8 years ago

If you inflate an EditText does it come back as AppCompatEditText?

kjeremy commented 8 years ago

I will say that there used to be an API check in here and now i'm wondering where that went.

Ryan-Palmer commented 8 years ago

I tried doing var et = FindViewById<EditText>(R.id.blah)'intoFindViewById<AppCompatEditText> and it seemed to allow it, on both versions. EditText isn't in that MvxAppCompatActivityHelper switch though is it?

Did you see that other commit I linked to? It seemed to change a lot of stuff like removing the MvxAppCompatActivityHelper altogether, but it's still in the project now. There is also MvxAppCompatViewFactory which seems do do a similar job (including edit texts and a version check). It does Radio buttons but not groups. I am so confused!

kjeremy commented 8 years ago

So essentially we are trying to do the equivalent of: https://github.com/android/platform_frameworks_support/blob/432f3317f8a9b8cf98277938ea5df4021e983055/v7/appcompat/src/android/support/v7/app/AppCompatDelegateImplV7.java#L997-L1013 which calls https://github.com/android/platform_frameworks_support/blob/432f3317f8a9b8cf98277938ea5df4021e983055/v7/appcompat/src/android/support/v7/app/AppCompatViewInflater.java#L80

Now those handle the base things like EditText to AppCompatEditText. We mimic that behavior for our special Mvx versions of those classes (ie MvxAppCompatSpinner). I think you're going to need to attach a sample.

Regarding the version check that used to be there: I don't think it applies or the docs are wrong since they will get into those calls with API 7+ anyway. That's probably why the check doesn't exist anymore.

Ryan-Palmer commented 8 years ago

Ok, i'll try and get one together when I get a moment, thanks for your help :)