-
Notifications
You must be signed in to change notification settings - Fork 354
feat(route) prototyping a new route checking directive #374
Conversation
modules/directives/route/route.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attrs.uiRoute evaluates to false all the time. I guess its because its getting treated as directive attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... you're right, but it's only if you use {{}} and even if you put text OUTSIDE of the brackets it's still undefined initially. So... why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only way around it I can think of is elm.attr(attrs.$attr.uiRoute) instead but this won't work if used as a class. I have NO idea why it's undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quickly groked ng-href directive. Even there, they are only observing the change on ng-href attr. In our case attrs.ngHref is available because we have set priority to 100.
Is it something to do with uiRoute and its expression are treated separately but with same priority 100. then in tat case order of execution is unknown (this is a wild wild guess..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably should restrict A
|
@ProLoser Looks good to me & much cleaner than my initial proposal.. |
|
Landed in 0f65bd9 |
|
What does it do exactly? Are there docs for it? |
|
Not really lol. I should expand the docblock. I was going to add a demo when I push the new build. @Narretz Want to help me out an open a PR? Here's basically how it works:
Declaring a route-match patternui-route <li ui-route="/dashboard">
or
<li ui-route="/user/[0-9]*">
or
<li ng-repeat="tab in tabs" ui-route="/{{tab}}">
or
<li ng-repeat="tab in tabs" ui-route="/{{tab}}(/[0-9]*)?">ng-href <a ng-href="/dashboard" ui-route>
or
<a ng-href="/user/{{userId}}" ui-route>href <a href="/dashboard" ui-route>Working with the flag<li ui-route="/dashboard" ng-class="{active:$uiRoute}">
<a href="/dashboard">Dash</a>
<ul>
<li ui-route="/dashboard/stats" ng-model="isStatsRoute">
<div ng-show="isStatsRoute && $uiRoute">Yes it's a shitty example |
|
@joseym Again, I'm not sure how much you're willing to take on, but if you are willing to help me out, The above comment ^^ needs to be turned into an open PR for the demo page that we can merge in once we push v0.4.0 It does not need to have an updated version of angular-ui in the PR as that will be changed already. |
|
Sure, this is great. |
In response to #348
This is just a WIP, DO NOT MERGE YET