search

JakeWharton / SdkSearch

An Android app and Chrome extension for searching the Android SDK documentation.
Apache License 2.0
2.06k stars 176 forks source link

RecyclerView's scroll position reset on config change #39

Closed oldergod closed 6 years ago

oldergod commented 6 years ago

sdksearch_config_change

JakeWharton commented 6 years ago

Far too much happens in onCreate, but I'll get it refactored eventually. I need to get a layer teased out between something pushing view models and the rendering so that rotation doesn't even trigger a new query.

oldergod commented 6 years ago

I need to get a layer teased out between something pushing view models and the rendering so that rotation doesn't even trigger a new query.

FWIW, I believe the recyclerview scroll position resets because of the thread switch; after rotation, the layout adapter applies the saved scroll position to a new, not yet defined, list of items.

JakeWharton commented 6 years ago

Right. So a view model available synchronously after rotation will solve that.

On Wed, Jan 17, 2018 at 11:33 PM Benoît Quenaudon notifications@github.com wrote:

I need to get a layer teased out between something pushing view models and the rendering so that rotation doesn't even trigger a new query.

FWIW, I believe the recyclerview scroll position resets because of the thread switch; after rotation, the layout adapter applies the saved scroll position to a new, not yet defined, list of items.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/JakeWharton/SdkSearch/issues/39#issuecomment-358534974, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEfjTTvkUTZJKy31XQF3HmgDTSkOqks5tLsmRgaJpZM4Rg5TW .

amanjeetsingh150 commented 6 years ago

Hi @JakeWharton , First of all thank you for this amazing project. Can we use onSavedInstanceState() method of the layout manager to save state of recycler view when activity is rotated in onSavedInstanceState method of activity and then restoring it appropriately. It will be less tedious I think. Like mentioned in this link: https://stackoverflow.com/questions/28236390/recyclerview-store-restore-state-between-activities/28262885 Do tell me if I am wrong somewhere. If this approach seems OK to you I will send a quick PR?

JakeWharton commented 6 years ago

That should be the default behavior. The problem is that we set an empty adapter before the actual data shows up on the next frame. A view model decoupled from the activity lifecycle will make the problem magically go away.

On Fri, Jan 26, 2018 at 7:19 PM Amanjeet Singh notifications@github.com wrote:

Hi @JakeWharton https://github.com/jakewharton , First of all thank you for this amazing project. Can we use onSavedInstanceState() method of the layout manager to save state of recycler view when activity is rotated in onSavedInstanceState method of activity and then restoring it appropriately. It will be less tedious I think. Like mentioned in this link: https://stackoverflow.com/questions/28236390/recyclerview-store-restore-state-between-activities/28262885 Do tell me if I am wrong somewhere. If this approach seems OK to you I will send a quick PR?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/JakeWharton/SdkSearch/issues/39#issuecomment-360941070, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEbidt3BRyqRBJZk8XraGzIJj-0BCks5tOmuYgaJpZM4Rg5TW .

amanjeetsingh150 commented 6 years ago

Agreed, with this case I'll definitely try the solution with viewmodel and will send PR if I solved it. Thanks :)

JakeWharton commented 6 years ago

That's lowercase "view model" not the architecture component ViewModel. I don't understand the purpose of the architecture component existing...

On Fri, Jan 26, 2018 at 7:38 PM Amanjeet Singh notifications@github.com wrote:

Agreed, with this case I'll definitely try the solution with ViewModel. Thanks :)

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/JakeWharton/SdkSearch/issues/39#issuecomment-360943242, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEaAGlGSrv-Dx7LDSttRbhy4-iFOpks5tOm_ngaJpZM4Rg5TW .

amanjeetsingh150 commented 6 years ago

Sorry my bad the small one I was also referring to the same but typed wrong. PS: Updated that 👍

michaldrabik commented 6 years ago

That's lowercase "view model" not the architecture component ViewModel. I don't understand the purpose of the architecture component existing...

Do you advise against AAC ViewModel solution here? It could emit view's states and survive config changes? Why would it be bad?

JakeWharton commented 6 years ago

This is almost fixed. The presenter is now retained across rotation and the old results are populated back into the UI. There's just a timing problem now of getting the first model after rotation.

JakeWharton commented 6 years ago

This is fixed by f9fc452, but the keyboard still pops up on rotation. It'd be nice to retain it's state by having something other than the query edit text hold focus when you interact with the list.

JakeWharton commented 6 years ago

This doesn't happen on my phone, only emulators. Closing as resolved.

Apsaliya commented 6 years ago

Keyboard pops up on rotation thingy happens on current play store version. (on real device)

JakeWharton commented 6 years ago

What device?

On Sun, Mar 11, 2018 at 5:34 AM Ankit Saliya notifications@github.com wrote:

Keyboard pops up on rotation thingy happens on current play store version. (on real device)

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/JakeWharton/SdkSearch/issues/39#issuecomment-372101738, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEdmocEgw7e5FdKbVh1w_ncaSsqgCks5tdO-hgaJpZM4Rg5TW .

Apsaliya commented 6 years ago

Xioami MI A1. It has stock 8.0.0 running on it.