firebase / FirebaseUI-Android

Optimized UI components for Firebase
https://firebaseopensource.com/projects/firebase/firebaseui-android/
Apache License 2.0
4.63k stars 1.84k forks source link

FirebaseUI 3: Adapter items reload, although there is no change in the data #947

Open NikiFoD opened 7 years ago

NikiFoD commented 7 years ago

I posted this on stackoverflow, but I think its better addressed here.

In FirebaseUI 3 release, I am creating an adapter within my activity using:

.setLifecycleOwner(this)

This starts and stops listening on onStart() and onStop() of my activity. The problem is that the items in the adapter reload even when there is no change in the data, cause onBindViewHolder is called every time the listener starts listening.

So for example, if I just put the app in the background and back in foreground, all items are reloaded (blinking screen). It also messes up with the shared element transition, cause when I return from the DetailsActivity to the MasterActivity, the adapter is reloading the data and the transition is not smooth.

In FirebaseUI 2.x.x release I didn't have this problem (populateViewHolder wasn't called unless there was a change in the data).

Is there a way, other than start listening in onCreate and stop listening in onDestroy to overcome this problem?

Thanks!

SUPERCILEX commented 7 years ago

@NikiFoD Yeah, setLifecycleOwner will clear listeners in onStop. Personally, I have a ViewModel which holds a FirestoreArray beyond config changes which I give to the adapter. When the ViewModel is initialized, I add an empty ChangeEventListener and then remove it in onCleared(). At some point, I'd like to implement a timeout that removes the listener after 30s of non-onStart so as not to waste the user's battery and network, but I haven't gotten around to that yet.

NikiFoD commented 7 years ago

@SUPERCILEX If I understand correctly, this solution without the timeout, is performance-wise (battery and network) the same as start listening in onCreate() and stop in onDestroy() and what was happening in FirebaseUI 2.x.x release, right? (mAdapter.cleanup(); in onDestroy())

On the other hand, if a timeout is added, the Shared Element Transitions will break, if the user stays in the Details screen longer than 30sec and then returns to the master screen (because he had stuff to do in the Details page, or he received a phone call, or returned to the app after a day to the Details screen that he had left it).

Any thoughts? :)

SUPERCILEX commented 7 years ago

@NikiFoD No, keeping your array in a ViewModel means it actually exists beyond onDestroy if the user is still on that screen and a config change occurs. (If they leave the screen completely, the view model will die as expected). This means it's even more performant, but yes, it's essentially like v2.0 which wasn't really a good idea in the first place for performance.

As for the timeout, I actually just saw ProcessLifecycleOwner which does exactly what I want with a 700ms timeout. When the app is put into the background, you could use that to kill your listeners and save the user's battery and then restore them later.

BTW, the manual methods are still there, just renamed: [start/stop]Listening().

Happy coding! 😄

ewaldbenes commented 6 years ago

I ran into the same issue. My app broke when I updated to version 3.

Is it possible to keep the snapshot array when stopListening is called instead of clearing it or would that introduce some other problems?

Any news on that?

SUPERCILEX commented 6 years ago

See https://github.com/firebase/FirebaseUI-Android/issues/998#issuecomment-342333313

samtstern commented 6 years ago

I believe we can close this issue as it's not a bug, just requires some special use to get the desired behavior. Plus it's pretty old.

NikiFoD commented 6 years ago

Hi @samtstern . It might not be a bug in terms of crashing, but the intended behavior is not achieved, unless some serious work is done from the developers. Even the official examples contain the same problem (blinking screen). Other problems include shared element transition breaking for the return transition and scroll position lost in orientation change. @SUPERCILEX details the work that the developers must do and proposes to provide the functionality out of the box in #998 (comment). I believe this would be a great addition to the library and a step forward to utilizing AAC to Firebase. Any chance for reconsidering?

samtstern commented 6 years ago

@NikiFoD good point, we can use this issue to consider improvements.

yashnagda04 commented 5 years ago

any solution?

SUPERCILEX commented 5 years ago

Here's a flushed out version of my comment above: https://proandroiddev.com/fui-rv-state-a66833c3bb88. Let me know what you think! 😊 Any potential improvements?

edgarmacas commented 4 years ago

@SUPERCILEX Thanks for the explanation, at the moment it is the only solution or it has already been solved in firebaseui because I cannot find any solution directly in firebaseui.