bluelinelabs / Conductor

A small, yet full-featured framework that allows building View-based Android applications
Apache License 2.0
3.9k stars 345 forks source link

How to do complex master-detail layout? #176

Open ZakTaccardi opened 7 years ago

ZakTaccardi commented 7 years ago

The example in the sample doesn't correctly survive configuration changes (you can get the portrait layout in landscape sometimes)

Example of layout I want to achieve

How should I be doing this? I've attempted to achieve this, but I am currently having issues restoring state.

I currently have the whole layout wrapped in a single HomeController, which has three childRouters via homeController.getChildRouter(view, TAG)

masterRouter's root is masterControllerA (or masterControllerB when toggled by nav drawer). detailControllerA's is either:

To avoid instantiating unnecessary controllers I'm always checking if the router/controller is already contained in the existing ViewGroup container/router by its tag.

Does any of this throw a red flag?

ZakTaccardi commented 7 years ago

I've tried using a single dagger module with all routers/controllers in the same scope - but I hit this error when swapping nav drawer options (aka changing the root of the masterRouter

Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.util.List com.bluelinelabs.conductor.Controller.getChildRouters()' on a null object reference
    at com.bluelinelabs.conductor.ControllerHostedRouter.getSiblingRouters(ControllerHostedRouter.java:187)
    at com.bluelinelabs.conductor.Router.removeAllExceptVisibleAndUnowned(Router.java:711)
    at com.bluelinelabs.conductor.Router.setRoot(Router.java:283)

I'm guessing it has something to do with swapping setting the root of the masterRouter while reusing the childRouter instance of a controller that was previously attached (but no longer attached) to the masterRouter

EricKuck commented 7 years ago

What's the problem with the way it's implemented in the demo app? The only time you can get the portrait layout in landscape is if you flip the phone when you're already in the detail view (I think). If there's another case I'm not aware of, please let me know.

Regarding your question, the only thing that throws a red flag is that it sounds like you're reusing potentially destroyed controllers. It all looks good besides that.

As for your crash, you're exactly right. If you try to use a Router that isn't attached to a host of any kind (ex: a child router whose controller isn't attached), you're not gonna have a good time.

ZakTaccardi commented 7 years ago

The only time you can get the portrait layout in landscape is if you flip the phone when you're already in the detail view (I think)

Is this considered a bug? (I think it should be)

EricKuck commented 7 years ago

Yeah, it probably should be considered a bug. I just wrote that as a quick demo without giving it too much thought. I've never had to use a master/detail view for much - how do apps typically handle this case? The master/detail example created by Android Studio's new project wizard seems to do what the demo does.

ZakTaccardi commented 7 years ago

how do apps typically handle this case

  1. viewing DetailController in portrait
  2. user rotates device into landscape
  3. Expected: both MasterController and DetailController are visible, with the sameDetailController active that was visible in portrait

My confusion stems around how I should be restoring the state of routers and controllers in a configuration change when you switch from portrait to landscape (single pane layout to two pane layout).

I see two general approaches:

  1. Have a separate masterRouter and detailRouter for landscape. In portrait, there would only be one masterRouter, which the detailController would be pushed on to. The problem here is that the detailController will be in the masterRouter in portrait, but in the detailRouter in landscape).

  2. A single masterRouter, which masterController gets the detailRouter as its child router. I think this approach is the recommended one, but I struggled with it for the following reason.

The demo utilizes a single, static masterController - like a navigation drawer, it's always there. My use case has a dynamic masterController, where I need to swap out the masterController based on the nav drawer selection. If you have used Google Play Music before, my use case is more like that. Think of how that app toggles between the two navigation drawer options, Home and Music Library, when the HomeFragment and MusicLibraryFragment would be two of the masterControllers, that each have their own separate detailFragment

tmtron commented 7 years ago

@ZakTaccardi did you ever find a good solution to this?

tmtron commented 7 years ago

I think currently there is no easy way to handle this in Conductor.

The workaround that I came up with is this:

I have an OverviewController which always has 2 child-routers for

landscape-layout is easy: MasterController and DetailController are shown side-by side (e.g. in a horizontal LinearLayout)

prortrait-layout needs more work

sdelaysam commented 5 years ago

@EricKuck I guess the issue is still valid and I've added my implementation just in case. https://github.com/bluelinelabs/Conductor/pull/564