search

ngOfficeUIFabric / ng-officeuifabric

Office UI Fabric (https://github.com/OfficeDev/office-ui-fabric) implementation for Angular
http://ngOfficeUiFabric.com
MIT License
321 stars 67 forks source link

breadcrumb with ng-repeat #330

Closed ReneHezser closed 8 years ago

ReneHezser commented 8 years ago

The breadcrumb control does not work with ng-repeat. This is what I have:

<uif-breadcrumb id="breadcrumbs" ng-controller="BreadcrumbController">
<uif-breadcrumb-link ng-href="{{breadcrumb.url}}" ng-repeat="breadcrumb in breadcrumbs">{{breadcrumb.name}}</uif-breadcrumb-link>
</uif-breadcrumb>

and $scope.breadcrumbs = [{ name: 'paul', url: 'test' }, { name: 'paul', url: 'test' }];

Expected Behavior

Two breadcrumbs will be rendered

Actual Behavior

Nothing happens

s-KaiNet commented 8 years ago

@ReneHezser thank you for submission!
Can you please create a plunkr or something demonstrating the issue?
From what I see in your code your are assigning ng-controller="BreadcrumbController" to uif-breadcrumb, but directive itself has a controller, conflict may occur. Try to put your controller on a higher level in the dom hierarchy.
And can you see any errors in dev console?

andikrueger commented 8 years ago

I was able to reproduce the error with this punklr: https://plnkr.co/edit/fKQsPUmQe7dAUkmOieQv?p=preview

The following error is logged in the console:

VM919 angular.js:12722 TypeError: Cannot read property 'insertBefore' of null at forEach.after (https://code.angularjs.org/1.4.9/angular.js:3510:13) at Object.JQLite.(anonymous function) as after at domInsert (https://code.angularjs.org/1.4.9/angular.js:5093:35) at Object.enter (https://code.angularjs.org/1.4.9/angular.js:5256:9) at ngRepeatTransclude (https://code.angularjs.org/1.4.9/angular.js:27928:26) at publicLinkFn (https://code.angularjs.org/1.4.9/angular.js:7808:29) at boundTranscludeFn (https://code.angularjs.org/1.4.9/angular.js:7947:16) at controllersBoundTransclude (https://code.angularjs.org/1.4.9/angular.js:8560:18) at ngRepeatAction (https://code.angularjs.org/1.4.9/angular.js:27921:15) at Object.$watchCollectionAction as fn

ghost commented 8 years ago

Probably because transclusion happens manually. Not sure how to fix this, I guess we could look at increase priority of breadcrumb and/or move it to prelink.

admanb commented 8 years ago

Any hope on this one? It would be really nice to be able to do dynamic breadcrumbs with this component.

ghost commented 8 years ago

I've been playing around, and this requires more work. Transclusion is a PITA and in the transclude function I don't seem to be able to grab the ng-repeated items. This may require a complete rework of this component. Not a 5 minute job...

andikrueger commented 8 years ago

Yesterday I built a playground on this topic: https://plnkr.co/

I think this will solve the ng-repeat issue,...

andikrueger commented 8 years ago

I updated my branch with my latest changes. The breadcrumb renders now with ng-repeat. There are some minor issues left:

andikrueger commented 8 years ago

Finally, I pushed my latest changes to this branch. By now tests for the regular and ng-repeat rendering are added. The ng-repeat functionality should work as expected...

Could anyone of you guys verify my changes? I will make the PR afterwards.

ghost commented 8 years ago

Just had a look at the source code, looks good to me! Nice work.

andrewconnell commented 8 years ago

Planning to merge the outstanding PR's later tonight (12hrs from now my time GMT -0500). @andikrueger if you want to submit a PR before that, I can get it into the next update.

Nice work!

andikrueger commented 8 years ago

@andrewconnell Thank you for the reminder. :)