Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Allow replace-transclude the sortable elements at link time #405

Open
sszdh opened this issue Nov 8, 2015 · 13 comments
Open

Allow replace-transclude the sortable elements at link time #405

sszdh opened this issue Nov 8, 2015 · 13 comments

Comments

@sszdh
Copy link

sszdh commented Nov 8, 2015

Code to reproduce issue:

app.js

var app = angular.module('App', ['ui.sortable']);

app.controller('MainCtrl', ['$scope', MainCtrl]);

function MainCtrl($scope) {
  $scope.items = [
    {id: 1, type: 'dog', name: 'galardo'},
    {id: 2, type: 'cat', name: 'loossi'},
    {id: 3, type: 'fish', name: 'sharkie'}
  ];
}

app.directive('myDirective', ['$compile', myDirective]);

function myDirective($compile) {
  return {
      restrict: 'EA',
      replace: true,
      //template: '<div ng-bind="item.name" item="item"></div>',
      scope: {
          type: "@",
          item: "="
      },
      link: link
  };

  function link(scope, element, attrs) {
      var view = $compile('<div ng-bind="item.name"></div>')(scope);
      element.replaceWith(view);
  }
}

index.html

<div ui-sortable ng-model="items">
    <my-directive item="item" ng-repeat="item in items"></my-directive>
 </div>

Console Error:

Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: item in items, Duplicate key: undefined:undefined, Duplicate value: undefined
http://errors.angularjs.org/1.4.7/ngRepeat/dupes?p0=item%20in%20items&p1=undefined%3Aundefined&p2=undefined
    at http://localhost/angular-tests/lib/angular/angular.js:68:12

P.S: If we use template property instead of link function, there is no issue.

Cheers.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 8, 2015

Great finding!!!
Does the problem get fixed if you add a track by id in your ng-repeat???
If yes, then the case might be that there is a digest loop where your
directive isn't yet linked and the element hasn't yet been replaced, so
since there is no "track by", angular uses the actual html for tracking,
resulting dupes.
Any chance you can make a codepen example of this?

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

Hi, @thgreasi

Does the problem get fixed if you add a track by id in your ng-repeat???

No it doesn't. Using track by item.id throw same error but using track by $index throw no error but no sortable functionality (in some case only one item could be moved)!

Any chance you can make a codepen example of this?

Yes , here it is.

Actually this issue raised in Directive-in-Directive too.

Cheers

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

See what happened in html source when we sorting one element intrack by $index mode:

<div ui-sortable="" ng-model="items" class="ng-pristine ng-untouched ng-valid ng-isolate-scope ui-sortable">
    <!-- ngRepeat: item in items track by $index -->
   <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>
    <!-- end ngRepeat: item in items track by $index -->
    <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>
    <!-- end ngRepeat: item in items track by $index -->
    <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>

   <!-- end ngRepeat: item in items track by $index -->
   <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">sharkie</div>
    <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">galardo</div>
    <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">loossi</div>
</div>

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

It seems that its a problem with the replacement of the original element...
It's true that ui-sortable is a little too sansitive about manipulations on
the repeated element.

