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

feat($location): Location change notifications can now be skipped #2398

Closed
wants to merge 1 commit into from

Conversation

lrlopez
Copy link
Contributor

@lrlopez lrlopez commented Apr 13, 2013

This adds a new notify method to $location that allows updating the location without triggering $locationChangeStart/$locationChangeSuccess events.

The method is chainable and must be called with a boolean parameter. Any falsy value will disable the notification procedure and will block any route update that may apply.

The skip flag will be reset after any digest cycle, so it must be set again on any subsequent change if needed.

Example: $location.path('/client/2').replace().notify(false);

Closes #1699

This adds a new method `notify` to `$location` that allows updating the
location without triggering `$locationChangeStart`/`$locationChangeSuccess`
events.

The method is chainable and must be called with a boolean parameter. Any
falsy value will disable the notification procedure and will block any
route update that may apply.

The skip flag will be reset after any digest cycle, so it must be set again
on any subsequent change if needed.

Example: `$location.path('/client/2').replace().notify(false);`

Closes angular#1699
@lrlopez
Copy link
Contributor Author

lrlopez commented Apr 13, 2013

I've already signed the CLA. This feature adds 114 bytes to the minified build. Issue #1699 describes the scenario that inspired this PR. The original idea came from @qualidafial.

Tests and documentation changes are also included.

@petebacondarwin
Copy link
Contributor

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
  • Feature improves existing core functionality
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@IgorMinar
Copy link
Contributor

-1

I talked to peter and explained to him how this breaks routing. he'll provide more details shortly

@lrlopez
Copy link
Contributor Author

lrlopez commented Apr 23, 2013

Ok! Let's see what's going on...

@petebacondarwin
Copy link
Contributor

Hi @lrlopez

Thanks once again for the work that you put into this PR. You did all the right things with the implementation, style, tests, docs, CLA, and so on.

As Igor mentioned, we discussed this PR yesterday. The problem is that if you use the $route service then you are signing up to a way of running your app, where the URL represents a specific route. By allowing people to change the URL, without changing the route, you are subverting the way that the routing system works.

The result is that you are then in an application state where the URL indicates a particular route is running but actually a different route is in place. It may be that you are able in your own application to manage this inconsistency but as a general rule it will likely lead to people abusing it or just getting themselves into trouble and creating difficult to identify bugs.

So where does this leave us for your particular issue?

The simplest solution is to change routes when you have saved the item, changing the URL and giving the application the chance to get itself into the correct state. Your concern with this is that when you change route from mything/new to mything/1234, the controller of the second route will reload all the data of the item that you have just created and so this is wasteful of bandwidth or visual performance (a flicker of unwanted reload)?

You could get around the former by using a service to store the current mything, which is checked when you enter the mything/:id route. If the current mything is the same as requested no server call need be made. This way you keep the controller as light as possible.

The latter, is more problematic in the current form of the routing service. Since the route will change, the ng-view will be destroyed and reloaded. It is possible that you could get around this by doing some manual work. For instance, you could try having a more general route for mythings then dealing with the edit/new state yourself, by watching the location path and using ng-includes.

Also, it might be worth taking a look at the AngularUI ui-router project, https://github.com/angular-ui/ui-router, which is more configurable and flexible, although still immature, and may be able to support what you need more easily.

I am going to close this PR for now. If you have further ideas or feel that we have come to the wrong conclusion the do please convince us otherwise of the use and suitability.

Thanks
Pete

@lrlopez
Copy link
Contributor Author

lrlopez commented Apr 25, 2013

I understand. Anyway, I'll leave the feature branch on my repository so it can be used by anyone who needs this. Just drop me a line if it gets unmergable and I'll rebase to latest master.

@mgcrea
Copy link
Contributor

mgcrea commented Jul 11, 2013

Would be great if you could reconsider this PR as this is really required for mobile applications where the ng-view create/destroy lifecycle is not appropriate (it kills performance and usability). Flexibility isn't a bad thing and clearly the current ngView paradigm is too strict for most people out there.

@cdmckay
Copy link

cdmckay commented Jul 12, 2013

Is there any reason why you can't make $location update the route information if the reload is suppressed?

@petebacondarwin
Copy link
Contributor

ngRoute is now a separate module from the core download (in 1.1.x). It has
never been easier for community to take a copy that module, fork it into a
new repository, add functionality like this, put it up on bower and see how
popular it gets.
If it gets lots of traction then perhaps the core team could be convinced
otherwise? In the meantime it would give people a very accessible solution.

On 12 July 2013 03:25, Cameron McKay notifications@github.com wrote:

Is there any reason why you can't make $location update the route
information if the reload is suppressed?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2398#issuecomment-20855035
.

@NaomiN
Copy link

NaomiN commented Nov 7, 2017

I read several issues yesterday but I don't understand as an AngularJs user what exactly is implemented and how should I achieve the functionality of changing just the URL without reloading. What should be my final implementation? Yesterday I used $state.go('someState', {notify: false, location: replace}); but I'd rather just change location if possible.

@petebacondarwin
Copy link
Contributor

@NaomiN you cannot do it with ngRoute. You may be able to do it with ui-router (which I notice you are using given the use of $state). Perhaps that is a question for them?

@NaomiN
Copy link

NaomiN commented Nov 13, 2017

I used switch to another state. Seems to be working fine.

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

Successfully merging this pull request may close these issues.

Option on $location to allow hash/path change w/o reloading the route
6 participants