ncuillery / angular-breadcrumb

Generate a breadcrumb from ui-router's states
http://ncuillery.github.io/angular-breadcrumb/
MIT License
785 stars 183 forks source link

Bound labels don't populate on browser refresh #42

Closed zpydee closed 9 years ago

zpydee commented 9 years ago

I am finding that breadcrumb labels bound to a controller's scope do not rebind on browser refresh.

ncuillery commented 9 years ago

The module is supposed to work with labels like Room {{room.number}} where room is a variable attached to the controller's scope.

Please can you provide a Plunker reproducing the bug ?

You can start a Plunker from scratch or fork this one.

zpydee commented 9 years ago

I do have the module working and binding correctly. the problem only occurs on browser refreshes. this is going to be difficult to show in a plunker as broweser refresh will be difficult to simulate.

Have you tested the module with controllerAs syntax?

On 28 Sep 2014, at 3:32 PM, Nicolas Cuillery notifications@github.com<mailto:notifications@github.com> wrote:

The module is supposed to work with labels like Room {{room.number}} where room is a variable attached to the controller's scope.

Please can you provide a Plunker reproducing the bug ?

You can start a Plunker from scratch or fork this onehttp://plnkr.co/edit/O8oOgnvcJ57k97f1fWRW?p=preview.

— Reply to this email directly or view it on GitHubhttps://github.com/ncuillery/angular-breadcrumb/issues/42#issuecomment-57085581.

ncuillery commented 9 years ago

Have you tested the module with controllerAs syntax?

No ;)

That's a clue. I'll try.

zpydee commented 9 years ago

i tried switching off controllerAs in one of my modules, and the problem still occurred, but perhaps its a point in the right direction. great module by the way!

on another topic, when will you be implementing the changes to allow abstract states to be included in the breadcrumb. if you need extra reason to do this, i'm happy to document the use-case.

let me know if you need any involvement from me in resolving...

regards, D

On 29 Sep 2014, at 10:51 AM, Nicolas Cuillery notifications@github.com<mailto:notifications@github.com> wrote:

Have you tested the module with controllerAs syntax? No ;)

That's a clue. I'll try.

— Reply to this email directly or view it on GitHubhttps://github.com/ncuillery/angular-breadcrumb/issues/42#issuecomment-57132647.

Stavrakakis commented 9 years ago

I'm experiencing this issue as well. We're also using controllerAs to define our controllers.

When the route is first generated, scope variables show up in the route correctly, but not on page refresh.

If you copy a url into a new tab and hit enter, the breadcrumb is correctly generated, it just seems to be when refreshing an already generated page.

Stavrakakis commented 9 years ago

I've been looking into this today and, for me, it was because $viewContentLoaded wasn't being picked up by the breadcrumb directive on page refresh.

I got round this by including a reference to the $breadcrumb provider in my app run() method:

angular.module('myApp')
    .run(['$rootScope', '$breadcrumb', myFunction]);

myFunction does some one time initialization stuff for my app.

It looks like $viewContentLoaded not firing might be a bug in Angular https://github.com/angular/angular.js/issues/1213.

zpydee commented 9 years ago

this does not solve my situation. i unfortunately have not had the time to explore the cause of my scenario yet.

On 06 Oct 2014, at 4:56 PM, Stavrakakis notifications@github.com<mailto:notifications@github.com> wrote:

I've been looking into this today and, for me, it was because $viewContentLoaded wasn't being picked up by the breadcrumb directive on page refresh.

I got round this by including a reference to the $breadcrumb provider in my app run() method:

angular.module('myApp') .run(['$rootScope', '$breadcrumb', myFunction]);

myFunction does some one time initialization stuff for my app.

It looks like $viewContentLoaded not firing might be a bug in Angular angular/angular.js#1213https://github.com/angular/angular.js/issues/1213.

— Reply to this email directly or view it on GitHubhttps://github.com/ncuillery/angular-breadcrumb/issues/42#issuecomment-58029308.

fkoehler commented 9 years ago

I run into the same issue. The reasons for me it did not work was because I did not put the <div ncy-breadcrumb></div> into the view of the main "home" state. After that the restoring worked... not sure why

m0ngr31 commented 9 years ago

I'm running into the same problem. Has anyone found a solution?

ansorensen commented 9 years ago

So I've been debugging this for the last day, and I've found that there are 2 breadcrumb elements, but the 2nd disappeared one is rendered as empty because thevierwScope is mismatched on reload.

More specifically, this block of code seems to be failing:

    // Early catch of $viewContentLoaded event
    $rootScope.$on('$viewContentLoaded', function (event) {
        // With nested views, the event occur several times, in "wrong" order
        if(isAOlderThanB(event.targetScope.$id, $lastViewScope.$id)) {
            $lastViewScope = event.targetScope;
        }
    });

It grabs the latest view scope when a view is loaded. When navigating normally to a page, that page is the newest viewScope. However, I've noticed that when I reload the problem page, it's scope is the 2nd to last. Another scope loads after the scope that the breadcrumb is bound to, so the breadcrumb is discarded. Not sure if there is a solution, but editing the code and see if I can tweak it to work, although I have a sinking feeling that what we probably need is a service that matches breadcrumbs steps to scope and can be used to look up the right scope to each step in case they fire out of order.

ncuillery commented 9 years ago

Another scope loads after the scope that the breadcrumb is bound to

That's interesting ! As you can see, I use the scope with the "higher" $id to interpolate all the labels. I did that because, in case of nested views, it is always the one bounded to the deepest view (i.e. the view corresponding to the current state).

