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

feat($routeProvider): allow setting caseInsensitiveMatch on the provider #9873

Closed

Conversation

pkozlowski-opensource
Copy link
Member

Fixes #6477

@@ -141,10 +141,13 @@ function $RouteProvider() {
* Adds a new route definition to the `$route` service.
*/
this.when = function(path, route) {
var routeWithDefaults = angular.extend({
Copy link
Contributor

Choose a reason for hiding this comment

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

this will need to change slightly if #9731 lands, and I think we want to land it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, but it shouldn't be a big deal. I can rebase / change this one when #9731 lands. I would like to get this one in as I was really surprised that we can't have provider-level definition for case-sensitive routes.

Copy link
Contributor

Choose a reason for hiding this comment

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

#9731 has landed, should rebase

@pkozlowski-opensource
Copy link
Member Author

@caitp it was rebased on master after #9731landed. Ready for the review / merging.

@caitp
Copy link
Contributor

caitp commented Nov 4, 2014

thanks, looking

@@ -146,6 +146,9 @@ function $RouteProvider() {
if (angular.isUndefined(routeCopy.reloadOnSearch)) {
routeCopy.reloadOnSearch = true;
}
if (angular.isUndefined(routeCopy.caseInsensitiveMatch)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should use if (!'prop' in routeCopy) instead, since that's really what we're testing. Either way, looks fine to me

@pkozlowski-opensource
Copy link
Member Author

@caitp thnx for the review. I guess based on your comments we could merge it but then we need to figure safe way of landing those types of things in 1.3.x / 1.4. Which goes back to the discussion we had yesterday. I guess this PR is a perfect example of little / low risk new features for which we need to figure out good strategy.

@caitp
Copy link
Contributor

caitp commented Nov 4, 2014

well it's not really changing existing behaviour, it's adding a new behaviour --- so it should be pretty safe.

@caitp
Copy link
Contributor

caitp commented Nov 11, 2014

@pkozlowski-opensource this looks good to me, merge at will

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.

Allow to set default value of caseInsensitiveMatch to true in routing for application
3 participants