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

feat($routeProvider): Allow using functions as template params in 'when' #1524

Closed
wants to merge 1 commit into from

Conversation

lrlopez
Copy link
Contributor

@lrlopez lrlopez commented Nov 3, 2012

This pull-request allows using custom functions when assigning templates in $routeProvider.when() method.

The new functionality could be used to generate templates on the fly based on the route parameters. Also, if used in templateUrl, the particular template URL that gets loaded can be customized.

P.S. I've just signed the CLA.

@lrlopez
Copy link
Contributor Author

lrlopez commented Nov 11, 2012

@IgorMinar , is there anything I should do to get this PR into consideration?

@lrlopez
Copy link
Contributor Author

lrlopez commented Dec 2, 2012

@pkozlowski-opensource . Igor seems to be MIA. Is this PR ok?

@pkozlowski-opensource
Copy link
Member

@lrlopez don't worry, he is not :-)

As for your PR - it looks good from what I can tell by very quickly looking at it. Having said this there are multiple PRs touching the routing system and we would like to look at them as a whole since some of them are a bit conflicting.

@lrlopez
Copy link
Contributor Author

lrlopez commented Dec 2, 2012

@pkozlowski-opensource, thanks for the feedback.

I've been reviewing some of the PRs related to the routing system and I think this particular one is fully compatible with them.

Anyway, I also think that routing could be further improved with lot of useful features (i.e. states-charts, subviews, etc).

Why don't we create an issue or a thread in the mailing list to throw in any ideas on this respect? We could enumerate the weakness in the current system and propose different potential solutions to them, along with their respective pro&cons and use cases. May be we could get one core team member assigned into it to coordinate the discussion. Of course, maintaining backward compatibility could be a requirement if the AngularJS team thinks so.

Do you like the idea?

@pkozlowski-opensource
Copy link
Member

@lrlopez Yes, I think that discussing this further is inevitable. As you've noticed there are several PRs touching the routing system (and another bunch of open issues). I think that it would be really beneficial to further discuss the requirements for the improved routing as well as the syntax that should be supported. As soon as this is settled the implementation should be easy :-)

So yes, if you could post a kick-off thread on the mailing list (linking to all the open PRs) it would be great. Maybe people will come to the same conclusions and agree on the needs / syntax.

@lrlopez
Copy link
Contributor Author

lrlopez commented Dec 2, 2012

Posted! I've created a thread for each aspect it could be enhanced about AngularJS routing.

https://groups.google.com/forum/?hl=en&fromgroups=#!topic/angular/dTw2PIpGqsk

@lrlopez
Copy link
Contributor Author

lrlopez commented Dec 4, 2012

We are discussing how to enhance parameter specification here: https://groups.google.com/forum/#!topicsearchin/angular/embedded/angular/w-LPkyrBWqY

Can you help us?

@mhevery
Copy link
Contributor

mhevery commented Jan 18, 2013

Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement).

CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html

@ghost ghost assigned mhevery Jan 18, 2013
@lrlopez
Copy link
Contributor Author

lrlopez commented Jan 18, 2013

@mhevery
Thanks for the review Misko, I already signed the CLA some weeks ago...

@mhevery
Copy link
Contributor

mhevery commented Jan 19, 2013

MERGED

@mhevery mhevery closed this Jan 19, 2013
@schmuli
Copy link

schmuli commented Jan 19, 2013

How difficult would it be to add this support for directives?

@lrlopez
Copy link
Contributor Author

lrlopez commented Jan 19, 2013

Hmmm.... I don't think it would be a problem... I'll try to create a new PR tomorrow

@olostan
Copy link
Contributor

olostan commented Jan 20, 2013

Another good change could be to be able to supply function to templaceUrl. I suppose that would not be a big change...

@schmuli
Copy link

schmuli commented Jan 20, 2013

Thanks, Luis.
Actually, after looking at the changes you made for Routing and then looking at the existing code in $compile, this should be almost the same code. I would do it myself, but I haven't yet signed the CLA nor set up a proper environment for creating a PR.
If you look at existing issues for AngularJS, you can see that there is a lot of discussion about making dynamic templates for directives. I believe this idea would cover all the issues. It will also make some existing templates simpler, for example those that use the ng-switch directive within the template.

@lrlopez
Copy link
Contributor Author

lrlopez commented Jan 20, 2013

I agree with you that it would be a powerful enhancement...

But I fear it will take some more time to implement because it is not as trivial as I thought. First of all, I need to think what parameters should be associated with the called function (take into account that we are constrained to the info available at compile time).

I'll keep working to have the PR ready ASAP.

@lrlopez
Copy link
Contributor Author

lrlopez commented Jan 20, 2013

Well, PR #1849 ready to go!

@lrlopez lrlopez deleted the template-functions branch January 20, 2013 20:10
@lrlopez
Copy link
Contributor Author

lrlopez commented Jan 23, 2013

Ping @pkozlowski-opensource @IgorMinar @mhevery @vojtajina

Sorry for bringing your attention in such a inconvenient way, but I'd rather not pollute the issues queue. I don't know if you are aware that a task group has been formed under the angular-ui project to discuss enhancing the AngularJS routing in a backward compatible way. We would really like someone from then AngularJS team to join us in order to, eventually, embed the changes into the main codebase so we can all benefit. Pawel requested something similar to this two months ago in this same PR.

What do you think? Most of the discussion is here: https://github.com/angular-ui/router/issues/1 https://github.com/angular-ui/router/issues/7

Thank you in advance!

@saurabhnanda
Copy link

Has this been merged? If yes, the docs don't talk about it at http://docs.angularjs.org/api/ng.$routeProvider

@lrlopez
Copy link
Contributor Author

lrlopez commented Feb 13, 2013

Yes, it has. The new functionality is available in 1.1.2, which is considered unstable. I think docs are built from the stable branch (1.0.4) so that could explain the missing piece of info from the docs.

@lrlopez lrlopez deleted the template-functions branch February 23, 2013 18:55
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.

6 participants