Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Novoda contributions #15

Closed
wants to merge 4 commits into from
Closed

Novoda contributions #15

wants to merge 4 commits into from

Conversation

Dorvaryn
Copy link
Contributor

Novoda had an internal repo that we were working against for some of our projects and planned on releasing soon but it looks more like something that should sit in RxAndroid itself so here is a PR with what we came up with.

Working with RxJava we realised that, depending on the project, different approaches were possible.
When the project can't use the traditional data layer approach with a UI that subscribes to an observable provided by that layer (UI driven by remote API). We encountered a need for tracking the response to a specific observable from an activity over rotation.

Thus here is tooling to help make that possible on Android:

  • ResumableSubscriber (looking for a better name since it is not a subscriber in the observer sense but performs the act of subscription), allows Loader like tracking over rotation

This is not useful for all architectures but it allows for some interesting usage:

  • defining a ReactiveDialog that allows for mapping actions after user interaction
  • reacting on ActivityResult in a simpler way

I'm not sure about conventions in this project and general direction so let's discuss this and make it happen.

Dorvaryn added 4 commits September 18, 2014 17:01
This is adding a resumable logic to handle Observables on rotation if
needed and add some wrapping logic around Dialogs and Navigation on
Android
@Dorvaryn Dorvaryn changed the title 0.x.1 Novoda contributions Sep 19, 2014
@staltz
Copy link
Member

staltz commented Sep 19, 2014

Would be good to have Javadocs for the most important parts. There are some confusing interfaces there such as ResumableObserver and ObserverOperator. And how does EventCachingOperator differ from RxJava operators replay or cache?

@mttkay
Copy link
Collaborator

mttkay commented Sep 19, 2014

Thanks for sharing this! I agree though that the scope of this PR is way too big. I don't understand what the responsibility of most of the classes even is. Can we break this down into a PR for each operator? Otherwise it'll be difficult to discuss any of this in a focused way. Maybe some samples / more context /more docs would help too.

On a general note, I don't think we should turn RxAndroid into a framework, but I see that many of the classes here depend on each other and seem to form a framework / app blueprint. One of the key things that makes RxJava so universally applicable is that it does the opposite: it provides very fundamental building blocks on which apps can build what makes sense for them. ObservableVault for instance for me is not universally applicable across all Android apps since it suggests that Observables are retained in this particular way. RxAndroid, in my opinion should be a thin layer on top of RxJava that lifts framework classes into Rx Observables or provides helpers for platform specific problems that affect everyone. It should not, however, provide a general blueprint for how to build an app.

Maybe we can open this to broader discussion?

@staltz
Copy link
Member

staltz commented Sep 19, 2014

RxAndroid, in my opinion should be a thin layer on top of RxJava that lifts framework classes into Rx Observables or provides helpers for platform specific problems that affect everyone.

I fully agree on that, pointing to AndroidSchedulers and ViewObservable as prime examples.

@Dorvaryn
Copy link
Contributor Author

Let's break this PR down and discuss the ideas a bit further.
The idea is not to create a framework but to add rotation tracking support
and that lead to a few classes overhead.
I tried to keep the changes at the Observer layers that way the composition
flow of the observables is not changed and it should not impact anything.

I'll break that down and we can discuss on subsequent PRs.

It might take me some time though since I am in France for droidcon Paris
until Wednesday.
On 19 Sep 2014 20:06, "André Staltz" notifications@github.com wrote:

RxAndroid, in my opinion should be a thin layer on top of RxJava that
lifts framework classes into Rx Observables or provides helpers for
platform specific problems that affect everyone.

I fully agree on that, pointing to AndroidSchedulers and ViewObservable as
prime examples.


Reply to this email directly or view it on GitHub
#15 (comment).

Disclaimer: The information in this e-mail and any attachments is the
property of Novoda Ltd and is confidential and may be legally privileged.
This e-mail is intended solely for the person or organisation to which it
is addressed. Any disclosure, copying or other use of the information by
any person or organisation who is not the intended recipient is strictly
prohibited and may be unlawful. If you have received this e-mail in error,
please inform the sender immediately and delete/destroy this e-mail and any
copies of it. Novoda Ltd has taken reasonable precautions to minimise the
risk of any software viruses which may damage your systems, but we advise
that you take the necessary steps to ensure that no virus contamination is
suffered. Novoda Ltd does not accept any liability for any loss or damage
caused by the transmission of any virus.

Novoda Ltd, Company No: 347444, Registered in Scotland Registered
Office: C/O Alexander Sloan, 38 Cadogan Street, Glasgow, G2 7HF, Scotland. VAT
Registration Number GB 984 2525 93

@staltz
Copy link
Member

staltz commented Sep 19, 2014

What is "rotation tracking"? Orientation changes? Or gyroscope tracking?

@Dorvaryn
Copy link
Contributor Author

Sorry, orientation change.
On 19 Sep 2014 20:51, "André Staltz" notifications@github.com wrote:

What is "rotation tracking"? Orientation changes? Or gyroscope tracking?


Reply to this email directly or view it on GitHub
#15 (comment).

