Android - Adding a new RetainedFragment example

[zsiegel]

I wanted to augment the current RetainedFragment example class to address some scenarios I have run into using the patterns defined in the current example.

I was hoping to get some feedback on a more complete scenario I have been working on which is the following.

On a typical login screen when the user taps a button we want to fire off our network request in an observable. We want to show a progress dialog and then show either success or an error dialog when it fails. We want to gracefully handle rotation and app switching in case a user does something else after firing off the login call. When the user returns we should be able to tell them there was an error or continue forward.

I tried to address this scenario above but specifically I wanted to try and complete the following.

• Get all the benefits from the current example - i.e. rotation support
• Make sure the callbacks are not triggered while the app is not in focus
• Make the example be triggered by a user action - i.e. a user tapping a button

Hoping to get some feedback on the example and see if there are any issues with the sample implementation.

Any issues you see here @mttkay or other android RX users?

Thanks

[mttkay]

This looks fine, I'm just wondering if it really adds much value over the existing sample? It's almost identical with the exception of moving subscription logic to onResume/onPause instead of onViewCreated/onDestroyView, and firing the observable manually (via a button click.)

How about this: to make this sample more interesting over the existing one, have the observable toggle the button state. You could map a completed notification to a boolean which in turn is used to set the enabled state. Then at least there's a new dimension to this sample: binding observables to views, not just screens.

Also, let's give it a more meaningful name. RetainedFragmentActivity2 doesn't convey much meaning to a reader. Or do you think it will get too verbose?

[zsiegel]

thanks @mttkay I will go ahead and update the PR with your suggestions this week.

I will try and come up with a less verbose but understandable name for the class itself.

[zsiegel]

My updated sample now uses the observable to make the data request and then maps that observable to a boolean value where a user can control the state of the UI.

I also updated the class documentation to closer match the current samples.

Let me know what you think.

[cloudbees-pull-request-builder]

RxJava-pull-requests #947 ABORTED

[zsiegel]

I was wondering is there a reason the samples don't use the AndroidObservable class for binding?

Are these examples safe in terms of being garbage collected properly? I assume since we are unsubscribing in onPause we are fine but I just want to make sure.

Ive been following the AndroidObservable classes but haven't seen the justification for using them if I am subscribing and unsubscribing.

Any insight on this would be great.

Thanks

[benjchristensen]

Is this ready for merging after the last commit?

[zsiegel]

We probably want to wait for @mttkay to sign off on the updated example. Not sure if he has been able to look at the latest update.

[zsiegel]

Upon further investigation this might actually get updated again based on the status of #1021.

[mttkay]

I think it's fine, just land it. I look at the samples module as an ever moving target anyway. I see some things here that we should take as clues for providing proper operators to bind sequences to views, where a notification's value is mapped to a view property. I believe this is what the original Reactive Extensions (and ports like ReactiveCocoa) already provide, but RxJava is lacking since RxJava is client-platform independent (unlike ReactiveCocoa, which was build with the Apple SDK in mind from the ground up.)

There have been a few attempts already at providing such view bindings for Android (cf. ViewObservable), but it's far from exhaustive. Hopefully, the core elements for Android (scheduling on the main looper, attaching sequences to screens) have settled and are stable going forward, so I'm open to explore new territory (I really wanted to nail the core use cases first, before looking into the "nice to haves".)

[benjchristensen]

Should this be merged?

[mttkay]

Merge it. We can iterate on samples frequently, it's not that anything
relies on them.
On Apr 15, 2014 11:58 PM, "Ben Christensen" notifications@github.com
wrote:

Should this be merged?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1009#issuecomment-40539718
.