So the rule is: The scope of the current state must define all the variables and functions used in the labels of the current state AND all its ancestors.

The reason of this limitation is because the events occur in a non-deterministic order during the page reload. See #10 and the associated commit e1f455b.

ncuillery commented 9 years ago

@zpydee @Stavrakakis @fkoehler @m0ngr31 I have been slow to answer here because I tried testing the module with the controllerAs syntax and several emplacements of the ncyBreadcrumb directive (on the controller-as branch and here ). Without success :(

Do you think your problem can be related to the restriction above ?

Please, can you try to define in scopes datas for every variables & functions present in labels (hard-coded or not) and tell me if the bug occur again ?

ansorensen commented 9 years ago

@ncuillery The strange thing about the latest id assumption is that this last $scope doesn't correspond a new ui-view within the current one (the current one has no children, as far as I can see). That's why it breaks on refresh but not on normal navigation. Since $scopes have so little identifying information, it's hard to tell what the $scope is, but I would guess it's actually a shallower scope that gets rendered slowly on refresh (where all scopes are being generated at once) and therefore mucks up the breadcrumb. On normal navigation, it's already be created and doesn't interfere.

There was a quick hack that works in my case: alter the if statement in the snippet above to exclude scope with no watchers. Since we add a watcher for each bound breadcrumb, we're never going to want to try to render a breadcrumb label on a scope without watchers.

However, still looking for a better solution for the refresh case that might actually be able to somehow link bound breadcrumbs to the correct scope, rather than assuming that the most recently rendered one is the correct one (and contains all variables for self and ancestor).

ncuillery commented 9 years ago

@ansorensen thanks for your analysis !

Here is the point : we must identify this $scope which come in late.

We are sure that this scope emits a $viewContentLoaded because it triggers the breadcrumb. And it's a ui-router specific event.

Please, can you follow these instructions:

By doing this, you will be able to identify this scope. I'm curious to know where it is located in your state hierarchy (is it within a multiple views state for example).

ghost commented 9 years ago

Hey,

just to "refresh" this conversation, I do have the same issue. I don't have controllers on all my views as some don't require one.

the event.targetScope.$id when refreshing is always: 00E but when coming from previous state it changes every time.

maku commented 9 years ago

Had the same issue on reloading a page. Passing $breadcrumb in the run function (like @Stavrakakis suggested) solved the problem.

ghost commented 9 years ago

OK, after some more digging, it seems that for me the analysis from @ansorensen was proper. If I check for watchers and only do the check if isAOlderThanB then, the label properly displays.

I've checked the $id from the targetScope and lastViewScope, and the conditions always end up being true. I'm not sure if only relying on that is sufficient. I have no $viewContentLoaded anywhere else in my app so that can't be polluted. However I do have some nested views within children states... Maybe that's why.

Issue is now, how can I port this modification to production..?

ghost commented 9 years ago

OK, with a bit more digging, it was because I had many ui-view on the same page. My index page looked like this:


  <div ui-view="menu"></div>

      <section class="wrapper">
        <div ui-view="topNav" class="topNav" id="return-top"></div>
        <section>
          <div ui-view="subheader" class="subheader"></div>

          <div ui-view="content" class="main"></div>
        </section>
    </section>
    <nav ui-view="footer" id="footer"></nav>

<div ui-view="root"></div>

Thing is, this way the last controllers being created were footer and root. Whereas my content comes from the content view (and its associated controller)

I now use ng-include for the footer as I can't move it, and I moved root to the top of the page as it doesn't display anything. Like so:


  <div ui-view="root"></div>

  <div ui-view="menu"></div>

      <section class="wrapper">
        <div ui-view="topNav" class="topNav" id="return-top"></div>
        <section>
          <div ui-view="subheader" class="subheader"></div>

          <div ui-view="content" class="main"></div>
        </section>
    </section>
    <nav ng-include="'views/navigation/navigationBottom.html'" id="footer"></nav>

Hope that helps some of you.

mateutek commented 9 years ago

I am using angular-fulstack generator when installing ncyBreadcrumb i added it to the navbar component. After refresh the {{impex.name}} dissappears

angular.module('impexatorApp')
  .config(function ($stateProvider) {
    $stateProvider
      .state('impexes', {
        url: '/impexes',
        templateUrl: 'app/impex/impex.html',
        controller: 'ImpexCtrl',
        ncyBreadcrumb: {
          label: 'Impexes'
        }
      })
      .state('impexes.detail',{
        url: '/{impexId}',
        views: {
          "@" : {
            templateUrl: 'app/impex/details.html',
            controller: 'ImpexDetailsCtrl'
          }
        },
        ncyBreadcrumb: {
          label: '{{impex.name}}'
        }
      })
  });
CaptainYouz commented 9 years ago

Hello, i have the same issue where i refresh the browser. The solutions mentionned in this topic didn't work for me. Does anyone found a patch ?

kazinov commented 9 years ago

This demonstrates common problem when there is an abstract base state which represents a footer

rkusuma commented 9 years ago

@kazinov so what is the solution? i'm still get this problem.

kazinov commented 9 years ago

@rkusuma To solve this issue the property ncyBreadcrumbIgnore should be set to true on the view controller scope:

function ViewController($scope) {
       $scope.ncyBreadcrumbIgnore = true;
}

This allows to deal with multiple views related to the same state.

patricknazar commented 8 years ago

@Stavrakakis 's solution worked for me but @kazinov 's did not.