Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ViewModel does not persist data through lifecycle changes as expected. #10

Open
githubmonkey opened this issue Apr 7, 2018 · 5 comments

Comments

@githubmonkey
Copy link

Thanks for publishing this sample. I learned a lot from studying your code. However, I am puzzled about one detail.

One of the biggest benefits of using a viewmodel is that model data survives destruction and recreation of the activity during configuration changes. This makes it easier to restore for example the position of the list. Scroll down a long restaurant list, rotate the phone, and watch the list being recreated at the same position.

Currently, your restaurant list does not respect the previous scroll position on rotation. I've added onSaveInstanceState and onRestoreInstanceState to capture the state of the layout manager during rotation. (See code snippet below.) However, this alone is not enough since the adapter is still empty at the time onRestoreInstanceState is called.

The adapter is only populated later through restaurant.observe which was set up in MainActivity.onCreate. This observer is triggered only when the repository has retrieved a new copy of the (unchanged) restaurant list from the firebase query.

IMHO this defeats the purpose of the ViewModel class. This layer is supposed to maintain a link to the data that survives rotation changes etc. By delegating data handling to Firebase the data is recreated after each lifecycle change.

Any idea how to make the restaurant data reference persist through the life time of the viewmodel?

Cheers!

P.S.: Here is the code snipped I tried to capture the layout manager state

@Override
protected void onSaveInstanceState(Bundle outState) {
    super.onSaveInstanceState(outState);

    if (binding.recycler != null) {
        RecyclerView.LayoutManager manager = binding.recycler.getLayoutManager();
        if (manager != null) {
            Parcelable parcel = manager.onSaveInstanceState();
            outState.putParcelable(BUNDLE_LAYOUT, parcel);
        }
    }
}

@Override
protected void onRestoreInstanceState(Bundle savedInstanceState) {
    super.onRestoreInstanceState(savedInstanceState);

    if (savedInstanceState != null) {
        // PROBLEM!! The adapter is not re-populated yet so the layout state is meaningless.
        Log.d("MainActivity", "current adapter size " + binding.recycler.getAdapter().getItemCount());
        
        RecyclerView.LayoutManager manager = binding.recycler.getLayoutManager();
        if (manager != null) {
            Parcelable savedRecyclerLayoutState = savedInstanceState.getParcelable(BUNDLE_LAYOUT);

            manager.onRestoreInstanceState(savedRecyclerLayoutState);
        }
    }
}
@amrro
Copy link
Owner

amrro commented Apr 8, 2018

You are very welcome.
But I guess i stumbled upon the same problem here, and that solved it.

Waiting your feedback....

@githubmonkey
Copy link
Author

Your solution is almost identical to what I did. However, it's not working in this case as the adapter (and with that the LayoutManager) does not have any data yet at the time onRestoreInstanceState() is called.

In fact, I found out after I compiled the initial comment that the problem runs deeper. ANY TIME the data changes on the server (let's say somebody else added a restaurant review), the firestore listener will trigger the adapter data to be replaced and the list view will automatically pop back to the top. Pretty irritating if you are the person just inspecting something at the bottom of the list.

I believe the only viable solution is to disallow firestore to trigger adapter updates and to let the viewmodel cash the firebase data and manage changes.

What do you think?

@thebatu
Copy link

thebatu commented Nov 28, 2018

any update on this issue? and how would you implement cashing firebase data and manage changes as you have pointed ?

Thanks...

@amrro
Copy link
Owner

amrro commented Nov 28, 2018

Hey @thebatu,

Currently, I am working on updating the project to Kotlin. I am using this pattern and I have never faced this issue; however, I will test that over the weekend and let you know.

@thebatu
Copy link

thebatu commented Nov 29, 2018

@amrro thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants