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

Dynamic ui-sref in directive #395

Closed
snikch opened this issue Sep 8, 2013 · 102 comments
Closed

Dynamic ui-sref in directive #395

snikch opened this issue Sep 8, 2013 · 102 comments
Assignees
Milestone

Comments

@snikch
Copy link

snikch commented Sep 8, 2013

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
Copy link
Contributor

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

Correct.

@snikch
Copy link
Author

snikch commented Sep 8, 2013

Technically difficult, or just not a priority?

@nateabele
Copy link
Contributor

More of an API design consideration.

@snikch
Copy link
Author

snikch commented Sep 8, 2013

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
Copy link
Contributor

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

@snikch
Copy link
Author

snikch commented Sep 8, 2013

Cool, thanks!

@nateabele
Copy link
Contributor

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
Copy link

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.

👍

@ghost
Copy link

ghost commented Sep 20, 2013

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
Copy link
Contributor

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

@ghost
Copy link

ghost commented Sep 20, 2013

@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
Copy link
Contributor

@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
Copy link

ghost commented Sep 20, 2013

@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
Copy link

KayEss commented Nov 15, 2013

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
Copy link

dmatteo commented Dec 2, 2013

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
Copy link

MrOrz commented Dec 19, 2013

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. 😢

@timkindberg
Copy link
Contributor

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

@timkindberg timkindberg reopened this Dec 19, 2013
@MrOrz
Copy link

MrOrz commented Dec 19, 2013

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
Copy link

Currently facing a need for this right now.
👍

@KrisBraun
Copy link

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

@imjoshholloway
Copy link

+1 for this.

@jlmagee
Copy link

jlmagee commented Jan 19, 2014

+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
Copy link

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

@tslater
Copy link

tslater commented Feb 7, 2014

+1...totally.

@maggiben
Copy link

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

@timkindberg
Copy link
Contributor

So hold on... does it work or doesn't it? @jlmagee is saying it does above: #395 (comment). Can anyone else validate that it works?

@ProLoser
Copy link
Member

I pretty much do this in every single app I ever build:

app.run(function($rootScope, $state) {
  $rootScope.$state = $state;
  // or
  $rootScope.go = $state.go.bind($state);
})

@ernestoalejo
Copy link

Ok, thanks. That's more reasonable

@homerjam
Copy link

@ProLoser that technique might create problems for those that use prerendering for crawlers. Besides that I've personally not encountered a project thats required more than around 20 active ui-srefs at any one time which is acceptable for the convenience, no?

@ProLoser
Copy link
Member

@homerjam honestly dude, you're going to have way bigger issues when dealing with crawlers. I just don't think it's worth the overhead for something you only interact with ONCE and is only relevant during interaction.

If it REALLY is important for you to have sexy urls you could do ng-href="myUtil" or something, but I don't think it's worth making everyone suffer the bloated overhead.

@ernestoalejo
Copy link

And what about a different directive, something like ui-sref-dynamic? It's the same as ng-bind-once or solutions like those.

@christopherthielen
Copy link
Contributor

Nate mentioned some API considerations. I'm not sure what those were so until I hear what they are,
I personally don't have problem with dynamic ui-sref. I was indeed surprised to learn they weren't supported when I started using UI-Router and tried implementing a data-driven navigation.

If performance is the concern, I bet we could find a way to make the common case fast, such as not observing if we know the value cant change.

That said, the workarounds are simple and easy, but it would be nice to have consistent linking mechanism in a UI-Router app.

@nateabele
Copy link
Contributor

I would opt for a separate directive, i.e.:

<div ng-controller="FooController">
  <a ui-state="state" ui-state-params="params" ui-state-opts="{ absolute: true }">
    Link
  </a>
</div>
function FooController($scope) {
  angular.extend($scope, {
    state: "person.view",
    params: { id: 1138 }
  });
}

No ugly syntax. No one-off micro-grammars. No complicated parsing rules. No inconsistent behavior. No degraded performance for default use cases.

If anyone would like to implement this, PRs are now being accepted. Please remember to consult the contributor guidelines.

@maruf89
Copy link

maruf89 commented Oct 14, 2014

+1

@Sawtaytoes
Copy link

It works for me on version 0.2.11:

a(ui-sref="accounts({accountId: '{{account.entityId}}'})")

@mirza99
Copy link

mirza99 commented Nov 4, 2014

+1 for the feature

If you have nested views and stated, you solved it by adding the $state to the scope.
I've done it for the "previous" button.

The view :
ui-sref="layout({layoutId: vm.state.params.layoutId } )"

@habfast
Copy link

habfast commented Nov 13, 2014

+1 i'd need that too.

@rapazz
Copy link

rapazz commented Nov 13, 2014

+1

@jrosseel
Copy link

+1 I need that too

@niemyjski
Copy link

+1 is this implemented already?

@quorak
Copy link

quorak commented Nov 23, 2014

+1

2 similar comments
@MaximeFradin
Copy link

+1

@GalGalGal
Copy link

+1

@angular-ui angular-ui locked and limited conversation to collaborators Dec 5, 2014
@nateabele
Copy link
Contributor

If anyone would like to implement this, please submit a PR and reference this issue, thanks.

@nateabele nateabele reopened this Aug 17, 2015
@christopherthielen christopherthielen modified the milestones: 0.2.16, future Aug 19, 2015
christopherthielen pushed a commit to nateabele/ui-router that referenced this issue May 9, 2020
 - Refactor StateRefDirective for better modularity
 - Drop key restrictions on ui-sref-opts
 - Improves performance over prior implementation with no extra $eval()’s

Fixes angular-ui#395, angular-ui#900, angular-ui#1932
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 a pull request may close this issue.