android / architecture-samples

A collection of samples to discuss and showcase different architectural tools and patterns for Android apps.
Apache License 2.0
44.46k stars 11.64k forks source link

Add tablet layout #3

Open JoseAlcerreca opened 8 years ago

JoseAlcerreca commented 8 years ago

A layout with multiple fragments per screen would showcase how to keep data fresh between views and send messages between presenters.

logo17 commented 8 years ago

Hi, do you have an example for this case?

JoseAlcerreca commented 8 years ago

Not yet :)

logo17 commented 8 years ago

Ok thanks.. I'm dealing with something like that. Do you have any recommendations about how to implement this communication between presenters?

Rainer-Lang commented 8 years ago

@JoseAlcerreca Will be a viewpager in the tablet layout? If this (viewpager / handle multiple fragments) is so "complicated" then I'm hoping to see an useful example here. :+1:

NikitaKozlov commented 8 years ago

Do you have some design mockups?

sergeykosik commented 8 years ago

@JoseAlcerreca Looking forward to see an example of the tablet layout.

JoseAlcerreca commented 8 years ago

It might be easier and cleaner to create a todo-mvp-tablet sample. Adding a tablet view to all variants is going to be hard and will pollute the code making it harder to compare between variants. :+1: or :-1: ?

flipper83 commented 8 years ago

I've prefered todo-mvp-tablet too! easy to read and for maintenance.

malmstein commented 8 years ago

same here, add tablet to every single sample would be a bit of overkill.

Rainer-Lang commented 8 years ago

Will a viewpager be used?

JoseAlcerreca commented 8 years ago

@Rainer-Lang no, no need.

JoseAlcerreca commented 8 years ago

I've been experimenting with this and basically have two solutions, each with pros and cons. Let me know what you think:

https://github.com/JoseAlcerreca/android-architecture/pull/1 Modifies each call to TasksPresenter or TaskDetailPresenter to be tablet-aware.

https://github.com/JoseAlcerreca/android-architecture/pull/2 Adds another layer of abstraction for presenters with a TasksTabletPresenter that talks to the list and detail presenters, keeping modifications to the original mvp classes low.

NikitaKozlov commented 8 years ago

I have small question, to your solutions. Why do you want so much, to have one Presenter per screen? If you keep one Presenter per fragment, then it will be easier to maintain and easier to understand. But you will have to create some kind of Navigator, who will decide what to open. I can create an example if you want.

Rainer-Lang commented 8 years ago

@NikitaKozlov Could your solution even be used for viewpager?

NikitaKozlov commented 8 years ago

Yeap. Even with retaining presenter, but this is completly out of scope.

JoseAlcerreca commented 8 years ago

Why do you want so much, to have one Presenter per screen?

One of the solution uses 2 presenters, the other one uses 3, actually. The reasoning for each is in the PR description.

But you will have to create some kind of Navigator, who will decide what to open.

Yes, that's what I've done in both, see TasksNavigator.java.

Sefford commented 8 years ago

IMO, #2 is a more correct, modular approach.

If each presenter handles the logic of a fragment (a "screen"), a "tablet fragment" is composed of several "screens" (eg, list/detail classic example).

As such, you would be interesting in creating a composite presenter (TasksTabletPresenter) which orchestrates the communication between all the individual Presenters.

This approach also requires to have separate logic to know when to employ the correct set of Presenters and tweak a little the navigation between the fragments.

lurbas commented 8 years ago

I don't think it's a Presenter job to talk to other Presenters... (#2). It should be handled be a different layer. Navigator looks like the best place for me. Navigator contains the logic to open a new Activity, or put a DetailFragment into DetailContainer (if exists). The reverse communication (action on DetailFragment like bookmarking, or favoriting object, that should affect visual changes on ListFragment) can be done be listening to database changes, and it's a ListPresenter job to update its view everytime database has a new dataset. To do it you can use libraries like sqlbrite, or it's not so difficult to write similar thing by yourself. If you're interested, I can provide some examples.

JoseAlcerreca commented 8 years ago

Yes that's how Data Binding and other libraries that observe changes work, but this is an vanilla MVP approach.

NikitaKozlov commented 8 years ago

@JoseAlcerreca I will try to rephrase my point. You are considering tablet layout as an advanced “Tasks screen”, so all orchestration logic is in the guy who implements TasksContract.Presenter: for #1 it is TasksPresenter, for #2 it is TasksTabletPresenter. I’m proposing to treat each fragment with a presenter as a block with it’s listeners and setters. And then create a controller on top that will handle the orchestration logic. This guy could be a Navigator, another Presenter or any kind of controller and will be responsible for connecting callbacks from different blocks(aka Presenters). He will also remove TaskDetailFragment on clearing if required, or tell proper Navigator to do that. Sort of “Presenter“ for Activity. That will keep changes to the minimum and make a code a little bit more decoupled. Once I finished with the example I will show it.

JoseAlcerreca commented 8 years ago

and then create a controller on top that will handle the orchestration logic. This guy could be a Navigator, another Presenter

Yeah, that's exactly https://github.com/JoseAlcerreca/android-architecture/pull/2 with the TasksTabletPresenter, that's why I think we're talking about the same thing. It does exactly what you're describing :)

NikitaKozlov commented 8 years ago

@JoseAlcerreca Yes and no. In JoseAlcerreca#2 your fragments are talking directly to TasksTabletPresenter. Instead of that I want, for example, TasksFragment talk to TasksPresenter, but TasksTabletPresenter will put some callbacks on TasksPresenter. It is your approach, but upside down. But the difference is significant if it comes to extendability. Let's image that you want to add a couple more methods to TasksContract.Presenter, in your case you will have to add them to TasksTabletPresenter as well, in my case I will have to change something only if it affects TaskDetailPresenter.

JoseAlcerreca commented 8 years ago

Right, but I think that's a good thing: You are forced to implement the tablet behavior. Even if it's just bypassing the event, you have the guarantee that that's the behavior you want in tablet mode.

JoseAlcerreca commented 8 years ago

https://github.com/JoseAlcerreca/android-architecture/pull/2 updated with some UI fixes and now I'm retaining presenters in retained fragments so that rotation is fast and easy. It's risky for a sample, since it can easily be misused, so it would need some documentation.