Skip to content

Commit

Permalink
fix(activation): Deactivate child viewModels when replacing a parent
Browse files Browse the repository at this point in the history
The `findDeactivatable` was looking for child viewModels to deactivate on `viewPortPlan.childNavigationInstruction` when present. This made sense in the case where you were replacing one child route with another, as the `BuildNavigationPlanStep` would properly identify which child route was being replaced and all the correct lifecycle methods would be invoked. However, this failed the not-so-edge case, seen here #411, where you were replacing a parent with an activated child. `BuildNavigationPlanStep` would not populate enough information on the child components to correctly deactivate them. The `LoadRouteStep` was inadvertently populating the missing information, and so the issue was only being seen on the `CanDeactivatePreviousStep`, which falls between the two. The `DeactivatePreviousStep` would proceed normally.

@jagonzalez opened #496 which does seem to solve the issue. However, the solution identified when the problem was taking place, specifically on canDeactivate and when certain other conditions are met, and wrote a solution around that. His original solution was closer to solving the underlying problem, but failed a unit test which I found to be erroneous. This fix builds upon the ideas of the earlier pull request but attempts to better identify the underlying problem and solve it.
  • Loading branch information
davismj committed Jul 24, 2017
1 parent ac6bf75 commit f73b6d5
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/activation.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ function findDeactivatable(plan, callbackName, list: Array<Object> = []): Array<
}
}

if (viewPortPlan.childNavigationInstruction) {
findDeactivatable(viewPortPlan.childNavigationInstruction.plan, callbackName, list);
} else if (prevComponent) {
if (viewPortPlan.strategy === activationStrategy.replace && prevComponent) {
addPreviousDeactivatable(prevComponent, callbackName, list);
} else if (viewPortPlan.childNavigationInstruction) {
findDeactivatable(viewPortPlan.childNavigationInstruction.plan, callbackName, list);
}
}

Expand Down

0 comments on commit f73b6d5

Please sign in to comment.