Skip to content
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

Add detail to Router NavigationCancel event doc #32140

Closed
wants to merge 1 commit into from

Conversation

jbogarthyde
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Description of event trigger is minimal.

Issue Number: #32096

What is the new behavior?

Expands description with cases where nav is canceled indirectly.

Does this PR introduce a breaking change?

  • Yes
  • No

@mary-poppins
Copy link

You can preview e135ad0 at https://pr32140-e135ad0.ngbuilds.io/.

@dzhavat
Copy link
Contributor

dzhavat commented Aug 14, 2019

Thanks for making the PR :)

I wanted to work on this issue as well in order to start contributing. Even prepared a small commit earlier today. My change looks like this:

/**
 * An event triggered when a navigation is canceled.
 *
 * The event is triggered when a route guard:
 * 
 * * returns `false`
 * * initiates a redirect by returning a {@link UrlTree}
 * 
 * @publicApi
 */

I've also deleted the sentence This is due to a Route Guard returning false during navigation. in the router page because that's not the only case.

Maybe you can do that as well?

@jbogarthyde
Copy link
Contributor Author

Thanks! Will do.

@mary-poppins
Copy link

You can preview 4b986f2 at https://pr32140-4b986f2.ngbuilds.io/.

@mary-poppins
Copy link

You can preview d5ee07b at https://pr32140-d5ee07b.ngbuilds.io/.

@jbogarthyde jbogarthyde added this to In Review in docs Aug 14, 2019
@jbogarthyde jbogarthyde force-pushed the jb-router-nav branch 2 times, most recently from d1a4d5e to 4a9518d Compare August 19, 2019 15:48
@mary-poppins
Copy link

You can preview d1a4d5e at https://pr32140-d1a4d5e.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 4a9518d at https://pr32140-4a9518d.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 3af85fd at https://pr32140-3af85fd.ngbuilds.io/.

@mary-poppins
Copy link

You can preview b8fbd7f at https://pr32140-b8fbd7f.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 14d9e0c at https://pr32140-14d9e0c.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 3a2427e at https://pr32140-3a2427e.ngbuilds.io/.

@jbogarthyde jbogarthyde added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Aug 27, 2019
@jbogarthyde
Copy link
Contributor Author

Doc only, should not need G3.
There doesn't seem to be a code-owner for Router anymore.

@jbogarthyde jbogarthyde moved this from In Review to Waiting for Approval in docs Aug 27, 2019
@jbogarthyde jbogarthyde moved this from Waiting for Approval to Done in docs Aug 27, 2019
@atscott atscott closed this in e3f4281 Aug 27, 2019
atscott pushed a commit that referenced this pull request Aug 27, 2019
@jbogarthyde jbogarthyde deleted the jb-router-nav branch August 28, 2019 16:56
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
arnehoek pushed a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview area: router cla: yes effort1: hours freq1: low merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risk: low target: patch This PR is targeted for the next patch release type: bug/fix
Projects
docs
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants