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

Parameter Typing -- URL Only #454

Closed
wants to merge 25 commits into from

Conversation

toddhgardner
Copy link
Contributor

I'm a bit concerned I'm not going down the right path here, so I'd love some feedback on this before I go farther. With this code, you can add types which will be encoded/decoded as they pass back and forth to the URL. Everything is handled with the URL property right now, no params structure.

mattbroekhuis and others added 6 commits September 10, 2013 14:42
@@ -107,6 +107,13 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory, $
$delegates: {}
};

// Create a proxy through to the Type registration on the UrlMatcherFactory
this.isTypeRegistered = $urlMatcherFactory.isTypeRegistered;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be better to make type() a getter if you only pass a name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but instead, I've just dropped it. I only needed it for test purposes, and the exposed type function has to return the state object so that the config can be chained.

@nateabele
Copy link
Contributor

So, it's looking like you're on the right track so far. In the issue thread you mentioned something about normalize(). That function can actually go away entirely, as type handling obviates its existence. The two calls to it can be replaced with filterByKeys().

Let me know if you have any other questions, but barring the one note I left, it looks good to me.

@timkindberg
Copy link
Contributor

Aren't we still missing a way to check the type before considering it a match or am I missing that? I don't see a test for it either.

Also in the Sample you added a custom date type but I don't think it's used in any urls, if it's not, we should add an example or not use it. We shouldn't have any extraneous code in the sample. Also you changed a param in the sample from regex to integer but you didn't change the comment above it which still talks about regex usage.

@nateabele
Copy link
Contributor

Aren't we still missing a way to check the type before considering it a match or am I missing that?

I showed how to do it in the issue thread.

Also in the Sample you added a custom date type but I don't think it's used in any urls, if it's not, we should add an example or not use it.

It's still a work-in-progress, he was just checking in.

@alejandroiglesias
Copy link

How's the status of this?

@leog
Copy link

leog commented Mar 14, 2014

+1

@iangilman
Copy link

I'm having an issue where I want to include the contents of my search input box in the route as a param, like so:

    $rootScope.$state.go('search', { query: $scope.searchTerm });

...but if the user types a slash, then it adds that slash to my URL, which routes me to a new location. I want the slash to be encoded, but if I manually encode it myself to %2F, ui-router then encodes the % so the result is %252F.

I just want to be able to tell ui-router to encode (and decode when appropriate) the slash as well. My understanding is that this patch will allow me to do that. If that's correct, here's my +1!

@nateabele
Copy link
Contributor

Btw everyone, here's the current status of this feature: https://github.com/angular-ui/ui-router/commits/types

@iangilman Feel free to submit an issue with a Plunkr that demonstrates the problem, or a PR with a unit test.

@iangilman
Copy link

@nateabele Thank you for the update! I'm not confident in my ability to boil the issue down to a Plunkr, but I'll post here if I make it happen.

@nateabele
Copy link
Contributor

This has been implemented.

@nateabele nateabele closed this Apr 23, 2014
@iangilman
Copy link

Awesome :-)

@ChrisCinelli
Copy link

Is it time to release v.2.11, then? =)

@phazei
Copy link

phazei commented Mar 23, 2016

Can someone point to the docs for this? I can't find anything for where parameter typing is :(

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

Successfully merging this pull request may close these issues.

9 participants