feat(router): add CanDeactivate next state argument #13560

DzmitryShylovich · 2016-12-18T23:15:56Z

vicb · 2016-12-20T03:31:01Z

modules/@angular/router/src/interfaces.ts

@@ -186,7 +186,8 @@ export interface CanActivateChild {
 *   canDeactivate(
 *     component: TeamComponent,
 *     route: ActivatedRouteSnapshot,


currentRoute

vicb · 2016-12-20T03:31:40Z

modules/@angular/router/test/integration.spec.ts

-                useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
+                useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot,
+                           nextState: RouterStateSnapshot) => {
+                  expect(nextState).toBeAnInstanceOf(RouterStateSnapshot);


we need a proper test

vicb · 2016-12-20T03:34:39Z

modules/@angular/router/test/integration.spec.ts

@@ -1624,7 +1625,9 @@ describe('Integration', () => {
            TestBed.configureTestingModule({
              providers: [{
                provide: 'CanDeactivate',
-                useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => {
+                useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot,


consistency b vs nextState

vicb · 2016-12-20T03:35:30Z

I'm not very clear on the use case but this is what should be tested in the integration test.

DzmitryShylovich · 2016-12-20T11:48:30Z

@vicb

should be tested in the integration test.

done

vicb · 2016-12-20T21:00:38Z

modules/@angular/router/test/integration.spec.ts

+            canDeactivate(
+                component: TeamCmp, currentRoute: ActivatedRouteSnapshot,
+                currentState: RouterStateSnapshot, nextState: RouterStateSnapshot): boolean {
+              expect(currentState.url).toEqual('/team/22');


maybe use a log and assert the content in test test ?

vicb · 2016-12-20T21:01:28Z

Could you please:

address the comment in the tests,
add a description of what this feature is in the commit comment.

Thanks !

DzmitryShylovich · 2016-12-20T23:16:32Z

@vicb done

vicb · 2016-12-21T00:52:27Z

Could you please reword it as

Adds a `nextState` argument to access the future url from `CanDeactivate`.

BEFORE:
    
    canDeactivate(component: T, route: ActivatedRouteSnapshot, state: RouterStateSnapshot)
        : Observable<boolean>|Promise<boolean>|boolean;

AFTER:

    canDeactivate(component: T, currentRoute: ActivatedRouteSnapshot, currentState: RouterStateSnapshot,
                  nextState?: RouterStateSnapshot): Observable<boolean>|Promise<boolean>|boolean;

DzmitryShylovich · 2016-12-21T01:09:57Z

@vicb could u reword it on merge? :)

vicb · 2016-12-21T01:58:01Z

I would but I'm not merging this week

nheidloff · 2016-12-21T10:14:28Z

This seems promising. I hope it will be merged soon. Thanks @DzmitryShylovich

Adds a `nextState` argument to access the future url from `CanDeactivate`. BEFORE: canDeactivate(component: T, route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean>|Promise<boolean>|boolean; AFTER: canDeactivate(component: T, currentRoute: ActivatedRouteSnapshot, currentState: RouterStateSnapshot, nextState?: RouterStateSnapshot): Observable<boolean>|Promise<boolean>|boolean; Closes angular#9853

DzmitryShylovich · 2016-12-21T11:04:23Z

@vicb

I would but I'm not merging this week

oke, I did it

angular-automatic-lock-bot · 2019-09-10T12:22:35Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Dec 18, 2016

DzmitryShylovich force-pushed the gh/9853 branch 2 times, most recently from 01455cd to 001d283 Compare December 19, 2016 11:03

vicb reviewed Dec 20, 2016

View reviewed changes

vicb added area: router action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews pr_state: LGTM labels Dec 20, 2016

DzmitryShylovich force-pushed the gh/9853 branch from 7cf6d8b to 18fd6e3 Compare December 20, 2016 11:37

vicb reviewed Dec 20, 2016

View reviewed changes

DzmitryShylovich force-pushed the gh/9853 branch 2 times, most recently from ea9f21d to 648b3c7 Compare December 20, 2016 23:06

DzmitryShylovich force-pushed the gh/9853 branch from 648b3c7 to f8d1445 Compare December 20, 2016 23:39

DzmitryShylovich force-pushed the gh/9853 branch from f8d1445 to f30a96a Compare December 21, 2016 11:00

DzmitryShylovich force-pushed the gh/9853 branch from f30a96a to 7f322e2 Compare December 21, 2016 11:03

vicb added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Dec 27, 2016

hansl merged commit 69fa3bb into angular:master Dec 27, 2016

DzmitryShylovich deleted the gh/9853 branch December 27, 2016 22:15

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): add CanDeactivate next state argument #13560

feat(router): add CanDeactivate next state argument #13560

DzmitryShylovich commented Dec 18, 2016

vicb Dec 20, 2016

vicb Dec 20, 2016

vicb Dec 20, 2016

vicb commented Dec 20, 2016

DzmitryShylovich commented Dec 20, 2016

vicb Dec 20, 2016

vicb commented Dec 20, 2016

DzmitryShylovich commented Dec 20, 2016

vicb commented Dec 21, 2016 •

edited

Loading

DzmitryShylovich commented Dec 21, 2016

vicb commented Dec 21, 2016

nheidloff commented Dec 21, 2016

DzmitryShylovich commented Dec 21, 2016 •

edited

Loading

angular-automatic-lock-bot bot commented Sep 10, 2019

feat(router): add CanDeactivate next state argument #13560

feat(router): add CanDeactivate next state argument #13560

Conversation

DzmitryShylovich commented Dec 18, 2016

vicb Dec 20, 2016

Choose a reason for hiding this comment

vicb Dec 20, 2016

Choose a reason for hiding this comment

vicb Dec 20, 2016

Choose a reason for hiding this comment

vicb commented Dec 20, 2016

DzmitryShylovich commented Dec 20, 2016

vicb Dec 20, 2016

Choose a reason for hiding this comment

vicb commented Dec 20, 2016

DzmitryShylovich commented Dec 20, 2016

vicb commented Dec 21, 2016 • edited Loading

DzmitryShylovich commented Dec 21, 2016

vicb commented Dec 21, 2016

nheidloff commented Dec 21, 2016

DzmitryShylovich commented Dec 21, 2016 • edited Loading

angular-automatic-lock-bot bot commented Sep 10, 2019

vicb commented Dec 21, 2016 •

edited

Loading

DzmitryShylovich commented Dec 21, 2016 •

edited

Loading