I saw that you use a "type" attribute. If that's your main use case, have
you seen the example in README (http://codepen.io/thgreasi/pen/uyHFC)
regarding dynamic templates?

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

@thgreasi Thanks, I already checked that.

Actually my main reason of using ng-model in directive is to track curriculum changes and then make an $http call to server!

But may I ask you how your tg-dynamic-directive can help me?

Actually I have a list like this:

var data = [
 {id: 1, type: 'chapter', title: 'chapter 1'},
 {id: 1, type: 'lecture', title: 'lecture 1-1'},
 {id: 2, type: 'lecture', title: 'lecture 1-2'},
 {id: 2, type: 'chapter', title: 'chapter 2'},
 ...
];

and a wrapper directive, something like that:

app.directive('curriculum', ['$compile', curriculum]);

function curriculum($compile) {
    return {
        restrict: 'EA',
        replace: true,
        scope: {
            type: "@",
            item: "="
        },
        link: link
    };

    function link(scope, element, attrs) {
        var view = $compile('<curriculum-'+ scope.type +' item="item"></curriculum-' + scope.type + '>')(scope);
        element.replaceWith(view);
    }
}

app.directive('curriculum-chapter', [curriculumChapter]);
function curriculumChapter() {
 return {
    ...
    template: '/templates/chapter.tpl.html'
    ...
 }
}

app.directive('curriculum-lecture', [curriculumLecture]);
function curriculumLecture() {
return {
    ...
    template: '/templates/lecture.tpl.html'
    ...
 }
}

So is it possible to use ng-ui-sortable in this case?

Thanks in advance.

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

Unfortunate tg-dynamic-directive does not replace its nested directives tag (<tg-dynamic-directive> & <div ng-include>) and it makes my HTML structure complicated and my CSS selectors does not work properly!

I thinks angularjs should allows nested directive with template: ..., replace: true options in both!

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

With the tg-dynamic-directive you could avoid creating a directive that would manually compile the appropriate directive based on the type parameter. You would only need to define a function that retrieves the template to be included based on the type provided.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

As you saw transclusions with replace: true can cause problems. If the suggested solution described in README works for you, then you should focus on fixing your CSS. I think this would be easier than solving the "replace: true & dynamic $compiling & linking" problem.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

My experience on a similar implementation convinced me that ng-include was
the way to go for this use case, since I experience similar problems with
"replaced transclusion" as well. That was the reason I created
tg-dynamic-directive, so that others wouldn't have to mess with the same
problem from scratch.
If you feel it is too restrictive, then just copy its implementation,
customize it (probably by adding some css classes to the elements) and
integrate it to your directives.

On Mon, Nov 9, 2015, 16:25 Soheil Samadzadeh notifications@github.com
wrote:

Unfortunate tg-dynamic-directive does not replace its nested directives
tag ( &

) and it makes my HTML
structure complicated and my CSS selectors does not work properly!

I thinks angularjs should allows nested directive with template: ...,
replace: true options in both!


Reply to this email directly or view it on GitHub
#405 (comment)
.

Thodoris Greasidis
Computer, Networking &
Communications Engineer

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

@thgreasi, this was a good discussion. and thank you.

Finally I ended up with this solution:

app.directive('curriculum', curriculum);

function curriculum() {
    return {
        restrict: 'EA',
        replace: true,
        template: '<div ng-include="templateUrl" include-replace></div>',
        link: link,
        scope: {
            type: '@',
            item: '='
        }
    };

    function link(scope) {
        scope.templateUrl = "/templates/curriculum-" + scope.type + ".tpl";
    }
}


app.directive('includeReplace', curriculum);

function includeReplace() {
    return {
        require: 'ngInclude',
        replace: true,
        restrict: 'A',
        link: function (scope, el) {
            el.replaceWith(el.children());
        }
    };
}
<curriculum ng-repeat="item in curriculum" type="[[item.type]]" item="item" ></curriculum>

It may be good solution for cases that only depends on templates.
But It prevents me to encapsulate directive-specific logic (such as controller, etc) into each directive! So Sad!

And I think we should get back and focus on main issue of ng-ui-sortable mentioned above "Sort-able directive with link function problem" and trying to fix it.

Thank you very much

@thgreasi thgreasi changed the title Sort-able directive with link function problem! Allow replace-transclude the sortable elements at link time Nov 9, 2015
@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

I'm glad that it worked for you 👍
That's more or less the solution that I was suggesting and that I followed in my case as well.
That's also what inspired me for the implementation of tg-dynamic-directive.
Unfortunately you can't directly define controllers or directives, but obviously you can do that inside your included template.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2015

I also changed the title to what I think better describes the problem. My opinion is that is not the linking that is causing the problem (since your directive and tg-dynamic-directive also have link functions) but the fact that we are replacing the sortable items that are generated by the ng-repeat.

@sszdh
Copy link
Author

sszdh commented Nov 9, 2015

👍

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

No branches or pull requests

2 participants