-
Notifications
You must be signed in to change notification settings - Fork 27k
feat(router): add a currentNavigation signal to the Router
#62971
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
Conversation
2bbc640 to
b53f184
Compare
b53f184 to
3e6e1c1
Compare
a9aca0b to
c1ed455
Compare
currentNavigation signal to the RoutercurrentNavigation signal to the Router
c1ed455 to
884fda4
Compare
884fda4 to
71c76a1
Compare
|
|
||
| await runMigration(); | ||
| expect(tree.readContent('/test.ts')).toContain( | ||
| 'const currentNavigation = this.router.currentNavigation();', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been wondering if the migration should also untrack the signal read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While that would technically be most equivalent to the current getCurrentNavigation behavior, I think it's probably best to assume it matters that things change and to recompute results when they do.
71c76a1 to
c59b7d4
Compare
This new signal property is convenient to derive a `isNavigating` state. `isNavigating = computed(() => !!this.router.currentNavigation())` DEPRECATED: The Router.getCurrentNavigation method is deprecated. Use the Router.currentNavigation signal instead. fixes angular#62958
c59b7d4 to
ced90b9
Compare
atscott
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api, size-tracking
thePunderWoman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: public-api
|
This PR was merged into the repository by commit d00b3fe. The changes were merged into the following branches: main |
…` service. (angular#62971)" This reverts commit d00b3fe.
…ngular#62971) This new signal property is convenient to derive a `isNavigating` state. `isNavigating = computed(() => !!this.router.currentNavigation())` DEPRECATED: The Router.getCurrentNavigation method is deprecated. Use the Router.currentNavigation signal instead. fixes angular#62958
|
Reopening since we had to roll it back. |
| @@ -0,0 +1,28 @@ | |||
| ## Remove `Router.getCurrentNavihation` migration | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo mistake
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This new signal property is convenient to derive a
isNavigatingstate.isNavigating = computed(() => !!this.router.currentNavigation())DEPRECATED: The
Router.getCurrentNavigationmethod is deprecated. Use theRouter.currentNavigationsignal instead.