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

feature (ngRoute) expose #parseRoute as public function #4192

Closed
wants to merge 1 commit into from

Conversation

geddski
Copy link
Contributor

@geddski geddski commented Sep 29, 2013

$route.parseRoute should be public, allowing you to determine which route a given path will resolve to. This lets you store meta data in your routes config, and take action (e.g. cancel route) before the route changes.

Example use case: before navigating, check if the route's feature is feature-switched on or off. If off, cancel the route and display a "feature disabled" popup.

Add a feature property to each route:

$routeProvider.when('/profile',
  feature: 'profile',
  template: 'profile.html',
  controller: 'ProfileCtrl'
});

In a main controller, cancel routing and display popup when a route's feature is disabled.

$scope.$on('$locationChangeStart', function(e) {
  // determine the route the user is trying to navigate to
  var nextRoute = $route.parseRoute($location.path());
  // get the feature metadata from the route
  var nextFeature = nextRoute.$$route.feature;
  // check if the feature is set to disabled using an app-specific
  // service
  var status = AppFeatures.status(nextFeature);
  if (status === 'disabled'){
    // cancel the route
    e.preventDefault();
    // display popup
    $scope.showFeatureDisabledModal();
  }
});

Making parseRoute public gives your application a lot of flexibility and control over routing.

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@geddski
Copy link
Contributor Author

geddski commented Nov 25, 2013

@petebacondarwin @btford I've updated this on top of the latest 1.2.2, please review. Thanks!

@geddski
Copy link
Contributor Author

geddski commented Dec 14, 2013

@petebacondarwin when you get a sec please review :)

@ghost ghost assigned btford Dec 19, 2013
@uglow
Copy link

uglow commented Dec 24, 2013

+1

@jraede
Copy link

jraede commented Jan 14, 2014

Has this been added to a release?

@btford
Copy link
Contributor

btford commented Feb 4, 2014

This would close #3288 and #738

@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

@IgorMinar cool if we merge this?

@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

I've updated it again to latest master.

@vojtajina
Copy link
Contributor

The code example in the commit msg is incomplete imho. It only shows how to add feature property to a route, which is something you can do without this change. Can you update the code example to show what you described, i.e. "... only allow navigation to that route if the user has permission for
that feature.". I think it will demonstrate better the purpose of this change, because I'm not entirely sold on this.

And while you touching the commit msg, please change the subject to feat(ngRoute): expose parseRoute as public method.

`$route.parseRoute` should be public, allowing you to determine which
route a given path will resolve to. This lets you store meta data in
your
routes config, and take action (e.g. cancel route) before the route
changes.

Example use case: before navigating, check if the route's feature is
feature-switched on or off. If off, cancel the route and display a
"feature disabled" popup.

Add a `feature` property to each route:

```js
$routeProvider.when('/profile',
  feature: 'profile',
  template: 'profile.html',
  controller: 'ProfileCtrl'
});
```

In a main controller, cancel routing and display popup when a route's
feature is disabled.

```js
$scope.$on('$locationChangeStart', function(e) {
  // determine the route the user is trying to navigate to
  var nextRoute = $route.parseRoute($location.path());
  // get the feature metadata from the route
  var nextFeature = nextRoute.$$route.feature;
  // check if the feature is set to disabled using an app-specific
  // service
  var status = AppFeatures.status(nextFeature);
  if (status === 'disabled'){
    // cancel the route
    e.preventDefault();
    // display popup
    $scope.showFeatureDisabledModal();
  }
});
```

Making `parseRoute` public gives your application a lot of flexibility
and control over routing.
@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

@vojtajina great feedback, thanks man. Done.

@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

Also closes #5581, #592, #738 and #3288

@IgorMinar
Copy link
Contributor

@vojtajina spoke to me about this issue. he's going to post an update here about our discussion and propose an alternative change that would address this use case.

But it comes down to: this change exposes a low level api in order to deal with a deficiency of the higher level api. it might be better to enable route cancelation from within the $routeChangeStart event.

@vojtajina
Copy link
Contributor

@geddski I thought about this some more and based on your code example, I think you want to be able to cancel route/url change based on metadata of the next route.

Rather than exposing $route.parseRoute so that you can do this. How about this change:
$route_change_start happens inside $locationChangeStart and if you cancel $routeChangeStart, it will cancel the location change. That should be much cleaner.

What do you think? Is there any other use case you are trying to cover?

@vojtajina
Copy link
Contributor

@caitp already implemented something similar in 2317794 - I think it would however be better to fire $routeChangeStart inside $locationChangeStart, so that we can cancel the url change, rather than rewriting (rewriting will still add a new item into the history stack).

@vojtajina
Copy link
Contributor

Btw, this does not fully solve #738.

@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

@vojtajina adding the ability to cancel $routeChangeStart leaving the history unchanged would solve this use case, yes. I don't understand what you mean however by:

$route_change_start happens inside $locationChangeStart

Would you mind providing an example like mine, but with this api?

@vojtajina
Copy link
Contributor

An example, assuming the same route definitions as yours:

$scope.$on('$routeChangeStart', function(e, currentRoute, nextRoute) {
  var nextFeature = nextRoute.$$route.feature;

  if (!somehowCheckIfThisFeatureIsAvailable(nextFeature)) {
    e.preventDefault();
    $scope.showFeatureDisabledModal();
  }
});

@geddski
Copy link
Contributor Author

geddski commented May 22, 2014

Ok yeah that would be great.

@geddski
Copy link
Contributor Author

geddski commented May 14, 2015

no longer needed

@geddski geddski closed this May 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants