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

[QUESTION] What is the idea of having a tag for child routers? #155

Closed
StephanSchuster opened this issue Nov 7, 2016 · 15 comments
Closed

Comments

@StephanSchuster
Copy link
Contributor

StephanSchuster commented Nov 7, 2016

I couln't find any example usage of tags for child routers. That's why I'm asking. Looking at the sources it seems that it is possible to have multiple routers with different tags for the same container. Right? Then, when I push a controller to each of them, the according views are all attached to the same container via change handler. Right? The order (z-index) in which the children are displayed in the container (e.g. FrameLayout) depends on the push order because change handler just use container.addView(). Right? Due to this it seems difficult to control which view is currently shown (on top). Maybe it makes more sense if the views of the different controllers/routers are not shown on top of each other but besides each other (e.g. with RelativeLayout or ConstraintLayout and according layout properties set for the views). But wouldn't it be clearer to use different containers when showing views side by side. I'm confused.

I'm sure tags for child routers have a reason and I'm just too stupid to see it. Or have I misunderstood something?

Any hints are appreciated.

@StephanSchuster StephanSchuster changed the title [Question] What is the idea of having a tag for child routers? [QUESTION] What is the idea of having a tag for child routers? Nov 7, 2016
@tmtron
Copy link
Contributor

tmtron commented Nov 8, 2016

There is actually an example: the "Navigation Demos" of the demo app uses a tag TAG_UP_TRANSACTION.

This tag is attached to "Controller #0" (in the HomeController.onModelRowClick() method).
When you click on "Next controller" mutliple times and then "Go Up", it will take you back to "Controller #0" via the tag: getRouter().popToTag(TAG_UP_TRANSACTION)

@StephanSchuster
Copy link
Contributor Author

I know this example. In my opinion it shows the usage of tags for RouterTransaction, not for Router.

I'm talking about this "tag":
public final Router getChildRouter(@nonnull ViewGroup container, String tag) { ... }

@PaulWoitaschek
Copy link
Collaborator

Conductor needs the tag so it can internally resolve the router you attached.

Lets say you have a Controller that displays a list of cards. The controller assigns each of these cards a child router so each card can get its own controller.

@StephanSchuster
Copy link
Contributor Author

From the sources I can see that a different child router is created depending on the tag in getChildRouter(container, tag). So far so good.

But I still don't get your example use case. Is your "list of cards" displayed in the same container or in different ones?

If you have just one container, but for whatever reasons want to attach several child routers to that single container, then you need the tag in getChildRouter(container, tag). But how do you manage stacking of the controllers' view in this single container then?

If you have several containers, one for each card, then you don't need the tag in getChildRouter(container, tag) at all.

What am I missing??? Sorry.

@PaulWoitaschek
Copy link
Collaborator

If you have just one container, but for whatever reasons want to attach several child routers to that single container

You don't do that. You have a single router for a view container and push controllers upon that.

If you have several containers, one for each card, then you don't need the tag in getChildRouter(container, tag) at all.

Conductor needs to have a unique tag so it can assign the right controller to the correct view.

@StephanSchuster
Copy link
Contributor Author

You don't do that. You have a single router for a view container and push controllers upon that.

Good. That was my understanding, too.

Conductor needs to have a unique tag so it can assign the right controller to the correct view.

Sorry. I still don't get your point. Please have look at the implementation of Controller.getChildRouter(container, tag). You'll see that "tag" is only used to create new instances of a ControllerHostedRouter or reuse existing ones. Except for this method the "tag" of a ControllerHostedRouter is never used within the whole framework.

As a result, after calling

getChildRouter(myContainer, null) // new
getChildRouter(myContainer, "tag_1") // new
getChildRouter(myContainer, "tag_2") // new
getChildRouter(myContainer, "tag_2") // reuse
getChildRouter(myContainer, null) // reuse

I will end up with 3 child router instances for the same container. According to you: "You don't do that. You have a single router for a view container and push controllers upon that."

So what's the point of this "tag" then???

@PaulWoitaschek
Copy link
Collaborator

PaulWoitaschek commented Nov 8, 2016

Well here

ControllerHostedRouter childRouter = null;
for (ControllerHostedRouter router: childRouters) {
    if (router.getHostId() == containerId && TextUtils.equals(tag, router.getTag())) {
        childRouter = router;
        break;
    }
}

it retrieves the router by its tag.

You'll see that "tag" is only used to create new instances of a ControllerHostedRouter or reuse existing ones. Except for this method the "tag" of a ControllerHostedRouter is never used within the whole framework.

So the point is to find the existing router. What else point should there be?

@EricKuck
Copy link
Member

EricKuck commented Nov 8, 2016

Thanks for starting this conversation @StephanSchuster. I wish I could say the tag had a great purpose that you hadn't seen yet, but unfortunately that isn't the case. The tag was used for some features that ended up being scrapped before this version went live. I should have removed this parameter, but apparently overlooked it. It's actually not needed at all, and actually probably shouldn't be used, as it gives the illusion that it's ok to have multiple routers in a single container.

I'll add another getChildRouter() method that only takes a container parameter and will deprecate the existing one.

Sorry for the confusion everyone! Not sure how this one slipped through for so long.

@PaulWoitaschek
Copy link
Collaborator

I don't get it. How will conductor find the right child router then?

@EricKuck
Copy link
Member

EricKuck commented Nov 8, 2016

In the block of code you sent, the relevant part is if (router.getHostId() == containerId). It'll know which child router is the right one based on the id of the container. I've actually passed null as the tag for all of my getChildRouter() calls on internal apps without really giving it any further thought and haven't had any issues at all.

@PaulWoitaschek
Copy link
Collaborator

Now that I see it it makes completely sense.

Btw, you should throw an exception if container.id == android.view.View.NO_ID

@EricKuck
Copy link
Member

EricKuck commented Nov 8, 2016

Another great point. Thanks @PaulWoitaschek!

@StephanSchuster
Copy link
Contributor Author

Okay, everything clarified. Thank you @EricKuck and @PaulWoitaschek for your time.

Maybe one of you could even look at my other question.

@PaulWoitaschek
Copy link
Collaborator

So in reference to the bug this arraised: #160
Shouldn't in this case the getChildController without a tag be @Deprecated or better: removed?

@EricKuck
Copy link
Member

The tag is only needed if you're going to have multiple child routers in the same host ViewGroup. The javadoc for the getChildRouter method that takes a tag mentions Note that multiple routers should not exist in the same container unless a lot of care is taken to maintain order between them. Avoid using the same container unless you have a great reason to do so (ex: ViewPagers).. Because of this, getting a child router without a tag should be the most common use-case.

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

No branches or pull requests

4 participants