Disclaimer: The information in this e-mail and any attachments is the
property of Novoda Ltd and is confidential and may be legally privileged.
This e-mail is intended solely for the person or organisation to which it
is addressed. Any disclosure, copying or other use of the information by
any person or organisation who is not the intended recipient is strictly
prohibited and may be unlawful. If you have received this e-mail in error,
please inform the sender immediately and delete/destroy this e-mail and any
copies of it. Novoda Ltd has taken reasonable precautions to minimise the
risk of any software viruses which may damage your systems, but we advise
that you take the necessary steps to ensure that no virus contamination is
suffered. Novoda Ltd does not accept any liability for any loss or damage
caused by the transmission of any virus.

Novoda Ltd, Company No: 347444, Registered in Scotland Registered
Office: C/O Alexander Sloan, 38 Cadogan Street, Glasgow, G2 7HF, Scotland. VAT
Registration Number GB 984 2525 93

@staltz
Copy link
Member

staltz commented Sep 19, 2014

Ah yes, that would be very useful :)

@mttkay
Copy link
Collaborator

mttkay commented Sep 19, 2014

No worries, take your time! We're still working on the build setup anyway
and are probably a few weeks away from the first RxAndroid release. No
rush.

@Wavesonics
Copy link

I agree that one big strength of RxJava is that it's a toolkit, not a framework. However having some built in, well tested solutions for dealing with orientation changes and other common problems on Android would be hugely beneficial to many of us I think.

@mttkay
Copy link
Collaborator

mttkay commented Sep 19, 2014

Maybe. We use retained fragments and the replay operator. That works pretty
well for us.

@Wavesonics
Copy link

@mttkay retained fragments aren't always an option. Anything using nested fragments with ChildFragmentManager can not be retained. Which means Fragments inside ViewPagers. In my code base at work we make extensive use of nested fragments, so currently without a good solution for handling configuration changes, using RxJava isn't viable.

@mttkay
Copy link
Collaborator

mttkay commented Sep 20, 2014

I don't think that's true. If the parent fragment is retained, so are all
child fragments (it's just that you can't retain them again). We do exactly
that.
On Sep 20, 2014 12:19 AM, "Adam Brown" notifications@github.com wrote:

@mttkay https://github.com/mttkay retained fragments aren't always an
option. Anything using nested fragments with ChildFragmentManager can not
be retained. Which means Fragments inside ViewPagers. In my code base at
work we make extensive use of nested fragments, so currently without a good
solution for handling configuration changes, using RxJava isn't viable.


Reply to this email directly or view it on GitHub
#15 (comment).

@Dorvaryn
Copy link
Contributor Author

The thing is sometimes you can't or don't want to rely on fragments.
The example we had was a Ui highly driven by remote events and data. It was
easier to have an observable retained and with replay driving the inflation
of custom views.
On rotation the replay would replace the Ui in the correct state even
though we could not know when coding the number of state existing.
On 20 Sep 2014 09:00, "Matthias Käppler" notifications@github.com wrote:

I don't think that's true. If the parent fragment is retained, so are all
child fragments (it's just that you can't retain them again). We do
exactly
that.
On Sep 20, 2014 12:19 AM, "Adam Brown" notifications@github.com wrote:

@mttkay https://github.com/mttkay retained fragments aren't always an
option. Anything using nested fragments with ChildFragmentManager can
not
be retained. Which means Fragments inside ViewPagers. In my code base at
work we make extensive use of nested fragments, so currently without a
good
solution for handling configuration changes, using RxJava isn't viable.


Reply to this email directly or view it on GitHub
#15 (comment).


Reply to this email directly or view it on GitHub
#15 (comment).

Disclaimer: The information in this e-mail and any attachments is the
property of Novoda Ltd and is confidential and may be legally privileged.
This e-mail is intended solely for the person or organisation to which it
is addressed. Any disclosure, copying or other use of the information by
any person or organisation who is not the intended recipient is strictly
prohibited and may be unlawful. If you have received this e-mail in error,
please inform the sender immediately and delete/destroy this e-mail and any
copies of it. Novoda Ltd has taken reasonable precautions to minimise the
risk of any software viruses which may damage your systems, but we advise
that you take the necessary steps to ensure that no virus contamination is
suffered. Novoda Ltd does not accept any liability for any loss or damage
caused by the transmission of any virus.

Novoda Ltd, Company No: 347444, Registered in Scotland Registered
Office: C/O Alexander Sloan, 38 Cadogan Street, Glasgow, G2 7HF, Scotland. VAT
Registration Number GB 984 2525 93

@meoyawn
Copy link

meoyawn commented Sep 29, 2014

I agree with @Dorvaryn, if we could ship some RX-driven solution to the horrible orientation problem on Android, the whole community would largely benefit from that

@mttkay retaining fragments is not an option when using backstack (start a retained fragment, replace it with another, change orientation, go back, see how sad the situation is)

@Dorvaryn Dorvaryn closed this Sep 30, 2014
@Dorvaryn
Copy link
Contributor Author

I'll reopen smaller PRs with the different components to help discussion.

@ggrell
Copy link

ggrell commented Sep 30, 2014

A quick comment: If this ends up being larger, or feeling more like a framework, there's nothing stopping it from being released as a standalone project (vs part of RxAndroid)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants