MOBILE-4686 user-menu: Fix routes with canLeave cannot leave #4218
Conversation
| @@ -726,7 +726,13 @@ export class CoreNavigatorService { | |||
| * @returns Whether the current route page can block leaving the route. | |||
| */ | |||
| currentRouteCanBlockLeave(): boolean { | |||
There was a problem hiding this comment.
You're reversing the result of this function for no reason. Before, this function returned true if the route could block, now it returns false if it blocks, which doesn't make sense IMO.
| return true; | ||
| } | ||
|
|
||
| return canDeactivate.every(method => typeof method === 'function' && method()); |
There was a problem hiding this comment.
This code doesn't work. The canDeactivate function (in here, method) expects to receive an instance of the component of the current route, and you aren't passing anything. This throws an error in console:
ERROR TypeError: Cannot use 'in' operator to search for 'canLeave' in undefined
at isCanLeave (can-leave.ts:25:37)
I checked the contents of getCurrentRoute() and I see we have access to the class of the component, but not the instance so I'm not sure if this is feasible.
| return true; | ||
| } | ||
|
|
||
| return canDeactivate.every(method => typeof method === 'function' && method()); |
There was a problem hiding this comment.
Even if we manage to fix this, I think this will cause a bad UX for the user. Your code is calling the canDeactivate function to check if the user can leave or not, which in most cases will display a confirm modal to the user, but you're not leaving the page.
After the user confirms that he wants to leave, the logout process will continue and this process performs a navigation to '/login/sites', which will trigger Angular's canDeactivate function again, displaying the confirm modal a second time. This is not what we want.
| @@ -726,7 +726,13 @@ export class CoreNavigatorService { | |||
| * @returns Whether the current route page can block leaving the route. | |||
| */ | |||
| currentRouteCanBlockLeave(): boolean { | |||
There was a problem hiding this comment.
Your change is a breaking change that will break the usage in behat-blocking. IMO you should leave the previous function as it was and create a new one for the logout/switch account use case.
No description provided.