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

Added support to use ncyBreadcrumb.label as an injector block #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thebigredgeek
Copy link

This allows more dynamic use of the label. Right now, the label is bound to the parent scope, even if the parent scope is destroyed (such as in the case that the child state is interpolated into the same view as the parent state was contained in). This breaks the breadcrumbs, as the label can no longer interpolate properties from the controller attached to it's state because the controller has been destroyed (as well as the scope).

@thebigredgeek
Copy link
Author

Bump?

@ncuillery
Copy link
Owner

Hi,

Apologies for the delay, I'll throw an eye on it soon 👼

@ncuillery
Copy link
Owner

Hi, apologies for the delay since my last apologies 😄

I think you did a great job here. If I understand well, it can be used by defining ncyBreadcrumb.label as a DI function or array like this:

      .state('room.detail.edit', {
        url: '/edit',
        views: {
          "@" : {
            templateUrl: 'views/room_form.html',
            controller: 'RoomDetailCtrl'
          }
        },
        ncyBreadcrumb: {
          label: function(rooms, $stateParams) {
            for(var index = 0; index < rooms.length; index++) {
              if(rooms[index].roomId === parseInt($stateParams.roomId)) {
                return 'Editing room ' + rooms[index].roomNumber;
              }
            }

            return 'Editing';
          }
        }
      });

It's a flexible way of defining a label and could be a workaround for the problem described here #89, right ?

I have a suggestion: We can easily make the current scope (the one attached to the $viewContentLoaded event, used by all the directives) available for DI by using the third arg locals of $injector.invoke (see docs):

step.ncyBreadcrumbLabel = $injector.invoke(
  step.ncyBreadcrumb.label, 
  undefined, 
  {'$scope': viewScope});

Then you can inject the scope:

        ncyBreadcrumb: {
          label: function($scope, rooms, $stateParams) {
            [...]
          }
        }

It could be useful if other situations, when the current scope hold desired informations. What you think ?

I could be great if we harmonize the syntax with the ncyBreadcrumb.parent where I "inject" the current scope manually here (I can do it later if you prefer, you paved the path here)

lastStep.ncyBreadcrumbLabel = $injector.invoke(lastStep.ncyBreadcrumb.label);
} else {
var parseLabel = $interpolate(lastStep.ncyBreadcrumb.label);
lastStep.ncyBreadcrumbLabel = parseLabel(viewScope);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this directive, we directly assign the variables to the scope of the directive.

scope.ncyBreadcrumbLabel = parseLabel(viewScope);

The same above (L273).

It's the cause of the test failure 😄

@SuPenguin
Copy link

Hello,
Any idea if this is going to be merged or if there is a workaround ?
Thanks

@stramel
Copy link

stramel commented Oct 7, 2016

Anyone have a status update on this?

@biltongza
Copy link

+1 Would really like this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants