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

Custom RecyclerViewAdapter: MvxAndroidBindingContextHelpers.Current() is null #222

Open sescandell opened 8 years ago

sescandell commented 8 years ago

Hi,

Steps to reproduce

  1. Clone my forked repo on branch issue-customRecyclerAdapter
  2. Run the application
  3. Go to Example RecyclerView screen

    Expected behavior

The screen should display a list of items using a custom adapter (see differences here)

Actual behavior

NullReference exception is thrown

Configuration

Version: 4.1.0

What's happening

On adapter creation, the workflow is the following:

ExampleRecyclerViewFragment::OnCreateView is calling new MyCustomAdapter()

MyCustomAdapter constructor is calling its base constructor from MvxRecyclerAdapter.

MvxRecyclerAdapter constructor without any argument is calling MvxAndroidBindingContextHelpers.Current() wich is null. So further call to BindingContext.LayoutInflaterHolder property into MyCustomAdapter is crashing.

The only way to make it works right now is to explicitly set the BindingContext on call on new MyCustomAdapter() using the one from the Fragment casting it to IMvxAndroidBindingContext. Something like new MyCustomAdapter(BindingContext as IMvxAndroidBindingContext) instead of simple new MyCustomAdapter().

My concerns are:

bspinner commented 8 years ago

Did you make any progress on this? We've stumbled upon the same thing.

thefex commented 8 years ago

as workaround pass (IMvxAndroidBindingContext)Binding Context from Activity/Fragment.

sescandell commented 8 years ago

@bspinner as suggested in my issue, for now, the only workaround I found is to use the BindingContext from the Fragment itself:

The only way to make it works right now is to explicitly set the BindingContext on call on new MyCustomAdapter() using the one from the Fragment casting it to IMvxAndroidBindingContext. Something like new MyCustomAdapter(BindingContext as IMvxAndroidBindingContext) instead of simple new MyCustomAdapter().

I didn't had time to look deeper onto that issue (and my knowledge of how binding is working behind the scene is for now to limited)

Maybe @martijn00 or/and @Cheesebaron have information about that.

martijn00 commented 8 years ago

I don't know why this is happening. I personally used the workaround described here before. Could you look into the source of this problem?

bspinner commented 8 years ago

After making sure our app isn't using too much memory, this problem vanished (for now). Some garbage collection foo?

kjeremy commented 8 years ago

I just hit this too. There must be something about layout inflation that is hiding this issue with the normal adapter. I know that we get the binding context here: https://github.com/MvvmCross/MvvmCross/blob/16f401378e11bc57b3ba08996e8d7ff114fdad14/MvvmCross/Binding/Droid/Views/MvxLayoutInflater.cs#L121 during inflation and it's not null (otherwise bindings wouldn't work).

Are there other cases where MvxAndroidBindingContextHelpers.Current() returns null when it shouldn't? When should MvxAndroidBindingContextHelpers.Current() NOT be null?

kjeremy commented 8 years ago

The stack is pushed and popped here: https://github.com/MvvmCross/MvvmCross/blob/3c735adc534a5df2d4730e9d58a08f7863c30cee/MvvmCross/Binding/Droid/BindingContext/MvxAndroidBindingContext.cs#L59-L68 during inflation.

            using (new MvxBindingContextStackRegistration<IMvxAndroidBindingContext>(this))
            {
                var layoutInflater = this._layoutInflaterHolder.LayoutInflater;
                {
                    // This is most likely a MvxLayoutInflater but it doesn't have to be.
                    // It handles setting the bindings and interacts with this instance of
                    // MvxAndroidBindingContext through the use of MvxAndroidBindingContextHelpers.Current().
                    return layoutInflater.Inflate(resourceId, viewGroup, attachToRoot);
                }
            }

This is why it works during normal inflation.

So the question is: What is the lifetime of the stack? Should MvxAndroidBindingContextHelpers.Current() return non-null while an MvxActivity is on top of the activity stack? I'm inclined to say no because we can mix with non-Mvx activities but I'm not sure.