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

Add tablet layout #3

Open
JoseAlcerreca opened this issue Mar 2, 2016 · 24 comments
Open

Add tablet layout #3

JoseAlcerreca opened this issue Mar 2, 2016 · 24 comments
Assignees
Milestone

Comments

@JoseAlcerreca
Copy link
Contributor

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

@JoseAlcerreca JoseAlcerreca self-assigned this Mar 2, 2016
@JoseAlcerreca JoseAlcerreca added this to the v2 milestone Mar 2, 2016
@logo17
Copy link

logo17 commented Mar 31, 2016

Hi, do you have an example for this case?

@JoseAlcerreca
Copy link
Contributor Author

Not yet :)

@logo17
Copy link

logo17 commented Mar 31, 2016

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

@Rainer-Lang
Copy link
Contributor

@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. 👍

@NikitaKozlov
Copy link

NikitaKozlov commented May 18, 2016

Do you have some design mockups?

@sergeykosik
Copy link

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

JoseAlcerreca pushed a commit that referenced this issue Jun 30, 2016
Pulled upstream changes in branch Todo databinding
@JoseAlcerreca
Copy link
Contributor Author

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.
👍 or 👎 ?

@flipper83
Copy link
Contributor

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

@malmstein
Copy link
Contributor

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

@Rainer-Lang
Copy link
Contributor

Will a viewpager be used?

@JoseAlcerreca
Copy link
Contributor Author

@Rainer-Lang no, no need.

@JoseAlcerreca
Copy link
Contributor Author

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

JoseAlcerreca#1
Modifies each call to TasksPresenter or TaskDetailPresenter to be tablet-aware.

JoseAlcerreca#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
Copy link

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
Copy link
Contributor

@NikitaKozlov Could your solution even be used for viewpager?

@NikitaKozlov
Copy link

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

@JoseAlcerreca
Copy link
Contributor Author

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
Copy link

Sefford commented Jul 26, 2016

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
Copy link

lurbas commented Jul 26, 2016

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
Copy link
Contributor Author

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

@NikitaKozlov
Copy link

@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
Copy link
Contributor Author

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 JoseAlcerreca#2 with the TasksTabletPresenter, that's why I think we're talking about the same thing. It does exactly what you're describing :)

@NikitaKozlov
Copy link

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

JoseAlcerreca#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.

@JoseAlcerreca JoseAlcerreca modified the milestones: v1, v2 Sep 1, 2016
JoseAlcerreca pushed a commit that referenced this issue Aug 17, 2017
Merges changes from todo-mvp and fixes UI tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants