Skip to content

Method for generating URLs from states #138

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

Merged
merged 1 commit into from
May 24, 2013
Merged

Conversation

nateabele
Copy link
Contributor

I put together a really simple method for reversing out a URL from a state + parameter hash, which should fix #128. If this looks good, I'll use it to implement #16.

@ksperling Let me know if this looks okay or if I missed anything.

@jeme
Copy link
Contributor

jeme commented May 23, 2013

👍

I couldn't see this clearly out of the code if it was already possible, would it be an improvement to allow things like below?:

expect($state.href("about.person", { person: "bob" })).toEqual("/about/bob");
expect($state.href("about.person.item", { person: "bob", id: null })).toEqual("/about/bob/");

$state.transitionTo("about.person", { person: "bob" });
//Current state contains "person: bob"...
expect($state.href("about.person.item", { id: 5 })).toEqual("/about/bob/5");

As in many cases about.person would be active when you get links to about.person.item...
In any case, just wondering if we should add that as a future improvement...

In any case, this one is good to merge from my perspective... But I will let @ksperling do this one as it's a bigger change.

@nateabele
Copy link
Contributor Author

@jeme Parameters don't cascade currently. It only goes off of what you pass it explicitly. I remember there was some concern that cascading parameters would be difficult to debug, but I think it makes sense if you're transitioning from a parent state to a child state, or more generically, inheriting parameters from the common ancestors of any two states.

If this looks good to start, I can work on parameter cascading/inheritance next.

@StephanHoyer
Copy link

+1, need this ;)

@jeme
Copy link
Contributor

jeme commented May 24, 2013

Ok... No yay or nay from @ksperling yet, so I will pull this in, after looking at the change, if he disagrees with it as is, it is so localized a change, and doesn't really affect the rest of the solution, and therefore can be removed again easily... After all, I think he agrees on the interface of the function at least.

jeme added a commit that referenced this pull request May 24, 2013
Method for generating URLs from states
@jeme jeme merged commit b027375 into angular-ui:master May 24, 2013
@ksperling
Copy link
Contributor

Looks good apart form the problem with state.navigable (probably a good idea to add a test for that case as well, i.e. have a non-navigable child of a navigable parent: calling href() on the child should throw)

@ksperling
Copy link
Contributor

In terms of "cascading" or "defaulting" from current parameters, I agree with @nateabele this should be limited to parameters that those states inherit between them (which can be either way around depending on if you're linking to an ancestor or a descendant). Unrelated parameters that just happen to have the same name shouldn't be defaulted. The parameter defaulting should be factored out into it's own (private) function so that we can also use it for transitionTo().

Ideally we'd also verify that all required parameters have been specified and are non-null, but this needs more work on the UrlMatcher side to actually produce that meta-data for each parameter -- at this point I'd say all path parameters are required and all query parameters are optional. So I guess we want 'params' and 'ownParams' to become maps from parameter names to meta-data, rather than just arrays of name.

@nateabele
Copy link
Contributor Author

@ksperling K, I'll work on this. Thanks for the feedback.

@fxck
Copy link

fxck commented Jun 6, 2013

What's the state of this and this #139 ? This was apparently merged, but I can't seem to be able to use it, using the latest snapshot, anyway..

//edit
scratch that, it is in the latest snapshot, still interested how's #139 coming..

..also thanks, once these both are done it's gonna make my life so much easier

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.

A reverse function that generates URI string from state name and params?
5 participants