Refactor Discovery to properly recover from stopped processes #48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pass along the root categories from the activity instead, via the pagerAdapter.
…eChanged -> pageSelected
This should fix a bug where restoring the first discovery page after the app has been stopped (not killed) results in a blank page. The previous approach used `pagerCreatedPage` and `pageSelections`. Unfortunately we can't rely entirely on `pageSelections` since it will emit page 0 on subscribe, even if a different page is eventually restored. `pagerCreatedPage` is also problematic because the fragment manager may not *create* the fragment instance again on reload; the fragment instance persists and we never get the create event. It seems like the best tool available to figure out when a page is ready to receive params is to hook into `setPrimaryItem` events. It fires many times, so we use `distinctUntilChanged` to de-dupe. The one last wrinkle is that it seems like a fragment may not be ready to receive params immediately after `setPrimaryItem` is called – I've hooked up a delay so that execution here is deferred until the next run loop. It's pretty janky and maybe we could spend more time thinking about it since it's not a great pattern.
…or Fragment creation delay
|
👏 |
christopherwright
approved these changes
Jan 18, 2017
There was a problem hiding this comment.
This is fab, nice and clean PR thanks to the refactor in #44.
| @@ -176,6 +176,9 @@ private Transformers() {} | |||
| /** | |||
| * If called on the main thread, schedule the work immediately. Otherwise delay execution of the work by adding it | |||
| * to a message queue, where it will be executed on the main thread. | |||
| * | |||
| * This is useful for `RecyclerView`s where new data can appear immediately rather than waiting for a frame | |||
There was a problem hiding this comment.
I might slightly rephrase this –
This is particularly useful for
RecyclerViews; if subscriptions in these views are delayed for a frame, then the views temporarily shows recycled content and frame rate stutters. To address that, we can useobserveForUIto execute the work immediately rather than wait for a frame.
PR 230 in the old repo goes into more detail.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
When an app goes into the background for a while, the Android system reclaims memory by stopping idle processes. Our app was not recovering correctly when relaunching from a stopped process state because we were using incorrect methods from our
DiscoveryPagerAdapterasinputs which did not give ourDiscoveryFragmenta proper resume kick.before:
how
DiscoveryViewModellogic via Pass categories from DiscoveryActivity to Fragment #44.skip(1)andtake(1)s that we did not feel great about. Simplify overallpagerSelectedPagelogic. Feel sound with ourRxJavaknowledge and operators/Subjects/Observables.FragmentPagerAdapterAPI again. This is how we found a better pager method, setPrimaryItem(). Why this is better:getItem()only pings once each time aFragmentis created. This would not ping on return from stopped state. See FragmentPagerAdapter docs for more info.RxViewPager'spageSelection's immediate ping on subscribe made it difficult to sync with when the pager was actually ready.setPrimaryItem()tells us the current page, and when the Fragment is ready. .:., better!0delay for thepagerAdapterinputs, since there is a frame of delay between when the Fragment is created and ready to receive the inputs. We also recently introduced the.compose(observeForUI)vs.observeOn(AndroidSchedulers.mainThread())pattern, and now know more about when to use which.after: