angular-ui / ui-router

The de-facto solution to flexible routing with nested views in AngularJS
http://ui-router.github.io/
MIT License
13.54k stars 3k forks source link

Dynamic ui-sref in directive #395

Closed snikch closed 9 years ago

snikch commented 11 years ago

I've written a breadcrumb directive which takes an array of crumbs, each with a name and sref property. I'm trying to bind the ui-sref directive ($StateRefDirective) to use the evaluated attribute, but it's not working. When the ui-sref directive's link function is called, the attrs.uiSref value is undefined, but immediately after the running of the function becomes available as the correct value.

angular.module('myApp').directive 'breadcrumb', ->
  scope: { crumbs: '=' }
  replace: true
  template: '
    <nav class="breadcrumbs">
      <a ui-sref="{{crumb.sref}}" ng-repeat="crumb in crumbs">{{crumb.name}}</a>
    </nav>
  '

Inside the $StateRefDirective link method, logging attrs gives

Attributes {… uiSref: undefined …}

Running the same log after a 100ms timeout gives:

Attributes {… uiSref: "apps.show({appId: crumb.app.id})" …}

Am I doing something wrong, or are dynamic ui-sref attributes not supported?

Note: Running v0.2.0

nateabele commented 11 years ago

[A]re dynamic ui-sref attributes not supported?

Correct.

snikch commented 11 years ago

Technically difficult, or just not a priority?

nateabele commented 11 years ago

More of an API design consideration.

snikch commented 11 years ago

Ah ok. Is there an official guideline as how one would implement dynamic links then? I'm guessing a function that calls the full $state.go(…), however it would have been nice to have the href tag set for when hovering over the link.

nateabele commented 11 years ago

Yeah, you can use $state.href() to generate the URL.

snikch commented 11 years ago

Cool, thanks!

nateabele commented 11 years ago

No worries. If there gets to be a big enough demand for this, we'll come back around and design a proper implementation, so I'll keep your notes here in mind.

julien-lafont commented 10 years ago

No worries. If there gets to be a big enough demand for this, we'll come back around and design a proper implementation, so I'll keep your notes here in mind.

:+1:

ghost commented 10 years ago

this directive was in this repository before v0.2.0, but it was not a part of the release installed with bower. I found it, used it and faced this bug. I fixed this issue for myself and thought it was no official part of ui-router.

Today - using v0.2.0 I faced the need of this feature again and I found my ui-sref.js in my git repository. Here is the code I used earlier:

$StateRefDirective.$inject = ['$state', '$interpolate'];
function $StateRefDirective($state, $interpolate) {

    function parseStateRef(ref) {
        var parsed = ref.match(/^([^(]+?)\s*(\((.*)\))?$/);
        if (!parsed || parsed.length !== 4) throw new Error("Invalid state ref '" + ref + "'");
        return { state: parsed[1], paramExpr: parsed[3] || null };
    }

    function hasToBeInterpolated(attributeValue) {
        return attributeValue.indexOf("{{") != -1
    }

    function isForm(element) {
        return element[0].nodeName === "FORM";
    }

    function buildNewHref(attributeValue, params) {
        return"/#" + $state.href(attributeValue.state, params, { lossy: true });
    }

    function setUrl(element, attr, newHref) {
        element[0][attr] = newHref;
    }

    return {
        restrict: 'A',
        priority: 1000,
        link: function (scope, element, attrs) {
            var attributeValue = attrs.uiSref;

            if (hasToBeInterpolated(attributeValue)) {
                attributeValue = $interpolate(attributeValue)(scope);
            }

            attributeValue = parseStateRef(attributeValue);

            var params = null
            var url = null;
            var attr = isForm(element) ? "action" : "href"; //, nav = true;

            var update = function (newParams) {
                if (newParams) {
                    params = newParams;
                }
                var newHref = buildNewHref(attributeValue, params);
                if (!newHref) {
                    nav = false;
                    return false;
                }
                setUrl(element, attr, newHref);
            };

            if (attributeValue.paramExpr) {
                scope.$watch(attributeValue.paramExpr, function (newParams, oldVal) {
                    if (newParams !== oldVal) {
                        update(newParams);
                    }
                }, true);
                params = scope.$eval(attributeValue.paramExpr);
            }
            update();
        }
    };
}

angular.module('ui.state').directive('uiSref', $StateRefDirective);

There could be some bugs, because I used it only once in testproject and It never got into production.

nateabele commented 10 years ago

@maklemenz Um, this isn't a bug. Also, that implementation doesn't even work for HTML5 mode.

ghost commented 10 years ago

@nateabele Yes, I said my code will not be production ready (or will contain bugs). It feels like a bug, because you cannot use this directive in any more complex application where you generalize things with directives. You need to resolve the url yourself with the services. For me this means I cannot use this directive in 99.9% of all my applications and need to do everything that directive is made for by hand. that feels like a bug IMHO.

nateabele commented 10 years ago

@maklemenz Please see my notes on #139, specifically:

I know there was some mention of a more dynamic way of referencing states. IMO that is out-of-scope for a shorthand syntax. If people want something more advanced, they can combine ng-href with $state.href().

ghost commented 10 years ago

@nateabele I see. A design decision. Personally I think ui-sref="{{stateName}}( {{stateParams}} )" is pretty ugly, but I could live with that. Ok, I will keep using the service for now.

KayEss commented 10 years ago

No worries. If there gets to be a big enough demand for this, we'll come back around and design a proper implementation, so I'll keep your notes here in mind.

Just wanted to say I came here looking for the exact same thing. I'll work something else out for now.

dmatteo commented 10 years ago

I was looking for the same exact feature, and wondering why it wasn't working...

@nateabele Actually, I can't see why you think this is out-of-scope for a routing service.

My use case is pretty straight forward. I just wanted a pointer to the caller state, and I passed it to the view using

    $rootScope.$on('$stateChangeStart', function (event, toState, toParams, fromState, fromParams) {
        $scope.callbackState = fromState.name
    })

And I would love to call it back with something like:

    <article ui-sref="{{callbackState}}">...</article>

It doesn't look out-of-scope to me, it's a "simple" route back to the caller

MrOrz commented 10 years ago

I come across this issue when I am refactoring out all my ng-href to ui-sref.

In my scenario I store the state name in a variable in the scope, which is the same as @dmatteo mentioned.

I guess I just cannot remove the URL helper I wrote that invokes $state.href from my rootScope. :cry:

timkindberg commented 10 years ago

So this has roughly +4 or 5 at this point... I think its worthy of reopening.

MrOrz commented 10 years ago

My current solution is to create a new directive called srefName, which looks like this when used:

<a sref-name="stateNameVariableInScope" sref-param="{...}">...</a>

srefName uses $scope.eval to read the state name and attrs.srefParam.

By separating the state name and state params, it looks slightly better than interpolation (though I think there is nothing bad with interpolation.)

clouddueling commented 10 years ago

Currently facing a need for this right now. :+1:

KrisBraun commented 10 years ago

Just hit a need for this. Adding a function on my model that uses $state.href wasn't too bad, though.

imjoshholloway commented 10 years ago

+1 for this.

jlmagee commented 10 years ago

+1 This would be unbelievably useful for creating data driven nav menus.

update 2014-01-19: this actually works fine -- as expected. see html snippet

<div class=" navbar-header navbar-collapse collapse navbar-responsive-collapse" id="bs-example-navbar-collapse-1"  ng-controller="topNav">
    <ul class="nav navbar-nav">
        <li class="dropdown" ng-repeat="menu in menus">
            <a class="dropdown-toggle {{ menu.setclass }}" data-toggle="dropdown" ui-sref="static({ thepage: '{{ menu.thepage }}' })">{{ menu.thetext }}</a>
            <ul class="dropdown-menu" ng-repeat="submenu in menu.submenus">
                <li>
                    <a class="{{ submenu.setclass }}" ui-sref="static({ thepage: '{{ submenu.thepage }}' })">{{ submenu.thetext }}</a>
                </li>
            </ul>
        </li>

..... .

elgerlambert commented 10 years ago

+1 I ran into this issue too today (By coincidence also for a breadcrumb directive) Kind regards

tslater commented 10 years ago

+1...totally.

maggiben commented 10 years ago

+1 (jlmagee) Is right on the money ! breadcrumbs & navigation toolbars indeed require this feature !

timkindberg commented 10 years ago

So hold on... does it work or doesn't it? @jlmagee is saying it does above: https://github.com/angular-ui/ui-router/issues/395#issuecomment-32699331. Can anyone else validate that it works?

dmatteo commented 10 years ago

@timkindberg I don't know how to test it, since there is no reference to the static() function @jlmagee use while creating the link.

<a class="{{ submenu.setclass }}" ui-sref="static({ thepage: '{{ submenu.thepage }}' })">{{ submenu.thetext }}</a>

With a little JS snippet that includes that function I would definitely test it.

jlmagee commented 10 years ago

static is just a state name in myApp.config(function($stateProvider, $urlRouterProvider) { // For any unmatched url, redirect to /state1 $urlRouterProvider.otherwise("/static/main"); // Now set up the states $stateProvider .state('static', { url: '/static/{thepage}', templateUrl: function (stateParams){ return 'partials/' + stateParams.thepage + '.html'; } }); });

hope this helps

tdg5 commented 10 years ago

I would agree that this seems to work fine now. It may be because I'm using the master version of ui-router, or it may be because I add the breadcrumb data to the scope during the prelink phase, I'm not sure.

Like everyone else, I was looking to use the state data to create a breadcrumbs control. You can look at what I put together here: https://github.com/interval-braining/angular-ui-router-breadcrumbs

timkindberg commented 10 years ago

@tdg5 awesome!!

cesarbarone commented 10 years ago

I think this issue can be close. I am using dynamic ui-sref inside a directive and it run normally, after i updated my angular from 1.0.8 to 1.2.15.

MrOrz commented 10 years ago

If it is currently working, I think it's time we close this issue with a PR that tests the said functionality. Is there already a spec that describes the desired behavior?

crisbeto commented 10 years ago

@cesarbarone Can you give an example of how you're using it?

DylanLukes commented 10 years ago

I would like to add, something like this does not work:

app.directive ...
   link: (scope, el, attrs) ->
    $(el).find('li > a').attr('ui-sref', attrs.sref) unless _.isUndefined(attrs.sref)

The attribute is added just fine, but it's not clickable and triggers no state transitions.

cesarbarone commented 10 years ago

The same code with:

Angular 1.0.8 http://plnkr.co/edit/iSrzoKKYIkoDfCOPCUUg?p=preview

Angular 1.2.15 http://plnkr.co/edit/MxCYfY2duMMAgs83Mnoy?p=preview

cesarbarone commented 10 years ago

@DylanLukes, can you tried to put ng-href to 'a' element?

kleinsch commented 10 years ago

Using the code @cesarbarone posted works for me, but doesn't live update when my scope variable changes.

1151174958 commented 10 years ago

good

DeepAnchor commented 10 years ago

this issue is still not resolved for me using angular 1.2.6 and the master ui-router. But, it's easily fixed adding an ng-click to the item and a $state.go in your controller

crisbeto commented 10 years ago

I solved it like this:

I'm using Angular 1.2.9 and UI-Router 0.2.10

.directive('someDirective', [function(){

    return {
        restrict:'EA',
        scope:{
            itemType: "=",
            itemId: "=",
            state: "@"
        },
        template:'\
            <a ui-sref="{{ state }}({type: type, id: id})">\
            </a>',
        replace:true,
        link: function(scope){}
    }

}]);
<div
    some-directive
    ng-repeat="item in items" 
    item-id="item.id"
    item-type="item.type"
    state="someStateName"
    ></div>
lee-tunnicliffe commented 10 years ago

I have fixed this by adding an observe to the uiSref attribute and re-parsing the state name when it changes then calling update which will consume the updated state name.

Any chance I could contribute this back?

ThiagoMiranda commented 10 years ago

I can't get the ui-sref to work at all with 1.2.9 ( tried .6 to .16 ) inside a directive or inside a nested view. I'm using ui-router 0.2.10. It builds the correct href but the click won't work ( debugging it looks like the $timeout call of the click bind in the ui-router doesn't fire ) line 2399 of the angular-ui-router.js:

console.info("arrives here");
$timeout(function() {
    console.info("doesn't arrives here");
        $state.go(ref.state, params, { relative: base });
      });          

Any thoughts?

panuhorsmalahti commented 10 years ago

+1, this is a pretty useful feature. This is often needed when working with state parameters and ui-sref's. For example, the state parameter includes a blog_id state parameter, and the page includes a link to the currently selected blog etc.

mcranston18 commented 10 years ago

+1

sseppola commented 10 years ago

+1

mellodev commented 10 years ago

+1. My state names are ugly (nested), so I moved them to constants with helper method to build them or shortcut straight to a specific state (ex: config.routes.home()). Thanks

homerjam commented 10 years ago

+1

dmatteo commented 10 years ago

Is the PR of @lee-tunnicliffe being worked on, or are we waiting for someone to submit a PR?

wawyed commented 10 years ago

The fix is there, we just need someone to review it and merge it into master.

lrodziewicz commented 10 years ago

+1 for that, I can't wait for that being fixed.