-
Notifications
You must be signed in to change notification settings - Fork 26.8k
docs(router): update navigation event example to use event.code #65236
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
| } else if (event instanceof NavigationCancel) { | ||
| console.warn('Navigation cancelled:', event.reason); | ||
| if (event.reason === NavigationCancellationCode.GuardRejected) { | ||
| console.warn('Navigation cancelled:', event.code); |
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.
Similar to what was mentioned earlier, I think event.reason should be preserved for the example on line 179
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.
On the other hand, you can amend the commit so it links correctly to the issue
See Commit Message Footer
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 have updated the line that i forgot to update when i was recreating the pr also now i have mentioned my two old pr which i closed and raised a new one.Please do consider this one.And sorry for the mistakes i have made.
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.
You can merge both commits into one.
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.
Yes I have merged both the pr in this one and created a new pr also I closed both my old pr I just mentioned that my previous pr refers to same issue so you can connect with this pr
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.
You should keep this as a single commit rather than the two separate commits currently shown.In the tab Commits
Please squash your changes into one commit before submitting.
b58def4 to
8b4ce3d
Compare
The example was comparing (string) against enum, causing a TypeScript type error (TS2367). Also, the import from was missing. This change updates the example to use , ensuring type compatibility and adding the missing import. See: https://next.angular.dev/guide/routing/lifecycle-and-events#error-handling
8b4ce3d to
78c406d
Compare
Update the code sample to use
event.codeinstead ofevent.reasonfor cancellation checks, while preservingevent.reasonin the console.warn line. This aligns the example with the NavigationCancellationCode enum and prevents confusion.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #65182
What is the new behavior?
Does this PR introduce a breaking change?
The previous example incorrectly compared event.code. Updated to use event.reason
to match the current NavigationCancel API. This fixes the type errors that were
caused by the old example.
Closes #65232
Closes #65235