Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat($route): ability to cancel $routeChangeStart event #9502

Closed
wants to merge 1 commit into from

Conversation

tbosch
Copy link
Contributor

@tbosch tbosch commented Oct 8, 2014

Calling preventDefault() on a $routeChangeStart event will
prevent the route change and also call preventDefault on the $locationChangeStart event, which prevents the location change as well.

BREAKING CHANGE:

Order of events has changed.
Previously: $locationChangeStart -> $locationChangeSuccess
-> $routeChangeStart -> $routeChangeSuccess

Now: $locationChangeStart -> $routeChangeStart
-> $locationChangeSuccess -> -> $routeChangeSuccess

Fixes #5581
Closes #5714

Calling `preventDefault()` on a `$routeChangeStart` event will
prevent the route change and also call `preventDefault` on the `$locationChangeStart` event, which prevents the location change as well.

BREAKING CHANGE:

Order of events has changed.
Previously: `$locationChangeStart` -> `$locationChangeSuccess` 
  -> `$routeChangeStart` -> `$routeChangeSuccess`

Now: `$locationChangeStart` -> `$routeChangeStart` 
  -> `$locationChangeSuccess` ->  -> `$routeChangeSuccess`

Fixes angular#5581
Closes angular#5714
@tbosch
Copy link
Contributor Author

tbosch commented Oct 8, 2014

@caitp Hi Caitlin, @IgorMinar and I just came up with this implementation to prevent route change. Can you take a look?

@tbosch tbosch closed this Oct 8, 2014
@tbosch tbosch deleted the feat/routeabort branch October 8, 2014 22:35
@btford btford removed the In Progress label Oct 8, 2014
tbosch added a commit that referenced this pull request Oct 8, 2014
Calling `preventDefault()` on a `$routeChangeStart` event will
prevent the route change and also call `preventDefault` on the `$locationChangeStart` event, which prevents the location change as well.

BREAKING CHANGE:

Order of events has changed.
Previously: `$locationChangeStart` -> `$locationChangeSuccess`
  -> `$routeChangeStart` -> `$routeChangeSuccess`

Now: `$locationChangeStart` -> `$routeChangeStart`
  -> `$locationChangeSuccess` ->  -> `$routeChangeSuccess`

Fixes #5581
Closes #5714
Closes #9502
@caitp
Copy link
Contributor

caitp commented Oct 9, 2014

Not keen on the breaking change since this was perfectly implementable without it, but hey

@IgorMinar
Copy link
Contributor

It wasn't without hacks that restored the location state.
On Oct 8, 2014 5:39 PM, "Caitlin Potter" notifications@github.com wrote:

Not keen on the breaking change since this was perfectly implementable
without it, but hey


Reply to this email directly or view it on GitHub
#9502 (comment).

@caitp
Copy link
Contributor

caitp commented Oct 9, 2014

cancelling $locationChangeStart should restore the location state, there is logic within $location itself to do that

bullgare pushed a commit to bullgare/angular.js that referenced this pull request Oct 9, 2014
Calling `preventDefault()` on a `$routeChangeStart` event will
prevent the route change and also call `preventDefault` on the `$locationChangeStart` event, which prevents the location change as well.

BREAKING CHANGE:

Order of events has changed.
Previously: `$locationChangeStart` -> `$locationChangeSuccess`
  -> `$routeChangeStart` -> `$routeChangeSuccess`

Now: `$locationChangeStart` -> `$routeChangeStart`
  -> `$locationChangeSuccess` ->  -> `$routeChangeSuccess`

Fixes angular#5581
Closes angular#5714
Closes angular#9502
@@ -481,7 +533,7 @@ describe('$route', function() {


describe('events', function() {
it('should not fire $after/beforeRouteChange during bootstrap (if no route)', function() {
it('should not fire $routeChangeStart/success during bootstrap (if no route)', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success ? (capital S)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks for catching this. Could you create a PR to fix it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to cancel $routeChangeStart event
6 participants