Details when saving scroll position

[kakai248]

I found a problem when saving scroll position due to the way we're doing things. Until now we thought the scroll position was being saved correctly because when we opened a new screen on top of the list screen (the list fragment view is destroyed) and go back, the scroll position was kept. But now we added a theme picker that recreates the activity when the user selects other theme. Upon changing theme, the scroll position was no longer kept.

Looking into it I found that using recyclerView.setControllerAndBuildModels is problematic because it forces the synchronous model build to happen even when we haven't set the models yet (due to LiveData observe being called later). Changing this to recyclerView.setController fixed this for some cases.

The cases that it didn't fix are cases where we build a screen with multiple different data and have multiple submits and requestModelBuilds. Imagine a screen with banners at the top and then the remaining list items. We load them separately because we don't want to wait for the banners to load before showing the already loaded list items. This means we have two separate LiveData, which creates the problem because as soon as one emits, we will call requestModelBuild and it will make the list different from what it was before during the synchronous model build.

There is no issue with epoxy here. I'm trying to find the best way to solve this and if I can leverage something in epoxy to help me.

My only idea right now (which is don't like) is to load everything in a combineLatest but, because this operator only starts emitting when every observable emitted at least once, I'll have to wrap everything in an Optional and use startWith(Optional.empty()) and then join all the Optional in a common model that I can pass to epoxy.

Ideally I would like a way to tell epoxy to group all the requestModelBuilds that happen before the synchronous one so when this last one happens, the list epoxy has is already complete.

Any ideas? Thanks.

[kakai248]

Apparently using requestDelayedModelBuild(1) for every controller submit except for one works. Could this create any problem I'm not seeing or not experiencing right now? Will it depend on the call order of something?

[elihart]

In order for scroll position to be restored the recyclerview content needs to set synchronously before Fragment#onViewStateRestored. This is an Android level requirement that you can't work around, unless you manually store the position and scroll to it later.

Given that view state restoration requirement, you have to have all of your data loaded synchronously when the fragment view is created, and epoxy model building must happen synchronously so models are ready before onViewStateRestored.

It sounds like your data layer needs to be changed to memory cache this information and be able to delivery the last data synchronously when the view is created

[kakai248]

@elihart That is already working with LiveData. The issue is that since we have different operations not depending on each other, each one of them emits to a different LiveData. Each observe on a LiveData will call controller.submitSomething which internally stores the new data inside the controller and calls requestModelBuild.

Upon view recreation, the LiveDatas will give their cached value, so there will be more than one requestModelBuild. The problem is that the first call to requestModelBuild will be the synchronous one (the one that RecyclerView needs to retain scroll position) and the remaining ones will be async. During the synchronous one the whole data won't be there yet, only the data that results from one of the LiveData's.

[elihart]

Gotcha. I would recommend not having live data trigger a model build unless the fragment is in the started lifecycle state. Then you can manually request the first model build in onViewCreated.

Ideally you have a base fragment and some infrastructure to take care of this automatically - for example we use MvRx which does this automatically.

EpoxyController could be modified to add something like freezeBuildRequests which would prevent any requests from starting until you unfreeze. I'm not sure it is necessary to do that though when it seems better to address this from the data/infra layer of you fragment

[kakai248]

@elihart Thanks for answering. When we receive the LiveData values the fragment is already STARTED.

I've looked into MvRx. There all your screens have an epoxy controller so that's easier to do. I do not want to go that route. I've thought of wrapping all the requestModelBuild calls and post them similarly to how it's done on MvRx using this. But since the fragment is already STARTED, I'm not really sure how this posted call executes before the recycler view layout pass. I don't see any calls to invalidate directly, only to postInvalidate. Why did you go with the approach of calling the runnable synchronously in epoxy instead of posting it in a similar way to MvRx?

[elihart]

Why did you go with the approach of calling the runnable synchronously in epoxy instead of posting it in a similar way to MvRx?

The posted runnable only runs after the fragment is "started", which is after view state restoration, which is too late to restore scroll position

[kakai248]

But the LiveData observer only receives the values if the fragment is started. And it keeps scroll. So it still runs before view state restoration. There's something I'm missing.

[elihart]

   f.performActivityCreated(f.mSavedFragmentState);
        dispatchOnFragmentActivityCreated(f, f.mSavedFragmentState, false);
        if (f.mView != null) {
            f.restoreViewState(f.mSavedFragmentState);
        }
        f.mSavedFragmentState = null;
    }
    // fall through
case Fragment.ACTIVITY_CREATED:
    if (newState > Fragment.ACTIVITY_CREATED) {
        if (DEBUG) Log.v(TAG, "moveto STARTED: " + f);
        f.performStart();
        dispatchOnFragmentStarted(f, false);

from fragment manager source. You can see restoreViewState is done before start is dispatched

[kakai248]

I debugged the execution and you're right. It goes (I'm using fragment 1.2.0-rc01 so the call names are a bit different):

1. activityCreated() -> this is where I subscribe to the LiveData
2. restoreViewState()
3. start() -> This is where the LiveData emits and subsequently where I'll call controller.requestModelBuild()

So restoreViewState always happens before the controller runs the synchronous model build. I'm still missing something.

[manueldidonna]

I solved that problem by building the models when all the data had been set at least once (in my case this means that they weren't null)

// onRestoreState
epoxyController.onNextModelBuild {
    recycler.layoutManager?.onRestoreInstanceState(bundle[SAVED_STATE])
}

inline fun EpoxyController.onNextModelBuild(crossinline action: (DiffResult) -> Unit) {
    addModelBuildListener(object : OnModelBuildFinishedListener {
        override fun onModelBuildFinished(result: DiffResult) {
            removeModelBuildListener(this)
            action(result)
        }
    })
}

[kakai248]

That didn't work for us because we didn't need all data to fill the screen. Some data could come later and it wouldn't be a problem. So null checking is not enough.

What we are working on is replacing all the LiveData with a single LiveData<ScreenData> that contains all the data that we want to pass to the controller. Submitting this ScreenData is the only place where we requestModelBuild. Inside this data we have some nullable objects (the optional data). In the ViewModel we subscribe to all the data sources in a combineLatest and map into this ScreenData. For the nullable fields, we have a startsWith. From our tests it keeps scroll properly.

This also mean't we had to build some behavior on our base controllers to enforce this pattern. I don't like it, but we didn't find any other way.

This is weird because looking at MvRx, it has the same problem. I even tried their solution of calling invalidate in onActivityCreated but this happens too soon. LiveData hasn't emitted by this point. It will only emit in onStart. So there's no hook between LiveData emission and view restoration.

[aantunovic]

I stumbled on medium link Looks like since RecyclerView 1.2.0 we can set StateRestorationPolicy . Maybe this can be leveraged, if not already...